Skip to content

Commit 3fe6b5b

Browse files
authored
fix(auth): treat transient code-access checks as indeterminate, not denied (#2931)
1 parent ace248d commit 3fe6b5b

2 files changed

Lines changed: 181 additions & 15 deletions

File tree

packages/core/src/auth/auth.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,9 @@ describe("AuthService", () => {
151151
string,
152152
{ name: string; projects: { id: number; name: string }[] }
153153
>;
154+
// Overrides the /api/code/invites/check-access/ response (defaults to
155+
// granting access). May throw to simulate a network error.
156+
checkAccess?: () => Response;
154157
} = {},
155158
) => {
156159
const accountKey = options.accountKey ?? "user-1";
@@ -186,6 +189,9 @@ describe("AuthService", () => {
186189
} as unknown as Response;
187190
}
188191

192+
if (options.checkAccess) {
193+
return options.checkAccess();
194+
}
189195
return {
190196
ok: true,
191197
json: vi.fn().mockResolvedValue({ has_access: true }),
@@ -1342,4 +1348,106 @@ describe("AuthService", () => {
13421348
expect(redeemCallCount).toBe(2);
13431349
});
13441350
});
1351+
1352+
describe("code access resilience", () => {
1353+
const okBody = (body: unknown): Response =>
1354+
({
1355+
ok: true,
1356+
json: vi.fn().mockResolvedValue(body),
1357+
}) as unknown as Response;
1358+
1359+
beforeEach(() => {
1360+
seedStoredSession();
1361+
oauthFlow.refreshToken.mockResolvedValue(
1362+
mockTokenResponse({
1363+
accessToken: "access-token",
1364+
refreshToken: "rotated-refresh-token",
1365+
}),
1366+
);
1367+
});
1368+
1369+
it.each([
1370+
{
1371+
name: "grants access when the server reports has_access true",
1372+
checkAccess: () => okBody({ has_access: true }),
1373+
expected: true,
1374+
},
1375+
{
1376+
name: "denies access when the server explicitly reports no access",
1377+
checkAccess: () => okBody({ has_access: false }),
1378+
expected: false,
1379+
},
1380+
{
1381+
name: "stays indeterminate when the check throws",
1382+
checkAccess: () => {
1383+
throw new Error("network down");
1384+
},
1385+
expected: null,
1386+
},
1387+
{
1388+
name: "stays indeterminate on a 2xx response without a has_access flag",
1389+
checkAccess: () => okBody({}),
1390+
expected: null,
1391+
},
1392+
])("$name", async ({ checkAccess, expected }) => {
1393+
stubAuthFetch({ checkAccess });
1394+
1395+
await service.initialize();
1396+
1397+
const state = service.getState();
1398+
expect(state.status).toBe("authenticated");
1399+
expect(state.hasCodeAccess).toBe(expected);
1400+
});
1401+
1402+
it.each([
1403+
{
1404+
name: "a network error",
1405+
fail: (): Response => {
1406+
throw new Error("network down");
1407+
},
1408+
},
1409+
{
1410+
name: "a 401",
1411+
fail: (): Response =>
1412+
({
1413+
ok: false,
1414+
status: 401,
1415+
json: vi.fn().mockResolvedValue({}),
1416+
}) as unknown as Response,
1417+
},
1418+
])(
1419+
"keeps a confirmed grant when a later check hits $name",
1420+
async ({ fail }) => {
1421+
let shouldFail = false;
1422+
stubAuthFetch({
1423+
checkAccess: () =>
1424+
shouldFail ? fail() : okBody({ has_access: true }),
1425+
});
1426+
1427+
await service.initialize();
1428+
expect(service.getState().hasCodeAccess).toBe(true);
1429+
1430+
shouldFail = true;
1431+
await service.refreshAccessToken();
1432+
1433+
expect(service.getState().hasCodeAccess).toBe(true);
1434+
},
1435+
);
1436+
1437+
it("recovers within the retry loop when a later attempt succeeds", async () => {
1438+
let attempts = 0;
1439+
stubAuthFetch({
1440+
checkAccess: () => {
1441+
attempts += 1;
1442+
if (attempts === 1) throw new Error("transient");
1443+
return okBody({ has_access: true });
1444+
},
1445+
});
1446+
1447+
await service.initialize();
1448+
1449+
expect(service.getState().hasCodeAccess).toBe(true);
1450+
expect(attempts).toBeGreaterThanOrEqual(2);
1451+
});
1452+
});
13451453
});

