Skip to content

Commit cdcda04

Browse files
committed
fix(oauth): distinguish required and discovered scopes
Persist required scopes separately from dynamically advertised capabilities so partial grants only warn when explicit integration or client policy is unsatisfied.
1 parent fff7ed6 commit cdcda04

2 files changed

Lines changed: 81 additions & 7 deletions

File tree

packages/core/sdk/src/oauth-scope-union.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,4 +801,52 @@ describe("oauth.start recorded scope fallback", () => {
801801
}),
802802
),
803803
);
804+
805+
it.effect("(m) does not report ungranted discovered capabilities as missing requirements", () =>
806+
Effect.scoped(
807+
Effect.gen(function* () {
808+
const server = yield* serveOAuthTestServer({
809+
scopes: ["read", "write", "*"],
810+
omitTokenResponseScopes: ["*"],
811+
});
812+
const plugins = [memoryCredentialsPlugin(), makeMcpScopePlugin({ scopes: null })] as const;
813+
const { executor } = yield* makeTestWorkspaceHarness({ plugins });
814+
yield* executor.mcp.seed();
815+
816+
yield* executor.oauth.createClient({
817+
owner: "org",
818+
slug: CLIENT,
819+
authorizationUrl: server.authorizationEndpoint,
820+
tokenUrl: server.tokenEndpoint,
821+
grant: "authorization_code",
822+
clientId: "test-client",
823+
clientSecret: "test-secret",
824+
resource: server.mcpResourceUrl,
825+
});
826+
827+
const started = yield* executor.oauth.start({
828+
owner: "org",
829+
client: CLIENT,
830+
clientOwner: "org",
831+
name: ConnectionName.make("main"),
832+
integration: INTEG,
833+
template: TEMPLATE,
834+
});
835+
expect(started.status).toBe("redirect");
836+
if (started.status !== "redirect") return;
837+
expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual(["read", "write", "*"]);
838+
839+
const callback = yield* server.completeAuthorizationCodeFlow({
840+
authorizationUrl: started.authorizationUrl,
841+
});
842+
const connection = yield* executor.oauth.complete({
843+
state: started.state,
844+
code: callback.code,
845+
});
846+
847+
expect(connection.oauthScope).toBe("read write");
848+
expect(connection.missingOAuthScopes).toEqual([]);
849+
}),
850+
),
851+
);
804852
});

packages/core/sdk/src/oauth-service.ts

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -378,21 +378,30 @@ export const missingGrantedOAuthScopes = (
378378

379379
const decodeJsonPayload = Schema.decodeUnknownOption(Schema.UnknownFromJsonString);
380380

381-
/** Extract the persisted `requestedScopes` from an `oauth_session.payload`. The
381+
/** Extract a persisted scope list from an `oauth_session.payload`. The
382382
* jsonColumn may surface as a parsed object (in-memory backends) or a JSON
383-
* string (serialized backends); decode strings before reading. Returns `null`
384-
* for legacy sessions written before `requestedScopes` was persisted, so
385-
* `complete` can fall back to the client's scopes. */
386-
const requestedScopesFromPayload = (payload: unknown): readonly string[] | null => {
383+
* string (serialized backends); decode strings before reading. */
384+
const scopesFromPayload = (payload: unknown, key: string): readonly string[] | null => {
387385
const decoded =
388386
typeof payload === "string"
389387
? decodeJsonPayload(payload).pipe(Option.getOrElse(() => payload))
390388
: payload;
391389
if (decoded === null || typeof decoded !== "object") return null;
392-
const value = (decoded as Record<string, unknown>).requestedScopes;
390+
const value = (decoded as Record<string, unknown>)[key];
393391
return Array.isArray(value) ? value.filter((s): s is string => typeof s === "string") : null;
394392
};
395393

394+
/** Returns `null` for legacy sessions written before `requestedScopes` was
395+
* persisted, so `complete` can fall back to the client's scopes. */
396+
const requestedScopesFromPayload = (payload: unknown): readonly string[] | null =>
397+
scopesFromPayload(payload, "requestedScopes");
398+
399+
/** Required scopes are distinct from dynamically discovered supported scopes.
400+
* Legacy sessions deliberately fall back to their requested set at completion,
401+
* preserving the verdict they would have received before this field existed. */
402+
const requiredScopesFromPayload = (payload: unknown): readonly string[] | null =>
403+
scopesFromPayload(payload, "requiredScopes");
404+
396405
/** Read the app owner `start` recorded on the session payload. Null when absent
397406
* (same-owner connects, or sessions written before this field), so `complete`
398407
* falls back to the session owner. */
@@ -1546,6 +1555,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
15461555
client,
15471556
token,
15481557
requestedScopes,
1558+
scopePolicy.kind === "scopes" ? requestedScopes : [],
15491559
input.clientOwner,
15501560
// client_credentials has no callback, so no regional rebind applies.
15511561
null,
@@ -1724,6 +1734,13 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
17241734
...authorizationRequestedScopes,
17251735
...(firstParty?.additionalAuthorizationScopes ?? []),
17261736
]);
1737+
// RFC 9728 `scopes_supported` advertises capabilities; it does not make
1738+
// every discovered value a requirement. Only integration/client policy
1739+
// that explicitly declares scopes can produce a missing-scope verdict.
1740+
const requiredAuthorizationScopes =
1741+
firstParty?.authorizationScopes !== undefined || scopePolicy.kind === "scopes"
1742+
? completeAuthorizationScopes
1743+
: dedupeScopes(firstParty?.additionalAuthorizationScopes ?? []);
17271744

