Skip to content

Commit 014285f

Browse files
committed
Keep refreshing a connection that has no recorded scope
The scope validation compared the provider's reported scope against the connection's recorded grant set. When that set is empty the comparison had no subset to accept, so every reported scope failed the whole refresh — and the failure is a retryable StorageError, so the connection would retry a grant that could never succeed, indefinitely. An empty grant set is a legitimate state, not a corrupt one: RFC 6749 §5.1 lets an authorization server omit the scope it granted, and the refresh request then omits the scope parameter entirely. With nothing recorded there is also nothing to widen away from, so the safe action is to keep the refresh and record no scope. The provider's string is still never persisted, which is the property the validation exists to hold.
1 parent c69cc52 commit 014285f

2 files changed

Lines changed: 36 additions & 0 deletions

File tree

packages/core/sdk/src/executor.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,6 +2045,15 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
20452045
reportedScope === null
20462046
? null
20472047
: [...new Set(reportedScope.split(/\s+/).filter(Boolean))];
2048+
// With no recorded grant there is nothing to validate against — and nothing to
2049+
// widen FROM either, since the request omits the scope parameter entirely. Failing
2050+
// here would strand a legitimate connection in a permanent retry loop, because
2051+
// RFC 6749 §5.1 lets an authorization server omit the scope it granted. So keep
2052+
// the refresh and simply record no scope: the reported value is still never
2053+
// persisted, which is the property this validation exists to hold.
2054+
if (trustedScopes.size === 0) {
2055+
return Effect.succeed({ expiresInSeconds, scope: null });
2056+
}
20482057
if (
20492058
reportedScopes !== null &&
20502059
reportedScopes.some((scope) => !trustedScopes.has(scope))

packages/core/sdk/src/oauth-refresh-grant-delegation.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,4 +879,31 @@ describe("provider-owned OAuth refresh grant", () => {
879879
}),
880880
),
881881
);
882+
883+
it.effect("keeps refreshing a connection that has no recorded scope", () =>
884+
Effect.scoped(
885+
Effect.gen(function* () {
886+
const { config, executor } = yield* scenario({ behaviour: SEALS });
887+
// RFC 6749 §5.1 lets an authorization server omit the granted scope, so a live connection
888+
// can legitimately carry none. The scope validation must not turn that into a permanent
889+
// failure: with nothing recorded there is nothing to widen from.
890+
yield* Effect.promise(() =>
891+
config.db.updateMany("connection", {
892+
where: (b) => b("name", "=", "main"),
893+
set: { oauth_scope: null },
894+
}),
895+
);
896+
897+
const out = yield* executor.execute(TOOL, {});
898+
expect(out).toEqual({ token: "delegated-access-token" });
899+
900+
// The provider's scope string is still never persisted — that is the property the
901+
// validation exists to hold, and it holds here by recording nothing at all.
902+
const row = yield* Effect.promise(() =>
903+
config.db.findFirst("connection", { where: (b) => b("name", "=", "main") }),
904+
);
905+
expect(row?.oauth_scope).toBeNull();
906+
}),
907+
),
908+
);
882909
});

0 commit comments

Comments
 (0)