Skip to content

Commit

Permalink
OAuth: Add authorization scopes & remove OpenID compatibility (#2734)
Browse files Browse the repository at this point in the history
* Re-use code definition of oauthResponseTypeSchema

* Generate proper invalid_authorization_details

* Remove OpenID compatibility

* tidy

* properly verify presence of jti claim in client assertion

* Remove non-standard "sub" from OAuthTokenResponse

* Remove nonce from authorization request

* tidy

* Enforce uniqueness of code_challenge

* remove unused "atproto" scope

* Improve reporting of validation errors

* Allow empty set of scopes

* Do not remove scopes not advertised in the AS's "scopes_supported" when building the authorization request.

* Prevent empty scope string

* Remove invalid check from token response

* remove un-necessary session refresh

* Validate scopes characters according to OAuth 2.1 spec

* Mandate the use of "atproto" scope

* Disable ability to list app passwords when using an app password

* Use locally defined authPassthru in com.atproto.admin.* handlers

* provide proper production handle resolver in example

* properly compote login method

* feat(oauth-provider): always rotate session cookie on sign-in

* feat(oauth-provider): do not require consent from first party apps

* update request parameter's prompt before other param validation checks

* feat(oauth-provider): rework display of client name

* feat(oauth-client-browser:example): add token info introspection

* feat(oauth-client-browser:example): allow defining scope globally

* Display requested scopes during the auth flow

* Add, and verify, a "typ" header to access and refresh tokens

* Ignore case when checking for dpop auth scheme

* Add "jwtAlg" option to verifySignature() function

* Verify service JWT header values. Add iat claim to service JWT

* Add support for "transition:generic" and "transition:chat.bsky" oauth scopes in PDS

* oauth-client-browser(example): add scope request

* Add missing "atproto" scope

* Allow missing 'typ' claim in service auth jwt

* Improved 401 feedback

Co-authored-by: devin ivy <[email protected]>

* Properly parse scopes upon verification

Co-authored-by: devin ivy <[email protected]>

* Rename "atp" to "credential" auth in oauth-client-browser example

* add key to iteration items

* Make CORS protection stronger

* Allow OAuthProvider to define its own CORS policies

* Revert "Allow missing 'typ' claim in service auth jwt"

This reverts commit 15c6b9e.

* Revert "Verify service JWT header values. Add iat claim to service JWT"

This reverts commit 08df8df.

* Revert "Add "jwtAlg" option to verifySignature() function"

This reverts commit d0f7735.

* Revert "Add, and verify, a "typ" header to access and refresh tokens"

This reverts commit 3e21be9.

* pds: implement protected service auth methods

* Prevent app password management using sessions initiated from an app password.

* Alphabetically sort PROTECTED_METHODS

* Revert changes to app password management permissions

* tidy

---------

Co-authored-by: devin ivy <[email protected]>
  • Loading branch information
matthieusieben and devinivy authored Aug 27, 2024
1 parent 70e2bff commit dee817b
Show file tree
Hide file tree
Showing 96 changed files with 1,096 additions and 1,118 deletions.
5 changes: 5 additions & 0 deletions .changeset/cool-toes-rescue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-provider": patch
---

Display requested scopes during the auth flow
5 changes: 5 additions & 0 deletions .changeset/fluffy-apples-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/pds": patch
---

Use locally defined authPassthru
5 changes: 5 additions & 0 deletions .changeset/green-bags-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/pds": patch
---

Add support for "transition:generic" and "transition:chat.bsky" oauth scopes
5 changes: 5 additions & 0 deletions .changeset/healthy-bottles-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-provider": patch
---

Generate proper invalid_authorization_details
6 changes: 6 additions & 0 deletions .changeset/lemon-mice-rule.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@atproto/oauth-provider": minor
"@atproto/oauth-client": minor
---

Remove "nonce" from authorization request
5 changes: 5 additions & 0 deletions .changeset/light-dingos-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/pds": patch
---

Ignore case when checking for dpop auth scheme
5 changes: 5 additions & 0 deletions .changeset/ninety-ants-collect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-provider": patch
---

Stronger CORS protections
5 changes: 5 additions & 0 deletions .changeset/odd-spies-boil.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-types": patch
---

Validate scopes characters according to OAuth 2.1 spec
6 changes: 6 additions & 0 deletions .changeset/polite-humans-sleep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@atproto/oauth-provider": minor
"@atproto/oauth-client": minor
---

Mandate the use of "atproto" scope
5 changes: 5 additions & 0 deletions .changeset/poor-socks-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-provider": patch
---

Do not require user consent during oauth flow for first party apps.
12 changes: 12 additions & 0 deletions .changeset/short-toes-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@atproto/oauth-provider": minor
"@atproto/oauth-client": minor
"@atproto/oauth-client-browser": minor
"@atproto/oauth-client-node": minor
---

Remove "openid" compatibility. The reason is that although we were technically "openid" compatible, ATProto identifiers are distributed identifiers. When a client relies on OpenID to authenticate users, it will use the auth provider in combination with the identifier to uniquely identify the user. Since ATProto identifiers are meant to be able to move from one provider to the other, OpenID compatibility could break authentication after a user was migrated to a different provider.

The way OpenID compliant clients would adapt to this particularity would typically be to remove the provider + identifier combination and use the identifier alone. While this is indeed the right way to handle ATProto identifiers, it requires more work to avoid impersonation. In particular, when obtaining a user identifier, the client **must** verify that the issuer of the identity token is indeed the server responsible for that user. This mechanism being not enforced by the OpenID standard, OpenID compatibility could lead to security issues. For this reason, we decided to remove OpenID compatibility from the OAuth provider.

Note that a trusted central authority could still offer OpenID compatibility by relying on ATProto's regular OAuth flow under the hood. This capability is out of the scope of this library.
5 changes: 5 additions & 0 deletions .changeset/six-ties-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-types": patch
---

Re-use code definition of oauthResponseTypeSchema
5 changes: 5 additions & 0 deletions .changeset/smart-drinks-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-client": patch
---

Do not remove scopes not advertised in the AS's "scopes_supported" when building the authorization request.
5 changes: 5 additions & 0 deletions .changeset/smart-gifts-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-types": patch
---

Remove non-standard "sub" from OAuthTokenResponse
5 changes: 5 additions & 0 deletions .changeset/thin-cycles-live.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/pds": patch
---

Allow OAuthProvider to define its own CORS policies
5 changes: 5 additions & 0 deletions .changeset/tidy-cars-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-provider": patch
---

Improve reporting of validation errors
5 changes: 2 additions & 3 deletions packages/api/OAUTH.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Here is an example client metadata.
"tos_uri": "https://example.com/tos",
"policy_uri": "https://example.com/policy",
"redirect_uris": ["https://example.com/callback"],
"scope": "offline_access",
"scope": "atproto",
"grant_types": ["authorization_code", "refresh_token"],
"response_types": ["code"],
"token_endpoint_auth_method": "none",
Expand Down Expand Up @@ -73,8 +73,7 @@ Here is an example client metadata.
the user during the authentication process.

- If you don't want or need the user to stay authenticated for long periods
(better for security), you can remove the `offline_access` scope, and
`refresh_token` from the `grant_types`.
(better for security), you can remove `refresh_token` from the `grant_types`.

> [!NOTE]
>
Expand Down
2 changes: 1 addition & 1 deletion packages/oauth/oauth-client-browser/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ needs of your application and must respect the [ATPROTO] spec.
"tos_uri": "https://my-app.com/tos",
"policy_uri": "https://my-app.com/policy",
"redirect_uris": ["https://my-app.com/callback"],
"scope": "profile email offline_access",
"scope": "atproto",
"grant_types": ["authorization_code", "refresh_token"],
"response_types": ["code"],
"token_endpoint_auth_method": "none",
Expand Down
23 changes: 23 additions & 0 deletions packages/oauth/oauth-client-browser/example/src/app.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import { useCallback, useState } from 'react'
import { useAuthContext } from './auth/auth-provider'
import { OAuthSession } from '@atproto/oauth-client'

function App() {
const { pdsAgent, signOut } = useAuthContext()

const hasTokenInfo = pdsAgent.sessionManager instanceof OAuthSession

const [tokeninfo, setTokeninfo] = useState<unknown>(undefined)
const loadTokeninfo = useCallback(async () => {
if (pdsAgent.sessionManager instanceof OAuthSession) {
setTokeninfo(await pdsAgent.sessionManager.getTokenInfo())
}
}, [pdsAgent])

// A call that requires to be authenticated
const [serviceAuth, setServiceAuth] = useState<unknown>(undefined)
const loadServiceAuth = useCallback(async () => {
Expand All @@ -30,6 +40,19 @@ function App() {
<div>
<p>Logged in!</p>

{hasTokenInfo && (
<>
<button onClick={loadTokeninfo}>Load token info</button>
<code>
<pre>
{tokeninfo !== undefined
? JSON.stringify(tokeninfo, undefined, 2)
: null}
</pre>
</code>
</>
)}

<button onClick={loadProfile}>Load profile</button>
<code>
<pre>
Expand Down
26 changes: 15 additions & 11 deletions packages/oauth/oauth-client-browser/example/src/auth/auth-form.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { useCallback, useEffect, useState } from 'react'
import { useEffect, useState } from 'react'

import { AtpSignIn, AtpSignInForm } from './atp/atp-sign-in-form'
import {
AtpSignIn,
CredentialSignInForm,
} from './credential/credential-sign-in-form'
import { OAuthSignIn, OAuthSignInForm } from './oauth/oauth-sign-in-form'

export function AuthForm({
Expand All @@ -10,19 +13,20 @@ export function AuthForm({
atpSignIn?: AtpSignIn
oauthSignIn?: OAuthSignIn
}) {
const defaultMethod = useCallback(
() => (oauthSignIn ? 'oauth' : atpSignIn ? 'atp' : undefined),
[],
)
const defaultMethod = oauthSignIn
? 'oauth'
: atpSignIn
? 'credential'
: undefined

const [method, setMethod] = useState<undefined | 'oauth' | 'atp'>(
const [method, setMethod] = useState<undefined | 'oauth' | 'credential'>(
defaultMethod,
)

useEffect(() => {
if (method === 'oauth' && !oauthSignIn) {
setMethod(defaultMethod)
} else if (method === 'atp' && !atpSignIn) {
} else if (method === 'credential' && !atpSignIn) {
setMethod(defaultMethod)
} else if (!method) {
setMethod(defaultMethod)
Expand All @@ -45,17 +49,17 @@ export function AuthForm({

<button
className={`bg-blue-500 hover:bg-blue-700 text-white font-bold py-1 px-4 rounded ${
method === 'atp' ? 'bg-blue-700' : ''
method === 'credential' ? 'bg-blue-700' : ''
}`}
onClick={() => atpSignIn && setMethod('atp')}
onClick={() => atpSignIn && setMethod('credential')}
disabled={!atpSignIn}
>
Credentials
</button>
</div>

{method === 'oauth' && <OAuthSignInForm signIn={oauthSignIn!} />}
{method === 'atp' && <AtpSignInForm signIn={atpSignIn!} />}
{method === 'credential' && <CredentialSignInForm signIn={atpSignIn!} />}
{method == null && <div>No auth method available</div>}
</div>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { Agent } from '@atproto/api'
import { createContext, ReactNode, useContext, useMemo } from 'react'

import { useAtpAuth } from './atp/use-atp-auth'
import { useCredentialAuth } from './credential/use-credential-auth'
import { AuthForm } from './auth-form'
import { useOAuth, UseOAuthOptions } from './oauth/use-oauth'

Expand All @@ -30,19 +30,19 @@ export const AuthProvider = ({
} = useOAuth(options)

const {
agent: atpAgent,
signIn: atpSignIn,
signOut: atpSignOut,
} = useAtpAuth()
agent: credentialAgent,
signIn: credentialSignIn,
signOut: credentialSignOut,
} = useCredentialAuth()

const value = useMemo<AuthContext | null>(
() =>
oauthAgent
? { pdsAgent: oauthAgent, signOut: oauthSignOut }
: atpAgent
? { pdsAgent: atpAgent, signOut: atpSignOut }
: credentialAgent
? { pdsAgent: credentialAgent, signOut: credentialSignOut }
: null,
[oauthAgent, oauthSignOut, atpAgent, atpSignOut],
[oauthAgent, oauthSignOut, credentialAgent, credentialSignOut],
)

if (isLoginPopup) {
Expand All @@ -56,7 +56,7 @@ export const AuthProvider = ({
if (!value) {
return (
<AuthForm
atpSignIn={atpSignIn}
atpSignIn={credentialSignIn}
oauthSignIn={oauthClient ? oauthSignIn : undefined}
/>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export type AtpSignIn = (input: {
* @returns Nice tailwind css form asking to enter either a handle or the host
* to use to login.
*/
export function AtpSignInForm({
export function CredentialSignInForm({
signIn,
...props
}: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useCallback, useMemo, useState } from 'react'

type Session = AtpSessionData & { service: string }

export function useAtpAuth() {
export function useCredentialAuth() {
const createAgent = useCallback((service: string) => {
const agent = new AtpAgent({
service,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ import {
OAuthSession,
} from '@atproto/oauth-client-browser'

type Gettable<T> = () => PromiseLike<T> | T

export type OnRestored = (session: OAuthSession | null) => void
export type OnSignedIn = (session: OAuthSession, state: null | string) => void
export type OnSignedOut = () => void
export type GetState = () =>
| undefined
| string
| PromiseLike<undefined | string>

function useCallbackRef<T extends (this: any, ...args: any[]) => any>(
fn: T,
Expand Down Expand Up @@ -145,13 +143,15 @@ export type UseOAuthOptions = ClientOptions & {
onRestored?: OnRestored
onSignedIn?: OnSignedIn
onSignedOut?: OnSignedOut
getState?: GetState
getScope?: Gettable<undefined | string>
getState?: Gettable<undefined | string>
}

export function useOAuth(options: UseOAuthOptions) {
const onRestored = useCallbackRef(options.onRestored)
const onSignedIn = useCallbackRef(options.onSignedIn)
const onSignedOut = useCallbackRef(options.onSignedOut)
const getScope = useCallbackRef(options.getScope)
const getState = useCallbackRef(options.getState)

const clientForInit = useOAuthClient(options)
Expand Down Expand Up @@ -243,9 +243,6 @@ export function useOAuth(options: UseOAuthOptions) {
)
}

// Force fetching the token info in order to trigger a token refresh
void session?.getTokenInfo(true)

return () => {
controller.abort()
}
Expand All @@ -256,11 +253,13 @@ export function useOAuth(options: UseOAuthOptions) {
if (!client) throw new Error('Client not initialized')

const state = options?.state ?? (await getState()) ?? undefined
const session = await client.signIn(input, { ...options, state })
const scope = options?.scope ?? (await getScope()) ?? 'atproto'

const session = await client.signIn(input, { ...options, scope, state })
setSession(session)
await onSignedIn(session, state ?? null)
},
[client, getState, onSignedIn],
[client, getScope, getState, onSignedIn],
)

// Memoize the return value to avoid re-renders in consumers
Expand Down
9 changes: 5 additions & 4 deletions packages/oauth/oauth-client-browser/example/src/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ ReactDOM.createRoot(document.getElementById('root')!).render(
<React.StrictMode>
<AuthProvider
// dev-env
// plcDirectoryUrl="http://localhost:2582"
// handleResolver="http://localhost:2584"
plcDirectoryUrl="http://localhost:2582"
handleResolver="http://localhost:2584"
// production
plcDirectoryUrl={undefined}
handleResolver="https://bsky.app"
// plcDirectoryUrl={undefined}
// handleResolver="https://bsky.social"
getScope={() => 'atproto transition:generic'}
>
<App />
</AuthProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export type Schema = {
dpopKey: EncodedKey

iss: string
nonce: string
verifier?: string
appState?: string
}>
Expand Down
3 changes: 1 addition & 2 deletions packages/oauth/oauth-client-node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const client = new NodeOAuthClientOptions({
tos_uri: 'https://my-app.com/tos',
policy_uri: 'https://my-app.com/policy',
redirect_uris: ['https://my-app.com/callback'],
scope: 'profile email offline_access',
grant_types: ['authorization_code', 'refresh_token'],
response_types: ['code'],
application_type: 'web',
Expand Down Expand Up @@ -158,7 +157,7 @@ The client metadata will typically contain:
"tos_uri": "https://my-app.com/tos",
"policy_uri": "https://my-app.com/policy",
"redirect_uris": ["https://my-app.com/atproto-oauth-callback"],
"scope": "profile email offline_access",
"scope": "atproto",
"grant_types": ["authorization_code", "refresh_token"],
"response_types": ["code"],
"application_type": "native",
Expand Down
Loading

0 comments on commit dee817b

Please sign in to comment.