From dee817b6e0fc02351d51ce310b5e65239b7c5ed7 Mon Sep 17 00:00:00 2001
From: Matthieu Sieben <matthieusieben@users.noreply.github.com>
Date: Tue, 27 Aug 2024 19:43:29 +0200
Subject: [PATCH] OAuth: Add authorization scopes & remove OpenID compatibility
(#2734)
* 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 <devinivy@gmail.com>
* Properly parse scopes upon verification
Co-authored-by: devin ivy <devinivy@gmail.com>
* 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 15c6b9e2197064eb5de61a96de6497060edb824e.
* Revert "Verify service JWT header values. Add iat claim to service JWT"
This reverts commit 08df8df322a3f4b631c4a63a61d55b2c84c60c11.
* Revert "Add "jwtAlg" option to verifySignature() function"
This reverts commit d0f77354e6904678e7f5d76bb026f07537443ba9.
* Revert "Add, and verify, a "typ" header to access and refresh tokens"
This reverts commit 3e21be9e4b5875caa5e862c11f2196786fb2366d.
* 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 <devinivy@gmail.com>
---
.changeset/cool-toes-rescue.md | 5 +
.changeset/fluffy-apples-do.md | 5 +
.changeset/green-bags-flash.md | 5 +
.changeset/healthy-bottles-hear.md | 5 +
.changeset/lemon-mice-rule.md | 6 +
.changeset/light-dingos-dream.md | 5 +
.changeset/ninety-ants-collect.md | 5 +
.changeset/odd-spies-boil.md | 5 +
.changeset/polite-humans-sleep.md | 6 +
.changeset/poor-socks-sniff.md | 5 +
.changeset/short-toes-battle.md | 12 +
.changeset/six-ties-arrive.md | 5 +
.changeset/smart-drinks-repeat.md | 5 +
.changeset/smart-gifts-itch.md | 5 +
.changeset/thin-cycles-live.md | 5 +
.changeset/tidy-cars-thank.md | 5 +
packages/api/OAUTH.md | 5 +-
packages/oauth/oauth-client-browser/README.md | 2 +-
.../oauth-client-browser/example/src/app.tsx | 23 ++
.../example/src/auth/auth-form.tsx | 26 +-
.../example/src/auth/auth-provider.tsx | 18 +-
.../credential-sign-in-form.tsx} | 2 +-
.../use-credential-auth.ts} | 2 +-
.../example/src/auth/oauth/use-oauth.ts | 19 +-
.../oauth-client-browser/example/src/main.tsx | 9 +-
.../src/browser-oauth-database.ts | 1 -
packages/oauth/oauth-client-node/README.md | 3 +-
packages/oauth/oauth-client/README.md | 1 -
.../oauth/oauth-client/src/oauth-client.ts | 32 +-
.../oauth-client/src/oauth-server-agent.ts | 26 +-
.../oauth/oauth-client/src/oauth-session.ts | 2 +-
packages/oauth/oauth-client/src/runtime.ts | 96 +----
.../oauth/oauth-client/src/state-store.ts | 1 -
packages/oauth/oauth-client/src/types.ts | 4 +-
packages/oauth/oauth-provider/package.json | 1 -
.../oauth-provider/src/account/account.ts | 18 +-
.../src/assets/app/backend-data.ts | 11 +-
.../src/assets/app/components/accept-form.tsx | 116 +++---
.../app/components/client-identifier.tsx | 31 --
.../src/assets/app/components/client-name.tsx | 40 ++-
.../src/assets/app/components/url-viewer.tsx | 6 +-
.../src/assets/app/views/accept-view.tsx | 11 +-
.../src/assets/app/views/authorize-view.tsx | 3 +-
.../src/assets/assets-middleware.ts | 16 +-
.../src/client/client-manager.ts | 138 +++----
.../oauth/oauth-provider/src/client/client.ts | 5 +-
.../oauth/oauth-provider/src/constants.ts | 3 +
.../src/device/device-manager.ts | 8 +-
.../invalid-authorization-details-error.ts | 13 +-
.../oauth-provider/src/lib/http/request.ts | 76 +++-
.../src/metadata/build-metadata.ts | 51 +--
.../oauth/oauth-provider/src/oauth-hooks.ts | 16 +-
.../oauth-provider/src/oauth-provider.ts | 340 ++++++++++--------
.../oauth-provider/src/oauth-verifier.ts | 3 +-
.../oauth/oauth-provider/src/oidc/claims.ts | 35 --
.../oauth/oauth-provider/src/oidc/userinfo.ts | 11 -
.../src/output/build-authorize-data.ts | 8 +
.../src/parameters/claims-requested.ts | 106 ------
.../src/parameters/oidc-payload.ts | 28 --
.../src/replay/replay-manager.ts | 9 +
.../oauth-provider/src/replay/replay-store.ts | 2 +-
.../src/request/request-info.ts | 2 +
.../src/request/request-manager.ts | 188 +++++-----
.../oauth/oauth-provider/src/signer/signer.ts | 63 ----
.../oauth-provider/src/token/token-manager.ts | 49 +--
.../src/atproto-loopback-client-metadata.ts | 5 +-
...oauth-authentication-request-parameters.ts | 22 +-
.../oauth-types/src/oauth-response-type.ts | 4 +-
.../oauth-types/src/oauth-token-response.ts | 1 -
.../src/api/com/atproto/admin/sendEmail.ts | 10 +-
.../com/atproto/admin/updateAccountEmail.ts | 2 +-
.../atproto/admin/updateAccountPassword.ts | 2 +-
.../identity/requestPlcOperationSignature.ts | 18 +-
.../com/atproto/identity/signPlcOperation.ts | 21 +-
.../api/com/atproto/identity/updateHandle.ts | 13 +-
.../api/com/atproto/server/activateAccount.ts | 19 +-
.../api/com/atproto/server/confirmEmail.ts | 18 +-
.../com/atproto/server/createAppPassword.ts | 15 +-
.../com/atproto/server/deactivateAccount.ts | 15 +-
.../atproto/server/getAccountInviteCodes.ts | 21 +-
.../api/com/atproto/server/getServiceAuth.ts | 23 +-
.../com/atproto/server/listAppPasswords.ts | 14 +-
.../atproto/server/requestAccountDelete.ts | 16 +-
.../server/requestEmailConfirmation.ts | 16 +-
.../com/atproto/server/requestEmailUpdate.ts | 17 +-
.../com/atproto/server/revokeAppPassword.ts | 13 +-
.../src/api/com/atproto/server/updateEmail.ts | 20 +-
packages/pds/src/api/proxy.ts | 6 +-
packages/pds/src/auth-routes.ts | 4 +-
packages/pds/src/auth-verifier.ts | 46 ++-
packages/pds/src/index.ts | 13 +-
packages/pds/src/oauth/provider.ts | 2 +
packages/pds/src/pipethrough.ts | 26 +-
packages/pds/tests/app-passwords.test.ts | 4 +-
packages/pds/tests/entryway.test.ts | 34 +-
pnpm-lock.yaml | 8 -
96 files changed, 1097 insertions(+), 1119 deletions(-)
create mode 100644 .changeset/cool-toes-rescue.md
create mode 100644 .changeset/fluffy-apples-do.md
create mode 100644 .changeset/green-bags-flash.md
create mode 100644 .changeset/healthy-bottles-hear.md
create mode 100644 .changeset/lemon-mice-rule.md
create mode 100644 .changeset/light-dingos-dream.md
create mode 100644 .changeset/ninety-ants-collect.md
create mode 100644 .changeset/odd-spies-boil.md
create mode 100644 .changeset/polite-humans-sleep.md
create mode 100644 .changeset/poor-socks-sniff.md
create mode 100644 .changeset/short-toes-battle.md
create mode 100644 .changeset/six-ties-arrive.md
create mode 100644 .changeset/smart-drinks-repeat.md
create mode 100644 .changeset/smart-gifts-itch.md
create mode 100644 .changeset/thin-cycles-live.md
create mode 100644 .changeset/tidy-cars-thank.md
rename packages/oauth/oauth-client-browser/example/src/auth/{atp/atp-sign-in-form.tsx => credential/credential-sign-in-form.tsx} (98%)
rename packages/oauth/oauth-client-browser/example/src/auth/{atp/use-atp-auth.ts => credential/use-credential-auth.ts} (98%)
delete mode 100644 packages/oauth/oauth-provider/src/assets/app/components/client-identifier.tsx
delete mode 100644 packages/oauth/oauth-provider/src/oidc/claims.ts
delete mode 100644 packages/oauth/oauth-provider/src/oidc/userinfo.ts
delete mode 100644 packages/oauth/oauth-provider/src/parameters/claims-requested.ts
delete mode 100644 packages/oauth/oauth-provider/src/parameters/oidc-payload.ts
diff --git a/.changeset/cool-toes-rescue.md b/.changeset/cool-toes-rescue.md
new file mode 100644
index 00000000..19ab0367
--- /dev/null
+++ b/.changeset/cool-toes-rescue.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-provider": patch
+---
+
+Display requested scopes during the auth flow
diff --git a/.changeset/fluffy-apples-do.md b/.changeset/fluffy-apples-do.md
new file mode 100644
index 00000000..22747fe3
--- /dev/null
+++ b/.changeset/fluffy-apples-do.md
@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Use locally defined authPassthru
diff --git a/.changeset/green-bags-flash.md b/.changeset/green-bags-flash.md
new file mode 100644
index 00000000..2bbc6286
--- /dev/null
+++ b/.changeset/green-bags-flash.md
@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Add support for "transition:generic" and "transition:chat.bsky" oauth scopes
diff --git a/.changeset/healthy-bottles-hear.md b/.changeset/healthy-bottles-hear.md
new file mode 100644
index 00000000..d210a116
--- /dev/null
+++ b/.changeset/healthy-bottles-hear.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-provider": patch
+---
+
+Generate proper invalid_authorization_details
diff --git a/.changeset/lemon-mice-rule.md b/.changeset/lemon-mice-rule.md
new file mode 100644
index 00000000..9bff8aa1
--- /dev/null
+++ b/.changeset/lemon-mice-rule.md
@@ -0,0 +1,6 @@
+---
+"@atproto/oauth-provider": minor
+"@atproto/oauth-client": minor
+---
+
+Remove "nonce" from authorization request
diff --git a/.changeset/light-dingos-dream.md b/.changeset/light-dingos-dream.md
new file mode 100644
index 00000000..a25c1025
--- /dev/null
+++ b/.changeset/light-dingos-dream.md
@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Ignore case when checking for dpop auth scheme
diff --git a/.changeset/ninety-ants-collect.md b/.changeset/ninety-ants-collect.md
new file mode 100644
index 00000000..8abe5d74
--- /dev/null
+++ b/.changeset/ninety-ants-collect.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-provider": patch
+---
+
+Stronger CORS protections
diff --git a/.changeset/odd-spies-boil.md b/.changeset/odd-spies-boil.md
new file mode 100644
index 00000000..5a01cf7f
--- /dev/null
+++ b/.changeset/odd-spies-boil.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-types": patch
+---
+
+Validate scopes characters according to OAuth 2.1 spec
diff --git a/.changeset/polite-humans-sleep.md b/.changeset/polite-humans-sleep.md
new file mode 100644
index 00000000..2879f0f6
--- /dev/null
+++ b/.changeset/polite-humans-sleep.md
@@ -0,0 +1,6 @@
+---
+"@atproto/oauth-provider": minor
+"@atproto/oauth-client": minor
+---
+
+Mandate the use of "atproto" scope
diff --git a/.changeset/poor-socks-sniff.md b/.changeset/poor-socks-sniff.md
new file mode 100644
index 00000000..a3332467
--- /dev/null
+++ b/.changeset/poor-socks-sniff.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-provider": patch
+---
+
+Do not require user consent during oauth flow for first party apps.
diff --git a/.changeset/short-toes-battle.md b/.changeset/short-toes-battle.md
new file mode 100644
index 00000000..cb5f9265
--- /dev/null
+++ b/.changeset/short-toes-battle.md
@@ -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.
diff --git a/.changeset/six-ties-arrive.md b/.changeset/six-ties-arrive.md
new file mode 100644
index 00000000..a930161f
--- /dev/null
+++ b/.changeset/six-ties-arrive.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-types": patch
+---
+
+Re-use code definition of oauthResponseTypeSchema
diff --git a/.changeset/smart-drinks-repeat.md b/.changeset/smart-drinks-repeat.md
new file mode 100644
index 00000000..37d86c1d
--- /dev/null
+++ b/.changeset/smart-drinks-repeat.md
@@ -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.
diff --git a/.changeset/smart-gifts-itch.md b/.changeset/smart-gifts-itch.md
new file mode 100644
index 00000000..08ea7f32
--- /dev/null
+++ b/.changeset/smart-gifts-itch.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-types": patch
+---
+
+Remove non-standard "sub" from OAuthTokenResponse
diff --git a/.changeset/thin-cycles-live.md b/.changeset/thin-cycles-live.md
new file mode 100644
index 00000000..8650de71
--- /dev/null
+++ b/.changeset/thin-cycles-live.md
@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Allow OAuthProvider to define its own CORS policies
diff --git a/.changeset/tidy-cars-thank.md b/.changeset/tidy-cars-thank.md
new file mode 100644
index 00000000..f0ecef96
--- /dev/null
+++ b/.changeset/tidy-cars-thank.md
@@ -0,0 +1,5 @@
+---
+"@atproto/oauth-provider": patch
+---
+
+Improve reporting of validation errors
diff --git a/packages/api/OAUTH.md b/packages/api/OAUTH.md
index 0f6bf4f9..e60c59f9 100644
--- a/packages/api/OAUTH.md
+++ b/packages/api/OAUTH.md
@@ -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",
@@ -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]
>
diff --git a/packages/oauth/oauth-client-browser/README.md b/packages/oauth/oauth-client-browser/README.md
index 4f3e16fb..ce10c240 100644
--- a/packages/oauth/oauth-client-browser/README.md
+++ b/packages/oauth/oauth-client-browser/README.md
@@ -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",
diff --git a/packages/oauth/oauth-client-browser/example/src/app.tsx b/packages/oauth/oauth-client-browser/example/src/app.tsx
index 3625d2f6..233c9f21 100644
--- a/packages/oauth/oauth-client-browser/example/src/app.tsx
+++ b/packages/oauth/oauth-client-browser/example/src/app.tsx
@@ -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 () => {
@@ -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>
diff --git a/packages/oauth/oauth-client-browser/example/src/auth/auth-form.tsx b/packages/oauth/oauth-client-browser/example/src/auth/auth-form.tsx
index 336ef420..acec2871 100644
--- a/packages/oauth/oauth-client-browser/example/src/auth/auth-form.tsx
+++ b/packages/oauth/oauth-client-browser/example/src/auth/auth-form.tsx
@@ -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({
@@ -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)
@@ -45,9 +49,9 @@ 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
@@ -55,7 +59,7 @@ export function AuthForm({
</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>
)
diff --git a/packages/oauth/oauth-client-browser/example/src/auth/auth-provider.tsx b/packages/oauth/oauth-client-browser/example/src/auth/auth-provider.tsx
index f083caba..5bb2e94e 100644
--- a/packages/oauth/oauth-client-browser/example/src/auth/auth-provider.tsx
+++ b/packages/oauth/oauth-client-browser/example/src/auth/auth-provider.tsx
@@ -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'
@@ -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) {
@@ -56,7 +56,7 @@ export const AuthProvider = ({
if (!value) {
return (
<AuthForm
- atpSignIn={atpSignIn}
+ atpSignIn={credentialSignIn}
oauthSignIn={oauthClient ? oauthSignIn : undefined}
/>
)
diff --git a/packages/oauth/oauth-client-browser/example/src/auth/atp/atp-sign-in-form.tsx b/packages/oauth/oauth-client-browser/example/src/auth/credential/credential-sign-in-form.tsx
similarity index 98%
rename from packages/oauth/oauth-client-browser/example/src/auth/atp/atp-sign-in-form.tsx
rename to packages/oauth/oauth-client-browser/example/src/auth/credential/credential-sign-in-form.tsx
index 00387181..40a651cb 100644
--- a/packages/oauth/oauth-client-browser/example/src/auth/atp/atp-sign-in-form.tsx
+++ b/packages/oauth/oauth-client-browser/example/src/auth/credential/credential-sign-in-form.tsx
@@ -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
}: {
diff --git a/packages/oauth/oauth-client-browser/example/src/auth/atp/use-atp-auth.ts b/packages/oauth/oauth-client-browser/example/src/auth/credential/use-credential-auth.ts
similarity index 98%
rename from packages/oauth/oauth-client-browser/example/src/auth/atp/use-atp-auth.ts
rename to packages/oauth/oauth-client-browser/example/src/auth/credential/use-credential-auth.ts
index 431b81db..1dd360b7 100644
--- a/packages/oauth/oauth-client-browser/example/src/auth/atp/use-atp-auth.ts
+++ b/packages/oauth/oauth-client-browser/example/src/auth/credential/use-credential-auth.ts
@@ -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,
diff --git a/packages/oauth/oauth-client-browser/example/src/auth/oauth/use-oauth.ts b/packages/oauth/oauth-client-browser/example/src/auth/oauth/use-oauth.ts
index 7256a8c4..057896a2 100644
--- a/packages/oauth/oauth-client-browser/example/src/auth/oauth/use-oauth.ts
+++ b/packages/oauth/oauth-client-browser/example/src/auth/oauth/use-oauth.ts
@@ -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,
@@ -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)
@@ -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()
}
@@ -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
diff --git a/packages/oauth/oauth-client-browser/example/src/main.tsx b/packages/oauth/oauth-client-browser/example/src/main.tsx
index aa85dcc1..ad148862 100644
--- a/packages/oauth/oauth-client-browser/example/src/main.tsx
+++ b/packages/oauth/oauth-client-browser/example/src/main.tsx
@@ -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>
diff --git a/packages/oauth/oauth-client-browser/src/browser-oauth-database.ts b/packages/oauth/oauth-client-browser/src/browser-oauth-database.ts
index 18ce8b58..b641649e 100644
--- a/packages/oauth/oauth-client-browser/src/browser-oauth-database.ts
+++ b/packages/oauth/oauth-client-browser/src/browser-oauth-database.ts
@@ -41,7 +41,6 @@ export type Schema = {
dpopKey: EncodedKey
iss: string
- nonce: string
verifier?: string
appState?: string
}>
diff --git a/packages/oauth/oauth-client-node/README.md b/packages/oauth/oauth-client-node/README.md
index 9a90afb6..1499d5b5 100644
--- a/packages/oauth/oauth-client-node/README.md
+++ b/packages/oauth/oauth-client-node/README.md
@@ -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',
@@ -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",
diff --git a/packages/oauth/oauth-client/README.md b/packages/oauth/oauth-client/README.md
index 7e9b9738..1057127e 100644
--- a/packages/oauth/oauth-client/README.md
+++ b/packages/oauth/oauth-client/README.md
@@ -341,7 +341,6 @@ or
```ts
const url = await client.authorize(handle, {
state,
- max_age: 600, // Require re-authentication after 10 minutes
})
```
diff --git a/packages/oauth/oauth-client/src/oauth-client.ts b/packages/oauth/oauth-client/src/oauth-client.ts
index 9187911f..b113fdb9 100644
--- a/packages/oauth/oauth-client/src/oauth-client.ts
+++ b/packages/oauth/oauth-client/src/oauth-client.ts
@@ -261,7 +261,6 @@ export class OAuthClient extends CustomEventTarget<OAuthClientEventMap> {
options,
)
- const nonce = await this.runtime.generateNonce()
const pkce = await this.runtime.generatePKCE()
const dpopKey = await this.runtime.generateKey(
metadata.dpop_signing_alg_values_supported || [FALLBACK_ALG],
@@ -272,17 +271,15 @@ export class OAuthClient extends CustomEventTarget<OAuthClientEventMap> {
await this.stateStore.set(state, {
iss: metadata.issuer,
dpopKey,
- nonce,
- verifier: pkce?.verifier,
+ verifier: pkce.verifier,
appState: options?.state,
})
const parameters = {
client_id: this.clientMetadata.client_id,
redirect_uri: redirectUri,
- code_challenge: pkce?.challenge,
- code_challenge_method: pkce?.method,
- nonce,
+ code_challenge: pkce.challenge,
+ code_challenge_method: pkce.method,
state,
login_hint: identity
? input // If input is a handle or a DID, use it as a login_hint
@@ -295,13 +292,8 @@ export class OAuthClient extends CustomEventTarget<OAuthClientEventMap> {
) ?? 'code',
display: options?.display,
- id_token_hint: options?.id_token_hint,
- max_age: options?.max_age, // this.clientMetadata.default_max_age
prompt: options?.prompt,
- scope: options?.scope
- ?.split(' ')
- .filter((s) => metadata.scopes_supported?.includes(s))
- .join(' '),
+ scope: options?.scope || undefined,
ui_locales: options?.ui_locales,
}
@@ -434,24 +426,12 @@ export class OAuthClient extends CustomEventTarget<OAuthClientEventMap> {
const tokenSet = await server.exchangeCode(codeParam, stateData.verifier)
try {
- if (tokenSet.id_token) {
- await this.runtime.validateIdTokenClaims(
- tokenSet.id_token,
- stateParam,
- stateData.nonce,
- codeParam,
- tokenSet.access_token,
- )
- }
-
- const { sub } = tokenSet
-
- await this.sessionGetter.setStored(sub, {
+ await this.sessionGetter.setStored(tokenSet.sub, {
dpopKey: stateData.dpopKey,
tokenSet,
})
- const session = this.createSession(server, sub)
+ const session = this.createSession(server, tokenSet.sub)
return { session, state: stateData.appState ?? null }
} catch (err) {
diff --git a/packages/oauth/oauth-client/src/oauth-server-agent.ts b/packages/oauth/oauth-client/src/oauth-server-agent.ts
index 07231dac..636bb749 100644
--- a/packages/oauth/oauth-client/src/oauth-server-agent.ts
+++ b/packages/oauth/oauth-client/src/oauth-server-agent.ts
@@ -1,6 +1,6 @@
import { Fetch, Json, bindFetch, fetchJsonProcessor } from '@atproto-labs/fetch'
import { SimpleStore } from '@atproto-labs/simple-store'
-import { Key, Keyset, SignedJwt } from '@atproto/jwk'
+import { Key, Keyset } from '@atproto/jwk'
import {
CLIENT_ASSERTION_TYPE_JWT_BEARER,
OAuthAuthorizationServerMetadata,
@@ -26,9 +26,8 @@ export type TokenSet = {
iss: string
sub: string
aud: string
- scope?: string
+ scope: string
- id_token?: SignedJwt
refresh_token?: string
access_token: string
token_type: OAuthTokenType
@@ -128,8 +127,17 @@ export class OAuthServerAgent {
tokenResponse: OAuthTokenResponse,
): Promise<TokenSet> {
const { sub } = tokenResponse
- // ATPROTO requires that the "sub" is always present in the token response.
- if (!sub) throw new TypeError(`Missing "sub" in token response`)
+
+ if (!sub || typeof sub !== 'string') {
+ throw new TypeError(`Unexpected ${typeof sub} "sub" in token response`)
+ }
+
+ // Using an array to check for the presence of the "atproto" scope (we don't
+ // want atproto to be a substring of another scope)
+ const scopes = tokenResponse.scope?.split(' ')
+ if (!scopes?.includes('atproto')) {
+ throw new TypeError('Missing "atproto" scope in token response')
+ }
// @TODO (?) make timeout configurable
using signal = timeoutSignal(10e3)
@@ -138,7 +146,7 @@ export class OAuthServerAgent {
signal,
})
- if (resolved.metadata.issuer !== this.serverMetadata.issuer) {
+ if (this.serverMetadata.issuer !== resolved.metadata.issuer) {
// Best case scenario; the user switched PDS. Worst case scenario; a bad
// actor is trying to impersonate a user. In any case, we must not allow
// this token to be used.
@@ -146,12 +154,12 @@ export class OAuthServerAgent {
}
return {
- sub,
aud: resolved.identity.pds.href,
iss: resolved.metadata.issuer,
- scope: tokenResponse.scope,
- id_token: tokenResponse.id_token,
+ sub,
+
+ scope: tokenResponse.scope!,
refresh_token: tokenResponse.refresh_token,
access_token: tokenResponse.access_token,
token_type: tokenResponse.token_type ?? 'Bearer',
diff --git a/packages/oauth/oauth-client/src/oauth-session.ts b/packages/oauth/oauth-client/src/oauth-session.ts
index 94b14e8f..4c5f25f0 100644
--- a/packages/oauth/oauth-client/src/oauth-session.ts
+++ b/packages/oauth/oauth-client/src/oauth-session.ts
@@ -138,7 +138,7 @@ export class OAuthSession {
if (isInvalidTokenResponse(finalResponse)) {
// TODO: Is there a "softer" way to handle this, e.g. by marking the
// session as "expired" in the session store, allowing the user to trigger
- // a new login (using login_hint/id_token_hint)?
+ // a new login (using login_hint)?
await this.sessionGetter.delStored(
this.sub,
new TokenInvalidError(this.sub),
diff --git a/packages/oauth/oauth-client/src/runtime.ts b/packages/oauth/oauth-client/src/runtime.ts
index 6de49d01..88ac002b 100644
--- a/packages/oauth/oauth-client/src/runtime.ts
+++ b/packages/oauth/oauth-client/src/runtime.ts
@@ -1,12 +1,8 @@
-import { JwtHeader, JwtPayload, Key, unsafeDecodeJwt } from '@atproto/jwk'
+import { Key } from '@atproto/jwk'
import { base64url } from 'multiformats/bases/base64'
import { requestLocalLock } from './lock.js'
-import {
- DigestAlgorithm,
- RuntimeImplementation,
- RuntimeLock,
-} from './runtime-implementation.js'
+import { RuntimeImplementation, RuntimeLock } from './runtime-implementation.js'
export class Runtime {
readonly hasImplementationLock: boolean
@@ -38,64 +34,6 @@ export class Runtime {
return base64url.baseEncode(bytes)
}
- public async validateIdTokenClaims(
- token: string,
- state: string,
- nonce: string,
- code?: string,
- accessToken?: string,
- ): Promise<{
- header: JwtHeader
- payload: JwtPayload
- }> {
- // It's fine to use unsafeDecodeJwt here because the token was received from
- // the server's token endpoint. The following checks are to ensure that the
- // oauth flow was indeed initiated by the client.
- const { header, payload } = unsafeDecodeJwt(token)
- if (!payload.nonce || payload.nonce !== nonce) {
- throw new TypeError('Nonce mismatch')
- }
- if (payload.c_hash) {
- await this.validateHashClaim(payload.c_hash, code, header)
- }
- if (payload.s_hash) {
- await this.validateHashClaim(payload.s_hash, state, header)
- }
- if (payload.at_hash) {
- await this.validateHashClaim(payload.at_hash, accessToken, header)
- }
- return { header, payload }
- }
-
- private async validateHashClaim(
- claim: unknown,
- source: unknown,
- header: { alg: string; crv?: string },
- ): Promise<void> {
- if (typeof claim !== 'string' || !claim) {
- throw new TypeError(`string "_hash" claim expected`)
- }
- if (typeof source !== 'string' || !source) {
- throw new TypeError(`string value expected`)
- }
- const expected = await this.generateHashClaim(source, header)
- if (expected !== claim) {
- throw new TypeError(`"_hash" does not match`)
- }
- }
-
- protected async generateHashClaim(
- source: string,
- header: { alg: string; crv?: string },
- ) {
- const algo = getHashAlgo(header)
- const bytes = new TextEncoder().encode(source)
- const digest = await this.implementation.digest(bytes, algo)
- if (digest.length % 2 !== 0) throw new TypeError('Invalid digest length')
- const digestHalf = digest.slice(0, digest.length / 2)
- return base64url.baseEncode(digestHalf)
- }
-
public async generatePKCE(byteLength?: number) {
const verifier = await this.generateVerifier(byteLength)
return {
@@ -127,36 +65,6 @@ export class Runtime {
}
}
-function getHashAlgo(header: { alg: string; crv?: string }): DigestAlgorithm {
- switch (header.alg) {
- case 'HS256':
- case 'RS256':
- case 'PS256':
- case 'ES256':
- case 'ES256K':
- return { name: 'sha256' }
- case 'HS384':
- case 'RS384':
- case 'PS384':
- case 'ES384':
- return { name: 'sha384' }
- case 'HS512':
- case 'RS512':
- case 'PS512':
- case 'ES512':
- return { name: 'sha512' }
- case 'EdDSA':
- switch (header.crv) {
- case 'Ed25519':
- return { name: 'sha512' }
- default:
- throw new TypeError('unrecognized or invalid EdDSA curve provided')
- }
- default:
- throw new TypeError('unrecognized or invalid JWS algorithm provided')
- }
-}
-
function extractJktComponents(jwk) {
const get = (field) => {
const value = jwk[field]
diff --git a/packages/oauth/oauth-client/src/state-store.ts b/packages/oauth/oauth-client/src/state-store.ts
index 928148f0..f49c36df 100644
--- a/packages/oauth/oauth-client/src/state-store.ts
+++ b/packages/oauth/oauth-client/src/state-store.ts
@@ -3,7 +3,6 @@ import { Key } from '@atproto/jwk'
export type InternalStateData = {
iss: string
- nonce: string
dpopKey: Key
verifier?: string
appState?: string
diff --git a/packages/oauth/oauth-client/src/types.ts b/packages/oauth/oauth-client/src/types.ts
index ed81aeb4..74161056 100644
--- a/packages/oauth/oauth-client/src/types.ts
+++ b/packages/oauth/oauth-client/src/types.ts
@@ -16,10 +16,8 @@ export type AuthorizeOptions = {
state?: string
signal?: AbortSignal
- // Only for OIDC compatible
+ // Borrowed from OIDC
ui_locales?: string
- id_token_hint?: string
- max_age?: number
}
export const clientMetadataSchema = oauthClientMetadataSchema.extend({
diff --git a/packages/oauth/oauth-provider/package.json b/packages/oauth/oauth-provider/package.json
index 242ba0df..f272e12a 100644
--- a/packages/oauth/oauth-provider/package.json
+++ b/packages/oauth/oauth-provider/package.json
@@ -44,7 +44,6 @@
"ioredis": "^5.3.2",
"jose": "^5.2.0",
"keygrip": "^1.1.0",
- "oidc-token-hash": "^5.0.3",
"psl": "^1.9.0",
"zod": "^3.23.8"
},
diff --git a/packages/oauth/oauth-provider/src/account/account.ts b/packages/oauth/oauth-provider/src/account/account.ts
index 07282323..7a850f79 100644
--- a/packages/oauth/oauth-provider/src/account/account.ts
+++ b/packages/oauth/oauth-provider/src/account/account.ts
@@ -1,10 +1,14 @@
-import { OIDCStandardPayload } from '../oidc/claims.js'
import { Sub } from '../oidc/sub.js'
import { Simplify } from '../lib/util/type.js'
-export type Account = Simplify<
- {
- sub: Sub // Account id
- aud: string | [string, ...string[]] // Resource server URL
- } & OIDCStandardPayload
->
+export type Account = Simplify<{
+ sub: Sub // Account id
+ aud: string | [string, ...string[]] // Resource server URL
+
+ // OIDC inspired
+ preferred_username?: string
+ email?: string
+ email_verified?: boolean
+ picture?: string
+ name?: string
+}>
diff --git a/packages/oauth/oauth-provider/src/assets/app/backend-data.ts b/packages/oauth/oauth-provider/src/assets/app/backend-data.ts
index 5bf4c4da..ae9f4874 100644
--- a/packages/oauth/oauth-provider/src/assets/app/backend-data.ts
+++ b/packages/oauth/oauth-provider/src/assets/app/backend-data.ts
@@ -14,6 +14,7 @@ export type Account = {
export type Session = {
account: Account
+ info?: never // Prevent relying on this in the frontend
selected: boolean
loginRequired: boolean
@@ -37,15 +38,21 @@ export type ErrorData = {
error_description: string
}
+export type ScopeDetail = {
+ scope: string
+ description?: string
+}
+
export type AuthorizeData = {
clientId: string
clientMetadata: OAuthClientMetadata
clientTrusted: boolean
requestUri: string
csrfCookie: string
- sessions: Session[]
- newSessionsRequireConsent: boolean
loginHint?: string
+ scopeDetails?: ScopeDetail[]
+ newSessionsRequireConsent: boolean
+ sessions: Session[]
}
// see "declareBackendData()" in the backend
diff --git a/packages/oauth/oauth-provider/src/assets/app/components/accept-form.tsx b/packages/oauth/oauth-provider/src/assets/app/components/accept-form.tsx
index d0f57af7..20449397 100644
--- a/packages/oauth/oauth-provider/src/assets/app/components/accept-form.tsx
+++ b/packages/oauth/oauth-provider/src/assets/app/components/accept-form.tsx
@@ -1,22 +1,23 @@
import { OAuthClientMetadata } from '@atproto/oauth-types'
import { FormEvent } from 'react'
-import { Account } from '../backend-data'
+import { Account, ScopeDetail } from '../backend-data'
import { Override } from '../lib/util'
import { AccountIdentifier } from './account-identifier'
import { Button } from './button'
-import { ClientIdentifier } from './client-identifier'
import { ClientName } from './client-name'
import { FormCard, FormCardProps } from './form-card'
-import { Fieldset } from './fieldset'
export type AcceptFormProps = Override<
FormCardProps,
{
- account: Account
clientId: string
clientMetadata: OAuthClientMetadata
clientTrusted: boolean
+
+ account: Account
+ scopeDetails?: ScopeDetail[]
+
onAccept: () => void
acceptLabel?: string
@@ -29,10 +30,13 @@ export type AcceptFormProps = Override<
>
export function AcceptForm({
- account,
clientId,
clientMetadata,
clientTrusted,
+
+ account,
+ scopeDetails,
+
onAccept,
acceptLabel = 'Accept',
onReject,
@@ -62,54 +66,64 @@ export function AcceptForm({
}
{...props}
>
- <Fieldset
- title={
- <ClientName clientId={clientId} clientMetadata={clientMetadata} />
- }
- >
- {clientTrusted && clientMetadata.logo_uri && (
- <div key="logo" className="flex items-center justify-center">
- <img
- crossOrigin="anonymous"
- src={clientMetadata.logo_uri}
- alt={clientMetadata.client_name}
- className="w-16 h-16 rounded-full"
- />
- </div>
- )}
+ {clientTrusted && clientMetadata.logo_uri && (
+ <div key="logo" className="flex items-center justify-center">
+ <img
+ crossOrigin="anonymous"
+ src={clientMetadata.logo_uri}
+ alt={clientMetadata.client_name}
+ className="w-16 h-16 rounded-full"
+ />
+ </div>
+ )}
+ <p>
+ <ClientName clientId={clientId} clientMetadata={clientMetadata} /> is
+ asking for permission to access your account (
+ <AccountIdentifier account={account} />
+ ).
+ </p>
- <p>
- <ClientIdentifier
- clientId={clientId}
- clientMetadata={clientMetadata}
- />{' '}
- is asking for permission to access your{' '}
- <AccountIdentifier account={account} /> account.
- </p>
+ <p>
+ By clicking <b>{acceptLabel}</b>, you allow this application to perform
+ the following actions in accordance to their{' '}
+ <a
+ href={clientMetadata.tos_uri}
+ rel="nofollow noopener"
+ target="_blank"
+ className="text-brand underline"
+ >
+ terms of service
+ </a>
+ {' and '}
+ <a
+ href={clientMetadata.policy_uri}
+ rel="nofollow noopener"
+ target="_blank"
+ className="text-brand underline"
+ >
+ privacy policy
+ </a>
+ :
+ </p>
- <p>
- By clicking <b>{acceptLabel}</b>, you allow this application to access
- your information in accordance to their{' '}
- <a
- href={clientMetadata.tos_uri}
- rel="nofollow noopener"
- target="_blank"
- className="text-brand underline"
- >
- terms of service
- </a>
- {' and '}
- <a
- href={clientMetadata.policy_uri}
- rel="nofollow noopener"
- target="_blank"
- className="text-brand underline"
- >
- privacy policy
- </a>
- .
- </p>
- </Fieldset>
+ {scopeDetails?.length ? (
+ <ul className="list-disc list-inside">
+ {scopeDetails.map(
+ ({ scope, description = getScopeDescription(scope) }) => (
+ <li key={scope}>{description}</li>
+ ),
+ )}
+ </ul>
+ ) : null}
</FormCard>
)
}
+
+function getScopeDescription(scope: string): string {
+ switch (scope) {
+ case 'atproto':
+ return 'Uniquely identify you'
+ default:
+ return scope
+ }
+}
diff --git a/packages/oauth/oauth-provider/src/assets/app/components/client-identifier.tsx b/packages/oauth/oauth-provider/src/assets/app/components/client-identifier.tsx
deleted file mode 100644
index ed8add61..00000000
--- a/packages/oauth/oauth-provider/src/assets/app/components/client-identifier.tsx
+++ /dev/null
@@ -1,31 +0,0 @@
-import {
- isOAuthClientIdDiscoverable,
- isOAuthClientIdLoopback,
- OAuthClientMetadata,
-} from '@atproto/oauth-types'
-import { HTMLAttributes } from 'react'
-
-import { UrlViewer } from './url-viewer'
-
-export type ClientIdentifierProps = {
- clientId: string
- clientMetadata: OAuthClientMetadata
- as?: keyof JSX.IntrinsicElements
-}
-
-export function ClientIdentifier({
- clientId,
- clientMetadata,
- as: As = 'span',
- ...attrs
-}: ClientIdentifierProps & HTMLAttributes<Element>) {
- if (isOAuthClientIdLoopback(clientId)) {
- return <As {...attrs}>An application on your device</As>
- }
-
- if (isOAuthClientIdDiscoverable(clientId)) {
- return <UrlViewer as={As} {...attrs} url={clientId} proto path />
- }
-
- return <As {...attrs}>{clientMetadata.client_name || clientId}</As>
-}
diff --git a/packages/oauth/oauth-provider/src/assets/app/components/client-name.tsx b/packages/oauth/oauth-provider/src/assets/app/components/client-name.tsx
index abafc87d..e47575df 100644
--- a/packages/oauth/oauth-provider/src/assets/app/components/client-name.tsx
+++ b/packages/oauth/oauth-provider/src/assets/app/components/client-name.tsx
@@ -1,30 +1,38 @@
-import { OAuthClientMetadata } from '@atproto/oauth-types'
+import {
+ isOAuthClientIdDiscoverable,
+ isOAuthClientIdLoopback,
+ OAuthClientMetadata,
+} from '@atproto/oauth-types'
import { HTMLAttributes } from 'react'
-import { ClientIdentifier } from './client-identifier'
+import { UrlViewer } from './url-viewer'
export type ClientNameProps = {
clientId: string
clientMetadata: OAuthClientMetadata
- as?: keyof JSX.IntrinsicElements
-}
+} & HTMLAttributes<Element>
export function ClientName({
clientId,
clientMetadata,
- as: As = 'span',
...attrs
-}: ClientNameProps & HTMLAttributes<Element>) {
- if (clientMetadata.client_name) {
- return <As {...attrs}>{clientMetadata.client_name}</As>
+}: ClientNameProps) {
+ if (isOAuthClientIdLoopback(clientId)) {
+ return <span {...attrs}>An application on your device</span>
}
- return (
- <ClientIdentifier
- clientId={clientId}
- clientMetadata={clientMetadata}
- as={As}
- {...attrs}
- />
- )
+ if (isOAuthClientIdDiscoverable(clientId)) {
+ if (clientMetadata.client_name) {
+ return (
+ <span {...attrs}>
+ {clientMetadata.client_name} (
+ <UrlViewer url={clientId} path />)
+ </span>
+ )
+ }
+
+ return <UrlViewer {...attrs} url={clientId} path />
+ }
+
+ return <span {...attrs}>{clientMetadata.client_name || clientId}</span>
}
diff --git a/packages/oauth/oauth-provider/src/assets/app/components/url-viewer.tsx b/packages/oauth/oauth-provider/src/assets/app/components/url-viewer.tsx
index 4879a5d8..fca5f848 100644
--- a/packages/oauth/oauth-provider/src/assets/app/components/url-viewer.tsx
+++ b/packages/oauth/oauth-provider/src/assets/app/components/url-viewer.tsx
@@ -1,4 +1,4 @@
-import { HTMLAttributes, useMemo } from 'react'
+import { Component, HTMLAttributes, useMemo } from 'react'
export type UrlPartRenderingOptions = {
faded?: boolean
@@ -28,7 +28,7 @@ export function UrlViewer({
const urlObj = useMemo(() => new URL(url), [url])
return (
- <As {...attrs}>
+ <Component as={As} {...attrs}>
{proto && (
<UrlPartViewer
value={`${urlObj.protocol}//`}
@@ -56,7 +56,7 @@ export function UrlViewer({
{hash && (
<UrlPartViewer value={urlObj.hash} {...(hash === true ? null : hash)} />
)}
- </As>
+ </Component>
)
}
diff --git a/packages/oauth/oauth-provider/src/assets/app/views/accept-view.tsx b/packages/oauth/oauth-provider/src/assets/app/views/accept-view.tsx
index 6601390f..daf99046 100644
--- a/packages/oauth/oauth-provider/src/assets/app/views/accept-view.tsx
+++ b/packages/oauth/oauth-provider/src/assets/app/views/accept-view.tsx
@@ -1,6 +1,6 @@
import { OAuthClientMetadata } from '@atproto/oauth-types'
-import { Session } from '../backend-data'
+import { Account, ScopeDetail } from '../backend-data'
import { AcceptForm } from '../components/accept-form'
import { LayoutTitlePage } from '../components/layout-title-page'
@@ -8,7 +8,9 @@ export type AcceptViewProps = {
clientId: string
clientMetadata: OAuthClientMetadata
clientTrusted: boolean
- session: Session
+
+ account: Account
+ scopeDetails?: ScopeDetail[]
onAccept: () => void
onReject: () => void
@@ -19,12 +21,12 @@ export function AcceptView({
clientId,
clientMetadata,
clientTrusted,
- session,
+ account,
+ scopeDetails,
onAccept,
onReject,
onBack,
}: AcceptViewProps) {
- const { account } = session
return (
<LayoutTitlePage
title="Authorize"
@@ -43,6 +45,7 @@ export function AcceptView({
clientMetadata={clientMetadata}
clientTrusted={clientTrusted}
account={account}
+ scopeDetails={scopeDetails}
onBack={onBack}
onAccept={onAccept}
onReject={onReject}
diff --git a/packages/oauth/oauth-provider/src/assets/app/views/authorize-view.tsx b/packages/oauth/oauth-provider/src/assets/app/views/authorize-view.tsx
index 4d523bf3..5955044a 100644
--- a/packages/oauth/oauth-provider/src/assets/app/views/authorize-view.tsx
+++ b/packages/oauth/oauth-provider/src/assets/app/views/authorize-view.tsx
@@ -79,10 +79,11 @@ export function AuthorizeView({
if (view === 'accept' && session) {
return (
<AcceptView
- session={session}
clientId={authorizeData.clientId}
clientMetadata={authorizeData.clientMetadata}
clientTrusted={authorizeData.clientTrusted}
+ account={session.account}
+ scopeDetails={authorizeData.scopeDetails}
onAccept={() => doAccept(session.account)}
onReject={doReject}
onBack={
diff --git a/packages/oauth/oauth-provider/src/assets/assets-middleware.ts b/packages/oauth/oauth-provider/src/assets/assets-middleware.ts
index 2471e4c9..f699f337 100644
--- a/packages/oauth/oauth-provider/src/assets/assets-middleware.ts
+++ b/packages/oauth/oauth-provider/src/assets/assets-middleware.ts
@@ -1,8 +1,13 @@
-import { writeStream } from '../lib/http/index.js'
+import {
+ Middleware,
+ validateFetchDest,
+ validateFetchSite,
+ writeStream,
+} from '../lib/http/index.js'
import { ASSETS_URL_PREFIX, getAsset } from './index.js'
-export function authorizeAssetsMiddleware() {
+export function authorizeAssetsMiddleware(): Middleware {
return async function assetsMiddleware(req, res, next): Promise<void> {
if (req.method !== 'GET' && req.method !== 'HEAD') return next()
if (!req.url?.startsWith(ASSETS_URL_PREFIX)) return next()
@@ -17,6 +22,13 @@ export function authorizeAssetsMiddleware() {
const asset = await getAsset(filename).catch(() => null)
if (!asset) return next()
+ try {
+ validateFetchSite(req, res, ['same-origin'])
+ validateFetchDest(req, res, ['style', 'script'])
+ } catch (err) {
+ return next(err)
+ }
+
if (req.headers['if-none-match'] === asset.sha256) {
return void res.writeHead(304).end()
}
diff --git a/packages/oauth/oauth-provider/src/client/client-manager.ts b/packages/oauth/oauth-provider/src/client/client-manager.ts
index e44f55b3..42ca7c32 100644
--- a/packages/oauth/oauth-provider/src/client/client-manager.ts
+++ b/packages/oauth/oauth-provider/src/client/client-manager.ts
@@ -17,6 +17,7 @@ import {
isLoopbackUrl,
isOAuthClientIdDiscoverable,
isOAuthClientIdLoopback,
+ OAuthAuthorizationServerMetadata,
OAuthClientIdDiscoverable,
OAuthClientIdLoopback,
OAuthClientMetadata,
@@ -55,9 +56,10 @@ export type LoopbackMetadataGetter = (
export class ClientManager {
protected readonly jwks: CachedGetter<string, Jwks>
- protected readonly metadata: CachedGetter<string, OAuthClientMetadata>
+ protected readonly metadataGetter: CachedGetter<string, OAuthClientMetadata>
constructor(
+ protected readonly serverMetadata: OAuthAuthorizationServerMetadata,
protected readonly keyset: Keyset,
protected readonly hooks: OAuthHooks,
protected readonly store: ClientStore | null,
@@ -76,7 +78,7 @@ export class ClientManager {
return jwks
}, clientJwksCache)
- this.metadata = new CachedGetter(async (uri, options) => {
+ this.metadataGetter = new CachedGetter(async (uri, options) => {
const metadata = await fetch(buildJsonGetRequest(uri, options)).then(
fetchMetadataHandler,
)
@@ -159,7 +161,7 @@ export class ClientManager {
): Promise<OAuthClientMetadata> {
const metadataUrl = parseDiscoverableClientId(clientId)
- const metadata = await this.metadata.get(metadataUrl.href)
+ const metadata = await this.metadataGetter.get(metadataUrl.href)
// Note: we do *not* re-validate the metadata here, as the metadata is
// validated within the getter. This is to avoid double validation.
@@ -195,6 +197,18 @@ export class ClientManager {
)
}
+ // Known OIDC specific parameters
+ for (const k of [
+ 'default_max_age',
+ 'userinfo_signed_response_alg',
+ 'id_token_signed_response_alg',
+ 'userinfo_encrypted_response_alg',
+ ] as const) {
+ if (metadata[k] != null) {
+ throw new InvalidClientMetadataError(`Unsupported "${k}" parameter`)
+ }
+ }
+
const clientUriUrl = metadata.client_uri
? new URL(metadata.client_uri)
: null
@@ -204,13 +218,27 @@ export class ClientManager {
throw new InvalidClientMetadataError('client_uri must be a valid URL')
}
- const scopes = metadata.scope?.split(' ')
- if (
- metadata.grant_types.includes('refresh_token') !==
- (scopes?.includes('offline_access') ?? false)
- ) {
+ const scopes = metadata.scope?.split(' ').filter(Boolean)
+
+ const dupScope = scopes?.find(isDuplicate)
+ if (dupScope) {
+ throw new InvalidClientMetadataError(`Duplicate scope "${dupScope}"`)
+ }
+
+ if (scopes) {
+ for (const scope of scopes) {
+ // Note, once we have dynamic scopes, this check will need to be
+ // updated to check against the server's supported scopes.
+ if (!this.serverMetadata.scopes_supported?.includes(scope)) {
+ throw new InvalidClientMetadataError(`Unsupported scope "${scope}"`)
+ }
+ }
+ }
+
+ const dupGrantType = metadata.grant_types.find(isDuplicate)
+ if (dupGrantType) {
throw new InvalidClientMetadataError(
- 'Grant type "refresh_token" requires scope "offline_access" (and vice versa)',
+ `Duplicate grant type "${dupGrantType}"`,
)
}
@@ -218,8 +246,8 @@ export class ClientManager {
switch (grantType) {
case 'authorization_code':
case 'refresh_token':
- case 'implicit': // Required by OIDC (for id_token)
continue
+ case 'implicit':
case 'password':
throw new InvalidClientMetadataError(
`Grant type "${grantType}" is not allowed`,
@@ -241,35 +269,6 @@ export class ClientManager {
)
}
- if (
- metadata.userinfo_signed_response_alg &&
- !this.keyset.signAlgorithms.includes(
- metadata.userinfo_signed_response_alg,
- )
- ) {
- throw new InvalidClientMetadataError(
- `Unsupported "userinfo_signed_response_alg" ${metadata.userinfo_signed_response_alg}`,
- )
- }
-
- if (
- metadata.id_token_signed_response_alg &&
- !this.keyset.signAlgorithms.includes(
- metadata.id_token_signed_response_alg,
- )
- ) {
- throw new InvalidClientMetadataError(
- `Unsupported "id_token_signed_response_alg" ${metadata.id_token_signed_response_alg}`,
- )
- }
-
- if (metadata.userinfo_encrypted_response_alg) {
- // We only support signature for now.
- throw new InvalidClientMetadataError(
- 'Encrypted userinfo response is not supported',
- )
- }
-
const method = metadata[`token_endpoint_auth_method`]
switch (method) {
case undefined:
@@ -338,37 +337,28 @@ export class ClientManager {
}
for (const responseType of metadata.response_types) {
- const rt = responseType.split(' ')
+ if (responseType.includes('id_token')) {
+ throw new InvalidClientMetadataError(
+ `OpenID Connect response type "${responseType}" is not supported`,
+ )
+ }
// ATPROTO spec requires the use of PKCE
- if (rt.includes('token')) {
+ if (responseType !== 'code') {
throw new InvalidClientMetadataError(
- '"token" response type is not compatible with PKCE (use "code" instead)',
+ `Unsupported response type "${responseType}"`,
)
}
// Consistency check
if (
- rt.includes('code') &&
+ responseType === 'code' &&
!metadata.grant_types.includes('authorization_code')
) {
throw new InvalidClientMetadataError(
`Response type "${responseType}" requires the "authorization_code" grant type`,
)
}
-
- // Asking for "code token" or "code id_token" is fine (as long as the
- // grant_types includes "authorization_code" and the scope includes
- // "openid"). Asking for "token" or "id_token" (without "code") requires
- // the "implicit" grant type.
- if (
- (rt.includes('token') || rt.includes('id_token')) &&
- !metadata.grant_types.includes('implicit')
- ) {
- throw new InvalidClientMetadataError(
- `Response type "${responseType}" requires the "implicit" grant type`,
- )
- }
}
if (metadata.application_type === 'native') {
@@ -383,11 +373,33 @@ export class ClientManager {
// > accordingly.
}
+ if (metadata.authorization_details_types?.length) {
+ const dupAuthDetailsType =
+ metadata.authorization_details_types.find(isDuplicate)
+ if (dupAuthDetailsType) {
+ throw new InvalidClientMetadataError(
+ `Duplicate authorization_details_type "${dupAuthDetailsType}"`,
+ )
+ }
+
+ const authorizationDetailsTypesSupported =
+ this.serverMetadata.authorization_details_types_supported
+ if (!authorizationDetailsTypesSupported) {
+ throw new InvalidClientMetadataError(
+ 'authorization_details_types are not supported',
+ )
+ }
+ for (const type of metadata.authorization_details_types) {
+ if (!authorizationDetailsTypesSupported.includes(type)) {
+ throw new InvalidClientMetadataError(
+ `Unsupported authorization_details_type "${type}"`,
+ )
+ }
+ }
+ }
+
if (!metadata.redirect_uris?.length) {
- // https://openid.net/specs/openid-connect-registration-1_0.html#rfc.section.2
- //
- // > OPs can require that request_uri values used be pre-registered with
- // > the require_request_uri_registration discovery parameter.
+ // ATPROTO spec requires that at least one redirect URI is provided
throw new InvalidClientMetadataError(
'At least one redirect_uri is required',
@@ -786,6 +798,12 @@ export class ClientManager {
}
}
+function isDuplicate<
+ T extends string | number | boolean | null | undefined | symbol,
+>(value: T, index: number, array: T[]) {
+ return array.includes(value, index + 1)
+}
+
function reverseDomain(domain: string) {
return domain.split('.').reverse().join('.')
}
diff --git a/packages/oauth/oauth-provider/src/client/client.ts b/packages/oauth/oauth-provider/src/client/client.ts
index 5034bf1f..e218aa34 100644
--- a/packages/oauth/oauth-provider/src/client/client.ts
+++ b/packages/oauth/oauth-provider/src/client/client.ts
@@ -140,6 +140,7 @@ export class Client {
audience: checks.audience,
subject: this.id,
maxTokenAge: CLIENT_ASSERTION_MAX_AGE / 1000,
+ requiredClaims: ['jti'],
}).catch((err) => {
if (err instanceof JOSEError) {
const msg = `Validation of "client_assertion" failed: ${err.message}`
@@ -153,10 +154,6 @@ export class Client {
throw new InvalidClientError(`"kid" required in client_assertion`)
}
- if (!result.payload.jti) {
- throw new InvalidClientError(`"jti" required in client_assertion`)
- }
-
const clientAuth: ClientAuth = {
method: CLIENT_ASSERTION_TYPE_JWT_BEARER,
jkt: await authJwkThumbprint(result.key),
diff --git a/packages/oauth/oauth-provider/src/constants.ts b/packages/oauth/oauth-provider/src/constants.ts
index 3804e790..97f25b16 100644
--- a/packages/oauth/oauth-provider/src/constants.ts
+++ b/packages/oauth/oauth-provider/src/constants.ts
@@ -67,3 +67,6 @@ export const DPOP_NONCE_MAX_AGE = 3 * MINUTE
/** 5 seconds */
export const SESSION_FIXATION_MAX_AGE = 5 * SECOND
+
+/** 1 day */
+export const CODE_CHALLENGE_REPLAY_TIMEFRAME = 1 * DAY
diff --git a/packages/oauth/oauth-provider/src/device/device-manager.ts b/packages/oauth/oauth-provider/src/device/device-manager.ts
index c2d69a9f..62431b44 100644
--- a/packages/oauth/oauth-provider/src/device/device-manager.ts
+++ b/packages/oauth/oauth-provider/src/device/device-manager.ts
@@ -100,10 +100,16 @@ export class DeviceManager {
public async load(
req: IncomingMessage,
res: ServerResponse,
+ forceRotate = false,
): Promise<{ deviceId: DeviceId }> {
const cookie = await this.getCookie(req)
if (cookie) {
- return this.refresh(req, res, cookie.value, cookie.mustRotate)
+ return this.refresh(
+ req,
+ res,
+ cookie.value,
+ forceRotate || cookie.mustRotate,
+ )
} else {
return this.create(req, res)
}
diff --git a/packages/oauth/oauth-provider/src/errors/invalid-authorization-details-error.ts b/packages/oauth/oauth-provider/src/errors/invalid-authorization-details-error.ts
index 89d9b073..b9be3d35 100644
--- a/packages/oauth/oauth-provider/src/errors/invalid-authorization-details-error.ts
+++ b/packages/oauth/oauth-provider/src/errors/invalid-authorization-details-error.ts
@@ -1,4 +1,5 @@
-import { OAuthError } from './oauth-error.js'
+import { OAuthAuthenticationRequestParameters } from '@atproto/oauth-types'
+import { AccessDeniedError } from './access-denied-error.js'
/**
* @see
@@ -15,8 +16,12 @@ import { OAuthError } from './oauth-error.js'
* - contains fields with invalid values for the authorization details type, or
* - is missing required fields for the authorization details type.
*/
-export class InvalidAuthorizationDetailsError extends OAuthError {
- constructor(error_description: string, cause?: unknown) {
- super('invalid_authorization_details', error_description, 400, cause)
+export class InvalidAuthorizationDetailsError extends AccessDeniedError {
+ constructor(
+ parameters: OAuthAuthenticationRequestParameters,
+ error_description: string,
+ cause?: unknown,
+ ) {
+ super(parameters, error_description, 'invalid_authorization_details', cause)
}
}
diff --git a/packages/oauth/oauth-provider/src/lib/http/request.ts b/packages/oauth/oauth-provider/src/lib/http/request.ts
index 23f3fb29..ac9b1add 100644
--- a/packages/oauth/oauth-provider/src/lib/http/request.ts
+++ b/packages/oauth/oauth-provider/src/lib/http/request.ts
@@ -28,6 +28,27 @@ export async function validateRequestPayload<S extends z.ZodTypeAny>(
return schema.parseAsync(payload, { path: ['body'] })
}
+export function validateHeaderValue(
+ req: IncomingMessage,
+ name: keyof IncomingMessage['headers'],
+ allowedValues: readonly (string | null)[],
+) {
+ const value = req.headers[name] ?? null
+
+ if (Array.isArray(value)) {
+ throw createHttpError(400, `Invalid ${name} header`)
+ }
+
+ if (!allowedValues.includes(value)) {
+ throw createHttpError(
+ 400,
+ value
+ ? `Forbidden ${name} header "${value}" (expected ${allowedValues})`
+ : `Missing ${name} header`,
+ )
+ }
+}
+
export function validateFetchMode(
req: IncomingMessage,
res: ServerResponse,
@@ -39,20 +60,45 @@ export function validateFetchMode(
| 'cors'
)[],
) {
- const reqMode = req.headers['sec-fetch-mode'] ?? null
+ validateHeaderValue(req, 'sec-fetch-mode', expectedMode)
+}
- if (Array.isArray(reqMode)) {
- throw createHttpError(400, `Invalid sec-fetch-mode header`)
- }
+export function validateFetchDest(
+ req: IncomingMessage,
+ res: ServerResponse,
+ expectedDest: readonly (
+ | null
+ | 'document'
+ | 'embed'
+ | 'font'
+ | 'image'
+ | 'manifest'
+ | 'media'
+ | 'object'
+ | 'report'
+ | 'script'
+ | 'serviceworker'
+ | 'sharedworker'
+ | 'style'
+ | 'worker'
+ | 'xslt'
+ )[],
+) {
+ validateHeaderValue(req, 'sec-fetch-dest', expectedDest)
+}
- if (!(expectedMode as (string | null)[]).includes(reqMode)) {
- throw createHttpError(
- 403,
- reqMode
- ? `Forbidden sec-fetch-mode "${reqMode}" (expected ${expectedMode})`
- : `Missing sec-fetch-mode (expected ${expectedMode})`,
- )
- }
+export function validateFetchSite(
+ req: IncomingMessage,
+ res: ServerResponse,
+ expectedSite: readonly (
+ | null
+ | 'same-origin'
+ | 'same-site'
+ | 'cross-site'
+ | 'none'
+ )[],
+) {
+ validateHeaderValue(req, 'sec-fetch-site', expectedSite)
}
export function validateReferer(
@@ -64,7 +110,7 @@ export function validateReferer(
const referer = req.headers['referer']
const refererUrl = referer ? new URL(referer) : null
if (refererUrl ? !urlMatch(refererUrl, reference) : !allowNull) {
- throw createHttpError(403, `Invalid referer ${referer}`)
+ throw createHttpError(400, `Invalid referer ${referer}`)
}
}
@@ -95,7 +141,7 @@ export function validateSameOrigin(
) {
const reqOrigin = req.headers['origin']
if (reqOrigin ? reqOrigin !== origin : !allowNull) {
- throw createHttpError(403, `Invalid origin ${reqOrigin}`)
+ throw createHttpError(400, `Invalid origin ${reqOrigin}`)
}
}
@@ -113,7 +159,7 @@ export function validateCsrfToken(
!cookieName ||
cookies[cookieName] !== csrfToken
) {
- throw createHttpError(403, `Invalid CSRF token`)
+ throw createHttpError(400, `Invalid CSRF token`)
}
if (clearCookie) {
diff --git a/packages/oauth/oauth-provider/src/metadata/build-metadata.ts b/packages/oauth/oauth-provider/src/metadata/build-metadata.ts
index c66820aa..0bbcd8c9 100644
--- a/packages/oauth/oauth-provider/src/metadata/build-metadata.ts
+++ b/packages/oauth/oauth-provider/src/metadata/build-metadata.ts
@@ -2,11 +2,9 @@ import { Keyset } from '@atproto/jwk'
import { OAuthAuthorizationServerMetadata } from '@atproto/oauth-types'
import { Client } from '../client/client.js'
-import { OIDC_STANDARD_CLAIMS } from '../oidc/claims.js'
import { VERIFY_ALGOS } from '../lib/util/crypto.js'
export type CustomMetadata = {
- claims_supported?: string[]
scopes_supported?: string[]
authorization_details_types_supported?: string[]
protected_resources?: string[]
@@ -25,35 +23,10 @@ export function buildMetadata(
issuer,
scopes_supported: [
- 'offline_access',
- 'openid',
- 'email',
- 'phone',
- 'profile',
-
+ 'atproto',
+ //
...(customMetadata?.scopes_supported ?? []),
],
- claims_supported: [
- /* IESG (Always provided) */
-
- 'sub', // did
- 'iss', // Authorization Server Origin
- 'aud',
- 'exp',
- 'iat',
- 'jti',
- 'client_id',
-
- /* OpenID */
-
- // 'acr', // "0"
- // 'amr',
- // 'azp',
- 'auth_time', // number - seconds since epoch
- 'nonce', // always required in "id_token", why would it not be supported?
-
- ...(customMetadata?.claims_supported ?? OIDC_STANDARD_CLAIMS),
- ],
subject_types_supported: [
//
'public', // The same "sub" is returned for all clients
@@ -62,15 +35,15 @@ export function buildMetadata(
response_types_supported: [
// OAuth
'code',
- 'token',
+ // 'token',
// OpenID
- 'none',
- 'code id_token token',
- 'code id_token',
- 'code token',
- 'id_token token',
- 'id_token',
+ // 'none',
+ // 'code id_token token',
+ // 'code id_token',
+ // 'code token',
+ // 'id_token token',
+ // 'id_token',
],
response_modes_supported: [
// https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html#ResponseModes
@@ -93,7 +66,6 @@ export function buildMetadata(
//
'en-US',
],
- id_token_signing_alg_values_supported: [...keyset.signAlgorithms],
display_values_supported: [
//
'page',
@@ -110,10 +82,6 @@ export function buildMetadata(
request_object_encryption_alg_values_supported: [], // None
request_object_encryption_enc_values_supported: [], // None
- // No claim makes sense to be translated
- claims_locales_supported: [],
-
- claims_parameter_supported: true,
request_parameter_supported: true,
request_uri_parameter_supported: true,
require_request_uri_registration: true,
@@ -130,7 +98,6 @@ export function buildMetadata(
introspection_endpoint: new URL('/oauth/introspect', issuer).href,
- userinfo_endpoint: new URL('/oauth/userinfo', issuer).href,
// end_session_endpoint: new URL('/oauth/logout', issuer).href,
// https://datatracker.ietf.org/doc/html/rfc9126#section-5
diff --git a/packages/oauth/oauth-provider/src/oauth-hooks.ts b/packages/oauth/oauth-provider/src/oauth-hooks.ts
index 2080d735..c4a4806d 100644
--- a/packages/oauth/oauth-provider/src/oauth-hooks.ts
+++ b/packages/oauth/oauth-provider/src/oauth-hooks.ts
@@ -11,6 +11,7 @@ import { ClientAuth } from './client/client-auth.js'
import { ClientId } from './client/client-id.js'
import { ClientInfo } from './client/client-info.js'
import { Client } from './client/client.js'
+import { InvalidAuthorizationDetailsError } from './errors/invalid-authorization-details-error.js'
import { Awaitable } from './lib/util/type.js'
// Make sure all types needed to implement the OAuthHooks are exported
@@ -20,6 +21,7 @@ export type {
ClientAuth,
ClientId,
ClientInfo,
+ InvalidAuthorizationDetailsError,
Jwks,
OAuthAuthenticationRequestParameters,
OAuthAuthorizationDetails,
@@ -42,7 +44,7 @@ export type OAuthHooks = {
/**
* Allows enriching the authorization details with additional information
- * before the tokens are issued.
+ * when the tokens are issued.
*
* @see {@link https://datatracker.ietf.org/doc/html/rfc9396 | RFC 9396}
*/
@@ -51,16 +53,4 @@ export type OAuthHooks = {
parameters: OAuthAuthenticationRequestParameters
account: Account
}) => Awaitable<undefined | OAuthAuthorizationDetails>
-
- /**
- * Allows altering the token response before it is sent to the client.
- */
- onTokenResponse?: (
- tokenResponse: OAuthTokenResponse,
- data: {
- client: Client
- parameters: OAuthAuthenticationRequestParameters
- account: Account
- },
- ) => Awaitable<void>
}
diff --git a/packages/oauth/oauth-provider/src/oauth-provider.ts b/packages/oauth/oauth-provider/src/oauth-provider.ts
index 237644fb..1df80bd9 100644
--- a/packages/oauth/oauth-provider/src/oauth-provider.ts
+++ b/packages/oauth/oauth-provider/src/oauth-provider.ts
@@ -1,7 +1,7 @@
import { safeFetchWrap } from '@atproto-labs/fetch-node'
import { SimpleStore } from '@atproto-labs/simple-store'
import { SimpleStoreMemory } from '@atproto-labs/simple-store-memory'
-import { Jwks, Keyset, SignedJwt, signedJwtSchema } from '@atproto/jwk'
+import { Jwks, Keyset } from '@atproto/jwk'
import {
AccessToken,
CLIENT_ASSERTION_TYPE_JWT_BEARER,
@@ -15,12 +15,11 @@ import {
oauthAuthenticationRequestParametersSchema,
} from '@atproto/oauth-types'
import { Redis, type RedisOptions } from 'ioredis'
-import { z } from 'zod'
+import z, { ZodError } from 'zod'
import { AccessTokenType } from './access-token/access-token-type.js'
import { AccountManager } from './account/account-manager.js'
import {
- AccountInfo,
AccountStore,
DeviceAccountInfo,
SignInCredentials,
@@ -58,12 +57,13 @@ import {
Middleware,
Router,
ServerResponse,
- acceptMiddleware,
combineMiddlewares,
setupCsrfToken,
staticJsonHandler,
validateCsrfToken,
+ validateFetchDest,
validateFetchMode,
+ validateFetchSite,
validateReferer,
validateRequestPayload,
validateSameOrigin,
@@ -74,7 +74,6 @@ import { Override } from './lib/util/type.js'
import { CustomMetadata, buildMetadata } from './metadata/build-metadata.js'
import { OAuthHooks } from './oauth-hooks.js'
import { OAuthVerifier, OAuthVerifierOptions } from './oauth-verifier.js'
-import { Userinfo } from './oidc/userinfo.js'
import { AuthorizationResultAuthorize } from './output/build-authorize-data.js'
import {
buildErrorPayload,
@@ -86,7 +85,6 @@ import {
AuthorizationResultRedirect,
sendAuthorizeRedirect,
} from './output/send-authorize-redirect.js'
-import { oidcPayload } from './parameters/oidc-payload.js'
import { ReplayStore, ifReplayStore } from './replay/replay-store.js'
import { RequestInfo } from './request/request-info.js'
import { RequestManager } from './request/request-manager.js'
@@ -103,7 +101,7 @@ import {
} from './request/types.js'
import { isTokenId } from './token/token-id.js'
import { TokenManager } from './token/token-manager.js'
-import { TokenInfo, TokenStore, asTokenStore } from './token/token-store.js'
+import { TokenStore, asTokenStore } from './token/token-store.js'
import {
CodeGrantRequest,
Introspect,
@@ -146,9 +144,7 @@ export type OAuthProviderOptions = Override<
{
/**
* Maximum age a device/account session can be before requiring
- * re-authentication. This can be overridden on a authorization request basis
- * using the `max_age` parameter and on a client basis using the
- * `default_max_age` client metadata.
+ * re-authentication.
*/
authenticationMaxAge?: number
@@ -286,6 +282,7 @@ export class OAuthProvider extends OAuthVerifier {
this.accountManager = new AccountManager(accountStore)
this.clientManager = new ClientManager(
+ this.metadata,
this.keyset,
rest,
clientStore || null,
@@ -326,14 +323,7 @@ export class OAuthProvider extends OAuthVerifier {
return true
}
- /** in seconds */
- const maxAge = parameters.max_age ?? client.metadata.default_max_age
-
- if (maxAge != null && maxAge < this.authenticationMaxAge) {
- return authAge >= maxAge
- } else {
- return authAge >= this.authenticationMaxAge
- }
+ return authAge >= this.authenticationMaxAge
}
protected async authenticateClient(
@@ -551,15 +541,14 @@ export class OAuthProvider extends OAuthVerifier {
throw new ConsentRequiredError(parameters)
}
- const redirect = await this.requestManager.setAuthorized(
+ const code = await this.requestManager.setAuthorized(
client,
uri,
deviceId,
ssoSession.account,
- ssoSession.info,
)
- return { issuer, client, parameters, redirect }
+ return { issuer, client, parameters, redirect: { code } }
}
// Automatic SSO when a did was provided
@@ -568,15 +557,14 @@ export class OAuthProvider extends OAuthVerifier {
if (ssoSessions.length === 1) {
const ssoSession = ssoSessions[0]!
if (!ssoSession.loginRequired && !ssoSession.consentRequired) {
- const redirect = await this.requestManager.setAuthorized(
+ const code = await this.requestManager.setAuthorized(
client,
uri,
deviceId,
ssoSession.account,
- ssoSession.info,
)
- return { issuer, client, parameters, redirect }
+ return { issuer, client, parameters, redirect: { code } }
}
}
}
@@ -585,7 +573,20 @@ export class OAuthProvider extends OAuthVerifier {
issuer,
client,
parameters,
- authorize: { uri, sessions },
+ authorize: {
+ uri,
+ sessions,
+ scopeDetails: parameters.scope
+ ?.split(/\s+/)
+ .filter(Boolean)
+ .sort((a, b) => a.localeCompare(b))
+ .map((scope) => ({
+ scope,
+ // @TODO Allow to customize the scope descriptions (e.g.
+ // using a hook)
+ description: undefined,
+ })),
+ },
}
} catch (err) {
await this.deleteRequest(uri, parameters)
@@ -652,6 +653,9 @@ export class OAuthProvider extends OAuthVerifier {
this.loginRequired(client, parameters, info),
consentRequired:
parameters.prompt === 'consent' ||
+ // @TODO the "authorizedClients" should also include the scopes that
+ // were already authorized for the client. Otherwise a client could
+ // use silent authentication to get additional scopes without consent.
!info.authorizedClients.includes(client.id),
matchesHint: hint == null || matchesHint(account),
@@ -660,9 +664,33 @@ export class OAuthProvider extends OAuthVerifier {
protected async signIn(
deviceId: DeviceId,
+ uri: RequestUri,
+ clientId: ClientId,
credentials: SignInCredentials,
- ): Promise<AccountInfo> {
- return this.accountManager.signIn(credentials, deviceId)
+ ): Promise<{
+ account: Account
+ consentRequired: boolean
+ }> {
+ const client = await this.clientManager.getClient(clientId)
+
+ // Ensure the request is still valid (and update the request expiration)
+ // @TODO use the returned scopes to determine if consent is required
+ await this.requestManager.get(uri, clientId, deviceId)
+
+ const { account, info } = await this.accountManager.signIn(
+ credentials,
+ deviceId,
+ )
+
+ return {
+ account,
+ consentRequired: client.info.isFirstParty
+ ? false
+ : // @TODO: the "authorizedClients" should also include the scopes that
+ // were already authorized for the client. Otherwise a client could
+ // use silent authentication to get additional scopes without consent.
+ !info.authorizedClients.includes(client.id),
+ }
}
protected async acceptRequest(
@@ -692,12 +720,11 @@ export class OAuthProvider extends OAuthVerifier {
)
}
- const redirect = await this.requestManager.setAuthorized(
+ const code = await this.requestManager.setAuthorized(
client,
uri,
deviceId,
account,
- info,
)
await this.accountManager.addAuthorizedClient(
@@ -707,7 +734,7 @@ export class OAuthProvider extends OAuthVerifier {
clientAuth,
)
- return { issuer, client, parameters, redirect }
+ return { issuer, client, parameters, redirect: { code } }
} catch (err) {
await this.deleteRequest(uri, parameters)
@@ -794,6 +821,32 @@ export class OAuthProvider extends OAuthVerifier {
input.code,
)
+ // the following check prevents re-use of PKCE challenges, enforcing the
+ // clients to generate a new challenge for each authorization request. The
+ // replay manager typically prevents replay over a certain time frame,
+ // which might not cover the entire lifetime of the token (depending on
+ // the implementation of the replay store). For this reason, we should
+ // ideally ensure that the code_challenge was not already used by any
+ // existing token or any other pending request.
+ //
+ // The current implementation will cause client devs not issuing a new
+ // code challenge for each authorization request to fail, which should be
+ // a good enough incentive to follow the best practices, until we have a
+ // better implementation.
+ //
+ // @TODO: Use tokenManager to ensure uniqueness of code_challenge
+ if (parameters.code_challenge) {
+ const unique = await this.replayManager.uniqueCodeChallenge(
+ parameters.code_challenge,
+ )
+ if (!unique) {
+ throw new InvalidGrantError(
+ 'code_challenge',
+ 'Code challenge already used',
+ )
+ }
+ }
+
const { account, info } = await this.accountManager.get(deviceId, sub)
return await this.tokenManager.create(
@@ -891,31 +944,6 @@ export class OAuthProvider extends OAuthVerifier {
}
}
- /**
- * @see {@link https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.3.2 Successful UserInfo Response}
- */
- protected async userinfo({ data, account }: TokenInfo): Promise<Userinfo> {
- return {
- ...oidcPayload(data.parameters, account),
-
- sub: account.sub,
-
- client_id: data.clientId,
- username: account.preferred_username,
- }
- }
-
- protected async signUserinfo(userinfo: Userinfo): Promise<SignedJwt> {
- const client = await this.clientManager.getClient(userinfo.client_id)
- return this.signer.sign(
- {
- alg: client.metadata.userinfo_signed_response_alg,
- typ: 'JWT',
- },
- userinfo,
- )
- }
-
protected override async authenticateToken(
tokenType: OAuthTokenType,
token: AccessToken,
@@ -979,6 +1007,8 @@ export class OAuthProvider extends OAuthVerifier {
combineMiddlewares([
function (req, res, next) {
res.setHeader('Access-Control-Allow-Origin', '*')
+ res.setHeader('Access-Control-Allow-Headers', '*')
+
res.setHeader('Cache-Control', 'max-age=300')
next()
},
@@ -995,6 +1025,7 @@ export class OAuthProvider extends OAuthVerifier {
): Handler<T, TReq, TRes> =>
async function (req, res) {
res.setHeader('Access-Control-Allow-Origin', '*')
+ res.setHeader('Access-Control-Allow-Headers', '*')
// https://www.rfc-editor.org/rfc/rfc6749.html#section-5.1
res.setHeader('Cache-Control', 'no-store')
@@ -1037,11 +1068,15 @@ export class OAuthProvider extends OAuthVerifier {
handler: (this: T, req: TReq, res: TRes) => void | Promise<void>,
): Handler<T, TReq, TRes> =>
async function (req, res) {
+ res.setHeader('Access-Control-Allow-Origin', '*')
+ res.setHeader('Access-Control-Allow-Headers', '*')
+
res.setHeader('Cache-Control', 'no-store')
res.setHeader('Pragma', 'no-cache')
try {
validateFetchMode(req, res, ['navigate'])
+ validateFetchDest(req, res, ['document'])
validateSameOrigin(req, res, issuerOrigin)
await handler.call(this, req, res)
@@ -1066,49 +1101,45 @@ export class OAuthProvider extends OAuthVerifier {
//- Public OAuth endpoints
- /*
- * Although OpenID compatibility is not required to implement the Atproto
- * OAuth2 specification, we do support OIDC discovery in this
- * implementation as we believe this may:
- * 1) Make the implementation of Atproto clients easier (since lots of
- * libraries support OIDC discovery)
- * 2) Allow self hosted PDS' to not implement authentication themselves
- * but rely on a trusted Atproto actor to act as their OIDC providers.
- * By supporting OIDC in the current implementation, Bluesky's
- * Authorization Server server can be used as an OIDC provider for
- * these users.
- */
- router.get('/.well-known/openid-configuration', staticJson(server.metadata))
-
router.get(
'/.well-known/oauth-authorization-server',
staticJson(server.metadata),
)
// CORS preflight
- router.options<{
- endpoint: 'jwks' | 'par' | 'token' | 'revoke' | 'introspect' | 'userinfo'
- }>(
- /^\/oauth\/(?<endpoint>jwks|par|token|revoke|introspect|userinfo)$/,
- function (req, res, _next) {
- res
- .writeHead(204, {
- 'Access-Control-Allow-Origin': req.headers['origin'] || '*',
- 'Access-Control-Allow-Methods':
- this.params.endpoint === 'jwks' ? 'GET' : 'POST',
- 'Access-Control-Allow-Headers': 'Content-Type,Authorization,DPoP',
- 'Access-Control-Max-Age': '86400', // 1 day
- })
- .end()
- },
- )
+ const corsPreflight: Middleware = function (req, res, _next) {
+ res
+ .writeHead(204, {
+ // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin
+ //
+ // > For requests without credentials, the literal value "*" can be
+ // > specified as a wildcard; the value tells browsers to allow
+ // > requesting code from any origin to access the resource.
+ // > Attempting to use the wildcard with credentials results in an
+ // > error.
+ //
+ // A "*" is safer to use than reflecting the request origin.
+ 'Access-Control-Allow-Origin': '*',
+ // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Methods
+ // > The value "*" only counts as a special wildcard value for
+ // > requests without credentials (requests without HTTP cookies or
+ // > HTTP authentication information). In requests with credentials,
+ // > it is treated as the literal method name "*" without special
+ // > semantics.
+ 'Access-Control-Allow-Methods': '*',
+ 'Access-Control-Allow-Headers': 'Content-Type,Authorization,DPoP',
+ 'Access-Control-Max-Age': '86400', // 1 day
+ })
+ .end()
+ }
router.get('/oauth/jwks', staticJson(server.jwks))
+ router.options('/oauth/par', corsPreflight)
router.post(
'/oauth/par',
jsonHandler(async function (req, _res) {
- const input = await validateRequestPayload(
+ const input = await validateRequest(
req,
pushedAuthorizationRequestSchema,
)
@@ -1124,14 +1155,18 @@ export class OAuthProvider extends OAuthVerifier {
)
// https://datatracker.ietf.org/doc/html/rfc9126#section-2.3
- router.addRoute('*', '/oauth/par', (req, res) => {
+ // > If the request did not use the POST method, the authorization server
+ // > responds with an HTTP 405 (Method Not Allowed) status code.
+ router.options('/oauth/par', corsPreflight)
+ router.all('/oauth/par', (req, res) => {
res.writeHead(405).end()
})
+ router.options('/oauth/token', corsPreflight)
router.post(
'/oauth/token',
jsonHandler(async function (req, _res) {
- const input = await validateRequestPayload(req, tokenRequestSchema)
+ const input = await validateRequest(req, tokenRequestSchema)
const dpopJkt = await server.checkDpopProof(
req.headers['dpop'],
@@ -1143,10 +1178,11 @@ export class OAuthProvider extends OAuthVerifier {
}),
)
+ router.options('/oauth/revoke', corsPreflight)
router.post(
'/oauth/revoke',
jsonHandler(async function (req, res) {
- const input = await validateRequestPayload(req, revokeSchema)
+ const input = await validateRequest(req, revokeSchema)
try {
await server.revoke(input)
@@ -1156,6 +1192,7 @@ export class OAuthProvider extends OAuthVerifier {
}),
)
+ router.options('/oauth/revoke', corsPreflight)
router.get(
'/oauth/revoke',
navigationHandler(async function (req, res) {
@@ -1180,69 +1217,11 @@ export class OAuthProvider extends OAuthVerifier {
router.post(
'/oauth/introspect',
jsonHandler(async function (req, _res) {
- const input = await validateRequestPayload(req, introspectSchema)
+ const input = await validateRequest(req, introspectSchema)
return server.introspect(input)
}),
)
- const userinfoBodySchema = z.object({
- access_token: signedJwtSchema.optional(),
- })
-
- router.addRoute(
- ['GET', 'POST'],
- '/oauth/userinfo',
- acceptMiddleware(
- async function (req, _res) {
- const body =
- req.method === 'POST'
- ? await validateRequestPayload(req, userinfoBodySchema)
- : null
-
- if (body?.access_token && req.headers['authorization']) {
- throw new InvalidRequestError(
- 'access token must be provided in either the authorization header or the request body',
- )
- }
-
- const auth = await server.authenticateRequest(
- req.method!,
- this.url,
- body?.access_token // Allow credentials to be parsed from body.
- ? {
- authorization: `Bearer ${body.access_token}`,
- dpop: undefined, // DPoP can only be used with headers
- }
- : req.headers,
- {
- scope: ['profile'],
- },
- )
-
- const tokenInfo: TokenInfo =
- 'tokenInfo' in auth
- ? (auth.tokenInfo as TokenInfo)
- : await server.tokenManager.getTokenInfo(
- auth.tokenType,
- auth.tokenId,
- )
-
- return server.userinfo(tokenInfo)
- },
- {
- '': 'application/json',
- 'application/json': jsonHandler(async function (_req, _res) {
- return this.data
- }),
- 'application/jwt': jsonHandler(async function (_req, res) {
- const jwt = await server.signUserinfo(this.data)
- res.writeHead(200, { 'Content-Type': 'application/jwt' }).end(jwt)
- return undefined
- }),
- },
- ),
- )
-
//- Private authorization endpoints
router.use(authorizeAssetsMiddleware())
@@ -1250,6 +1229,8 @@ export class OAuthProvider extends OAuthVerifier {
router.get(
'/oauth/authorize',
navigationHandler(async function (req, res) {
+ validateFetchSite(req, res, ['cross-site', 'none'])
+
const query = Object.fromEntries(this.url.searchParams)
const input = await authorizationRequestQuerySchema.parseAsync(query, {
path: ['query'],
@@ -1281,13 +1262,15 @@ export class OAuthProvider extends OAuthVerifier {
credentials: signInCredentialsSchema,
})
+ router.options('/oauth/authorize/sign-in', corsPreflight)
router.post(
'/oauth/authorize/sign-in',
jsonHandler(async function (req, res) {
validateFetchMode(req, res, ['same-origin'])
+ validateFetchSite(req, res, ['same-origin'])
validateSameOrigin(req, res, issuerOrigin)
- const input = await validateRequestPayload(req, signInPayloadSchema)
+ const input = await validateRequest(req, signInPayloadSchema)
validateReferer(req, res, {
origin: issuerOrigin,
@@ -1300,20 +1283,14 @@ export class OAuthProvider extends OAuthVerifier {
csrfCookie(input.request_uri),
)
- const { deviceId } = await deviceManager.load(req, res)
+ const { deviceId } = await deviceManager.load(req, res, true)
- const { account, info } = await server.signIn(
+ return server.signIn(
deviceId,
+ input.request_uri,
+ input.client_id,
input.credentials,
)
-
- // Prevent fixation attacks
- await deviceManager.rotate(req, res, deviceId)
-
- return {
- account,
- consentRequired: !info.authorizedClients.includes(input.client_id),
- }
}),
)
@@ -1324,9 +1301,20 @@ export class OAuthProvider extends OAuthVerifier {
account_sub: z.string(),
})
+ // Though this is a "no-cors" request, meaning that the browser will allow
+ // any cross-origin request, with credentials, to be sent, the handler will
+ // 1) validate the request origin,
+ // 2) validate the CSRF token,
+ // 3) validate the referer,
+ // 4) validate the sec-fetch-site header,
+ // 4) validate the sec-fetch-mode header,
+ // 5) validate the sec-fetch-dest header (see navigationHandler).
+ // And will error if any of these checks fail.
router.get(
'/oauth/authorize/accept',
navigationHandler(async function (req, res) {
+ validateFetchSite(req, res, ['same-origin'])
+
const query = Object.fromEntries(this.url.searchParams)
const input = await acceptQuerySchema.parseAsync(query, {
path: ['query'],
@@ -1367,9 +1355,20 @@ export class OAuthProvider extends OAuthVerifier {
client_id: clientIdSchema,
})
+ // Though this is a "no-cors" request, meaning that the browser will allow
+ // any cross-origin request, with credentials, to be sent, the handler will
+ // 1) validate the request origin,
+ // 2) validate the CSRF token,
+ // 3) validate the referer,
+ // 4) validate the sec-fetch-site header,
+ // 4) validate the sec-fetch-mode header,
+ // 5) validate the sec-fetch-dest header (see navigationHandler).
+ // And will error if any of these checks fail.
router.get(
'/oauth/authorize/reject',
navigationHandler(async function (req, res) {
+ validateFetchSite(req, res, ['same-origin'])
+
const query = Object.fromEntries(this.url.searchParams)
const input = await rejectQuerySchema.parseAsync(query, {
path: ['query'],
@@ -1406,3 +1405,26 @@ export class OAuthProvider extends OAuthVerifier {
return router
}
}
+
+async function validateRequest<S extends z.ZodTypeAny>(
+ req: IncomingMessage,
+ schema: S,
+): Promise<z.TypeOf<S>> {
+ try {
+ return await validateRequestPayload(req, schema)
+ } catch (err) {
+ if (err instanceof ZodError) {
+ const issue = err.issues[0]
+ if (issue?.path.length) {
+ // "part" will typically be
+ const [part, ...path] = issue.path
+ throw new InvalidRequestError(
+ `Validation of ${part}'s "${path.join('.')}" with error: ${issue.message}`,
+ err,
+ )
+ }
+ }
+
+ throw new InvalidRequestError('Input validation error', err)
+ }
+}
diff --git a/packages/oauth/oauth-provider/src/oauth-verifier.ts b/packages/oauth/oauth-provider/src/oauth-verifier.ts
index 4d8cefa4..a9d9414a 100644
--- a/packages/oauth/oauth-provider/src/oauth-verifier.ts
+++ b/packages/oauth/oauth-provider/src/oauth-verifier.ts
@@ -36,8 +36,7 @@ export type OAuthVerifierOptions = Override<
issuer: URL | string
/**
- * The keyset used to sign tokens. Note that OIDC requires that at least one
- * RS256 key is present in the keyset. ATPROTO requires ES256.
+ * The keyset used to sign access tokens.
*/
keyset: Keyset | Iterable<Key | undefined | null | false>
diff --git a/packages/oauth/oauth-provider/src/oidc/claims.ts b/packages/oauth/oauth-provider/src/oidc/claims.ts
deleted file mode 100644
index cbcbfe2d..00000000
--- a/packages/oauth/oauth-provider/src/oidc/claims.ts
+++ /dev/null
@@ -1,35 +0,0 @@
-import { JwtPayload } from '@atproto/jwk'
-
-/**
- * @see {@link https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims | OpenID Connect Core 1.0, 5.4. Requesting Claims using Scope Values}
- */
-export const OIDC_SCOPE_CLAIMS = Object.freeze({
- email: Object.freeze(['email', 'email_verified'] as const),
- phone: Object.freeze(['phone_number', 'phone_number_verified'] as const),
- address: Object.freeze(['address'] as const),
- profile: Object.freeze([
- 'name',
- 'family_name',
- 'given_name',
- 'middle_name',
- 'nickname',
- 'preferred_username',
- 'gender',
- 'picture',
- 'profile',
- 'website',
- 'birthdate',
- 'zoneinfo',
- 'locale',
- 'updated_at',
- ] as const),
-})
-
-export const OIDC_STANDARD_CLAIMS = Object.freeze(
- Object.values(OIDC_SCOPE_CLAIMS).flat(),
-)
-
-export type OIDCStandardClaim = (typeof OIDC_STANDARD_CLAIMS)[number]
-export type OIDCStandardPayload = Partial<{
- [K in OIDCStandardClaim]?: JwtPayload[K]
-}>
diff --git a/packages/oauth/oauth-provider/src/oidc/userinfo.ts b/packages/oauth/oauth-provider/src/oidc/userinfo.ts
deleted file mode 100644
index 3810c6e3..00000000
--- a/packages/oauth/oauth-provider/src/oidc/userinfo.ts
+++ /dev/null
@@ -1,11 +0,0 @@
-import { OIDCStandardPayload } from './claims.js'
-
-export type Userinfo = OIDCStandardPayload & {
- // "The sub (subject) Claim MUST always be returned in the UserInfo Response."
- sub: string
-
- // client_id is not mandatory per spec, but we require it here for convenience
- client_id: string
-
- username?: string
-}
diff --git a/packages/oauth/oauth-provider/src/output/build-authorize-data.ts b/packages/oauth/oauth-provider/src/output/build-authorize-data.ts
index 722fa9a1..225ccb02 100644
--- a/packages/oauth/oauth-provider/src/output/build-authorize-data.ts
+++ b/packages/oauth/oauth-provider/src/output/build-authorize-data.ts
@@ -8,12 +8,18 @@ import { Account } from '../account/account.js'
import { Client } from '../client/client.js'
import { RequestUri } from '../request/request-uri.js'
+export type ScopeDetail = {
+ scope: string
+ description?: string
+}
+
export type AuthorizationResultAuthorize = {
issuer: string
client: Client
parameters: OAuthAuthenticationRequestParameters
authorize: {
uri: RequestUri
+ scopeDetails?: ScopeDetail[]
sessions: readonly {
account: Account
info: DeviceAccountInfo
@@ -44,6 +50,7 @@ export type AuthorizeData = {
requestUri: string
csrfCookie: string
loginHint?: string
+ scopeDetails?: ScopeDetail[]
newSessionsRequireConsent: boolean
sessions: Session[]
}
@@ -59,6 +66,7 @@ export function buildAuthorizeData(
csrfCookie: `csrf-${data.authorize.uri}`,
loginHint: data.parameters.login_hint,
newSessionsRequireConsent: data.parameters.prompt === 'consent',
+ scopeDetails: data.authorize.scopeDetails,
sessions: data.authorize.sessions.map(
(session): Session => ({
account: session.account,
diff --git a/packages/oauth/oauth-provider/src/parameters/claims-requested.ts b/packages/oauth/oauth-provider/src/parameters/claims-requested.ts
deleted file mode 100644
index c02d5ab6..00000000
--- a/packages/oauth/oauth-provider/src/parameters/claims-requested.ts
+++ /dev/null
@@ -1,106 +0,0 @@
-import {
- OAuthAuthenticationRequestParameters,
- OidcClaimsParameter,
- OidcEntityType,
-} from '@atproto/oauth-types'
-import { InvalidRequestError } from '../errors/invalid-request-error.js'
-
-export function claimRequested(
- parameters: OAuthAuthenticationRequestParameters,
- entityType: OidcEntityType,
- claimName: OidcClaimsParameter,
- value: unknown,
-): boolean {
- if (claimAvailable(parameters, entityType, claimName, value)) {
- return true
- }
-
- const entityClaims = parameters.claims?.[entityType]
- if (entityClaims?.[claimName]?.essential === true) {
- // https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.5.1
- //
- // > By requesting Claims as Essential Claims, the RP indicates to the
- // > End-User that releasing these Claims will ensure a smooth
- // > authorization for the specific task requested by the End-User. Note
- // > that even if the Claims are not available because the End-User did
- // > not authorize their release or they are not present, the
- // > Authorization Server MUST NOT generate an error when Claims are not
- // > returned, whether they are Essential or Voluntary, unless otherwise
- // > specified in the description of the specific claim.
- switch (claimName) {
- case 'acr':
- // https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.5.1.1
- //
- // > If this is an Essential Claim and the requirement cannot be met,
- // > then the Authorization Server MUST treat that outcome as a failed
- // > authentication attempt.
- throw new InvalidRequestError(
- `Unable to provide essential claim: ${claimName}`,
- )
- }
- }
-
- return false
-}
-
-function claimAvailable(
- parameters: OAuthAuthenticationRequestParameters,
- entityType: OidcEntityType,
- claimName: OidcClaimsParameter,
- value: unknown,
-): boolean {
- if (value === undefined) return false
-
- if (parameters.claims) {
- const entityClaims = parameters.claims[entityType]
- if (entityClaims === undefined) return false
-
- const claimConfig = entityClaims[claimName]
- if (claimConfig === undefined) return false
- if (claimConfig === null) return true
-
- if (
- claimConfig.value !== undefined &&
- !compareClaimValue(claimConfig.value, value)
- ) {
- return false
- }
-
- if (
- claimConfig?.values !== undefined &&
- !claimConfig.values.some((v) => compareClaimValue(v, value))
- ) {
- return false
- }
- }
-
- return true
-}
-
-type DefinedValue = NonNullable<unknown> | null
-
-function compareClaimValue(
- expectedValue: DefinedValue,
- value: DefinedValue,
-): boolean {
- const expectedType = typeof expectedValue
- const valueType = typeof value
-
- if (expectedType !== valueType) return false
-
- switch (typeof expectedValue) {
- case 'undefined':
- case 'string':
- case 'number':
- case 'boolean':
- return expectedValue === value
- case 'object':
- if (expectedValue === null) return value === null
- // @TODO (?): allow object comparison
- // falls through
- default:
- throw new InvalidRequestError(
- `Unable to compare claim value of type ${expectedType}`,
- )
- }
-}
diff --git a/packages/oauth/oauth-provider/src/parameters/oidc-payload.ts b/packages/oauth/oauth-provider/src/parameters/oidc-payload.ts
deleted file mode 100644
index 5077890e..00000000
--- a/packages/oauth/oauth-provider/src/parameters/oidc-payload.ts
+++ /dev/null
@@ -1,28 +0,0 @@
-import { OAuthAuthenticationRequestParameters } from '@atproto/oauth-types'
-import { Account } from '../account/account.js'
-import { OIDCStandardPayload, OIDC_SCOPE_CLAIMS } from '../oidc/claims.js'
-import { claimRequested } from './claims-requested.js'
-
-export function oidcPayload(
- params: OAuthAuthenticationRequestParameters,
- account: Account,
-) {
- const payload: OIDCStandardPayload = {}
-
- const scopes = params.scope ? params.scope?.split(' ') : undefined
- if (scopes) {
- for (const [scope, claims] of Object.entries(OIDC_SCOPE_CLAIMS)) {
- const allowed = scopes.includes(scope)
- for (const claim of claims) {
- const value = allowed ? account[claim] : undefined
- // Should not throw as RequestManager should have already checked
- // that all the essential claims are available.
- if (claimRequested(params, 'id_token', claim, value)) {
- payload[claim] = value as any // All good as long as the account props match the claims
- }
- }
- }
- }
-
- return payload
-}
diff --git a/packages/oauth/oauth-provider/src/replay/replay-manager.ts b/packages/oauth/oauth-provider/src/replay/replay-manager.ts
index 0433a7a0..3d8f71cb 100644
--- a/packages/oauth/oauth-provider/src/replay/replay-manager.ts
+++ b/packages/oauth/oauth-provider/src/replay/replay-manager.ts
@@ -2,6 +2,7 @@ import { ClientId } from '../client/client-id.js'
import {
CLIENT_ASSERTION_MAX_AGE,
DPOP_NONCE_MAX_AGE,
+ CODE_CHALLENGE_REPLAY_TIMEFRAME,
JAR_MAX_AGE,
} from '../constants.js'
import { ReplayStore } from './replay-store.js'
@@ -35,4 +36,12 @@ export class ReplayManager {
asTimeFrame(DPOP_NONCE_MAX_AGE),
)
}
+
+ async uniqueCodeChallenge(challenge: string): Promise<boolean> {
+ return this.replayStore.unique(
+ 'CodeChallenge',
+ challenge,
+ asTimeFrame(CODE_CHALLENGE_REPLAY_TIMEFRAME),
+ )
+ }
}
diff --git a/packages/oauth/oauth-provider/src/replay/replay-store.ts b/packages/oauth/oauth-provider/src/replay/replay-store.ts
index 95b83a96..67dc324a 100644
--- a/packages/oauth/oauth-provider/src/replay/replay-store.ts
+++ b/packages/oauth/oauth-provider/src/replay/replay-store.ts
@@ -9,7 +9,7 @@ export interface ReplayStore {
* strictly necessary for security purposes, the namespace should be used to
* mitigate denial of service attacks from one client to the other.
*
- * @param timeFrame expressed in milliseconds. Will never exceed 24 hours.
+ * @param timeFrame expressed in milliseconds.
*/
unique(
namespace: string,
diff --git a/packages/oauth/oauth-provider/src/request/request-info.ts b/packages/oauth/oauth-provider/src/request/request-info.ts
index eb4f76b7..533339c6 100644
--- a/packages/oauth/oauth-provider/src/request/request-info.ts
+++ b/packages/oauth/oauth-provider/src/request/request-info.ts
@@ -1,4 +1,5 @@
import { OAuthAuthenticationRequestParameters } from '@atproto/oauth-types'
+import { ClientId } from '../client/client-id.js'
import { ClientAuth } from '../client/client-auth.js'
import { RequestId } from './request-id.js'
import { RequestUri } from './request-uri.js'
@@ -8,5 +9,6 @@ export type RequestInfo = {
uri: RequestUri
parameters: Readonly<OAuthAuthenticationRequestParameters>
expiresAt: Date
+ clientId: ClientId
clientAuth: ClientAuth
}
diff --git a/packages/oauth/oauth-provider/src/request/request-manager.ts b/packages/oauth/oauth-provider/src/request/request-manager.ts
index d8d38bd4..6d8bdcd2 100644
--- a/packages/oauth/oauth-provider/src/request/request-manager.ts
+++ b/packages/oauth/oauth-provider/src/request/request-manager.ts
@@ -4,7 +4,6 @@ import {
OAuthAuthorizationServerMetadata,
} from '@atproto/oauth-types'
-import { DeviceAccountInfo } from '../account/account-store.js'
import { Account } from '../account/account.js'
import { ClientAuth } from '../client/client-auth.js'
import { ClientId } from '../client/client-id.js'
@@ -17,12 +16,12 @@ import {
import { DeviceId } from '../device/device-id.js'
import { AccessDeniedError } from '../errors/access-denied-error.js'
import { ConsentRequiredError } from '../errors/consent-required-error.js'
+import { InvalidAuthorizationDetailsError } from '../errors/invalid-authorization-details-error.js'
import { InvalidGrantError } from '../errors/invalid-grant-error.js'
import { InvalidParametersError } from '../errors/invalid-parameters-error.js'
import { InvalidRequestError } from '../errors/invalid-request-error.js'
import { compareRedirectUri } from '../lib/util/redirect-uri.js'
import { OAuthHooks } from '../oauth-hooks.js'
-import { OIDC_SCOPE_CLAIMS } from '../oidc/claims.js'
import { Signer } from '../signer/signer.js'
import { Code, generateCode } from './code.js'
import {
@@ -44,7 +43,6 @@ export class RequestManager {
protected readonly signer: Signer,
protected readonly metadata: OAuthAuthorizationServerMetadata,
protected readonly hooks: OAuthHooks,
- protected readonly pkceRequired = true,
protected readonly tokenMaxAge = TOKEN_MAX_AGE,
) {}
@@ -83,7 +81,7 @@ export class RequestManager {
})
const uri = encodeRequestUri(id)
- return { id, uri, expiresAt, parameters, clientAuth }
+ return { id, uri, expiresAt, parameters, clientId: client.id, clientAuth }
}
async validate(
@@ -91,8 +89,28 @@ export class RequestManager {
clientAuth: ClientAuth,
parameters: Readonly<OAuthAuthenticationRequestParameters>,
dpopJkt: null | string,
- pkceRequired = this.pkceRequired,
): Promise<Readonly<OAuthAuthenticationRequestParameters>> {
+ for (const k of [
+ // Known unsupported OIDC parameters
+ 'claims',
+ 'id_token_hint',
+ 'nonce', // note that OIDC "nonce" is redundant with PKCE
+ ] as const) {
+ if (parameters[k]) {
+ throw new InvalidParametersError(
+ parameters,
+ `Unsupported "${k}" parameter`,
+ )
+ }
+ }
+
+ if (parameters.response_type !== 'code') {
+ throw new InvalidParametersError(
+ parameters,
+ 'Only "code" response type is allowed',
+ )
+ }
+
// https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-10#section-1.4.1
// > The authorization server MAY fully or partially ignore the scope
// > requested by the client, based on the authorization server policy or
@@ -101,59 +119,75 @@ export class RequestManager {
// > server MUST include the scope response parameter in the token response
// > (Section 3.2.3) to inform the client of the actual scope granted.
- const cScopes = client.metadata.scope?.split(' ')
+ const cScopes = client.metadata.scope?.split(' ').filter(Boolean)
const sScopes = this.metadata.scopes_supported
- const scopes =
- (parameters.scope || client.metadata.scope)
- ?.split(' ')
- .filter((scope) => !!scope && (sScopes?.includes(scope) ?? true)) ?? []
+ const scopes = new Set(
+ parameters.scope?.split(' ').filter(Boolean) || cScopes,
+ )
+
+ if (scopes.has('openid')) {
+ throw new InvalidParametersError(
+ parameters,
+ 'OpenID Connect is not supported',
+ )
+ }
+
+ if (!scopes.has('atproto')) {
+ throw new InvalidParametersError(
+ parameters,
+ 'The "atproto" scope is required',
+ )
+ }
for (const scope of scopes) {
- if (!cScopes?.includes(scope)) {
+ // Loopback clients do not define any scope in their metadata
+ if (cScopes && !cScopes.includes(scope)) {
throw new InvalidParametersError(
parameters,
`Scope "${scope}" is not registered for this client`,
)
}
- }
- for (const [scope, claims] of Object.entries(OIDC_SCOPE_CLAIMS)) {
- for (const claim of claims) {
- if (
- parameters?.claims?.id_token?.[claim]?.essential === true ||
- parameters?.claims?.userinfo?.[claim]?.essential === true
- ) {
- if (!scopes?.includes(scope)) {
- throw new InvalidParametersError(
- parameters,
- `Essential ${claim} claim requires "${scope}" scope`,
- )
- }
- }
+ // Currently, the implementation requires all the scopes to be statically
+ // defined in the server metadata. In the future, we might add support
+ // for dynamic scopes.
+ if (!sScopes?.includes(scope)) {
+ throw new InvalidParametersError(
+ parameters,
+ `Scope "${scope}" is not supported by this server`,
+ )
}
}
- parameters = { ...parameters, scope: scopes.join(' ') }
-
- const responseTypes = parameters.response_type.split(' ')
+ parameters = { ...parameters, scope: [...scopes].join(' ') || undefined }
if (parameters.authorization_details) {
const clientAuthDetailsTypes = client.metadata.authorization_details_types
if (!clientAuthDetailsTypes) {
- throw new InvalidParametersError(
+ throw new InvalidAuthorizationDetailsError(
parameters,
'Client Metadata does not declare any "authorization_details"',
)
}
for (const detail of parameters.authorization_details) {
- if (!clientAuthDetailsTypes?.includes(detail.type)) {
- throw new InvalidParametersError(
+ if (
+ !this.metadata.authorization_details_types_supported?.includes(
+ detail.type,
+ )
+ ) {
+ throw new InvalidAuthorizationDetailsError(
parameters,
`Unsupported "authorization_details" type "${detail.type}"`,
)
}
+ if (!clientAuthDetailsTypes?.includes(detail.type)) {
+ throw new InvalidAuthorizationDetailsError(
+ parameters,
+ `Client Metadata does not declare any "authorization_details" of type "${detail.type}"`,
+ )
+ }
}
}
@@ -197,16 +231,9 @@ export class RequestManager {
)
}
- if (pkceRequired && responseTypes.includes('token')) {
- throw new InvalidParametersError(
- parameters,
- `Response type "${parameters.response_type}" is incompatible with PKCE`,
- 'unsupported_response_type',
- )
- }
-
// https://datatracker.ietf.org/doc/html/rfc7636#section-4.4.1
- if (pkceRequired && !parameters.code_challenge) {
+ // PKCE is mandatory
+ if (!parameters.code_challenge) {
throw new InvalidParametersError(parameters, 'code_challenge is required')
}
@@ -229,50 +256,15 @@ export class RequestManager {
)
}
- // https://openid.net/specs/openid-connect-core-1_0.html#HybridAuthRequest
- //
- // > nonce: REQUIRED if the Response Type of the request is "code id_token" or
- // > "code id_token token" and OPTIONAL when the Response Type of the
- // > request is "code token". It is a string value used to associate a
- // > Client session with an ID Token, and to mitigate replay attacks. The
- // > value is passed through unmodified from the Authentication Request to
- // > the ID Token. Sufficient entropy MUST be present in the nonce values
- // > used to prevent attackers from guessing values. For implementation
- // > notes, see Section 15.5.2.
- if (responseTypes.includes('id_token') && !parameters.nonce) {
- throw new InvalidParametersError(
- parameters,
- 'nonce is required for implicit and hybrid flows',
- )
- }
-
- // Make "expensive" checks after the "cheaper" checks
-
- if (parameters.id_token_hint != null) {
- const { payload } = await this.signer.verify(parameters.id_token_hint, {
- // these are meant to be outdated when used as a hint
- clockTolerance: Infinity,
- })
-
- if (!payload.sub) {
- throw new InvalidParametersError(
- parameters,
- `Unexpected empty id_token_hint "sub"`,
- )
- } else if (parameters.login_hint == null) {
- parameters = { ...parameters, login_hint: payload.sub }
- } else if (parameters.login_hint !== payload.sub) {
- throw new InvalidParametersError(
- parameters,
- 'login_hint does not match "sub" of id_token_hint',
- )
- }
- }
-
- // ATPROTO extension: if the client is not trusted, force users to consent
- // to authorization requests. We do this to avoid unauthenticated clients
- // from being able to silently re-authenticate users.
- if (clientAuth.method === 'none' && !client.info.isFirstParty) {
+ // ATPROTO extension: if the client is not trusted, and not authenticated,
+ // force users to consent to authorization requests. We do this to avoid
+ // unauthenticated clients from being able to silently re-authenticate
+ // users.
+ if (
+ !client.info.isTrusted &&
+ !client.info.isFirstParty &&
+ clientAuth.method === 'none'
+ ) {
if (parameters.prompt === 'none') {
throw new ConsentRequiredError(
parameters,
@@ -346,6 +338,7 @@ export class RequestManager {
uri,
expiresAt: updates.expiresAt || data.expiresAt,
parameters: data.parameters,
+ clientId: data.clientId,
clientAuth: data.clientAuth,
}
}
@@ -355,8 +348,7 @@ export class RequestManager {
uri: RequestUri,
deviceId: DeviceId,
account: Account,
- info: DeviceAccountInfo,
- ): Promise<{ code?: Code; token?: string; id_token?: string }> {
+ ): Promise<Code> {
const id = decodeRequestUri(uri)
const data = await this.store.readRequest(id)
@@ -385,18 +377,8 @@ export class RequestManager {
)
}
- const responseType = data.parameters.response_type.split(' ')
-
- if (responseType.includes('token')) {
- throw new AccessDeniedError(
- data.parameters,
- 'Implicit "token" forbidden (use "code" with PKCE instead)',
- )
- }
-
- const code = responseType.includes('code')
- ? await generateCode()
- : undefined
+ // Only response_type=code is supported
+ const code = await generateCode()
// Bind the request to the account, preventing it from being used again.
await this.store.updateRequest(id, {
@@ -406,15 +388,7 @@ export class RequestManager {
expiresAt: new Date(Date.now() + AUTHORIZATION_INACTIVITY_TIMEOUT),
})
- const id_token = responseType.includes('id_token')
- ? await this.signer.idToken(client, data.parameters, account, {
- auth_time: info.authenticatedAt,
- exp: this.createTokenExpiry(),
- code,
- })
- : undefined
-
- return { code, id_token }
+ return code
} catch (err) {
await this.store.deleteRequest(id)
throw err
diff --git a/packages/oauth/oauth-provider/src/signer/signer.ts b/packages/oauth/oauth-provider/src/signer/signer.ts
index 54af407e..8a4d3bb5 100644
--- a/packages/oauth/oauth-provider/src/signer/signer.ts
+++ b/packages/oauth/oauth-provider/src/signer/signer.ts
@@ -1,5 +1,3 @@
-import { randomBytes } from 'node:crypto'
-
import {
JwtPayload,
JwtPayloadGetter,
@@ -12,14 +10,10 @@ import {
OAuthAuthenticationRequestParameters,
OAuthAuthorizationDetails,
} from '@atproto/oauth-types'
-import { generate as hash } from 'oidc-token-hash'
import { Account } from '../account/account.js'
import { Client } from '../client/client.js'
-import { InvalidClientMetadataError } from '../errors/invalid-client-metadata-error.js'
import { dateToEpoch } from '../lib/util/date.js'
-import { claimRequested } from '../parameters/claims-requested.js'
-import { oidcPayload } from '../parameters/oidc-payload.js'
import { TokenId } from '../token/token-id.js'
import {
SignedTokenPayload,
@@ -105,61 +99,4 @@ export class Signer {
return result
}
-
- async idToken(
- client: Client,
- params: OAuthAuthenticationRequestParameters,
- account: Account,
- extra: {
- exp: Date
- iat?: Date
- auth_time?: Date
- code?: string
- access_token?: string
- },
- ): Promise<SignedJwt> {
- // This can happen when a client is using password_grant. If a client is
- // using password_grant, it should not set "require_auth_time" to true.
- if (client.metadata.require_auth_time && extra.auth_time == null) {
- throw new InvalidClientMetadataError(
- '"require_auth_time" metadata is not compatible with "password_grant" flow',
- )
- }
-
- return this.sign(
- {
- alg: client.metadata.id_token_signed_response_alg,
- typ: 'JWT',
- },
- async ({ alg }, key) => ({
- ...oidcPayload(params, account),
-
- aud: client.id,
- iat: dateToEpoch(extra.iat),
- exp: dateToEpoch(extra.exp),
- sub: account.sub,
- jti: randomBytes(16).toString('hex'),
- scope: params.scope,
- nonce: params.nonce,
-
- s_hash: params.state //
- ? await hash(params.state, alg, key.crv)
- : undefined,
- c_hash: extra.code //
- ? await hash(extra.code, alg, key.crv)
- : undefined,
- at_hash: extra.access_token //
- ? await hash(extra.access_token, alg, key.crv)
- : undefined,
-
- // https://openid.net/specs/openid-provider-authentication-policy-extension-1_0.html#rfc.section.5.2
- auth_time:
- client.metadata.require_auth_time ||
- (extra.auth_time != null && params.max_age != null) ||
- claimRequested(params, 'id_token', 'auth_time', extra.auth_time)
- ? dateToEpoch(extra.auth_time!)
- : undefined,
- }),
- )
- }
}
diff --git a/packages/oauth/oauth-provider/src/token/token-manager.ts b/packages/oauth/oauth-provider/src/token/token-manager.ts
index 9f9ea3cf..46a75e6d 100644
--- a/packages/oauth/oauth-provider/src/token/token-manager.ts
+++ b/packages/oauth/oauth-provider/src/token/token-manager.ts
@@ -1,4 +1,4 @@
-import { isSignedJwt, SignedJwt } from '@atproto/jwk'
+import { isSignedJwt } from '@atproto/jwk'
import {
AccessToken,
CLIENT_ASSERTION_TYPE_JWT_BEARER,
@@ -140,6 +140,10 @@ export class TokenManager {
if (!('code_verifier' in input) || !input.code_verifier) {
throw new InvalidGrantError('code_verifier is required')
}
+ // Prevent client from generating too short code_verifiers
+ if (input.code_verifier.length < 43) {
+ throw new InvalidGrantError('code_verifier too short')
+ }
switch (parameters.code_challenge_method) {
case undefined: // Default is "plain" (per spec)
case 'plain': {
@@ -181,8 +185,7 @@ export class TokenManager {
}
const tokenId = await generateTokenId()
- const scopes = parameters.scope?.split(' ')
- const refreshToken = scopes?.includes('offline_access')
+ const refreshToken = client.metadata.grant_types.includes('refresh_token')
? await generateRefreshToken()
: undefined
@@ -222,22 +225,10 @@ export class TokenManager {
authorization_details: authorizationDetails,
})
- const idToken = scopes?.includes('openid')
- ? await this.signer.idToken(client, parameters, account, {
- exp: expiresAt,
- iat: now,
- // If there is no deviceInfo, we are in a "password_grant" context
- auth_time: device?.info.authenticatedAt || new Date(),
- access_token: accessToken,
- code,
- })
- : undefined
-
return this.buildTokenResponse(
client,
accessToken,
refreshToken,
- idToken,
expiresAt,
parameters,
account,
@@ -249,7 +240,6 @@ export class TokenManager {
client: Client,
accessToken: AccessToken,
refreshToken: string | undefined,
- idToken: SignedJwt | undefined,
expiresAt: Date,
parameters: OAuthAuthenticationRequestParameters,
account: Account,
@@ -259,8 +249,7 @@ export class TokenManager {
access_token: accessToken,
token_type: parameters.dpop_jkt ? 'DPoP' : 'Bearer',
refresh_token: refreshToken,
- id_token: idToken,
- scope: parameters.scope ?? '',
+ scope: parameters.scope,
authorization_details: authorizationDetails,
get expires_in() {
return dateToRelativeSeconds(expiresAt)
@@ -272,12 +261,6 @@ export class TokenManager {
sub: account.sub,
}
- await this.hooks.onTokenResponse?.call(null, tokenResponse, {
- client,
- parameters,
- account,
- })
-
return tokenResponse
}
@@ -316,7 +299,7 @@ export class TokenManager {
throw new InvalidGrantError(`Invalid refresh token`)
}
- const { account, info, data } = tokenInfo
+ const { account, data } = tokenInfo
const { parameters } = data
try {
@@ -400,26 +383,10 @@ export class TokenManager {
authorization_details,
})
- // https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.3.1.3.3
- //
- // > In addition to the response parameters specified by OAuth 2.0, the
- // > following parameters MUST be included in the response:
- // > - id_token: ID Token value associated with the authenticated session.
- const scopes = parameters.scope?.split(' ')
- const idToken = scopes?.includes('openid')
- ? await this.signer.idToken(client, parameters, account, {
- exp: expiresAt,
- iat: now,
- auth_time: info?.authenticatedAt,
- access_token: accessToken,
- })
- : undefined
-
return this.buildTokenResponse(
client,
accessToken,
nextRefreshToken,
- idToken,
expiresAt,
parameters,
account,
diff --git a/packages/oauth/oauth-types/src/atproto-loopback-client-metadata.ts b/packages/oauth/oauth-types/src/atproto-loopback-client-metadata.ts
index f6a0346b..e93327b9 100644
--- a/packages/oauth/oauth-types/src/atproto-loopback-client-metadata.ts
+++ b/packages/oauth/oauth-types/src/atproto-loopback-client-metadata.ts
@@ -21,9 +21,8 @@ export function atprotoLoopbackClientMetadata(
return {
client_id: clientId,
client_name: 'Loopback client',
- response_types: ['code id_token', 'code'],
- grant_types: ['authorization_code', 'implicit', 'refresh_token'],
- scope: 'openid profile offline_access',
+ response_types: ['code'],
+ grant_types: ['authorization_code', 'refresh_token'],
redirect_uris: (redirectUris.length
? redirectUris
: (['127.0.0.1', '[::1]'] as const).map(
diff --git a/packages/oauth/oauth-types/src/oauth-authentication-request-parameters.ts b/packages/oauth/oauth-types/src/oauth-authentication-request-parameters.ts
index be02c606..2d060f53 100644
--- a/packages/oauth/oauth-types/src/oauth-authentication-request-parameters.ts
+++ b/packages/oauth/oauth-types/src/oauth-authentication-request-parameters.ts
@@ -3,6 +3,7 @@ import { z } from 'zod'
import { oauthAuthorizationDetailsSchema } from './oauth-authorization-details.js'
import { oauthClientIdSchema } from './oauth-client-id.js'
+import { oauthResponseTypeSchema } from './oauth-response-type.js'
import { oidcClaimsParameterSchema } from './oidc-claims-parameter.js'
import { oidcClaimsPropertiesSchema } from './oidc-claims-properties.js'
import { oidcEntityTypeSchema } from './oidc-entity-type.js'
@@ -17,19 +18,7 @@ export const oauthAuthenticationRequestParametersSchema = z.object({
nonce: z.string().optional(),
dpop_jkt: z.string().optional(),
- response_type: z.enum([
- // OAuth2 (https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-10#section-4.1.1)
- 'code',
- 'token',
-
- // OIDC (https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html)
- 'id_token',
- 'none',
- 'code token',
- 'code id_token',
- 'id_token token',
- 'code id_token token',
- ]),
+ response_type: oauthResponseTypeSchema,
// Default depend on response_type
response_mode: z.enum(['query', 'fragment', 'form_post']).optional(),
@@ -40,10 +29,13 @@ export const oauthAuthenticationRequestParametersSchema = z.object({
redirect_uri: z.string().url().optional(),
- // email profile openid (other?)
+ // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-11#section-1.4.1
+ // scope = scope-token *( SP scope-token )
+ // scope-token = 1*( %x21 / %x23-5B / %x5D-7E )
+ // = Basically most ASCII characters except backslash and double quote
scope: z
.string()
- .regex(/^[a-zA-Z0-9_]+( [a-zA-Z0-9_]+)*$/)
+ .regex(/^[!\x23-\x5B\x5D-\x7E]+( [!\x23-\x5B\x5D-\x7E]+)*$/)
.optional(),
// OIDC
diff --git a/packages/oauth/oauth-types/src/oauth-response-type.ts b/packages/oauth/oauth-types/src/oauth-response-type.ts
index 1f65d223..5f0269a3 100644
--- a/packages/oauth/oauth-types/src/oauth-response-type.ts
+++ b/packages/oauth/oauth-types/src/oauth-response-type.ts
@@ -1,11 +1,11 @@
import { z } from 'zod'
export const oauthResponseTypeSchema = z.enum([
- // OAuth
+ // OAuth2 (https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-10#section-4.1.1)
'code', // Authorization Code Grant
'token', // Implicit Grant
- // OpenID
+ // OIDC (https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html)
'none',
'code id_token token',
'code id_token',
diff --git a/packages/oauth/oauth-types/src/oauth-token-response.ts b/packages/oauth/oauth-types/src/oauth-token-response.ts
index 198f37c5..1f8cf772 100644
--- a/packages/oauth/oauth-types/src/oauth-token-response.ts
+++ b/packages/oauth/oauth-types/src/oauth-token-response.ts
@@ -12,7 +12,6 @@ export const oauthTokenResponseSchema = z
access_token: z.string(),
token_type: oauthTokenTypeSchema,
issuer: z.string().url().optional(),
- sub: z.string().optional(),
scope: z.string().optional(),
id_token: signedJwtSchema.optional(),
refresh_token: z.string().optional(),
diff --git a/packages/pds/src/api/com/atproto/admin/sendEmail.ts b/packages/pds/src/api/com/atproto/admin/sendEmail.ts
index 2cd48307..491cf1b7 100644
--- a/packages/pds/src/api/com/atproto/admin/sendEmail.ts
+++ b/packages/pds/src/api/com/atproto/admin/sendEmail.ts
@@ -26,14 +26,14 @@ export default function (server: Server, ctx: AppContext) {
if (ctx.entrywayAgent) {
assert(ctx.cfg.entryway)
return resultPassthru(
- await ctx.entrywayAgent.com.atproto.admin.sendEmail(input.body, {
- encoding: 'application/json',
- ...(await ctx.serviceAuthHeaders(
+ await ctx.entrywayAgent.com.atproto.admin.sendEmail(
+ input.body,
+ await ctx.serviceAuthHeaders(
recipientDid,
ctx.cfg.entryway.did,
ids.ComAtprotoAdminSendEmail,
- )),
- }),
+ ),
+ ),
)
}
diff --git a/packages/pds/src/api/com/atproto/admin/updateAccountEmail.ts b/packages/pds/src/api/com/atproto/admin/updateAccountEmail.ts
index cf1aefa0..421cc307 100644
--- a/packages/pds/src/api/com/atproto/admin/updateAccountEmail.ts
+++ b/packages/pds/src/api/com/atproto/admin/updateAccountEmail.ts
@@ -1,7 +1,7 @@
import { InvalidRequestError } from '@atproto/xrpc-server'
import { Server } from '../../../../lexicon'
import AppContext from '../../../../context'
-import { authPassthru } from '../../../proxy'
+import { authPassthru } from './util'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.admin.updateAccountEmail({
diff --git a/packages/pds/src/api/com/atproto/admin/updateAccountPassword.ts b/packages/pds/src/api/com/atproto/admin/updateAccountPassword.ts
index f5536783..0c7a0f8c 100644
--- a/packages/pds/src/api/com/atproto/admin/updateAccountPassword.ts
+++ b/packages/pds/src/api/com/atproto/admin/updateAccountPassword.ts
@@ -1,6 +1,6 @@
import { Server } from '../../../../lexicon'
import AppContext from '../../../../context'
-import { authPassthru } from '../../../proxy'
+import { authPassthru } from './util'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.admin.updateAccountPassword({
diff --git a/packages/pds/src/api/com/atproto/identity/requestPlcOperationSignature.ts b/packages/pds/src/api/com/atproto/identity/requestPlcOperationSignature.ts
index 4aab6520..d611eddc 100644
--- a/packages/pds/src/api/com/atproto/identity/requestPlcOperationSignature.ts
+++ b/packages/pds/src/api/com/atproto/identity/requestPlcOperationSignature.ts
@@ -1,16 +1,24 @@
-import { Server } from '../../../../lexicon'
-import AppContext from '../../../../context'
+import assert from 'node:assert'
+
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { authPassthru } from '../../../proxy'
+
+import AppContext from '../../../../context'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.identity.requestPlcOperationSignature({
auth: ctx.authVerifier.accessFull(),
- handler: async ({ auth, req }) => {
+ handler: async ({ auth }) => {
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
await ctx.entrywayAgent.com.atproto.identity.requestPlcOperationSignature(
undefined,
- authPassthru(req),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoIdentityRequestPlcOperationSignature,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/identity/signPlcOperation.ts b/packages/pds/src/api/com/atproto/identity/signPlcOperation.ts
index eb2d05e9..3b1f646c 100644
--- a/packages/pds/src/api/com/atproto/identity/signPlcOperation.ts
+++ b/packages/pds/src/api/com/atproto/identity/signPlcOperation.ts
@@ -1,19 +1,28 @@
-import { Server } from '../../../../lexicon'
-import AppContext from '../../../../context'
-import * as plc from '@did-plc/lib'
+import assert from 'node:assert'
+
import { check } from '@atproto/common'
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { authPassthru, resultPassthru } from '../../../proxy'
+import * as plc from '@did-plc/lib'
+
+import AppContext from '../../../../context'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
+import { resultPassthru } from '../../../proxy'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.identity.signPlcOperation({
auth: ctx.authVerifier.accessFull(),
- handler: async ({ auth, input, req }) => {
+ handler: async ({ auth, input }) => {
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
return resultPassthru(
await ctx.entrywayAgent.com.atproto.identity.signPlcOperation(
input.body,
- authPassthru(req, true),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoIdentitySignPlcOperation,
+ ),
),
)
}
diff --git a/packages/pds/src/api/com/atproto/identity/updateHandle.ts b/packages/pds/src/api/com/atproto/identity/updateHandle.ts
index 591488f4..7b0863a2 100644
--- a/packages/pds/src/api/com/atproto/identity/updateHandle.ts
+++ b/packages/pds/src/api/com/atproto/identity/updateHandle.ts
@@ -1,10 +1,11 @@
+import assert from 'node:assert'
import { InvalidRequestError } from '@atproto/xrpc-server'
import { DAY, MINUTE } from '@atproto/common'
import { normalizeAndValidateHandle } from '../../../../handle'
import { Server } from '../../../../lexicon'
import AppContext from '../../../../context'
import { httpLogger } from '../../../../logger'
-import { authPassthru } from '../../../proxy'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.identity.updateHandle({
@@ -21,16 +22,22 @@ export default function (server: Server, ctx: AppContext) {
calcKey: ({ auth }) => auth.credentials.did,
},
],
- handler: async ({ auth, input, req }) => {
+ handler: async ({ auth, input }) => {
const requester = auth.credentials.did
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
+
// the full flow is:
// -> entryway(identity.updateHandle) [update handle, submit plc op]
// -> pds(admin.updateAccountHandle) [track handle, sequence handle update]
await ctx.entrywayAgent.com.atproto.identity.updateHandle(
{ did: requester, handle: input.body.handle },
- authPassthru(req, true),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoIdentityUpdateHandle,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/server/activateAccount.ts b/packages/pds/src/api/com/atproto/server/activateAccount.ts
index c46c73e5..8d7c5047 100644
--- a/packages/pds/src/api/com/atproto/server/activateAccount.ts
+++ b/packages/pds/src/api/com/atproto/server/activateAccount.ts
@@ -1,20 +1,29 @@
+import assert from 'node:assert'
+
import { CidSet } from '@atproto/repo'
-import { InvalidRequestError } from '@atproto/xrpc-server'
import { INVALID_HANDLE } from '@atproto/syntax'
-import { Server } from '../../../../lexicon'
+import { InvalidRequestError } from '@atproto/xrpc-server'
+
import AppContext from '../../../../context'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
import { assertValidDidDocumentForService } from './util'
-import { authPassthru } from '../../../proxy'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.activateAccount({
auth: ctx.authVerifier.accessFull(),
- handler: async ({ auth, req }) => {
+ handler: async ({ auth }) => {
// in the case of entryway, the full flow is activateAccount (PDS) -> activateAccount (Entryway) -> updateSubjectStatus(PDS)
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
+
await ctx.entrywayAgent.com.atproto.server.activateAccount(
undefined,
- authPassthru(req),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerActivateAccount,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/server/confirmEmail.ts b/packages/pds/src/api/com/atproto/server/confirmEmail.ts
index 11a935ce..d01891e2 100644
--- a/packages/pds/src/api/com/atproto/server/confirmEmail.ts
+++ b/packages/pds/src/api/com/atproto/server/confirmEmail.ts
@@ -1,12 +1,15 @@
-import { Server } from '../../../../lexicon'
-import AppContext from '../../../../context'
+import assert from 'node:assert'
+
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { authPassthru } from '../../../proxy'
+
+import AppContext from '../../../../context'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.confirmEmail({
auth: ctx.authVerifier.accessStandard({ checkTakedown: true }),
- handler: async ({ auth, input, req }) => {
+ handler: async ({ auth, input }) => {
const did = auth.credentials.did
const user = await ctx.accountManager.getAccount(did, {
@@ -17,9 +20,14 @@ export default function (server: Server, ctx: AppContext) {
}
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
await ctx.entrywayAgent.com.atproto.server.confirmEmail(
input.body,
- authPassthru(req, true),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerConfirmEmail,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/server/createAppPassword.ts b/packages/pds/src/api/com/atproto/server/createAppPassword.ts
index 2edf2c42..3dcd1c6e 100644
--- a/packages/pds/src/api/com/atproto/server/createAppPassword.ts
+++ b/packages/pds/src/api/com/atproto/server/createAppPassword.ts
@@ -1,18 +1,27 @@
+import assert from 'node:assert'
+
import AppContext from '../../../../context'
import { Server } from '../../../../lexicon'
-import { authPassthru, resultPassthru } from '../../../proxy'
+import { ids } from '../../../../lexicon/lexicons'
+import { resultPassthru } from '../../../proxy'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.createAppPassword({
auth: ctx.authVerifier.accessFull({
checkTakedown: true,
}),
- handler: async ({ auth, input, req }) => {
+ handler: async ({ auth, input }) => {
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
+
return resultPassthru(
await ctx.entrywayAgent.com.atproto.server.createAppPassword(
input.body,
- authPassthru(req, true),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerCreateAppPassword,
+ ),
),
)
}
diff --git a/packages/pds/src/api/com/atproto/server/deactivateAccount.ts b/packages/pds/src/api/com/atproto/server/deactivateAccount.ts
index 48f64c97..a73bce60 100644
--- a/packages/pds/src/api/com/atproto/server/deactivateAccount.ts
+++ b/packages/pds/src/api/com/atproto/server/deactivateAccount.ts
@@ -1,16 +1,23 @@
-import { Server } from '../../../../lexicon'
+import assert from 'node:assert'
+
import AppContext from '../../../../context'
-import { authPassthru } from '../../../proxy'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.deactivateAccount({
auth: ctx.authVerifier.accessFull(),
- handler: async ({ auth, input, req }) => {
+ handler: async ({ auth, input }) => {
// in the case of entryway, the full flow is deactivateAccount (PDS) -> deactivateAccount (Entryway) -> updateSubjectStatus(PDS)
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
await ctx.entrywayAgent.com.atproto.server.deactivateAccount(
input.body,
- authPassthru(req, true),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerDeactivateAccount,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/server/getAccountInviteCodes.ts b/packages/pds/src/api/com/atproto/server/getAccountInviteCodes.ts
index da848214..655921e9 100644
--- a/packages/pds/src/api/com/atproto/server/getAccountInviteCodes.ts
+++ b/packages/pds/src/api/com/atproto/server/getAccountInviteCodes.ts
@@ -1,19 +1,28 @@
+import assert from 'node:assert'
+
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { Server } from '../../../../lexicon'
-import AppContext from '../../../../context'
-import { genInvCodes } from './util'
+
import { CodeDetail } from '../../../../account-manager/helpers/invite'
-import { authPassthru, resultPassthru } from '../../../proxy'
+import AppContext from '../../../../context'
+import { Server } from '../../../../lexicon'
+import { resultPassthru } from '../../../proxy'
+import { genInvCodes } from './util'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.getAccountInviteCodes({
auth: ctx.authVerifier.accessFull({ checkTakedown: true }),
- handler: async ({ params, auth, req }) => {
+ handler: async ({ params, auth }) => {
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
return resultPassthru(
await ctx.entrywayAgent.com.atproto.server.getAccountInviteCodes(
params,
- authPassthru(req),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerGetAccountInviteCodes,
+ ),
),
)
}
diff --git a/packages/pds/src/api/com/atproto/server/getServiceAuth.ts b/packages/pds/src/api/com/atproto/server/getServiceAuth.ts
index 6436db59..7a9a2c7a 100644
--- a/packages/pds/src/api/com/atproto/server/getServiceAuth.ts
+++ b/packages/pds/src/api/com/atproto/server/getServiceAuth.ts
@@ -2,7 +2,7 @@ import { InvalidRequestError, createServiceJwt } from '@atproto/xrpc-server'
import { HOUR, MINUTE } from '@atproto/common'
import AppContext from '../../../../context'
import { Server } from '../../../../lexicon'
-import { PRIVILEGED_METHODS } from '../../../../pipethrough'
+import { PRIVILEGED_METHODS, PROTECTED_METHODS } from '../../../../pipethrough'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.getServiceAuth({
@@ -30,15 +30,20 @@ export default function (server: Server, ctx: AppContext) {
)
}
}
- if (
- !auth.credentials.isPrivileged &&
- lxm &&
- PRIVILEGED_METHODS.has(lxm)
- ) {
- throw new InvalidRequestError(
- `cannot request a service auth token for the following method with an app password: ${lxm}`,
- )
+
+ if (lxm) {
+ if (PROTECTED_METHODS.has(lxm)) {
+ throw new InvalidRequestError(
+ `cannot request a service auth token for the following protected method: ${lxm}`,
+ )
+ }
+ if (!auth.credentials.isPrivileged && PRIVILEGED_METHODS.has(lxm)) {
+ throw new InvalidRequestError(
+ `insufficient access to request a service auth token for the following method: ${lxm}`,
+ )
+ }
}
+
const keypair = await ctx.actorStore.keypair(did)
const token = await createServiceJwt({
diff --git a/packages/pds/src/api/com/atproto/server/listAppPasswords.ts b/packages/pds/src/api/com/atproto/server/listAppPasswords.ts
index 65db42c3..7414e345 100644
--- a/packages/pds/src/api/com/atproto/server/listAppPasswords.ts
+++ b/packages/pds/src/api/com/atproto/server/listAppPasswords.ts
@@ -1,16 +1,24 @@
+import assert from 'node:assert'
+
import AppContext from '../../../../context'
import { Server } from '../../../../lexicon'
-import { authPassthru, resultPassthru } from '../../../proxy'
+import { ids } from '../../../../lexicon/lexicons'
+import { resultPassthru } from '../../../proxy'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.listAppPasswords({
auth: ctx.authVerifier.accessStandard(),
- handler: async ({ auth, req }) => {
+ handler: async ({ auth }) => {
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
return resultPassthru(
await ctx.entrywayAgent.com.atproto.server.listAppPasswords(
undefined,
- authPassthru(req),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerListAppPasswords,
+ ),
),
)
}
diff --git a/packages/pds/src/api/com/atproto/server/requestAccountDelete.ts b/packages/pds/src/api/com/atproto/server/requestAccountDelete.ts
index e00f1e57..3068ff45 100644
--- a/packages/pds/src/api/com/atproto/server/requestAccountDelete.ts
+++ b/packages/pds/src/api/com/atproto/server/requestAccountDelete.ts
@@ -1,8 +1,11 @@
+import assert from 'node:assert'
+
import { DAY, HOUR } from '@atproto/common'
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { Server } from '../../../../lexicon'
+
import AppContext from '../../../../context'
-import { authPassthru } from '../../../proxy'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.requestAccountDelete({
@@ -19,7 +22,7 @@ export default function (server: Server, ctx: AppContext) {
},
],
auth: ctx.authVerifier.accessFull({ checkTakedown: true }),
- handler: async ({ auth, req }) => {
+ handler: async ({ auth }) => {
const did = auth.credentials.did
const account = await ctx.accountManager.getAccount(did, {
includeDeactivated: true,
@@ -30,9 +33,14 @@ export default function (server: Server, ctx: AppContext) {
}
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
await ctx.entrywayAgent.com.atproto.server.requestAccountDelete(
undefined,
- authPassthru(req),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerRequestAccountDelete,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/server/requestEmailConfirmation.ts b/packages/pds/src/api/com/atproto/server/requestEmailConfirmation.ts
index bebd2d32..6e1724f4 100644
--- a/packages/pds/src/api/com/atproto/server/requestEmailConfirmation.ts
+++ b/packages/pds/src/api/com/atproto/server/requestEmailConfirmation.ts
@@ -1,8 +1,11 @@
+import assert from 'node:assert'
+
import { DAY, HOUR } from '@atproto/common'
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { Server } from '../../../../lexicon'
+
import AppContext from '../../../../context'
-import { authPassthru } from '../../../proxy'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.requestEmailConfirmation({
@@ -19,7 +22,7 @@ export default function (server: Server, ctx: AppContext) {
},
],
auth: ctx.authVerifier.accessStandard({ checkTakedown: true }),
- handler: async ({ auth, req }) => {
+ handler: async ({ auth }) => {
const did = auth.credentials.did
const account = await ctx.accountManager.getAccount(did, {
includeDeactivated: true,
@@ -30,9 +33,14 @@ export default function (server: Server, ctx: AppContext) {
}
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
await ctx.entrywayAgent.com.atproto.server.requestEmailConfirmation(
undefined,
- authPassthru(req),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerRequestEmailConfirmation,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/server/requestEmailUpdate.ts b/packages/pds/src/api/com/atproto/server/requestEmailUpdate.ts
index 5639deba..fa7b0f22 100644
--- a/packages/pds/src/api/com/atproto/server/requestEmailUpdate.ts
+++ b/packages/pds/src/api/com/atproto/server/requestEmailUpdate.ts
@@ -1,8 +1,12 @@
+import assert from 'node:assert'
+
import { DAY, HOUR } from '@atproto/common'
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { Server } from '../../../../lexicon'
+
import AppContext from '../../../../context'
-import { authPassthru, resultPassthru } from '../../../proxy'
+import { Server } from '../../../../lexicon'
+import { resultPassthru } from '../../../proxy'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.requestEmailUpdate({
@@ -19,7 +23,7 @@ export default function (server: Server, ctx: AppContext) {
},
],
auth: ctx.authVerifier.accessStandard({ checkTakedown: true }),
- handler: async ({ auth, req }) => {
+ handler: async ({ auth }) => {
const did = auth.credentials.did
const account = await ctx.accountManager.getAccount(did, {
includeDeactivated: true,
@@ -30,10 +34,15 @@ export default function (server: Server, ctx: AppContext) {
}
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
return resultPassthru(
await ctx.entrywayAgent.com.atproto.server.requestEmailUpdate(
undefined,
- authPassthru(req),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerRequestEmailUpdate,
+ ),
),
)
}
diff --git a/packages/pds/src/api/com/atproto/server/revokeAppPassword.ts b/packages/pds/src/api/com/atproto/server/revokeAppPassword.ts
index 3506cdc2..2d1c9f2e 100644
--- a/packages/pds/src/api/com/atproto/server/revokeAppPassword.ts
+++ b/packages/pds/src/api/com/atproto/server/revokeAppPassword.ts
@@ -1,15 +1,22 @@
+import assert from 'node:assert'
+
import AppContext from '../../../../context'
import { Server } from '../../../../lexicon'
-import { authPassthru } from '../../../proxy'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.revokeAppPassword({
auth: ctx.authVerifier.accessStandard(),
- handler: async ({ auth, input, req }) => {
+ handler: async ({ auth, input }) => {
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
await ctx.entrywayAgent.com.atproto.server.revokeAppPassword(
input.body,
- authPassthru(req, true),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerRevokeAppPassword,
+ ),
)
return
}
diff --git a/packages/pds/src/api/com/atproto/server/updateEmail.ts b/packages/pds/src/api/com/atproto/server/updateEmail.ts
index 161950c4..f6bc781b 100644
--- a/packages/pds/src/api/com/atproto/server/updateEmail.ts
+++ b/packages/pds/src/api/com/atproto/server/updateEmail.ts
@@ -1,14 +1,17 @@
-import disposable from 'disposable-email'
+import assert from 'node:assert'
+
import { InvalidRequestError } from '@atproto/xrpc-server'
-import { Server } from '../../../../lexicon'
-import AppContext from '../../../../context'
-import { authPassthru } from '../../../proxy'
+import disposable from 'disposable-email'
+
import { UserAlreadyExistsError } from '../../../../account-manager/helpers/account'
+import AppContext from '../../../../context'
+import { Server } from '../../../../lexicon'
+import { ids } from '../../../../lexicon/lexicons'
export default function (server: Server, ctx: AppContext) {
server.com.atproto.server.updateEmail({
auth: ctx.authVerifier.accessFull({ checkTakedown: true }),
- handler: async ({ auth, input, req }) => {
+ handler: async ({ auth, input }) => {
const did = auth.credentials.did
const { token, email } = input.body
if (!disposable.validate(email)) {
@@ -24,9 +27,14 @@ export default function (server: Server, ctx: AppContext) {
}
if (ctx.entrywayAgent) {
+ assert(ctx.cfg.entryway)
await ctx.entrywayAgent.com.atproto.server.updateEmail(
input.body,
- authPassthru(req, true),
+ await ctx.serviceAuthHeaders(
+ auth.credentials.did,
+ ctx.cfg.entryway.did,
+ ids.ComAtprotoServerUpdateEmail,
+ ),
)
return
}
diff --git a/packages/pds/src/api/proxy.ts b/packages/pds/src/api/proxy.ts
index ad16a337..fc51115e 100644
--- a/packages/pds/src/api/proxy.ts
+++ b/packages/pds/src/api/proxy.ts
@@ -37,7 +37,11 @@ export function authPassthru(req: IncomingMessage, withEncoding?: boolean) {
// This is fine since app views are usually called using the requester's
// credentials when "auth.credentials.type === 'access'", which is the only
// case were DPoP is used.
- if (authorization.startsWith('DPoP ') || req.headers['dpop']) {
+ const [type] = authorization.split(' ', 1)
+ if (!type) {
+ throw new InvalidRequestError('Invalid authorization header')
+ }
+ if (type.toLowerCase() === 'dpop' || req.headers['dpop']) {
throw new InvalidRequestError('DPoP requests cannot be proxied')
}
diff --git a/packages/pds/src/auth-routes.ts b/packages/pds/src/auth-routes.ts
index 77fc23aa..ef513bce 100644
--- a/packages/pds/src/auth-routes.ts
+++ b/packages/pds/src/auth-routes.ts
@@ -11,11 +11,13 @@ export const createRouter = ({ authProvider, cfg }: AppContext): Router => {
resource: cfg.service.publicUrl,
authorization_servers: [cfg.entryway?.url ?? cfg.service.publicUrl],
bearer_methods_supported: ['header'],
- scopes_supported: ['profile', 'email', 'phone'],
+ scopes_supported: [],
resource_documentation: 'https://atproto.com',
})
router.get('/.well-known/oauth-protected-resource', (req, res) => {
+ res.setHeader('Access-Control-Allow-Origin', '*')
+ res.setHeader('Access-Control-Allow-Method', '*')
res.status(200).json(oauthProtectedResourceMetadata)
})
diff --git a/packages/pds/src/auth-verifier.ts b/packages/pds/src/auth-verifier.ts
index 3b764e4e..02aefdd4 100644
--- a/packages/pds/src/auth-verifier.ts
+++ b/packages/pds/src/auth-verifier.ts
@@ -452,13 +452,6 @@ export class AuthVerifier {
ctx: ReqCtx,
scopes: AuthScope[],
): Promise<AccessOutput> {
- if (!scopes.includes(AuthScope.Access)) {
- throw new InvalidRequestError(
- 'DPoP access token cannot be used for this request',
- 'InvalidToken',
- )
- }
-
this.setAuthHeaders(ctx)
const { req } = ctx
@@ -489,13 +482,48 @@ export class AuthVerifier {
throw new InvalidRequestError('Malformed token', 'InvalidToken')
}
+ const tokenScopes = new Set(result.claims.scope?.split(' '))
+
+ if (!tokenScopes.has('transition:generic')) {
+ throw new AuthRequiredError(
+ 'Missing required scope: transition:generic',
+ 'InvalidToken',
+ )
+ }
+
+ const scopeEquivalent: AuthScope = tokenScopes.has('transition:chat.bsky')
+ ? AuthScope.AppPassPrivileged
+ : AuthScope.AppPass
+
+ if (!scopes.includes(scopeEquivalent)) {
+ // AppPassPrivileged is sufficient but was not provided "transition:chat.bsky"
+ if (scopes.includes(AuthScope.AppPassPrivileged)) {
+ throw new InvalidRequestError(
+ 'Missing required scope: transition:chat.bsky',
+ 'InvalidToken',
+ )
+ }
+
+ // AuthScope.Access and AuthScope.SignupQueued do not have an OAuth
+ // scope equivalent.
+ throw new InvalidRequestError(
+ 'DPoP access token cannot be used for this request',
+ 'InvalidToken',
+ )
+ }
+
+ const isPrivileged = [
+ AuthScope.Access,
+ AuthScope.AppPassPrivileged,
+ ].includes(scopeEquivalent)
+
return {
credentials: {
type: 'access',
did: result.claims.sub,
- scope: AuthScope.Access,
+ scope: scopeEquivalent,
audience: this.dids.pds,
- isPrivileged: true,
+ isPrivileged,
},
artifacts: result.token,
}
diff --git a/packages/pds/src/index.ts b/packages/pds/src/index.ts
index 493120a8..1a13b68d 100644
--- a/packages/pds/src/index.ts
+++ b/packages/pds/src/index.ts
@@ -54,12 +54,6 @@ export class PDS {
secrets: ServerSecrets,
overrides?: Partial<AppContextOptions>,
): Promise<PDS> {
- const app = express()
- app.set('trust proxy', true)
- app.use(cors({ maxAge: DAY / SECOND }))
- app.use(loggerMiddleware)
- app.use(compression())
-
const ctx = await AppContext.fromConfig(cfg, secrets, overrides)
const xrpcOpts: XrpcServerOptions = {
@@ -100,7 +94,12 @@ export class PDS {
server = API(server, ctx)
- app.use(authRoutes.createRouter(ctx))
+ const app = express()
+ app.set('trust proxy', true)
+ app.use(loggerMiddleware)
+ app.use(compression())
+ app.use(authRoutes.createRouter(ctx)) // Before CORS
+ app.use(cors({ maxAge: DAY / SECOND }))
app.use(basicRoutes.createRouter(ctx))
app.use(wellKnown.createRouter(ctx))
app.use(server.xrpc.router)
diff --git a/packages/pds/src/oauth/provider.ts b/packages/pds/src/oauth/provider.ts
index 3430c463..2fb039dd 100644
--- a/packages/pds/src/oauth/provider.ts
+++ b/packages/pds/src/oauth/provider.ts
@@ -44,6 +44,8 @@ export class PdsOAuthProvider extends OAuthProvider {
// & resource server, in which case the issuer origin is also the
// resource server uri.
protected_resources: [new URL(issuer).origin],
+
+ scopes_supported: ['transition:generic', 'transition:chat.bsky'],
},
accountStore: new DetailedAccountStore(
diff --git a/packages/pds/src/pipethrough.ts b/packages/pds/src/pipethrough.ts
index f48b1821..1d9d348b 100644
--- a/packages/pds/src/pipethrough.ts
+++ b/packages/pds/src/pipethrough.ts
@@ -22,7 +22,10 @@ export const proxyHandler = (ctx: AppContext): CatchallHandler => {
try {
const { url, aud, nsid } = await formatUrlAndAud(ctx, req)
const auth = await accessStandard({ req, res })
- if (!auth.credentials.isPrivileged && PRIVILEGED_METHODS.has(nsid)) {
+ if (
+ PROTECTED_METHODS.has(nsid) ||
+ (!auth.credentials.isPrivileged && PRIVILEGED_METHODS.has(nsid))
+ ) {
throw new InvalidRequestError('Bad token method', 'InvalidToken')
}
const headers = await formatHeaders(ctx, req, {
@@ -276,6 +279,27 @@ export const PRIVILEGED_METHODS = new Set([
ids.ComAtprotoServerCreateAccount,
])
+// These endpoints are related to account management and must be used directly,
+// not proxied or service-authed. Service auth may be utilized between PDS and
+// entryway for these methods.
+export const PROTECTED_METHODS = new Set([
+ ids.ComAtprotoAdminSendEmail,
+ ids.ComAtprotoIdentityRequestPlcOperationSignature,
+ ids.ComAtprotoIdentitySignPlcOperation,
+ ids.ComAtprotoIdentityUpdateHandle,
+ ids.ComAtprotoServerActivateAccount,
+ ids.ComAtprotoServerConfirmEmail,
+ ids.ComAtprotoServerCreateAppPassword,
+ ids.ComAtprotoServerDeactivateAccount,
+ ids.ComAtprotoServerGetAccountInviteCodes,
+ ids.ComAtprotoServerListAppPasswords,
+ ids.ComAtprotoServerRequestAccountDelete,
+ ids.ComAtprotoServerRequestEmailConfirmation,
+ ids.ComAtprotoServerRequestEmailUpdate,
+ ids.ComAtprotoServerRevokeAppPassword,
+ ids.ComAtprotoServerUpdateEmail,
+])
+
const defaultService = (
ctx: AppContext,
nsid: string,
diff --git a/packages/pds/tests/app-passwords.test.ts b/packages/pds/tests/app-passwords.test.ts
index 00130dc6..35bae752 100644
--- a/packages/pds/tests/app-passwords.test.ts
+++ b/packages/pds/tests/app-passwords.test.ts
@@ -118,7 +118,7 @@ describe('app_passwords', () => {
lxm: 'com.atproto.server.createAccount',
})
await expect(attempt).rejects.toThrow(
- /cannot request a service auth token for the following method with an app password/,
+ /insufficient access to request a service auth token for the following method/,
)
})
@@ -159,7 +159,7 @@ describe('app_passwords', () => {
lxm: 'com.atproto.server.createAccount',
})
await expect(priviAttempt).rejects.toThrow(
- /cannot request a service auth token for the following method with an app password/,
+ /insufficient access to request a service auth token for the following method/,
)
// allows only full access auth
diff --git a/packages/pds/tests/entryway.test.ts b/packages/pds/tests/entryway.test.ts
index ceb72784..30c5c376 100644
--- a/packages/pds/tests/entryway.test.ts
+++ b/packages/pds/tests/entryway.test.ts
@@ -1,6 +1,9 @@
import * as os from 'node:os'
import * as path from 'node:path'
+import assert from 'node:assert'
+import { decodeJwt } from 'jose'
import * as plcLib from '@did-plc/lib'
+import { parseReqNsid } from '@atproto/xrpc-server'
import { AtpAgent } from '@atproto/api'
import { Secp256k1Keypair, randomStr } from '@atproto/crypto'
import { SeedClient, TestPds, TestPlc, mockResolvers } from '@atproto/dev-env'
@@ -114,10 +117,11 @@ describe('entryway', () => {
it('updates handle from entryway.', async () => {
await entrywayAgent.api.com.atproto.identity.updateHandle(
{ handle: 'alice3.test' },
- {
- headers: SeedClient.getHeaders(accessToken),
- encoding: 'application/json',
- },
+ await pds.ctx.serviceAuthHeaders(
+ alice,
+ 'did:example:entryway',
+ 'com.atproto.identity.updateHandle',
+ ),
)
const doc = await entryway.ctx.idResolver.did.resolve(alice)
const handleToDid =
@@ -182,6 +186,28 @@ const createEntryway = async (
const server = await pdsEntryway.PDS.create(cfg, secrets)
await server.ctx.db.migrateToLatestOrThrow()
await server.start()
+ // patch entryway access token verification to handle internal service auth pds -> entryway
+ const origValidateAccessToken =
+ server.ctx.authVerifier.validateAccessToken.bind(server.ctx.authVerifier)
+ server.ctx.authVerifier.validateAccessToken = async (req, scopes) => {
+ const jwt = req.headers.authorization?.replace('Bearer ', '') ?? ''
+ const claims = decodeJwt(jwt)
+ if (claims.aud === 'did:example:entryway') {
+ assert(claims.lxm === parseReqNsid(req), 'bad lxm claim in service auth')
+ assert(claims.aud, 'missing aud claim in service auth')
+ assert(claims.iss, 'missing iss claim in service auth')
+ return {
+ artifacts: jwt,
+ credentials: {
+ type: 'access',
+ scope: 'com.atproto.access' as any,
+ audience: claims.aud,
+ did: claims.iss,
+ },
+ }
+ }
+ return origValidateAccessToken(req, scopes)
+ }
// @TODO temp hack because entryway teardown calls signupActivator.run() by mistake
server.ctx.signupActivator.run = server.ctx.signupActivator.destroy
return server
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index d48c2a5e..80304df1 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -963,9 +963,6 @@ importers:
jose:
specifier: ^5.2.0
version: 5.3.0
- oidc-token-hash:
- specifier: ^5.0.3
- version: 5.0.3
psl:
specifier: ^1.9.0
version: 1.9.0
@@ -10497,11 +10494,6 @@ packages:
object-keys: 1.1.1
dev: true
- /oidc-token-hash@5.0.3:
- resolution: {integrity: sha512-IF4PcGgzAr6XXSff26Sk/+P4KZFJVuHAJZj3wgO3vX2bMdNVp/QXTP3P7CEm9V1IdG8lDLY3HhiqpsE/nOwpPw==}
- engines: {node: ^10.13.0 || >=12.0.0}
- dev: false
-
/on-exit-leak-free@2.1.0:
resolution: {integrity: sha512-VuCaZZAjReZ3vUwgOB8LxAosIurDiAW0s13rI1YwmaP++jvcxP77AWoQvenZebpCA2m8WC1/EosPYPMjnRAp/w==}