Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions packages/core/sdk/src/oauth-scope-union.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -801,4 +801,52 @@ describe("oauth.start recorded scope fallback", () => {
}),
),
);

it.effect("(m) does not report ungranted discovered capabilities as missing requirements", () =>
Effect.scoped(
Effect.gen(function* () {
const server = yield* serveOAuthTestServer({
scopes: ["read", "write", "*"],
omitTokenResponseScopes: ["*"],
});
const plugins = [memoryCredentialsPlugin(), makeMcpScopePlugin({ scopes: null })] as const;
const { executor } = yield* makeTestWorkspaceHarness({ plugins });
yield* executor.mcp.seed();

yield* executor.oauth.createClient({
owner: "org",
slug: CLIENT,
authorizationUrl: server.authorizationEndpoint,
tokenUrl: server.tokenEndpoint,
grant: "authorization_code",
clientId: "test-client",
clientSecret: "test-secret",
resource: server.mcpResourceUrl,
});

const started = yield* executor.oauth.start({
owner: "org",
client: CLIENT,
clientOwner: "org",
name: ConnectionName.make("main"),
integration: INTEG,
template: TEMPLATE,
});
expect(started.status).toBe("redirect");
if (started.status !== "redirect") return;
expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual(["read", "write", "*"]);

const callback = yield* server.completeAuthorizationCodeFlow({
authorizationUrl: started.authorizationUrl,
});
const connection = yield* executor.oauth.complete({
state: started.state,
code: callback.code,
});

expect(connection.oauthScope).toBe("read write");
expect(connection.missingOAuthScopes).toEqual([]);
}),
),
);
});
40 changes: 33 additions & 7 deletions packages/core/sdk/src/oauth-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,21 +378,30 @@ export const missingGrantedOAuthScopes = (

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

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

/** Returns `null` for legacy sessions written before `requestedScopes` was
* persisted, so `complete` can fall back to the client's scopes. */
const requestedScopesFromPayload = (payload: unknown): readonly string[] | null =>
scopesFromPayload(payload, "requestedScopes");

/** Required scopes are distinct from dynamically discovered supported scopes.
* Legacy sessions deliberately fall back to their requested set at completion,
* preserving the verdict they would have received before this field existed. */
const requiredScopesFromPayload = (payload: unknown): readonly string[] | null =>
scopesFromPayload(payload, "requiredScopes");

/** Read the app owner `start` recorded on the session payload. Null when absent
* (same-owner connects, or sessions written before this field), so `complete`
* falls back to the session owner. */
Expand Down Expand Up @@ -1546,6 +1555,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
client,
token,
requestedScopes,
scopePolicy.kind === "scopes" ? requestedScopes : [],
input.clientOwner,
// client_credentials has no callback, so no regional rebind applies.
null,
Expand Down Expand Up @@ -1724,6 +1734,13 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
...authorizationRequestedScopes,
...(firstParty?.additionalAuthorizationScopes ?? []),
]);
// RFC 9728 `scopes_supported` advertises capabilities; it does not make
// every discovered value a requirement. Only integration/client policy
// that explicitly declares scopes can produce a missing-scope verdict.
const requiredAuthorizationScopes =
firstParty?.authorizationScopes !== undefined || scopePolicy.kind === "scopes"
? completeAuthorizationScopes
: dedupeScopes(firstParty?.additionalAuthorizationScopes ?? []);

// authorization_code: persist a session + build the authorize URL.
const verifier = createPkceCodeVerifier();
Expand Down Expand Up @@ -1786,6 +1803,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
owner: input.owner,
clientOwner: input.clientOwner,
requestedScopes: completeAuthorizationScopes,
requiredScopes: requiredAuthorizationScopes,
},
expires_at: expiresAt,
created_at: now,
Expand Down Expand Up @@ -1852,6 +1870,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
// recorded-scope fallback when the AS omits `scope`. Missing/legacy
// payloads fall back to the client's scopes below.
requestedScopes: requestedScopesFromPayload(sessionRow.payload),
requiredScopes: requiredScopesFromPayload(sessionRow.payload),
// The app's owner, recorded by `start` — reload the SAME app at
// completion by explicit owner (no derivation). Defaults to the session
// owner for same-owner connects.
Expand Down Expand Up @@ -1957,6 +1976,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
// The scopes `start` requested (the integration's declared set), persisted
// on the session. Empty only for a corrupt/legacy session with no payload.
session.requestedScopes ?? [],
// Legacy sessions predate the required/supported distinction and retain
// their historical requested-scope verdict.
session.requiredScopes ?? session.requestedScopes ?? [],
session.clientOwner,
// Persist the regional token endpoint ONLY when it differs from the
// client's configured one, so refresh redeems against the same region.
Expand Down Expand Up @@ -2030,6 +2052,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
* declared or discovered scopes) — the recorded-scope fallback when the AS
* omits `scope`. */
requestedScopes: readonly string[],
/** Scopes explicitly required by integration/client policy. Dynamically
* discovered `scopes_supported` values are capabilities, not requirements. */
requiredScopes: readonly string[],
/** The owner of `client` — persisted so refresh loads it by explicit owner. */
clientOwner: Owner,
/** Regional token endpoint override to persist when the code was redeemed
Expand Down Expand Up @@ -2057,7 +2082,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
const oauthScope = recordedOAuthScope(token, requestedScopes);
const missingScopes =
client.grant === "authorization_code"
? missingGrantedOAuthScopes(requestedScopes, oauthScope)
? missingGrantedOAuthScopes(requiredScopes, oauthScope)
: [];
// The freshness facts of this connection AT BIRTH, on the enclosing
// span (executor.oauth.complete, or the reconnect path's request
Expand All @@ -2069,6 +2094,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
// customer resource names on some providers.
yield* Effect.annotateCurrentSpan({
"executor.oauth.scope_requested_count": requestedScopes.length,
"executor.oauth.scope_required_count": requiredScopes.length,
"executor.oauth.scope_missing_count": missingScopes.length,
"executor.oauth.has_refresh_token": token.refresh_token !== undefined,
"executor.oauth.has_advertised_expiry": typeof token.expires_in === "number",
Expand Down
Loading