packages/core/src/auth/auth.ts

Lines changed: 73 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -910,26 +910,84 @@ export class AuthService extends TypedEventEmitter<AuthServiceEvents> {
910910
return;
911911
}
912912

913-
try {
914-
const apiHost = getCloudUrlFromRegion(this.session.cloudRegion);
915-
const response = await this.executeAuthenticatedFetch(
916-
fetch,
917-
`${apiHost}/api/code/invites/check-access/`,
918-
{},
919-
this.session.accessToken,
920-
);
921-
const data = (await response.json().catch(() => ({}))) as {
922-
has_access?: boolean;
923-
};
913+
const hasAccess = await this.checkCodeAccess(this.session);
924914

925-
this.updateState({ hasCodeAccess: data.has_access === true });
926-
} catch (error) {
927-
this.logger.warn("Failed to update code access state", { error });
928-
this.updateState({ hasCodeAccess: false });
915+
if (hasAccess !== null) {
916+
this.updateState({ hasCodeAccess: hasAccess });
917+
return;
929918
}
919+
920+
// Indeterminate: a transient/unauthorized failure isn't proof the invite
921+
// was revoked, so keep the prior value and let the next sync re-check.
922+
this.logger.warn(
923+
"Code access check was inconclusive; keeping previous value",
924+
{ hasCodeAccess: this.state.hasCodeAccess },
925+
);
926+
}
927+
928+
/**
929+
* Resolves Code invite access. Only a 2xx response with an explicit boolean
930+
* `has_access` is authoritative; everything else (offline, network error,
931+
* non-2xx, malformed body) is indeterminate, retried with backoff, then
932+
* returned as `null` so the caller keeps the prior value. Uses the synced
933+
* token directly rather than `authenticatedFetch`, which would re-enter the
934+
* refresh flow this runs inside and deadlock.
935+
*/
936+
private async checkCodeAccess(
937+
session: InMemorySession,
938+
): Promise<boolean | null> {
939+
const url = `${getCloudUrlFromRegion(session.cloudRegion)}/api/code/invites/check-access/`;
940+
941+
for (
942+
let attempt = 0;
943+
attempt < AuthService.CODE_ACCESS_MAX_ATTEMPTS;
944+
attempt++
945+
) {
946+
if (!this.connectivity.getStatus().isOnline) {
947+
return null;
948+
}
949+
950+
try {
951+
const response = await this.executeAuthenticatedFetch(
952+
fetch,
953+
url,
954+
{},
955+
session.accessToken,
956+
);
957+
958+
if (response.ok) {
959+
const data = (await response.json().catch(() => null)) as {
960+
has_access?: unknown;
961+
} | null;
962+
if (data && typeof data.has_access === "boolean") {
963+
return data.has_access;
964+
}
965+
this.logger.warn("Code access response missing has_access flag", {
966+
status: response.status,
967+
});
968+
} else {
969+
this.logger.warn("Code access check returned non-OK status", {
970+
status: response.status,
971+
});
972+
}
973+
} catch (error) {
974+
this.logger.warn("Code access check request failed", {
975+
error,
976+
attempt,
977+
});
978+
}
979+
980+
const isLastAttempt =
981+
attempt === AuthService.CODE_ACCESS_MAX_ATTEMPTS - 1;
982+
if (isLastAttempt) break;
983+
await sleepWithBackoff(attempt, AuthService.REFRESH_BACKOFF);
984+
}
985+
986+
return null;
930987
}
931988
private static readonly REFRESH_MAX_ATTEMPTS = 3;
932989
private static readonly ORG_FETCH_MAX_ATTEMPTS = 3;
990+
private static readonly CODE_ACCESS_MAX_ATTEMPTS = 3;
933991
private static readonly ORG_RECOVERY_MAX_ATTEMPTS = 5;
934992
private static readonly REFRESH_BACKOFF: BackoffOptions = {
935993
initialDelayMs: 1_000,

0 commit comments

Comments
 (0)