Skip to content

Consolidate access-token storage to the server-side session #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devin-ai-integration wants to merge 1 commit into from
Closed

Consolidate access-token storage to the server-side session #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 45 additions & 1 deletion src/client/tokenStore.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,13 @@ describe('TokenStore', () => {
};
const refreshedToken = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.${btoa(JSON.stringify(refreshedPayload))}.mock-signature`;

document.cookie = `workos-access-token=${encodeURIComponent(initialToken)}`;
// Seed the initial token through the session-bound server RPC (the only
// legitimate token source) rather than a client-readable cookie.
vi.mocked(getAccessTokenAction).mockResolvedValue(initialToken);
vi.mocked(refreshAccessTokenAction).mockResolvedValue(refreshedToken);

const localStore = new TokenStore();
await localStore.getAccessTokenSilently();
expect(refreshAccessTokenAction).not.toHaveBeenCalled();

// Advance past the originally scheduled fire time. If the schedule buffer
Expand All @@ -366,4 +369,45 @@ describe('TokenStore', () => {
localStore.reset();
});
});

describe('does not trust client-readable cookies (SEC-1348)', () => {
afterEach(() => {
document.cookie
.split(';')
.map((c) => c.split('=')[0].trim())
.filter(Boolean)
.forEach((name) => {
document.cookie = `${name}=; Path=/; Max-Age=0`;
document.cookie = `${name}=; Max-Age=0`;
});
});

it('ignores a planted workos-access-token cookie and uses the session-bound RPC instead', async () => {
const now = Math.floor(Date.now() / 1000);
const makeToken = (sub: string) =>
`eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.${btoa(
JSON.stringify({ sub, sid: `session_${sub}`, iat: now, exp: now + 3600 }),
)}.mock-signature`;

const victimToken = makeToken('victim');
const attackerToken = makeToken('attacker');

// Attacker plants their own token via a cookie-write primitive.
document.cookie = `workos-access-token=${encodeURIComponent(attackerToken)}; Path=/`;

// The legitimate, session-bound token source returns the victim's token.
vi.mocked(getAccessTokenAction).mockResolvedValue(victimToken);
vi.mocked(refreshAccessTokenAction).mockResolvedValue(victimToken);

const localStore = new TokenStore();
// Construction must not adopt the cookie.
expect(localStore.getSnapshot().token).toBeUndefined();

const served = await localStore.getAccessToken();
expect(served).toBe(victimToken);
expect(served).not.toBe(attackerToken);

localStore.reset();
});
});
});
107 changes: 1 addition & 106 deletions src/client/tokenStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const SHORT_TOKEN_EXPIRY_BUFFER_SECONDS = 30;
const MIN_REFRESH_DELAY_SECONDS = 15;
const MAX_REFRESH_DELAY_SECONDS = 24 * 60 * 60;
const RETRY_DELAY_SECONDS = 300;
const jwtCookieName = 'workos-access-token';

function getExpiryBuffer(totalTokenLifetime: number): number {
return totalTokenLifetime <= SHORT_TOKEN_LIFETIME_SECONDS
Expand All @@ -26,9 +25,8 @@ export class TokenStore {
private serverSnapshot: TokenState;

constructor() {
const initialToken = typeof window !== 'undefined' ? this.getInitialTokenFromCookie() : undefined;
this.state = {
token: initialToken,
token: undefined,
loading: false,
error: null,
};
Expand All @@ -38,20 +36,11 @@ export class TokenStore {
loading: false,
error: null,
};

if (initialToken) {
this.fastCookieConsumed = true;
const tokenData = this.parseToken(initialToken);
if (tokenData) {
this.scheduleRefresh(tokenData);
}
}
}

private listeners = new Set<() => void>();
private refreshPromise: Promise<string | undefined> | null = null;
private refreshTimeout: ReturnType<typeof setTimeout> | undefined;
private fastCookieConsumed = false;

subscribe = (listener: () => void) => {
this.listeners.add(listener);
Expand Down Expand Up @@ -108,79 +97,6 @@ export class TokenStore {
return Math.min(Math.max(idealDelay, MIN_REFRESH_DELAY_SECONDS * 1000), MAX_REFRESH_DELAY_SECONDS * 1000);
}

private deleteCookie() {
const isSecure = window.location.protocol === 'https:';

const deletionString = isSecure
? `${jwtCookieName}=; SameSite=Lax; Max-Age=0; Secure`
: `${jwtCookieName}=; SameSite=Lax; Max-Age=0`;

document.cookie = deletionString;
}

private getInitialTokenFromCookie(): string | undefined {
if (typeof document === 'undefined' || typeof document.cookie === 'undefined') {
return;
}

const cookies = document.cookie.split(';').reduce(
(acc, cookie) => {
const [name, ...valueParts] = cookie.trim().split('=');
if (name && valueParts.length > 0) {
const value = valueParts.join('=');
acc[name.trim()] = decodeURIComponent(value);
}
return acc;
},
{} as Record<string, string>,
);

const token = cookies[jwtCookieName];
if (!token) {
return;
}

this.deleteCookie();

return token;
}

private consumeFastCookie(): string | undefined {
if (this.fastCookieConsumed) {
return;
}

if (typeof document === 'undefined' || typeof document.cookie === 'undefined') {
return;
}

const cookies = document.cookie.split(';').reduce(
(acc, cookie) => {
const [name, ...valueParts] = cookie.trim().split('=');
if (name && valueParts.length > 0) {
const value = valueParts.join('=');
acc[name.trim()] = decodeURIComponent(value);
}
return acc;
},
{} as Record<string, string>,
);

const newToken = cookies[jwtCookieName];
if (!newToken) {
this.fastCookieConsumed = true;
return;
}

this.fastCookieConsumed = true;

this.deleteCookie();

if (newToken !== this.state.token) {
return newToken;
}
}

parseToken(token: string | undefined) {
if (!token) return null;

Expand Down Expand Up @@ -223,13 +139,6 @@ export class TokenStore {
}

async getAccessToken(): Promise<string | undefined> {
const fastToken = this.consumeFastCookie();

if (fastToken) {
this.setState({ token: fastToken, loading: false, error: null });
return fastToken;
}

const tokenData = this.parseToken(this.state.token);

if (tokenData && !tokenData.isExpiring) {
Expand All @@ -244,19 +153,6 @@ export class TokenStore {
}

async getAccessTokenSilently(): Promise<string | undefined> {
const fastToken = this.consumeFastCookie();

if (fastToken) {
this.setState({ token: fastToken, loading: false, error: null });

const tokenData = this.parseToken(fastToken);
if (tokenData) {
this.scheduleRefresh(tokenData);
}

return fastToken;
}

const tokenData = this.parseToken(this.state.token);

if (tokenData && !tokenData.isExpiring) {
Expand Down Expand Up @@ -355,7 +251,6 @@ export class TokenStore {
reset() {
this.state = { token: undefined, loading: false, error: null };
this.refreshPromise = null;
this.fastCookieConsumed = false;
if (this.refreshTimeout) {
clearTimeout(this.refreshTimeout);
this.refreshTimeout = undefined;
Expand Down
Loading