diff --git a/.changeset/angry-spiders-hunt.md b/.changeset/angry-spiders-hunt.md new file mode 100644 index 00000000000..5b837103e31 --- /dev/null +++ b/.changeset/angry-spiders-hunt.md @@ -0,0 +1,5 @@ +--- +"@atproto/oauth-types": patch +--- + +Add oauthClientIdLoopbackSchema and oauthClientIdDiscoverableSchema schemas diff --git a/.changeset/cyan-parrots-obey.md b/.changeset/cyan-parrots-obey.md new file mode 100644 index 00000000000..2e0f748716d --- /dev/null +++ b/.changeset/cyan-parrots-obey.md @@ -0,0 +1,5 @@ +--- +"@atproto/oauth-client": patch +--- + +Verify authorization_endpoint URL protocol diff --git a/.changeset/green-cameras-grin.md b/.changeset/green-cameras-grin.md new file mode 100644 index 00000000000..d944cb14c3e --- /dev/null +++ b/.changeset/green-cameras-grin.md @@ -0,0 +1,5 @@ +--- +"@atproto/oauth-client": patch +--- + +Ensure that client-id is a web url diff --git a/.changeset/little-pets-sit.md b/.changeset/little-pets-sit.md new file mode 100644 index 00000000000..ac87f8c03d5 --- /dev/null +++ b/.changeset/little-pets-sit.md @@ -0,0 +1,5 @@ +--- +"@atproto/oauth-client": patch +--- + +Improve message of OAuthResolverError in case of metadata validation error diff --git a/.changeset/new-badgers-cross.md b/.changeset/new-badgers-cross.md new file mode 100644 index 00000000000..b4b8873c22f --- /dev/null +++ b/.changeset/new-badgers-cross.md @@ -0,0 +1,5 @@ +--- +"@atproto/pds": patch +--- + +Prevent use of non https: resource uri in production environments diff --git a/.changeset/small-apes-retire.md b/.changeset/small-apes-retire.md new file mode 100644 index 00000000000..b14c53e0467 --- /dev/null +++ b/.changeset/small-apes-retire.md @@ -0,0 +1,5 @@ +--- +"@atproto/oauth-types": patch +--- + +Enforce use of http and https url where applicable diff --git a/.changeset/spotty-owls-drum.md b/.changeset/spotty-owls-drum.md new file mode 100644 index 00000000000..6f667035d04 --- /dev/null +++ b/.changeset/spotty-owls-drum.md @@ -0,0 +1,5 @@ +--- +"@atproto/oauth-types": patch +--- + +Strong validation or redirect_uri diff --git a/packages/oauth/oauth-client/src/atproto-token-response.ts b/packages/oauth/oauth-client/src/atproto-token-response.ts index 5c5b9e81967..08c0c56ed01 100644 --- a/packages/oauth/oauth-client/src/atproto-token-response.ts +++ b/packages/oauth/oauth-client/src/atproto-token-response.ts @@ -1,6 +1,6 @@ import { atprotoDidSchema } from '@atproto/did' import { oauthTokenResponseSchema } from '@atproto/oauth-types' -import { z } from 'zod' +import { z, TypeOf } from 'zod' import { includesSpaceSeparatedValue, SpaceSeparatedValue } from './util' @@ -19,4 +19,4 @@ export const atprotoTokenResponseSchema = oauthTokenResponseSchema.extend({ id_token: z.never().optional(), }) -export type AtprotoTokenResponse = z.infer +export type AtprotoTokenResponse = TypeOf diff --git a/packages/oauth/oauth-client/src/oauth-client.ts b/packages/oauth/oauth-client/src/oauth-client.ts index 9680b6cdccc..6d500674925 100644 --- a/packages/oauth/oauth-client/src/oauth-client.ts +++ b/packages/oauth/oauth-client/src/oauth-client.ts @@ -316,6 +316,19 @@ export class OAuthClient extends CustomEventTarget { scope: options?.scope ?? this.clientMetadata.scope, } + const authorizationUrl = new URL(metadata.authorization_endpoint) + + // Since the user will be redirected to the authorization_endpoint url using + // a browser, we need to make sure that the url is valid. + if ( + authorizationUrl.protocol !== 'https:' && + authorizationUrl.protocol !== 'http:' + ) { + throw new TypeError( + `Invalid authorization endpoint protocol: ${authorizationUrl.protocol}`, + ) + } + if (metadata.pushed_authorization_request_endpoint) { const server = await this.serverFactory.fromMetadata(metadata, dpopKey) const parResponse = await server.request( @@ -323,7 +336,6 @@ export class OAuthClient extends CustomEventTarget { parameters, ) - const authorizationUrl = new URL(metadata.authorization_endpoint) authorizationUrl.searchParams.set( 'client_id', this.clientMetadata.client_id, @@ -335,7 +347,6 @@ export class OAuthClient extends CustomEventTarget { 'Server requires pushed authorization requests (PAR) but no PAR endpoint is available', ) } else { - const authorizationUrl = new URL(metadata.authorization_endpoint) for (const [key, value] of Object.entries(parameters)) { if (value) authorizationUrl.searchParams.set(key, String(value)) } diff --git a/packages/oauth/oauth-client/src/oauth-resolver-error.ts b/packages/oauth/oauth-client/src/oauth-resolver-error.ts index 6541b4b1c4e..15138d4c0d6 100644 --- a/packages/oauth/oauth-client/src/oauth-resolver-error.ts +++ b/packages/oauth/oauth-client/src/oauth-resolver-error.ts @@ -1,3 +1,5 @@ +import { ZodError } from 'zod' + export class OAuthResolverError extends Error { constructor(message: string, options?: { cause?: unknown }) { super(message, options) @@ -5,7 +7,14 @@ export class OAuthResolverError extends Error { static from(cause: unknown, message?: string): OAuthResolverError { if (cause instanceof OAuthResolverError) return cause - return new OAuthResolverError(message ?? `Unable to resolve identity`, { + const validationReason = + cause instanceof ZodError + ? `${cause.errors[0].path} ${cause.errors[0].message}` + : null + const fullMessage = + (message ?? `Unable to resolve identity`) + + (validationReason ? ` (${validationReason})` : '') + return new OAuthResolverError(fullMessage, { cause, }) } diff --git a/packages/oauth/oauth-client/src/types.ts b/packages/oauth/oauth-client/src/types.ts index 33fa2dacc14..81b9ed8eafd 100644 --- a/packages/oauth/oauth-client/src/types.ts +++ b/packages/oauth/oauth-client/src/types.ts @@ -1,9 +1,10 @@ import { + oauthClientIdLoopbackSchema, OAuthAuthorizationRequestParameters, - oauthClientIdSchema, + oauthClientIdDiscoverableSchema, oauthClientMetadataSchema, } from '@atproto/oauth-types' -import z from 'zod' +import { TypeOf, z } from 'zod' import { Simplify } from './util.js' @@ -26,7 +27,10 @@ export type AuthorizeOptions = Simplify< > export const clientMetadataSchema = oauthClientMetadataSchema.extend({ - client_id: oauthClientIdSchema.url(), + client_id: z.union([ + oauthClientIdDiscoverableSchema, + oauthClientIdLoopbackSchema, + ]), }) -export type ClientMetadata = z.infer +export type ClientMetadata = TypeOf diff --git a/packages/oauth/oauth-provider/src/client/client.ts b/packages/oauth/oauth-provider/src/client/client.ts index c7ed67d9944..4dd881c2af9 100644 --- a/packages/oauth/oauth-provider/src/client/client.ts +++ b/packages/oauth/oauth-provider/src/client/client.ts @@ -4,6 +4,7 @@ import { OAuthAuthorizationRequestParameters, OAuthClientCredentials, OAuthClientMetadata, + OAuthRedirectUri, } from '@atproto/oauth-types' import { UnsecuredJWT, @@ -323,7 +324,7 @@ export class Client { return parameters } - get defaultRedirectUri(): string | undefined { + get defaultRedirectUri(): OAuthRedirectUri | undefined { const { redirect_uris } = this.metadata return redirect_uris.length === 1 ? redirect_uris[0] : undefined } diff --git a/packages/oauth/oauth-provider/src/oauth-verifier.ts b/packages/oauth/oauth-provider/src/oauth-verifier.ts index 3403cbb1352..2b152e02c1a 100644 --- a/packages/oauth/oauth-provider/src/oauth-verifier.ts +++ b/packages/oauth/oauth-provider/src/oauth-verifier.ts @@ -1,6 +1,7 @@ import { Key, Keyset, isSignedJwt } from '@atproto/jwk' import { OAuthAccessToken, + OAuthIssuerIdentifier, OAuthTokenType, oauthIssuerIdentifierSchema, } from '@atproto/oauth-types' @@ -77,7 +78,7 @@ export { } export class OAuthVerifier { - public readonly issuer: string + public readonly issuer: OAuthIssuerIdentifier public readonly keyset: Keyset protected readonly accessTokenType: AccessTokenType diff --git a/packages/oauth/oauth-provider/src/request/request-uri.ts b/packages/oauth/oauth-provider/src/request/request-uri.ts index 18f69548038..af2a5c41b22 100644 --- a/packages/oauth/oauth-provider/src/request/request-uri.ts +++ b/packages/oauth/oauth-provider/src/request/request-uri.ts @@ -6,7 +6,6 @@ export const REQUEST_URI_PREFIX = 'urn:ietf:params:oauth:request_uri:' export const requestUriSchema = z .string() - .url() .refinement( (data): data is `${typeof REQUEST_URI_PREFIX}${RequestId}` => data.startsWith(REQUEST_URI_PREFIX) && 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 3fdbc2667c3..017cbb6843f 100644 --- a/packages/oauth/oauth-types/src/atproto-loopback-client-metadata.ts +++ b/packages/oauth/oauth-types/src/atproto-loopback-client-metadata.ts @@ -1,16 +1,21 @@ -import { parseOAuthLoopbackClientId } from './oauth-client-id-loopback.js' +import { + OAuthClientIdLoopback, + parseOAuthLoopbackClientId, +} from './oauth-client-id-loopback.js' import { OAuthClientMetadataInput } from './oauth-client-metadata.js' export function atprotoLoopbackClientMetadata( clientId: string, -): OAuthClientMetadataInput { +): OAuthClientMetadataInput & { + client_id: OAuthClientIdLoopback +} { const { scope = 'atproto', redirect_uris = [`http://127.0.0.1/`, `http://[::1]/`], } = parseOAuthLoopbackClientId(clientId) return { - client_id: clientId, + client_id: clientId as OAuthClientIdLoopback, scope, redirect_uris, client_name: 'Loopback client', diff --git a/packages/oauth/oauth-types/src/index.ts b/packages/oauth/oauth-types/src/index.ts index 38508a9cded..18d310c1536 100644 --- a/packages/oauth/oauth-types/src/index.ts +++ b/packages/oauth/oauth-types/src/index.ts @@ -1,4 +1,5 @@ export * from './constants.js' +export * from './uri.js' export * from './util.js' export * from './atproto-loopback-client-metadata.js' @@ -25,6 +26,7 @@ export * from './oauth-issuer-identifier.js' export * from './oauth-par-response.js' export * from './oauth-password-grant-token-request.js' export * from './oauth-protected-resource-metadata.js' +export * from './oauth-redirect-uri.js' export * from './oauth-refresh-token-grant-token-request.js' export * from './oauth-refresh-token.js' export * from './oauth-request-uri.js' diff --git a/packages/oauth/oauth-types/src/oauth-authorization-code-grant-token-request.ts b/packages/oauth/oauth-types/src/oauth-authorization-code-grant-token-request.ts index f1d30ca4923..8e71a4c21bd 100644 --- a/packages/oauth/oauth-types/src/oauth-authorization-code-grant-token-request.ts +++ b/packages/oauth/oauth-types/src/oauth-authorization-code-grant-token-request.ts @@ -1,9 +1,10 @@ import { z } from 'zod' +import { oauthRedirectUriSchema } from './oauth-redirect-uri.js' export const oauthAuthorizationCodeGrantTokenRequestSchema = z.object({ grant_type: z.literal('authorization_code'), code: z.string().min(1), - redirect_uri: z.string().url(), + redirect_uri: oauthRedirectUriSchema, /** @see {@link https://datatracker.ietf.org/doc/html/rfc7636#section-4.1} */ code_verifier: z .string() diff --git a/packages/oauth/oauth-types/src/oauth-authorization-details.ts b/packages/oauth/oauth-types/src/oauth-authorization-details.ts index a8e1b34236c..d805f797c76 100644 --- a/packages/oauth/oauth-types/src/oauth-authorization-details.ts +++ b/packages/oauth/oauth-types/src/oauth-authorization-details.ts @@ -1,14 +1,34 @@ import { z } from 'zod' +import { dangerousUriSchema } from './uri.js' /** * @see {@link https://datatracker.ietf.org/doc/html/rfc9396#section-2 | RFC 9396, Section 2} */ export const oauthAuthorizationDetailSchema = z.object({ type: z.string(), - locations: z.array(z.string().url()).optional(), + /** + * An array of strings representing the location of the resource or RS. These + * strings are typically URIs identifying the location of the RS. + */ + locations: z.array(dangerousUriSchema).optional(), + /** + * An array of strings representing the kinds of actions to be taken at the + * resource. + */ actions: z.array(z.string()).optional(), + /** + * An array of strings representing the kinds of data being requested from the + * resource. + */ datatypes: z.array(z.string()).optional(), + /** + * A string identifier indicating a specific resource available at the API. + */ identifier: z.string().optional(), + /** + * An array of strings representing the types or levels of privilege being + * requested at the resource. + */ privileges: z.array(z.string()).optional(), }) diff --git a/packages/oauth/oauth-types/src/oauth-authorization-request-parameters.ts b/packages/oauth/oauth-types/src/oauth-authorization-request-parameters.ts index 036a570bafa..965a2278b5f 100644 --- a/packages/oauth/oauth-types/src/oauth-authorization-request-parameters.ts +++ b/packages/oauth/oauth-types/src/oauth-authorization-request-parameters.ts @@ -4,6 +4,7 @@ import { z } from 'zod' import { oauthAuthorizationDetailsSchema } from './oauth-authorization-details.js' import { oauthClientIdSchema } from './oauth-client-id.js' import { oauthCodeChallengeMethodSchema } from './oauth-code-challenge-method.js' +import { oauthRedirectUriSchema } from './oauth-redirect-uri.js' import { oauthResponseTypeSchema } from './oauth-response-type.js' import { oauthScopeSchema } from './oauth-scope.js' import { oidcClaimsParameterSchema } from './oidc-claims-parameter.js' @@ -16,7 +17,7 @@ import { oidcEntityTypeSchema } from './oidc-entity-type.js' export const oauthAuthorizationRequestParametersSchema = z.object({ client_id: oauthClientIdSchema, state: z.string().optional(), - redirect_uri: z.string().url().optional(), + redirect_uri: oauthRedirectUriSchema.optional(), scope: oauthScopeSchema.optional(), response_type: oauthResponseTypeSchema, diff --git a/packages/oauth/oauth-types/src/oauth-authorization-server-metadata.ts b/packages/oauth/oauth-types/src/oauth-authorization-server-metadata.ts index e62f7daf484..dfec6c3a435 100644 --- a/packages/oauth/oauth-types/src/oauth-authorization-server-metadata.ts +++ b/packages/oauth/oauth-types/src/oauth-authorization-server-metadata.ts @@ -2,9 +2,13 @@ import { z } from 'zod' import { oauthCodeChallengeMethodSchema } from './oauth-code-challenge-method.js' import { oauthIssuerIdentifierSchema } from './oauth-issuer-identifier.js' +import { webUriSchema } from './uri.js' /** * @see {@link https://datatracker.ietf.org/doc/html/rfc8414} + * @note we do not enforce https: scheme in URIs to support development + * environments. Make sure to validate the URIs before using it in a production + * environment. */ export const oauthAuthorizationServerMetadataSchema = z.object({ issuer: oauthIssuerIdentifierSchema, @@ -37,31 +41,31 @@ export const oauthAuthorizationServerMetadataSchema = z.object({ .array(z.string()) .optional(), - jwks_uri: z.string().url().optional(), + jwks_uri: webUriSchema.optional(), - authorization_endpoint: z.string().url(), // .optional(), + authorization_endpoint: webUriSchema, // .optional(), - token_endpoint: z.string().url(), // .optional(), + token_endpoint: webUriSchema, // .optional(), token_endpoint_auth_methods_supported: z.array(z.string()).optional(), token_endpoint_auth_signing_alg_values_supported: z .array(z.string()) .optional(), - revocation_endpoint: z.string().url().optional(), - introspection_endpoint: z.string().url().optional(), - pushed_authorization_request_endpoint: z.string().url().optional(), + revocation_endpoint: webUriSchema.optional(), + introspection_endpoint: webUriSchema.optional(), + pushed_authorization_request_endpoint: webUriSchema.optional(), require_pushed_authorization_requests: z.boolean().optional(), - userinfo_endpoint: z.string().url().optional(), - end_session_endpoint: z.string().url().optional(), - registration_endpoint: z.string().url().optional(), + userinfo_endpoint: webUriSchema.optional(), + end_session_endpoint: webUriSchema.optional(), + registration_endpoint: webUriSchema.optional(), // https://datatracker.ietf.org/doc/html/rfc9449#section-5.1 dpop_signing_alg_values_supported: z.array(z.string()).optional(), // https://datatracker.ietf.org/doc/html/draft-ietf-oauth-resource-metadata-05#section-4 - protected_resources: z.array(z.string().url()).optional(), + protected_resources: z.array(webUriSchema).optional(), // https://drafts.aaronpk.com/draft-parecki-oauth-client-id-metadata-document/draft-parecki-oauth-client-id-metadata-document.html client_id_metadata_document_supported: z.boolean().optional(), diff --git a/packages/oauth/oauth-types/src/oauth-client-id-discoverable.ts b/packages/oauth/oauth-types/src/oauth-client-id-discoverable.ts index f7a439c4077..0e720b65143 100644 --- a/packages/oauth/oauth-types/src/oauth-client-id-discoverable.ts +++ b/packages/oauth/oauth-types/src/oauth-client-id-discoverable.ts @@ -1,69 +1,87 @@ -import { OAuthClientId } from './oauth-client-id.js' +import { TypeOf, z } from 'zod' +import { oauthClientIdSchema } from './oauth-client-id.js' +import { httpsUriSchema } from './uri.js' import { extractUrlPath, isHostnameIP } from './util.js' /** * @see {@link https://drafts.aaronpk.com/draft-parecki-oauth-client-id-metadata-document/draft-parecki-oauth-client-id-metadata-document.html} */ -export type OAuthClientIdDiscoverable = OAuthClientId & `https://${string}` +export const oauthClientIdDiscoverableSchema = z + .intersection(oauthClientIdSchema, httpsUriSchema) + .superRefine((value, ctx): value is `https://${string}/${string}` => { + const url = new URL(value) + + if (url.username || url.password) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'ClientID must not contain credentials', + }) + return false + } + + if (url.hash) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'ClientID must not contain a fragment', + }) + return false + } + + if (url.pathname === '/') { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + 'ClientID must contain a path component (e.g. "/client-metadata.json")', + }) + return false + } + + if (url.pathname.endsWith('/')) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'ClientID path must not end with a trailing slash', + }) + return false + } + + if (isHostnameIP(url.hostname)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'ClientID hostname must not be an IP address', + }) + return false + } + + // URL constructor normalizes the URL, so we extract the path manually to + // avoid normalization, then compare it to the normalized path to ensure + // that the URL does not contain path traversal or other unexpected characters + if (extractUrlPath(value) !== url.pathname) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `ClientID must be in canonical form ("${url.href}", got "${value}")`, + }) + return false + } + + return true + }) + +export type OAuthClientIdDiscoverable = TypeOf< + typeof oauthClientIdDiscoverableSchema +> export function isOAuthClientIdDiscoverable( clientId: string, ): clientId is OAuthClientIdDiscoverable { - try { - parseOAuthDiscoverableClientId(clientId) - return true - } catch { - return false - } + return oauthClientIdDiscoverableSchema.safeParse(clientId).success } export function assertOAuthDiscoverableClientId( value: string, ): asserts value is OAuthClientIdDiscoverable { - void parseOAuthDiscoverableClientId(value) + void oauthClientIdDiscoverableSchema.parse(value) } export function parseOAuthDiscoverableClientId(clientId: string): URL { - const url = new URL(clientId) - - if (url.protocol !== 'https:') { - throw new TypeError('ClientID must use the "https:" protocol') - } - - if (url.username || url.password) { - throw new TypeError('ClientID must not contain credentials') - } - - if (url.hash) { - throw new TypeError('ClientID must not contain a fragment') - } - - if (url.hostname === 'localhost') { - throw new TypeError('ClientID hostname must not be "localhost"') - } - - if (url.pathname === '/') { - throw new TypeError( - 'ClientID must contain a path component (e.g. "/client-metadata.json")', - ) - } - - if (url.pathname.endsWith('/')) { - throw new TypeError('ClientID path must not end with a trailing slash') - } - - if (isHostnameIP(url.hostname)) { - throw new TypeError('ClientID hostname must not be an IP address') - } - - // URL constructor normalizes the URL, so we extract the path manually to - // avoid normalization, then compare it to the normalized path to ensure - // that the URL does not contain path traversal or other unexpected characters - if (extractUrlPath(clientId) !== url.pathname) { - throw new TypeError( - `ClientID must be in canonical form ("${url.href}", got "${clientId}")`, - ) - } - - return url + return new URL(oauthClientIdDiscoverableSchema.parse(clientId)) } diff --git a/packages/oauth/oauth-types/src/oauth-client-id-loopback.ts b/packages/oauth/oauth-types/src/oauth-client-id-loopback.ts index 27a8808bd0a..8226c1be637 100644 --- a/packages/oauth/oauth-types/src/oauth-client-id-loopback.ts +++ b/packages/oauth/oauth-types/src/oauth-client-id-loopback.ts @@ -1,11 +1,33 @@ -import { OAuthClientId } from './oauth-client-id.js' +import { TypeOf, ZodIssueCode } from 'zod' +import { oauthClientIdSchema } from './oauth-client-id.js' +import { + OAuthLoopbackRedirectURI, + oauthLoopbackRedirectURISchema, + OAuthRedirectUri, +} from './oauth-redirect-uri.js' import { OAuthScope, oauthScopeSchema } from './oauth-scope.js' -import { isLoopbackHost, safeUrl } from './util.js' -const OAUTH_CLIENT_ID_LOOPBACK_URL = 'http://localhost' +const PREFIX = 'http://localhost' -export type OAuthClientIdLoopback = OAuthClientId & - `${typeof OAUTH_CLIENT_ID_LOOPBACK_URL}${'' | '/'}${'' | `?${string}`}` +export const oauthClientIdLoopbackSchema = oauthClientIdSchema.superRefine( + (value, ctx): value is `${typeof PREFIX}${'' | '/'}${'' | `?${string}`}` => { + try { + assertOAuthLoopbackClientId(value) + return true + } catch (error) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: + error instanceof TypeError + ? error.message + : 'Invalid loopback client ID', + }) + return false + } + }, +) + +export type OAuthClientIdLoopback = TypeOf export function isOAuthClientIdLoopback( clientId: string, @@ -28,21 +50,18 @@ export function assertOAuthLoopbackClientId( // validation functions) export function parseOAuthLoopbackClientId(clientId: string): { scope?: OAuthScope - redirect_uris?: [string, ...string[]] + redirect_uris?: [OAuthRedirectUri, ...OAuthRedirectUri[]] } { - if (!clientId.startsWith(OAUTH_CLIENT_ID_LOOPBACK_URL)) { - throw new TypeError( - `Loopback ClientID must start with "${OAUTH_CLIENT_ID_LOOPBACK_URL}"`, - ) - } else if (clientId.includes('#', OAUTH_CLIENT_ID_LOOPBACK_URL.length)) { + if (!clientId.startsWith(PREFIX)) { + throw new TypeError(`Loopback ClientID must start with "${PREFIX}"`) + } else if (clientId.includes('#', PREFIX.length)) { throw new TypeError('Loopback ClientID must not contain a hash component') } const queryStringIdx = - clientId.length > OAUTH_CLIENT_ID_LOOPBACK_URL.length && - clientId[OAUTH_CLIENT_ID_LOOPBACK_URL.length] === '/' - ? OAUTH_CLIENT_ID_LOOPBACK_URL.length + 1 - : OAUTH_CLIENT_ID_LOOPBACK_URL.length + clientId.length > PREFIX.length && clientId[PREFIX.length] === '/' + ? PREFIX.length + 1 + : PREFIX.length if (clientId.length === queryStringIdx) { return {} // no query string to parse @@ -72,33 +91,14 @@ export function parseOAuthLoopbackClientId(clientId: string): { } const redirect_uris = searchParams.has('redirect_uri') - ? (searchParams.getAll('redirect_uri') as [string, ...string[]]) + ? (searchParams + .getAll('redirect_uri') + .map((value) => oauthLoopbackRedirectURISchema.parse(value)) as [ + OAuthLoopbackRedirectURI, + ...OAuthLoopbackRedirectURI[], + ]) : undefined - if (redirect_uris) { - for (const uri of redirect_uris) { - const url = safeUrl(uri) - if (!url) { - throw new TypeError(`Invalid redirect_uri in client ID: ${uri}`) - } - if (url.protocol !== 'http:') { - throw new TypeError( - `Loopback ClientID must use "http:" redirect_uri's (got ${uri})`, - ) - } - if (url.hostname === 'localhost') { - throw new TypeError( - `Loopback ClientID must not use "localhost" as redirect_uri hostname (got ${uri})`, - ) - } - if (!isLoopbackHost(url.hostname)) { - throw new TypeError( - `Loopback ClientID must use loopback addresses as redirect_uri's (got ${uri})`, - ) - } - } - } - return { scope, redirect_uris, diff --git a/packages/oauth/oauth-types/src/oauth-client-metadata.ts b/packages/oauth/oauth-types/src/oauth-client-metadata.ts index c74dba8cd23..1b005dbaadc 100644 --- a/packages/oauth/oauth-types/src/oauth-client-metadata.ts +++ b/packages/oauth/oauth-types/src/oauth-client-metadata.ts @@ -4,13 +4,23 @@ import { z } from 'zod' import { oauthClientIdSchema } from './oauth-client-id.js' import { oauthEndpointAuthMethod } from './oauth-endpoint-auth-method.js' import { oauthGrantTypeSchema } from './oauth-grant-type.js' +import { oauthRedirectUriSchema } from './oauth-redirect-uri.js' import { oauthResponseTypeSchema } from './oauth-response-type.js' import { oauthScopeSchema } from './oauth-scope.js' +import { webUriSchema } from './uri.js' -// https://openid.net/specs/openid-connect-registration-1_0.html -// https://datatracker.ietf.org/doc/html/rfc7591 +/** + * @see {@link https://openid.net/specs/openid-connect-registration-1_0.html} + * @see {@link https://datatracker.ietf.org/doc/html/rfc7591} + * @note we do not enforce https: scheme in URIs to support development + * environments. Make sure to validate the URIs before using it in a production + * environment. + */ export const oauthClientMetadataSchema = z.object({ - redirect_uris: z.array(z.string().url()).nonempty(), + /** + * @note redirect_uris require additional validation + */ + redirect_uris: z.array(oauthRedirectUriSchema).nonempty(), response_types: z .array(oauthResponseTypeSchema) .nonempty() @@ -30,7 +40,7 @@ export const oauthClientMetadataSchema = z.object({ token_endpoint_auth_signing_alg: z.string().optional(), userinfo_signed_response_alg: z.string().optional(), userinfo_encrypted_response_alg: z.string().optional(), - jwks_uri: z.string().url().optional(), + jwks_uri: webUriSchema.optional(), jwks: jwksPubSchema.optional(), application_type: z.enum(['web', 'native']).default('web').optional(), // default, per spec, is "web" subject_type: z.enum(['public', 'pairwise']).default('public').optional(), @@ -41,10 +51,10 @@ export const oauthClientMetadataSchema = z.object({ authorization_encrypted_response_alg: z.string().optional(), client_id: oauthClientIdSchema.optional(), client_name: z.string().optional(), - client_uri: z.string().url().optional(), - policy_uri: z.string().url().optional(), - tos_uri: z.string().url().optional(), - logo_uri: z.string().url().optional(), + client_uri: webUriSchema.optional(), + policy_uri: webUriSchema.optional(), + tos_uri: webUriSchema.optional(), + logo_uri: webUriSchema.optional(), // TODO: allow data: uri ? /** * Default Maximum Authentication Age. Specifies that the End-User MUST be diff --git a/packages/oauth/oauth-types/src/oauth-issuer-identifier.ts b/packages/oauth/oauth-types/src/oauth-issuer-identifier.ts index 0e99809122e..5b965076336 100644 --- a/packages/oauth/oauth-types/src/oauth-issuer-identifier.ts +++ b/packages/oauth/oauth-types/src/oauth-issuer-identifier.ts @@ -1,9 +1,8 @@ import { z } from 'zod' -import { safeUrl } from './util.js' +import { webUriSchema } from './uri.js' -export const oauthIssuerIdentifierSchema = z - .string() - .superRefine((value, ctx): value is `${'http' | 'https'}://${string}` => { +export const oauthIssuerIdentifierSchema = webUriSchema.superRefine( + (value, ctx) => { // Validate the issuer (MIX-UP attacks) if (value.endsWith('/')) { @@ -14,22 +13,7 @@ export const oauthIssuerIdentifierSchema = z return false } - const url = safeUrl(value) - if (!url) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: 'Invalid url', - }) - return false - } - - if (url.protocol !== 'https:' && url.protocol !== 'http:') { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `Invalid issuer URL protocol "${url.protocol}"`, - }) - return false - } + const url = new URL(value) if (url.username || url.password) { ctx.addIssue({ @@ -57,6 +41,7 @@ export const oauthIssuerIdentifierSchema = z } return true - }) + }, +) export type OAuthIssuerIdentifier = z.infer diff --git a/packages/oauth/oauth-types/src/oauth-protected-resource-metadata.ts b/packages/oauth/oauth-types/src/oauth-protected-resource-metadata.ts index ce3ff6cca8a..b61ec794e8d 100644 --- a/packages/oauth/oauth-types/src/oauth-protected-resource-metadata.ts +++ b/packages/oauth/oauth-types/src/oauth-protected-resource-metadata.ts @@ -1,6 +1,7 @@ import { z } from 'zod' import { oauthIssuerIdentifierSchema } from './oauth-issuer-identifier.js' +import { webUriSchema } from './uri.js' /** * @see {@link https://datatracker.ietf.org/doc/html/draft-ietf-oauth-resource-metadata-05#name-protected-resource-metadata-r} @@ -10,8 +11,17 @@ export const oauthProtectedResourceMetadataSchema = z.object({ * REQUIRED. The protected resource's resource identifier, which is a URL that * uses the https scheme and has no query or fragment components. Using these * well-known resources is described in Section 3. + * + * @note This schema allows non https URLs for testing & development purposes. + * Make sure to validate the URL before using it in a production environment. */ - resource: z.string().url(), + resource: webUriSchema + .refine((url) => !url.includes('?'), { + message: 'Resource URL must not contain query parameters', + }) + .refine((url) => !url.includes('#'), { + message: 'Resource URL must not contain a fragment', + }), /** * OPTIONAL. JSON array containing a list of OAuth authorization server issuer @@ -31,7 +41,7 @@ export const oauthProtectedResourceMetadataSchema = z.object({ * available, a use (public key use) parameter value is REQUIRED for all keys * in the referenced JWK Set to indicate each key's intended usage. */ - jwks_uri: z.string().url().optional(), + jwks_uri: webUriSchema.optional(), /** * RECOMMENDED. JSON array containing a list of the OAuth 2.0 [RFC6749] scope @@ -64,20 +74,20 @@ export const oauthProtectedResourceMetadataSchema = z.object({ * OPTIONAL. URL of a page containing human-readable information that * developers might want or need to know when using the protected resource */ - resource_documentation: z.string().url().optional(), + resource_documentation: webUriSchema.optional(), /** * OPTIONAL. URL that the protected resource provides to read about the * protected resource's requirements on how the client can use the data * provided by the protected resource */ - resource_policy_uri: z.string().url().optional(), + resource_policy_uri: webUriSchema.optional(), /** * OPTIONAL. URL that the protected resource provides to read about the * protected resource's terms of service */ - resource_tos_uri: z.string().url().optional(), + resource_tos_uri: webUriSchema.optional(), }) export type OAuthProtectedResourceMetadata = z.infer< diff --git a/packages/oauth/oauth-types/src/oauth-redirect-uri.ts b/packages/oauth/oauth-types/src/oauth-redirect-uri.ts new file mode 100644 index 00000000000..aaf37cf8405 --- /dev/null +++ b/packages/oauth/oauth-types/src/oauth-redirect-uri.ts @@ -0,0 +1,56 @@ +import { TypeOf, z, ZodIssueCode } from 'zod' +import { + httpsUriSchema, + LoopbackUri, + loopbackUriSchema, + privateUseUriSchema, +} from './uri.js' + +export const oauthLoopbackRedirectURISchema = loopbackUriSchema.superRefine( + (value, ctx): value is Exclude => { + if (value.startsWith('http://localhost')) { + // https://datatracker.ietf.org/doc/html/rfc8252#section-8.3 + // + // > While redirect URIs using localhost (i.e., + // > "http://localhost:{port}/{path}") function similarly to loopback IP + // > redirects described in Section 7.3, the use of localhost is NOT + // > RECOMMENDED. Specifying a redirect URI with the loopback IP literal + // > rather than localhost avoids inadvertently listening on network + // > interfaces other than the loopback interface. It is also less + // > susceptible to client-side firewalls and misconfigured host name + // > resolution on the user's device. + ctx.addIssue({ + code: ZodIssueCode.custom, + message: + 'Use of "localhost" hostname is not allowed (RFC 8252), use a loopback IP such as "127.0.0.1" instead', + }) + return false + } + + return true + }, +) +export type OAuthLoopbackRedirectURI = TypeOf< + typeof oauthLoopbackRedirectURISchema +> + +export const oauthHttpsRedirectURISchema = httpsUriSchema +export type OAuthHttpsRedirectURI = TypeOf + +export const oauthPrivateUseRedirectURISchema = privateUseUriSchema +export type OAuthPrivateUseRedirectURI = TypeOf< + typeof oauthPrivateUseRedirectURISchema +> + +export const oauthRedirectUriSchema = z.union( + [ + oauthLoopbackRedirectURISchema, + oauthHttpsRedirectURISchema, + oauthPrivateUseRedirectURISchema, + ], + { + message: `URL must use the "https:" or "http:" protocol, or a private-use URI scheme (RFC 8252)`, + }, +) + +export type OAuthRedirectUri = TypeOf diff --git a/packages/oauth/oauth-types/src/uri.ts b/packages/oauth/oauth-types/src/uri.ts new file mode 100644 index 00000000000..d60918bf5d0 --- /dev/null +++ b/packages/oauth/oauth-types/src/uri.ts @@ -0,0 +1,171 @@ +import { TypeOf, z, ZodIssueCode } from 'zod' +import { isHostnameIP, isLoopbackHost } from './util.js' + +/** + * Valid, but potentially dangerous URL (`data:`, `file:`, `javascript:`, etc.). + * + * Any value that matches this schema is safe to parse using `new URL()`. + */ +export const dangerousUriSchema = z + .string() + .refine( + (data): data is `${string}:${string}` => + data.includes(':') && URL.canParse(data), + { + message: 'Invalid URL', + }, + ) + +/** + * Valid, but potentially dangerous URL (`data:`, `file:`, `javascript:`, etc.). + */ +export type DangerousUrl = TypeOf + +export const loopbackUriSchema = dangerousUriSchema.superRefine( + ( + value, + ctx, + ): value is + | `http://[::1]${string}` + | `http://localhost${'' | `${':' | '/' | '?' | '#'}${string}`}` + | `http://127.0.0.1${'' | `${':' | '/' | '?' | '#'}${string}`}` => { + // Loopback url must use the "http:" protocol + if (!value.startsWith('http://')) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'URL must use the "http:" protocol', + }) + return false + } + + const url = new URL(value) + + if (!isLoopbackHost(url.hostname)) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'URL must use "localhost", "127.0.0.1" or "[::1]" as hostname', + }) + return false + } + + return true + }, +) + +export type LoopbackUri = TypeOf + +export const httpsUriSchema = dangerousUriSchema.superRefine( + (value, ctx): value is `https://${string}` => { + if (!value.startsWith('https://')) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'URL must use the "https:" protocol', + }) + return false + } + + const url = new URL(value) + + // Disallow loopback URLs with the `https:` protocol + if (isLoopbackHost(url.hostname)) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'https: URL must not use a loopback host', + }) + return false + } + + if (isHostnameIP(url.hostname)) { + // Hostname is an IP address + } else { + // Hostname is a domain name + if (!url.hostname.includes('.')) { + // we don't depend on PSL here, so we only check for a dot + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'Domain name must contain at least two segments', + }) + return false + } + + if (url.hostname.endsWith('.local')) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'Domain name must not end with ".local"', + }) + return false + } + } + + return true + }, +) + +export type HttpsUri = TypeOf + +export const webUriSchema = z + .string() + .superRefine((value, ctx): value is LoopbackUri | HttpsUri => { + // discriminated union of `loopbackUriSchema` and `httpsUriSchema` + if (value.startsWith('http://')) { + const result = loopbackUriSchema.safeParse(value) + if (!result.success) result.error.issues.forEach(ctx.addIssue, ctx) + return result.success + } + + if (value.startsWith('https://')) { + const result = httpsUriSchema.safeParse(value) + if (!result.success) result.error.issues.forEach(ctx.addIssue, ctx) + return result.success + } + + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'URL must use the "http:" or "https:" protocol', + }) + return false + }) + +export type WebUri = TypeOf + +export const privateUseUriSchema = dangerousUriSchema.superRefine( + (value, ctx): value is `${string}.${string}:/${string}` => { + const dotIdx = value.indexOf('.') + const colonIdx = value.indexOf(':') + + // Optimization: avoid parsing the URL if the protocol does not contain a "." + if (dotIdx === -1 || colonIdx === -1 || dotIdx > colonIdx) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: + 'Private-use URI scheme requires a "." as part of the protocol', + }) + return false + } + + const url = new URL(value) + + // Should be covered by the check before, but let's be extra sure + if (!url.protocol.includes('.')) { + ctx.addIssue({ + code: ZodIssueCode.custom, + message: 'Invalid private-use URI scheme', + }) + return false + } + + if (url.hostname) { + // https://datatracker.ietf.org/doc/html/rfc8252#section-7.1 + ctx.addIssue({ + code: ZodIssueCode.custom, + message: + 'Private-use URI schemes must not include a hostname (only one "/" is allowed after the protocol, as per RFC 8252)', + }) + return false + } + + return true + }, +) + +export type PrivateUseUri = TypeOf diff --git a/packages/pds/src/auth-routes.ts b/packages/pds/src/auth-routes.ts index 8aa2358c8f1..059c61b126d 100644 --- a/packages/pds/src/auth-routes.ts +++ b/packages/pds/src/auth-routes.ts @@ -15,6 +15,13 @@ export const createRouter = ({ authProvider, cfg }: AppContext): Router => { resource_documentation: 'https://atproto.com', }) + if ( + !cfg.service.devMode && + !oauthProtectedResourceMetadata.resource.startsWith('https://') + ) { + throw new Error('Resource URL must use the https scheme') + } + router.get('/.well-known/oauth-protected-resource', (req, res) => { res.setHeader('Access-Control-Allow-Origin', '*') res.setHeader('Access-Control-Allow-Method', '*')