Repository navigation
feat(api): add scoped MCP OAuth consent - #1032
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 21 files
Confidence score: 2/5
tool-context.tsblocks OAuth connections granted onlyread:linksfrom using organization-wide link tools because website discovery and validation requireread:data. Allow those steps for the selected grant.mcp.tsleavesskipConsentclients without a valid authorization path: the guard now requires a grant claim these tokens do not carry. Preserve an explicit path for these clients.consent/page.tsxalways requestsoffline_access, so users cannot approve API permissions without also approving offline access. Add a separate choice and omit the scope unless approved.with-workspace.tsrejectsremoveGitHubRepofor every selected-website grant, including when the requested website is approved, because the call passes only the organization. Pass the knownwebsiteIdto the guard.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/routes/mcp.ts">
<violation number="1" location="apps/api/src/routes/mcp.ts:41">
P2: `oauthClient.skipConsent = true` clients no longer have a valid path: the previous handler explicitly accepted them without an `oauthConsent` row, but this guard requires a grant claim that such tokens do not carry, so requests return 401. Preserve a scoped-grant path or reject these clients during authorization/token issuance.</violation>
</file>
<file name="packages/ai/src/ai/mcp/tool-context.ts">
<violation number="1" location="packages/ai/src/ai/mcp/tool-context.ts:200">
P1: OAuth connections approved only `read:links` cannot use the organization-wide link tools because website discovery and validation both require `read:data`. Allow selected-grant website discovery and validation for link tools, then enforce the tool’s declared `read:links` or `write:links` scope instead.</violation>
</file>
<file name="apps/dashboard/test/e2e/specs/regressions/mcp-consent.spec.ts">
<violation number="1" location="apps/dashboard/test/e2e/specs/regressions/mcp-consent.spec.ts:27">
P3: The websites mock returns the same site list for both organizations, and after switching to Org Two the test never re-enters "Choose websites", so the per-organization website refetch is never displayed or asserted. A regression where sites from the previously selected organization leak into the new organization would pass unnoticed. Have the mock return distinct sites per organization and re-select "Choose websites" after the switch to actually exercise the org-scoped list query.</violation>
</file>
<file name="apps/dashboard/app/(auth)/consent/page.tsx">
<violation number="1" location="apps/dashboard/app/(auth)/consent/page.tsx:141">
P2: The consent form always includes requested `offline_access`, even though users cannot opt out of it. Add a separate choice and omit this scope unless the user approves it, so approving API permissions does not also authorize persistent access.</violation>
</file>
<file name="packages/rpc/src/procedures/with-workspace.ts">
<violation number="1" location="packages/rpc/src/procedures/with-workspace.ts:190">
P2: `removeGitHubRepo` passes only the organization to `withWorkspace`, so this guard rejects the call for every selected-website OAuth grant—even when the requested website is approved. Pass the known `websiteId` to the authorization check.</violation>
</file>
Architecture diagram
sequenceDiagram
participant C as Client (Claude)
participant BA as Better Auth Server
participant MB as MCP/RPC Backend
participant DB as Database
participant UI as Consent Page
Note over C,UI: NEW: Scoped OAuth Consent Flow
C->>BA: Request /authorize with scopes & PKCE
BA->>C: Redirect to /consent
C->>UI: Load consent page
UI->>UI: Parse requested scopes
UI->>UI: Check read scopes by default
UI->>UI: Load organizations & websites
UI->>UI: User selects org, websites, permissions
UI->>BA: POST /oauth2/consent
BA->>BA: Validate organization & membership
BA->>BA: Validate selected websites
BA->>BA: Generate grant reference with UUID
BA->>DB: Store consent with grant reference
BA->>C: Redirect with auth code
C->>BA: POST /oauth2/token (code + PKCE)
BA->>BA: Generate JWT with grant hash claim
BA-->>C: Access token
C->>MB: MCP request with bearer token
MB->>MB: Extract grant hash from JWT
MB->>DB: Fetch consent by grant hash
DB-->>MB: Grant + scopes
alt Valid grant
MB->>MB: Filter accessible websites by grant
MB->>MB: Check scopes & permissions
MB->>MB: Execute tool / RPC
MB-->>C: Response
else Invalid/no grant
MB-->>C: 401 Unauthorized
end
Note over C,BA: Disconnect / Reconnect Flow
C->>BA: POST /oauth2/delete-consent
BA->>DB: Revoke refresh tokens
BA->>DB: Delete all consent rows
C->>BA: POST /oauth2/consent (new grant)
BA->>BA: Generate new grant reference
BA->>DB: Store new consent
BA-->>C: New access token with new grant hash
Note over MB: Old JWTs rejected - grant hash mismatch
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| }; | ||
| const { oauthUserId } = principal; | ||
| if (oauthUserId) { | ||
| if (principal.oauthScopes && !principal.oauthScopes.includes("read:data")) { |
There was a problem hiding this comment.
P1: OAuth connections approved only read:links cannot use the organization-wide link tools because website discovery and validation both require read:data. Allow selected-grant website discovery and validation for link tools, then enforce the tool’s declared read:links or write:links scope instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/ai/src/ai/mcp/tool-context.ts, line 200:
<comment>OAuth connections approved only `read:links` cannot use the organization-wide link tools because website discovery and validation both require `read:data`. Allow selected-grant website discovery and validation for link tools, then enforce the tool’s declared `read:links` or `write:links` scope instead.</comment>
<file context>
@@ -175,25 +189,46 @@ export async function getCachedAccessibleWebsites(
};
const { oauthUserId } = principal;
+ if (oauthUserId) {
+ if (principal.oauthScopes && !principal.oauthScopes.includes("read:data")) {
+ return [];
+ }
</file context>
| !(subject && clientId && (await hasActiveOAuthGrant(subject, clientId))) | ||
| ) { | ||
| const grantHash = claims[MCP_GRANT_CLAIM]; | ||
| if (!(subject && clientId && typeof grantHash === "string")) { |
There was a problem hiding this comment.
P2: oauthClient.skipConsent = true clients no longer have a valid path: the previous handler explicitly accepted them without an oauthConsent row, but this guard requires a grant claim that such tokens do not carry, so requests return 401. Preserve a scoped-grant path or reject these clients during authorization/token issuance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/api/src/routes/mcp.ts, line 41:
<comment>`oauthClient.skipConsent = true` clients no longer have a valid path: the previous handler explicitly accepted them without an `oauthConsent` row, but this guard requires a grant claim that such tokens do not carry, so requests return 401. Preserve a scoped-grant path or reject these clients during authorization/token issuance.</comment>
<file context>
@@ -66,22 +37,32 @@ const handleOAuthMcpRequest = createMcpProtectedRequestHandler(
- !(subject && clientId && (await hasActiveOAuthGrant(subject, clientId)))
- ) {
+ const grantHash = claims[MCP_GRANT_CLAIM];
+ if (!(subject && clientId && typeof grantHash === "string")) {
+ return createMcpUnauthorizedResponse();
+ }
</file context>
| oauth_query: oauthQuery, | ||
| organizationId: organization?.id, | ||
| websiteIds, | ||
| scope: [...identityScopes, ...approvedActions].join(" "), |
There was a problem hiding this comment.
P2: The consent form always includes requested offline_access, even though users cannot opt out of it. Add a separate choice and omit this scope unless the user approves it, so approving API permissions does not also authorize persistent access.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/dashboard/app/(auth)/consent/page.tsx, line 141:
<comment>The consent form always includes requested `offline_access`, even though users cannot opt out of it. Add a separate choice and omit this scope unless the user approves it, so approving API permissions does not also authorize persistent access.</comment>
<file context>
@@ -66,20 +89,68 @@ function ConsentPage() {
+ oauth_query: oauthQuery,
+ organizationId: organization?.id,
+ websiteIds,
+ scope: [...identityScopes, ...approvedActions].join(" "),
+ }
+ : { accept, oauth_query: oauthQuery },
</file context>
| websiteIds && | ||
| (input.websiteId | ||
| ? !websiteIds.includes(input.websiteId) | ||
| : resource !== "link") |
There was a problem hiding this comment.
P2: removeGitHubRepo passes only the organization to withWorkspace, so this guard rejects the call for every selected-website OAuth grant—even when the requested website is approved. Pass the known websiteId to the authorization check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/rpc/src/procedures/with-workspace.ts, line 190:
<comment>`removeGitHubRepo` passes only the organization to `withWorkspace`, so this guard rejects the call for every selected-website OAuth grant—even when the requested website is approved. Pass the known `websiteId` to the authorization check.</comment>
<file context>
@@ -170,6 +170,45 @@ async function resolveGrant(
+ websiteIds &&
+ (input.websiteId
+ ? !websiteIds.includes(input.websiteId)
+ : resource !== "link")
+ ) {
+ return {
</file context>
| ], | ||
| }) | ||
| ); | ||
| await page.route("**/rpc/websites/list", (route) => |
There was a problem hiding this comment.
P3: The websites mock returns the same site list for both organizations, and after switching to Org Two the test never re-enters "Choose websites", so the per-organization website refetch is never displayed or asserted. A regression where sites from the previously selected organization leak into the new organization would pass unnoticed. Have the mock return distinct sites per organization and re-select "Choose websites" after the switch to actually exercise the org-scoped list query.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At apps/dashboard/test/e2e/specs/regressions/mcp-consent.spec.ts, line 27:
<comment>The websites mock returns the same site list for both organizations, and after switching to Org Two the test never re-enters "Choose websites", so the per-organization website refetch is never displayed or asserted. A regression where sites from the previously selected organization leak into the new organization would pass unnoticed. Have the mock return distinct sites per organization and re-select "Choose websites" after the switch to actually exercise the org-scoped list query.</comment>
<file context>
@@ -0,0 +1,111 @@
+ ],
+ })
+ );
+ await page.route("**/rpc/websites/list", (route) =>
+ route.fulfill({
+ json: {
</file context>
railway-app
Bot
temporarily deployed
to
Databuddy Self Host / production
October 1, 2026 17:28
Inactive
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MCP OAuth connections previously inherited access across the signed-in account's organizations. The consent page now lets the user choose one organization, all or selected websites, and a subset of the client's requested permissions. Read permissions start selected; write permissions require an explicit choice. Connected apps shows the approved access.
The auth configuration delegates codes, consent, tokens, and refresh to Better Auth. A focused grant module validates the selection and binds it to the native consent reference; access tokens carry its hash. Shared MCP and RPC authorization checks enforce that grant alongside scopes and current organization roles, including cached discovery and ID-only actions. Disconnecting an app removes its grants and revokes its refresh tokens; reconnecting creates a fresh grant so old JWTs cannot regain access.
No new database tables, migrations, or dependencies. Existing unscoped OAuth connections must reconnect after deployment. Short-link permissions remain organization-wide and the selector explains this.
Validation:
bun run lintandbun run check-typespassed across the repository.bun run testpassed through the repository pre-push hook (all 31 tasks succeeded).Dependencies: none. Other open auth work (#692) concerns Redis session initialization; this slice changes MCP OAuth consent and grant enforcement.
Implementation and review assisted by Codex.
Summary by cubic
Adds scoped consent to MCP OAuth connections so approvals no longer inherit access across all the signed-in account's organizations. Users now pick one organization, all or selected websites, and which requested permissions to approve, with read permissions pre-selected and write permissions requiring an explicit choice.
New Features
Migration
Written for commit ee0438b. Summary will update on new commits.