From dcbff13aa5826d9e36d8b95fbe9664365f3db2cf Mon Sep 17 00:00:00 2001 From: johnyeo Date: Mon, 8 Jun 2026 15:29:17 +0100 Subject: [PATCH] fix: oauth scopes --- packages/auth/src/oauth/index.ts | 1 + packages/auth/src/oauth/leafOAuth.ts | 15 +++++++ packages/auth/src/oauth/mcpOAuth.ts | 16 -------- .../auth/actions/registerMcpOAuthClient.ts | 6 +-- .../auth/oauth/handleOAuthConsentWithEnv.ts | 35 ++++++---------- .../auth/oauth/handleOAuthTokenWithApiKey.ts | 41 +++++++++++++------ .../src/internal/auth/oauth/mcpOAuthScopes.ts | 38 ++++++++++++----- .../internal/auth/oauth/oauthConsentScopes.ts | 34 +++++++++++++++ .../auth/repos/oauthAccessTokenRepo.ts | 15 +++++++ .../auth/repos/oauthRefreshTokenRepo.ts | 15 +++++++ .../src/internal/dev/cli/oauthApiKeyUtils.ts | 2 + .../unit/auth/registerMcpOAuthClient.test.ts | 4 +- 12 files changed, 155 insertions(+), 67 deletions(-) create mode 100644 packages/auth/src/oauth/leafOAuth.ts create mode 100644 server/src/internal/auth/oauth/oauthConsentScopes.ts diff --git a/packages/auth/src/oauth/index.ts b/packages/auth/src/oauth/index.ts index b9b8860c3..553c3c91e 100644 --- a/packages/auth/src/oauth/index.ts +++ b/packages/auth/src/oauth/index.ts @@ -1,2 +1,3 @@ +export * from "./leafOAuth.js"; export * from "./mcpOAuth.js"; export * from "./oauthUrls.js"; diff --git a/packages/auth/src/oauth/leafOAuth.ts b/packages/auth/src/oauth/leafOAuth.ts new file mode 100644 index 000000000..b91ef7de3 --- /dev/null +++ b/packages/auth/src/oauth/leafOAuth.ts @@ -0,0 +1,15 @@ +import { LEAF_OAUTH_SCOPES } from "@autumn/shared/leafOAuthScopes"; +import type { ScopeString } from "@autumn/shared/scopeDefinitions"; + +const leafScopeSet = new Set(LEAF_OAUTH_SCOPES); + +export const getDefaultOAuthScopes = (requestedScopes?: string[] | null) => { + const requested = + requestedScopes && requestedScopes.length > 0 + ? requestedScopes + : [...LEAF_OAUTH_SCOPES]; + + return [...new Set(requested)].filter((scope): scope is ScopeString => + leafScopeSet.has(scope), + ); +}; diff --git a/packages/auth/src/oauth/mcpOAuth.ts b/packages/auth/src/oauth/mcpOAuth.ts index af1df6599..756894f08 100644 --- a/packages/auth/src/oauth/mcpOAuth.ts +++ b/packages/auth/src/oauth/mcpOAuth.ts @@ -1,6 +1,3 @@ -import { LEAF_OAUTH_SCOPES } from "@autumn/shared/leafOAuthScopes"; -import type { ScopeString } from "@autumn/shared/scopeDefinitions"; - export const MCP_CLIENT_KIND = "mcp_client"; export const SLACK_MCP_OAUTH_CLIENT_ID = "autumn_mcp_slack"; export const AUTUMN_ADMIN_OAUTH_CLIENT_ID = "autumn_admin"; @@ -71,19 +68,6 @@ export const isMcpOAuthResource = (resource: string | null | undefined) => { return new URL(resource).pathname.replace(/\/+$/, "").endsWith("/mcp"); }; -const leafScopeSet = new Set(LEAF_OAUTH_SCOPES); - -export const getLeafMcpOAuthScopes = (requestedScopes?: string[] | null) => { - const requested = - requestedScopes && requestedScopes.length > 0 - ? requestedScopes - : [...LEAF_OAUTH_SCOPES]; - - return [...new Set(requested)].filter((scope): scope is ScopeString => - leafScopeSet.has(scope), - ); -}; - export const getResourceFromOAuthTokenRequest = async (request: Request) => { const contentType = request.headers.get("content-type") ?? ""; const rawBody = await request.text(); diff --git a/server/src/internal/auth/actions/registerMcpOAuthClient.ts b/server/src/internal/auth/actions/registerMcpOAuthClient.ts index a9649a636..ef59f4c36 100644 --- a/server/src/internal/auth/actions/registerMcpOAuthClient.ts +++ b/server/src/internal/auth/actions/registerMcpOAuthClient.ts @@ -1,5 +1,5 @@ import { - getLeafMcpOAuthScopes, + getDefaultOAuthScopes, MCP_CLIENT_KIND, MCP_OAUTH_CLIENTS, type MpcClientInfo, @@ -126,9 +126,9 @@ export const getRequestedScopesForMcpClient = ({ scope: unknown; }) => { if (typeof scope !== "string" || !scope.trim()) { - return getLeafMcpOAuthScopes(); + return getDefaultOAuthScopes(); } - return getLeafMcpOAuthScopes(scope.split(" ")); + return getDefaultOAuthScopes(scope.split(" ")); }; const mergeMetadata = ({ diff --git a/server/src/internal/auth/oauth/handleOAuthConsentWithEnv.ts b/server/src/internal/auth/oauth/handleOAuthConsentWithEnv.ts index 5dcb2c1d2..c927374a9 100644 --- a/server/src/internal/auth/oauth/handleOAuthConsentWithEnv.ts +++ b/server/src/internal/auth/oauth/handleOAuthConsentWithEnv.ts @@ -1,11 +1,10 @@ import { AppEnv, RecaseError } from "@autumn/shared"; import type { Context } from "hono"; import { db } from "@/db/initDrizzle.js"; -import type { AutumnContext } from "@/honoUtils/HonoEnv.js"; import { auth } from "@/utils/auth.js"; import { oauthConsentRepo } from "../repos/index.js"; import { isAtmnOAuthClientId } from "./atmnOAuthClients.js"; -import { assertMcpOAuthScopeGrant } from "./mcpOAuthScopes.js"; +import { getOAuthConsentScopeGrant } from "./oauthConsentScopes.js"; type RequestFields = Record; @@ -68,10 +67,6 @@ const getRedirectUriFromFields = (fields: RequestFields) => getString(fields.redirectUri) ?? getNestedOAuthField(fields.oauth_query, "redirect_uri"); -const getResourceFromFields = (fields: RequestFields) => - getString(fields.resource) ?? - getNestedOAuthField(fields.oauth_query, "resource"); - const getScopesFromFields = (fields: RequestFields) => { const rawScope = getString(fields.scope) ?? getNestedOAuthField(fields.oauth_query, "scope"); @@ -159,25 +154,19 @@ export const handleOAuthConsentWithEnv = async (c: Context) => { const orgId = session?.session?.activeOrganizationId; if (userId && orgId) { try { - const scopeGrant = await assertMcpOAuthScopeGrant({ - clientId, - ctx: { - db, - oauthResource: getResourceFromFields(fields) ?? undefined, - org: { id: orgId }, - userId, - } as AutumnContext, + const scopeGrant = await getOAuthConsentScopeGrant({ + db, + organizationId: orgId, requestedScopes: getScopesFromFields(fields), + userId, + }); + grantedScopes = scopeGrant; + request = withScope({ + contentType, + request, + fields, + scope: scopeGrant.join(" "), }); - if (scopeGrant) { - grantedScopes = scopeGrant; - request = withScope({ - contentType, - request, - fields, - scope: scopeGrant.join(" "), - }); - } } catch (error) { if (error instanceof RecaseError) { return jsonOAuthError({ error }); diff --git a/server/src/internal/auth/oauth/handleOAuthTokenWithApiKey.ts b/server/src/internal/auth/oauth/handleOAuthTokenWithApiKey.ts index 73a2300ef..48aca2658 100644 --- a/server/src/internal/auth/oauth/handleOAuthTokenWithApiKey.ts +++ b/server/src/internal/auth/oauth/handleOAuthTokenWithApiKey.ts @@ -6,14 +6,15 @@ import { import { RecaseError } from "@autumn/shared"; import type { Context } from "hono"; import { db } from "@/db/initDrizzle.js"; -import type { AutumnContext } from "@/honoUtils/HonoEnv.js"; import { auth } from "@/utils/auth.js"; -import { assertMcpOAuthScopeGrant } from "./mcpOAuthScopes.js"; +import { oauthAccessTokenRepo, oauthRefreshTokenRepo } from "../repos/index.js"; +import { isMcpOAuthClient } from "./mcpOAuthScopes.js"; import { getExternalOAuthApiKeyForToken, getOAuthAccessTokenRecord, scopesFromOAuthScopeString, } from "./oauthAccessTokenApiKey.js"; +import { getOAuthConsentScopeGrant } from "./oauthConsentScopes.js"; const getString = (value: unknown) => typeof value === "string" && value.length > 0 ? value : null; @@ -131,25 +132,41 @@ export const handleOAuthTokenWithApiKey = async (c: Context) => { resource, requestedScopes, }); - const mcpScopeGrant = await assertMcpOAuthScopeGrant({ - clientId: tokenRecord.clientId, - ctx: { - db, - oauthResource: resource ?? undefined, - org: { id: tokenRecord.referenceId }, - userId: tokenRecord.userId, - } as AutumnContext, + const issuedScopes = await getOAuthConsentScopeGrant({ + db, + organizationId: tokenRecord.referenceId, requestedScopes: tokenRecord.scopes, + userId: tokenRecord.userId, + }); + tokenRecord.scopes = issuedScopes; + if (tokenRecord.id) { + await oauthAccessTokenRepo.updateScopes({ + db, + id: tokenRecord.id, + scopes: issuedScopes, + }); + } + if (tokenRecord.refreshId) { + await oauthRefreshTokenRepo.updateScopes({ + db, + id: tokenRecord.refreshId, + scopes: issuedScopes, + }); + } + const isMcpClient = await isMcpOAuthClient({ + clientId: tokenRecord.clientId, + db, + resource: resource ?? undefined, }); if ( - mcpScopeGrant || + isMcpClient || returnsOAuthAccessTokenForClientId({ clientId: tokenRecord.clientId }) ) { return jsonTokenResponse({ body: rewriteOAuthAccessTokenBody({ accessToken: prefixOAuthToken({ token: accessToken }), body, - scopes: mcpScopeGrant ?? tokenRecord.scopes, + scopes: tokenRecord.scopes, }), response, status: response.status, diff --git a/server/src/internal/auth/oauth/mcpOAuthScopes.ts b/server/src/internal/auth/oauth/mcpOAuthScopes.ts index b5fd16e5e..3c494d955 100644 --- a/server/src/internal/auth/oauth/mcpOAuthScopes.ts +++ b/server/src/internal/auth/oauth/mcpOAuthScopes.ts @@ -1,29 +1,45 @@ import { - getLeafMcpOAuthScopes, + getDefaultOAuthScopes, isKnownMcpOAuthClientId, isMcpOAuthClientRecord, isMcpOAuthResource, } from "@autumn/auth/oauth"; import { ErrCode, isScopeSubset, RecaseError } from "@autumn/shared"; +import type { DrizzleCli } from "@/db/initDrizzle.js"; import type { AutumnContext } from "@/honoUtils/HonoEnv.js"; import { getScopesForUserInOrg } from "@/utils/authUtils/customSessionScopes.js"; import { oauthClientRepo } from "../repos/index.js"; +export const isMcpOAuthClient = async ({ + clientId, + db, + resource, +}: { + clientId: string; + db: DrizzleCli; + resource?: string; +}) => { + if (isMcpOAuthResource(resource)) return true; + if (isKnownMcpOAuthClientId({ clientId })) return true; + + const client = await oauthClientRepo.getByClientId({ db, clientId }); + if (!client) return false; + + return isMcpOAuthClientRecord(client); +}; + export const isMcpOAuthClientId = async ({ clientId, ctx, }: { clientId: string; ctx: AutumnContext; -}) => { - if (isMcpOAuthResource(ctx.oauthResource)) return true; - if (isKnownMcpOAuthClientId({ clientId })) return true; - - const client = await oauthClientRepo.getByClientId({ db: ctx.db, clientId }); - if (!client) return false; - - return isMcpOAuthClientRecord(client); -}; +}) => + isMcpOAuthClient({ + clientId, + db: ctx.db, + resource: ctx.oauthResource, + }); export const getMcpOAuthScopeGrant = async ({ clientId, @@ -44,7 +60,7 @@ export const getMcpOAuthScopeGrant = async ({ statusCode: 400, }); } - const leafScopes = getLeafMcpOAuthScopes(requestedScopes); + const leafScopes = getDefaultOAuthScopes(requestedScopes); const { scopes: userScopes } = await getScopesForUserInOrg({ db: ctx.db, userId: ctx.userId, diff --git a/server/src/internal/auth/oauth/oauthConsentScopes.ts b/server/src/internal/auth/oauth/oauthConsentScopes.ts new file mode 100644 index 000000000..0ba219810 --- /dev/null +++ b/server/src/internal/auth/oauth/oauthConsentScopes.ts @@ -0,0 +1,34 @@ +import { getDefaultOAuthScopes } from "@autumn/auth/oauth"; +import { ErrCode, isScopeSubset, RecaseError } from "@autumn/shared"; +import type { DrizzleCli } from "@/db/initDrizzle.js"; +import { getScopesForUserInOrg } from "@/utils/authUtils/customSessionScopes.js"; + +export const getOAuthConsentScopeGrant = async ({ + db, + organizationId, + requestedScopes, + userId, +}: { + db: DrizzleCli; + organizationId: string; + requestedScopes?: string[] | null; + userId: string; +}) => { + const requestedLeafScopes = getDefaultOAuthScopes(requestedScopes); + const { scopes: userScopes } = await getScopesForUserInOrg({ + db, + userId, + organizationId, + }); + + const grant = requestedLeafScopes.filter((scope) => + isScopeSubset([scope], userScopes), + ); + if (grant.length > 0) return grant; + + throw new RecaseError({ + message: "No requested scopes can be granted to this OAuth client", + code: ErrCode.InsufficientScopes, + statusCode: 403, + }); +}; diff --git a/server/src/internal/auth/repos/oauthAccessTokenRepo.ts b/server/src/internal/auth/repos/oauthAccessTokenRepo.ts index 674f6d2ea..51772147b 100644 --- a/server/src/internal/auth/repos/oauthAccessTokenRepo.ts +++ b/server/src/internal/auth/repos/oauthAccessTokenRepo.ts @@ -43,7 +43,22 @@ export const deleteOAuthAccessTokensByClientAndReference = async ({ ), ); +export const updateOAuthAccessTokenScopes = async ({ + db, + id, + scopes, +}: { + db: DrizzleCli; + id: string; + scopes: string[]; +}) => + db + .update(oauthAccessToken) + .set({ scopes }) + .where(eq(oauthAccessToken.id, id)); + export const oauthAccessTokenRepo = { getValidByTokenValues: getValidOAuthAccessTokenByTokenValues, deleteByClientAndReference: deleteOAuthAccessTokensByClientAndReference, + updateScopes: updateOAuthAccessTokenScopes, }; diff --git a/server/src/internal/auth/repos/oauthRefreshTokenRepo.ts b/server/src/internal/auth/repos/oauthRefreshTokenRepo.ts index c92fc5bac..6ff1db70a 100644 --- a/server/src/internal/auth/repos/oauthRefreshTokenRepo.ts +++ b/server/src/internal/auth/repos/oauthRefreshTokenRepo.ts @@ -22,6 +22,21 @@ export const deleteOAuthRefreshTokensByClientAndReference = async ({ ), ); +export const updateOAuthRefreshTokenScopes = async ({ + db, + id, + scopes, +}: { + db: DrizzleCli; + id: string; + scopes: string[]; +}) => + db + .update(oauthRefreshToken) + .set({ scopes }) + .where(eq(oauthRefreshToken.id, id)); + export const oauthRefreshTokenRepo = { deleteByClientAndReference: deleteOAuthRefreshTokensByClientAndReference, + updateScopes: updateOAuthRefreshTokenScopes, }; diff --git a/server/src/internal/dev/cli/oauthApiKeyUtils.ts b/server/src/internal/dev/cli/oauthApiKeyUtils.ts index 3ac45f452..2257bd910 100644 --- a/server/src/internal/dev/cli/oauthApiKeyUtils.ts +++ b/server/src/internal/dev/cli/oauthApiKeyUtils.ts @@ -12,6 +12,8 @@ export type OAuthApiKeyRequestBody = { }; export type ResourceAccessTokenRecord = { + id?: string; + refreshId?: string | null; userId: string | null; referenceId: string | null; clientId: string; diff --git a/server/tests/unit/auth/registerMcpOAuthClient.test.ts b/server/tests/unit/auth/registerMcpOAuthClient.test.ts index b7e7703f5..4819fd086 100644 --- a/server/tests/unit/auth/registerMcpOAuthClient.test.ts +++ b/server/tests/unit/auth/registerMcpOAuthClient.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { getLeafMcpOAuthScopes } from "@autumn/auth/oauth"; +import { getDefaultOAuthScopes } from "@autumn/auth/oauth"; import { LEAF_OAUTH_SCOPES } from "@autumn/shared"; import { Scopes } from "@autumn/shared/scopeDefinitions"; import { getRequestedScopesForMcpClient } from "@/internal/auth/actions/registerMcpOAuthClient.js"; @@ -37,7 +37,7 @@ describe("getRequestedScopesForMcpClient", () => { test("caps OAuth grants to Leaf scopes", () => { expect( - getLeafMcpOAuthScopes([ + getDefaultOAuthScopes([ Scopes.Customers.Read, Scopes.ApiKeys.Write, Scopes.Analytics.Read,