17281745
// authorization_code: persist a session + build the authorize URL.
17291746
const verifier = createPkceCodeVerifier();
@@ -1786,6 +1803,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
17861803
owner: input.owner,
17871804
clientOwner: input.clientOwner,
17881805
requestedScopes: completeAuthorizationScopes,
1806+
requiredScopes: requiredAuthorizationScopes,
17891807
},
17901808
expires_at: expiresAt,
17911809
created_at: now,
@@ -1852,6 +1870,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
18521870
// recorded-scope fallback when the AS omits `scope`. Missing/legacy
18531871
// payloads fall back to the client's scopes below.
18541872
requestedScopes: requestedScopesFromPayload(sessionRow.payload),
1873+
requiredScopes: requiredScopesFromPayload(sessionRow.payload),
18551874
// The app's owner, recorded by `start` — reload the SAME app at
18561875
// completion by explicit owner (no derivation). Defaults to the session
18571876
// owner for same-owner connects.
@@ -1957,6 +1976,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
19571976
// The scopes `start` requested (the integration's declared set), persisted
19581977
// on the session. Empty only for a corrupt/legacy session with no payload.
19591978
session.requestedScopes ?? [],
1979+
// Legacy sessions predate the required/supported distinction and retain
1980+
// their historical requested-scope verdict.
1981+
session.requiredScopes ?? session.requestedScopes ?? [],
19601982
session.clientOwner,
19611983
// Persist the regional token endpoint ONLY when it differs from the
19621984
// client's configured one, so refresh redeems against the same region.
@@ -2030,6 +2052,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
20302052
* declared or discovered scopes) — the recorded-scope fallback when the AS
20312053
* omits `scope`. */
20322054
requestedScopes: readonly string[],
2055+
/** Scopes explicitly required by integration/client policy. Dynamically
2056+
* discovered `scopes_supported` values are capabilities, not requirements. */
2057+
requiredScopes: readonly string[],
20332058
/** The owner of `client` — persisted so refresh loads it by explicit owner. */
20342059
clientOwner: Owner,
20352060
/** Regional token endpoint override to persist when the code was redeemed
@@ -2057,7 +2082,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
20572082
const oauthScope = recordedOAuthScope(token, requestedScopes);
20582083
const missingScopes =
20592084
client.grant === "authorization_code"
2060-
? missingGrantedOAuthScopes(requestedScopes, oauthScope)
2085+
? missingGrantedOAuthScopes(requiredScopes, oauthScope)
20612086
: [];
20622087
// The freshness facts of this connection AT BIRTH, on the enclosing
20632088
// span (executor.oauth.complete, or the reconnect path's request
@@ -2069,6 +2094,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
20692094
// customer resource names on some providers.
20702095
yield* Effect.annotateCurrentSpan({
20712096
"executor.oauth.scope_requested_count": requestedScopes.length,
2097+
"executor.oauth.scope_required_count": requiredScopes.length,
20722098
"executor.oauth.scope_missing_count": missingScopes.length,
20732099
"executor.oauth.has_refresh_token": token.refresh_token !== undefined,
20742100
"executor.oauth.has_advertised_expiry": typeof token.expires_in === "number",

0 commit comments

Comments
 (0)