diff --git a/.changeset/oauth-provider-managed-client-resources.md b/.changeset/oauth-provider-managed-client-resources.md new file mode 100644 index 0000000000..7e67ce2294 --- /dev/null +++ b/.changeset/oauth-provider-managed-client-resources.md @@ -0,0 +1,5 @@ +--- +"@better-auth/oauth-provider": patch +--- + +If you register OAuth clients from your server with `adminCreateOAuthClient`, you can now pass `resources` to link the new client to protected resources in the same call. Previously you created the client first and then called `adminLinkClientResource` for each resource. When `resourcePrivileges` is configured, it is called with the `link` action for each requested resource, and a missing or disabled resource rejects the registration with `invalid_target`. Dynamic client registration and `adminLinkClientResource` are unchanged. diff --git a/.changeset/oauth-provider-resource-identifier-validation.md b/.changeset/oauth-provider-resource-identifier-validation.md new file mode 100644 index 0000000000..be04179d6c --- /dev/null +++ b/.changeset/oauth-provider-resource-identifier-validation.md @@ -0,0 +1,5 @@ +--- +"@better-auth/oauth-provider": patch +--- + +Resource identifiers created with `adminCreateOAuthResource` or listed in the `resources` option now follow the same rules as the `resource` parameter in token and registration requests. An identifier with an empty fragment (`https://api.example.com/#`) or a `javascript:`, `data:`, or `vbscript:` scheme is rejected, and startup seeding skips it with a warning. Token requests already rejected these identifiers, so no working resource is affected. Setups with a custom `identifierValidator` are unaffected. diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index de4780b0b8..814a78b707 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -729,7 +729,7 @@ The caller must authenticate as a registered client ([RFC 7662 §2.1](https://da * it issued the token, or * it is a resource server linked to one of the token's resources. -The second case is the common one: a frontend client gets the token, and your API validates it. To set this up, register the API as a resource and link the client to it. Dynamic client registration with a `resources` field creates the link for you; otherwise add a row to the `oauthClientResource` table. +The second case is the common one: a frontend client gets the token, and your API validates it. To set this up, register the API as a resource and link the client to it. Dynamic registration and server-only `adminCreateOAuthClient` can link resources during registration; use `adminLinkClientResource` for an existing client. Any other authenticated client gets `{ active: false }`, the same response as an unknown or expired token, so introspection can't be used to fish for valid tokens. Refresh tokens are stricter: only the client that requested one can introspect it. @@ -1269,7 +1269,27 @@ oauthProvider({ }) ``` -Explicit resource requests are closed by default: when both registration-resource options are omitted, no resource may be requested. A requested resource outside the effective allowlist is rejected with `invalid_target`; missing and disabled resources are rejected too. Registrations with resolved resources return the final `resources` list and create the corresponding `oauthClientResource` links atomically. +Dynamic-registration resource requests are closed by default: when both registration-resource options are omitted, no resource may be requested. A requested resource outside the effective allowlist is rejected with `invalid_target`; missing and disabled resources are rejected too. Registrations with resolved resources return the final `resources` list and create the corresponding `oauthClientResource` links atomically. + +To link resources to a client your server registers, pass `resources` to the server-only `adminCreateOAuthClient` call. The dynamic-registration allowlist does not apply here. Instead, `resourcePrivileges` is called with the `link` action for each requested resource before the client is created. `clientRegistrationDefaultResources` are still added, without a privilege check. + +```ts +await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: ["https://app.example.com/callback"], + resources: ["https://api.example.com/default"], + }, +}); +``` + +The call creates nothing when a resource is rejected: + +* A resource that is not an absolute URI, contains a fragment, or uses a `javascript:`, `data:`, or `vbscript:` scheme fails request validation with `VALIDATION_ERROR`. +* A resource that does not exist or is disabled returns `400` with `invalid_target`. +* A `resourcePrivileges` denial returns `401`. + +The client and its resource links are written in one transaction when the database adapter supports transactions. On an adapter without them, such as standalone MongoDB, a failed link can leave the client registered without its links. To link a resource to an existing client, use `adminLinkClientResource`. ### Scopes diff --git a/packages/oauth-provider/src/oauthClient/index.ts b/packages/oauth-provider/src/oauthClient/index.ts index cc062e9898..d17608e79d 100644 --- a/packages/oauth-provider/src/oauthClient/index.ts +++ b/packages/oauth-provider/src/oauthClient/index.ts @@ -3,7 +3,11 @@ import * as z from "zod"; import { publicSessionMiddleware } from "../middleware"; import { createOAuthClientEndpoint } from "../register"; import type { OAuthOptions, Scope } from "../types"; -import { clientJwksSchema, SafeUrlSchema } from "../types/zod"; +import { + clientJwksSchema, + ResourceUriSchema, + SafeUrlSchema, +} from "../types/zod"; import { deleteClientEndpoint, getClientEndpoint, @@ -42,6 +46,7 @@ export const adminCreateOAuthClient = (opts: OAuthOptions) => jwks_uri: z.string().optional(), grant_types: grantTypesSchema.optional(), client_credentials_scopes: z.array(z.string().trim().min(1)).optional(), + resources: z.array(ResourceUriSchema).optional(), response_types: z.array(z.enum(["code"])).optional(), // SERVER_ONLY applicable fields client_secret_expires_at: z diff --git a/packages/oauth-provider/src/oauthResource/endpoints.test.ts b/packages/oauth-provider/src/oauthResource/endpoints.test.ts index 6ca496ca44..37365042c7 100644 --- a/packages/oauth-provider/src/oauthResource/endpoints.test.ts +++ b/packages/oauth-provider/src/oauthResource/endpoints.test.ts @@ -97,6 +97,24 @@ describe("resource admin CRUD", () => { }); }); + it.each([ + "https://api.example.com/x#", + "javascript:alert(1)", + "data:text/plain,hello", + "vbscript:msgbox(1)", + ])("rejects a resource identifier disallowed by the shared URI schema: %s", async (identifier) => { + const instance = await boot(); + const headers = await signedInHeaders(instance); + await expect( + instance.auth.api.adminCreateOAuthResource({ + body: { identifier }, + headers, + }), + ).rejects.toMatchObject({ + body: expect.objectContaining({ error: "invalid_target" }), + }); + }); + it("rejects duplicate identifiers", async () => { const instance = await boot(); const headers = await signedInHeaders(instance); diff --git a/packages/oauth-provider/src/oauthResource/endpoints.ts b/packages/oauth-provider/src/oauthResource/endpoints.ts index 75d584b988..35e1eaff0e 100644 --- a/packages/oauth-provider/src/oauthResource/endpoints.ts +++ b/packages/oauth-provider/src/oauthResource/endpoints.ts @@ -1,6 +1,5 @@ import type { GenericEndpointContext } from "@better-auth/core"; import { APIError, getSessionFromCtx } from "better-auth/api"; -import type { Session, User } from "better-auth/types"; import { assertIdentifierValid, invalidateResourceCache } from "../resources"; import type { OAuthOptions, @@ -8,52 +7,7 @@ import type { OAuthResourceInput, Scope, } from "../types"; - -/** - * Action types passed to {@link OAuthOptions.resourcePrivileges}. Mirrors - * the `clientPrivileges` action vocabulary so admins can reuse the same - * RBAC patterns. - */ -type ResourceAction = - | "create" - | "read" - | "update" - | "delete" - | "list" - | "link" - | "unlink"; - -/** - * Gate every admin resource endpoint. Mirrors `assertClientPrivileges`: - * a missing session → 401; a defined `resourcePrivileges` callback that - * returns falsy → 401 with the original action context preserved. - * - * When `resourcePrivileges` is undefined, the gate degrades to "any - * authenticated session can manage resources" — same forgiving default - * as `clientPrivileges`. Operators who care about RBAC must define the - * callback. - * - * @internal - */ -async function assertResourcePrivileges( - ctx: GenericEndpointContext, - session: { session: Session; user: User } | null, - opts: OAuthOptions, - action: ResourceAction, - resourceId?: string, -): Promise { - if (!session) throw new APIError("UNAUTHORIZED"); - if (!ctx.headers) throw new APIError("BAD_REQUEST"); - if (!opts.resourcePrivileges) return; - const allowed = await opts.resourcePrivileges({ - headers: ctx.headers, - action, - session: session.session, - user: session.user, - resourceId, - }); - if (!allowed) throw new APIError("UNAUTHORIZED"); -} +import { assertResourcePrivileges } from "./privileges"; const resourceModel = (opts: OAuthOptions) => opts.schema?.oauthResource?.modelName ?? "oauthResource"; diff --git a/packages/oauth-provider/src/oauthResource/privileges.ts b/packages/oauth-provider/src/oauthResource/privileges.ts new file mode 100644 index 0000000000..ba3dccf76e --- /dev/null +++ b/packages/oauth-provider/src/oauthResource/privileges.ts @@ -0,0 +1,52 @@ +import type { GenericEndpointContext } from "@better-auth/core"; +import { APIError } from "better-auth/api"; +import type { Session, User } from "better-auth/types"; +import type { OAuthOptions, Scope } from "../types"; + +/** + * Action types passed to {@link OAuthOptions.resourcePrivileges}. Mirrors + * the `clientPrivileges` action vocabulary so admins can reuse the same + * RBAC patterns. + */ +type ResourceAction = + | "create" + | "read" + | "update" + | "delete" + | "list" + | "link" + | "unlink"; + +/** + * Authorizes a resource action against the configured `resourcePrivileges` + * hook. Gates every admin resource endpoint and the resource links selected + * during managed client registration. Mirrors `assertClientPrivileges`: + * a missing session → 401; a defined `resourcePrivileges` callback that + * returns falsy → 401 with the original action context preserved. + * + * When `resourcePrivileges` is undefined, the gate degrades to "any + * authenticated session can manage resources" — same forgiving default + * as `clientPrivileges`. Operators who care about RBAC must define the + * callback. + * + * @internal + */ +export async function assertResourcePrivileges( + ctx: GenericEndpointContext, + session: { session: Session; user: User } | null, + opts: OAuthOptions, + action: ResourceAction, + resourceId?: string, +): Promise { + if (!session) throw new APIError("UNAUTHORIZED"); + if (!ctx.headers) throw new APIError("BAD_REQUEST"); + if (!opts.resourcePrivileges) return; + const allowed = await opts.resourcePrivileges({ + headers: ctx.headers, + action, + session: session.session, + user: session.user, + resourceId, + }); + if (!allowed) throw new APIError("UNAUTHORIZED"); +} diff --git a/packages/oauth-provider/src/register.ts b/packages/oauth-provider/src/register.ts index 0ae50804e9..acbf44c82a 100644 --- a/packages/oauth-provider/src/register.ts +++ b/packages/oauth-provider/src/register.ts @@ -20,6 +20,7 @@ import { validateClientCredentialsScopes, } from "./oauthClient/client-credentials"; import { assertClientPrivileges } from "./oauthClient/privileges"; +import { assertResourcePrivileges } from "./oauthResource/privileges"; import { getResource } from "./resources"; import type { ClientRegistrationRequest, @@ -152,14 +153,12 @@ function getRawHttpHostname(redirectUri: string): string | null { return (hostAndPort.split(":")[0] ?? "").toLowerCase(); } -async function resolveClientRegistrationResources( - ctx: GenericEndpointContext, +function assertDynamicRegistrationResourcesAllowed( opts: OAuthOptions, requestedResources: readonly string[], -): Promise { - const defaultResources = opts.clientRegistrationDefaultResources ?? []; +) { const allowedResources = new Set([ - ...defaultResources, + ...(opts.clientRegistrationDefaultResources ?? []), ...(opts.clientRegistrationAllowedResources ?? []), ]); for (const identifier of requestedResources) { @@ -170,7 +169,14 @@ async function resolveClientRegistrationResources( }); } } +} +async function resolveClientRegistrationResources( + ctx: GenericEndpointContext, + opts: OAuthOptions, + requestedResources: readonly string[], +): Promise { + const defaultResources = opts.clientRegistrationDefaultResources ?? []; const resources = [...new Set([...defaultResources, ...requestedResources])]; for (const identifier of resources) { const row = await getResource(ctx, opts, identifier); @@ -720,7 +726,11 @@ export type CreateOAuthClientRegistrationInput = }) | (CreateOAuthClientRegistrationBaseInput & { registrationSource: "managed"; - requestedResources?: never; + /** + * Resources to link to the new client. The caller must authorize the + * `link` resource privilege for each one before persisting. + */ + requestedResources?: string[]; /** Server-owned scope ceiling configured by an administrator. */ clientCredentialsScopes?: Scope[]; }); @@ -900,12 +910,17 @@ async function persistOAuthClientRegistration( ) { schema.applicationType = null; } + const requestedResources = + input.registrationSource === "clientMetadataDocument" + ? [] + : (input.requestedResources ?? []); + if (input.registrationSource === "dynamic") { + assertDynamicRegistrationResourcesAllowed(opts, requestedResources); + } const resources = await resolveClientRegistrationResources( ctx, opts, - input.registrationSource === "dynamic" - ? (input.requestedResources ?? []) - : [], + requestedResources, ); const clientModel = opts.schema?.oauthClient?.modelName ?? "oauthClient"; const clientResourceModel = @@ -1255,12 +1270,17 @@ export async function createOAuthClientEndpoint( "configure-client-credentials-scopes", ); } + const requestedResources = settings?.admin ? metadata.resources : undefined; + for (const identifier of new Set(requestedResources ?? [])) { + await assertResourcePrivileges(ctx, session, opts, "link", identifier); + } const registrationInput: CreateOAuthClientRegistrationInput = { metadata, registrationSource: "managed", userId: referenceId ? undefined : session.session.userId, referenceId, clientCredentialsScopes, + ...(requestedResources ? { requestedResources } : {}), }; const responseBody = settings?.admin ? await createOAuthClientAdministrativeRegistration( diff --git a/packages/oauth-provider/src/resource-discovery.test.ts b/packages/oauth-provider/src/resource-discovery.test.ts index 8b90443388..d955c4e0c8 100644 --- a/packages/oauth-provider/src/resource-discovery.test.ts +++ b/packages/oauth-provider/src/resource-discovery.test.ts @@ -209,6 +209,187 @@ describe("DCR — resources field (RFC 7591 §2 extension)", () => { }); describe("managed client registration resources", () => { + it("links explicitly selected resources during managed client creation", async () => { + const resource = "https://api.example.com/managed-requested"; + const resourcePrivileges = vi.fn( + ({ action }: { action: string }) => action === "link", + ); + const instance = await boot( + { + resources: [resource], + scopes: ["read:business"], + clientPrivileges: () => true, + resourcePrivileges, + }, + true, + ); + const { headers } = await instance.signInWithTestUser(); + + const result = await instance.auth.api.adminCreateOAuthClient({ + headers, + body: { + grant_types: ["client_credentials"], + client_credentials_scopes: ["read:business"], + resources: [resource, resource], + }, + }); + + expect(result.resources).toEqual([resource]); + const links = await instance.ctx.adapter.findMany({ + model: "oauthClientResource", + where: [{ field: "clientId", value: result.client_id }], + }); + expect(links.map((link) => link.resourceId)).toEqual([resource]); + expect(resourcePrivileges).toHaveBeenCalledWith( + expect.objectContaining({ action: "link", resourceId: resource }), + ); + }); + + it("checks resource-link privileges before creating a managed client", async () => { + const firstResource = "https://api.example.com/managed-allowed"; + const deniedResource = "https://api.example.com/managed-denied"; + const resourcePrivileges = vi.fn( + ({ action, resourceId }: { action: string; resourceId?: string }) => + action !== "link" || resourceId !== deniedResource, + ); + const instance = await boot( + { resources: [firstResource, deniedResource], resourcePrivileges }, + true, + ); + const { headers } = await instance.signInWithTestUser(); + const clientName = "denied-resource-link"; + + await expect( + instance.auth.api.adminCreateOAuthClient({ + headers, + body: { + client_name: clientName, + resources: [firstResource, deniedResource], + }, + }), + ).rejects.toMatchObject({ statusCode: 401 }); + expect(resourcePrivileges).toHaveBeenCalledWith( + expect.objectContaining({ action: "link", resourceId: firstResource }), + ); + expect(resourcePrivileges).toHaveBeenCalledWith( + expect.objectContaining({ action: "link", resourceId: deniedResource }), + ); + const storedClient = await instance.ctx.adapter.findOne({ + model: "oauthClient", + where: [{ field: "name", value: clientName }], + }); + expect(storedClient).toBeNull(); + const links = await instance.ctx.adapter.findMany({ + model: "oauthClientResource", + where: [{ field: "resourceId", value: firstResource }], + }); + expect(links).toEqual([]); + }); + + it.each([ + ["missing", "https://api.example.com/managed-missing", "does not exist"], + ["disabled", "https://api.example.com/managed-unavailable", "disabled"], + ])("rejects a %s resource before creating a managed client", async (kind, requested, reason) => { + const configured = "https://api.example.com/managed-unavailable"; + const instance = await boot({ resources: [configured] }, true); + if (kind === "disabled") { + await instance.ctx.adapter.update({ + model: "oauthResource", + where: [{ field: "identifier", value: configured }], + update: { disabled: true }, + }); + } + const { headers } = await instance.signInWithTestUser(); + const clientName = `managed-${kind}-resource`; + await expect( + instance.auth.api.adminCreateOAuthClient({ + headers, + body: { + client_name: clientName, + grant_types: ["client_credentials"], + resources: [requested], + }, + }), + ).rejects.toMatchObject({ + statusCode: 400, + body: { + error: "invalid_target", + error_description: expect.stringContaining(reason), + }, + }); + expect( + await instance.ctx.adapter.findOne({ + model: "oauthClient", + where: [{ field: "name", value: clientName }], + }), + ).toBeNull(); + }); + + it.each([ + ["relative", "managed-relative", "must be an absolute URI"], + [ + "fragment", + "https://api.example.com/managed-unavailable#part", + "must not contain a fragment", + ], + ])("rejects a %s resource URI in the request body", async (kind, requested, reason) => { + const instance = await boot( + { resources: ["https://api.example.com/managed-unavailable"] }, + true, + ); + const { headers } = await instance.signInWithTestUser(); + const clientName = `managed-${kind}-resource`; + await expect( + instance.auth.api.adminCreateOAuthClient({ + headers, + body: { + client_name: clientName, + grant_types: ["client_credentials"], + resources: [requested], + }, + }), + ).rejects.toMatchObject({ + statusCode: 400, + body: { + code: "VALIDATION_ERROR", + message: expect.stringContaining(reason), + }, + }); + expect( + await instance.ctx.adapter.findOne({ + model: "oauthClient", + where: [{ field: "name", value: clientName }], + }), + ).toBeNull(); + }); + + it("ignores resources sent to the session-facing create endpoint", async () => { + const resource = "https://api.example.com/session-facing"; + const resourcePrivileges = vi.fn(() => true); + const instance = await boot( + { resources: [resource], resourcePrivileges }, + true, + ); + const { headers } = await instance.signInWithTestUser(); + const body = { + redirect_uris: ["https://app.example.com/callback"], + resources: [resource], + }; + + const result = (await instance.auth.api.createOAuthClient({ + headers, + body, + })) as { client_id: string; resources?: string[] }; + + expect(result.resources).toBeUndefined(); + expect(resourcePrivileges).not.toHaveBeenCalled(); + const links = await instance.ctx.adapter.findMany({ + model: "oauthClientResource", + where: [{ field: "clientId", value: result.client_id }], + }); + expect(links).toEqual([]); + }); + it("links server-owned defaults during managed client creation", async () => { const defaultResource = "https://api.example.com/managed-default"; const instance = await boot( @@ -242,14 +423,8 @@ describe("managed client registration resources", () => { it("rolls back the client when a resource link cannot be inserted", async () => { const clientName = "transaction-rollback-client"; - const defaultResource = "https://api.example.com/rollback-default"; - const instance = await boot( - { - resources: [defaultResource], - clientRegistrationDefaultResources: [defaultResource], - }, - true, - ); + const resource = "https://api.example.com/rollback-requested"; + const instance = await boot({ resources: [resource] }, true); const { headers } = await instance.signInWithTestUser(); const originalTransaction = instance.ctx.adapter.transaction.bind( instance.ctx.adapter, @@ -277,6 +452,7 @@ describe("managed client registration resources", () => { body: { client_name: clientName, redirect_uris: ["https://app.example.com/callback"], + resources: [resource], }, }), ).rejects.toThrow("forced resource-link insertion failure"); diff --git a/packages/oauth-provider/src/resources.ts b/packages/oauth-provider/src/resources.ts index 9880d758a8..6e4ff8dc88 100644 --- a/packages/oauth-provider/src/resources.ts +++ b/packages/oauth-provider/src/resources.ts @@ -9,6 +9,7 @@ import type { OAuthResourceInput, Scope, } from "./types"; +import { ResourceUriSchema } from "./types/zod"; /** * Source-of-truth list of asymmetric JWS algorithms supported by the JWT @@ -93,20 +94,11 @@ async function checkIdentifier( } return { ok: true }; } - // Strict default: must parse as absolute URI, must not have a fragment. - let url: URL; - try { - url = new URL(identifier); - } catch { + const parsed = ResourceUriSchema.safeParse(identifier); + if (!parsed.success) { return { ok: false, - reason: `resource identifier ${identifier} must be an absolute URI (RFC 8707 §2)`, - }; - } - if (url.hash) { - return { - ok: false, - reason: `resource identifier ${identifier} must not contain a URI fragment (RFC 8707 §2)`, + reason: `resource identifier ${identifier} is invalid: ${parsed.error.issues[0]?.message ?? "not an absolute URI"}`, }; } return { ok: true };