Skip to content

Commit c37b7e9

Browse files
authored
fix: persist the mfa session cookie outside the browser (#52)
`credentials: 'include'` is browser-only, so node dropped every Set-Cookie the server returned. Since server 2.4.0 MFA is on by default: signup/login withhold the access token, return "Proceed to mfa setup", and identify the pending user by an mfa_session cookie. skipMfaSetup / verifyOtp / the webauthn MFA-setup path resolve it only if that cookie comes back, so all of them failed with "invalid session" — the entire MFA surface was unreachable from node while the methods existed and read as correct. Store the mfa_session cookies per instance and replay them on the graphql/rest choke point. Scoped to those cookies rather than a general jar on purpose: the server resolves identity from the `cookie` session before the Authorization header, so replaying a login session would silently override a caller-supplied bearer token.
1 parent 9853cc6 commit c37b7e9

2 files changed

Lines changed: 136 additions & 5 deletions

File tree

‎__test__/mfaMethods.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import { Authorizer } from '../lib';
55

66
const mockFetch = crossFetch as unknown as jest.Mock;
77

8-
const jsonResponse = (body: unknown) =>
8+
const jsonResponse = (body: unknown, setCookie: string[] = []) =>
99
Promise.resolve({
1010
ok: true,
1111
status: 200,
1212
text: () => Promise.resolve(JSON.stringify(body)),
13+
// cross-fetch on node is node-fetch v2, whose Headers exposes raw().
14+
headers: { raw: () => ({ 'set-cookie': setCookie }) },
1315
});
1416

1517
describe('MFA setup/skip/lock SDK methods', () => {
@@ -44,6 +46,53 @@ describe('MFA setup/skip/lock SDK methods', () => {
4446
expect(body.variables.data.state).toBe('oidc-state');
4547
});
4648

49+
// Regression: node's fetch has no cookie store, so the mfa_session cookie
50+
// signup/login set was dropped and skipMfaSetup always failed with
51+
// "invalid session".
52+
it('replays the mfa_session cookie signup set on the follow-up skipMfaSetup', async () => {
53+
const authz = new Authorizer({
54+
authorizerURL: 'http://localhost:8080',
55+
redirectURL: 'http://localhost:8080/app',
56+
});
57+
mockFetch.mockReturnValueOnce(
58+
jsonResponse(
59+
{ data: { signup: { message: 'Proceed to mfa setup', access_token: null } } },
60+
[
61+
'mfa_session=sess-1; Path=/; Domain=localhost; Max-Age=179; HttpOnly',
62+
'mfa_session_domain=sess-1; Path=/; Domain=localhost; Max-Age=179; HttpOnly',
63+
// The login session cookie must NOT be replayed: the server resolves
64+
// identity from it before the Authorization header, so replaying it
65+
// would override a caller-supplied bearer token.
66+
'cookie=login-session; Path=/; Domain=localhost; HttpOnly',
67+
],
68+
),
69+
);
70+
await authz.signup({
71+
email: 'user@example.com',
72+
password: 'Test@123#',
73+
confirm_password: 'Test@123#',
74+
});
75+
expect(mockFetch.mock.calls[0][1].headers.Cookie).toBeUndefined();
76+
77+
mockFetch.mockReturnValueOnce(
78+
jsonResponse({ data: { skip_mfa_setup: { access_token: 'tok-123' } } }, [
79+
// consuming the session expires the cookie
80+
'mfa_session=; Path=/; Max-Age=0',
81+
'mfa_session_domain=; Path=/; Max-Age=0',
82+
]),
83+
);
84+
const res = await authz.skipMfaSetup({ email: 'user@example.com' });
85+
expect(res.data?.access_token).toBe('tok-123');
86+
expect(mockFetch.mock.calls[1][1].headers.Cookie).toBe(
87+
'mfa_session=sess-1; mfa_session_domain=sess-1',
88+
);
89+
90+
// expired cookies are dropped, not replayed
91+
mockFetch.mockReturnValueOnce(jsonResponse({ data: { logout: {} } }));
92+
await authz.logout();
93+
expect(mockFetch.mock.calls[2][1].headers.Cookie).toBeUndefined();
94+
});
95+
4796
it('lockMfa sends email/phone_number and returns the message', async () => {
4897
mockFetch.mockReturnValueOnce(
4998
jsonResponse({

‎src/index.ts‎

Lines changed: 86 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ const authTokenFragment = `message access_token expires_in refresh_token id_toke
2727
// set fetch based on window object. Cross fetch have issues with umd build
2828
const getFetcher = () => (hasWindow() ? window.fetch : crossFetch);
2929

30+
// Name prefix of the MFA-gate cookies the server sets on a token-withheld
31+
// signup/login (`mfa_session` and its domain-scoped twin `mfa_session_domain`
32+
// — see the backend's internal/cookie/mfa_session.go).
33+
const MFA_COOKIE_PREFIX = 'mfa_session';
34+
3035
function toErrorList(errors: unknown): Types.AuthorizerSDKError[] {
3136
if (Array.isArray(errors)) {
3237
return errors.map(toSDKError);
@@ -82,6 +87,23 @@ export class Authorizer {
8287
// it can be aborted before a modal ceremony starts - the browser allows only
8388
// one outstanding navigator.credentials.get() at a time.
8489
private conditionalPasskeyAbort?: AbortController;
90+
// MFA-session cookie store for non-browser runtimes. `credentials:
91+
// 'include'` is a browser-only mechanism: node's fetch has no cookie store,
92+
// so every Set-Cookie the server returns is dropped. That store is
93+
// REQUIRED, not an optimisation - since server 2.4.0 MFA is on by default,
94+
// so signup/login withhold the access token ("Proceed to mfa setup") and
95+
// identify the pending user by an `mfa_session` cookie. skipMfaSetup /
96+
// verifyOtp / the webauthn MFA-setup path resolve it only if that cookie
97+
// comes back, so without a store they always fail with "invalid session"
98+
// and the whole MFA surface is unreachable from node.
99+
//
100+
// Deliberately scoped to the MFA-gate cookies (MFA_COOKIE_PREFIX) rather
101+
// than being a general cookie jar: the server resolves a request's identity
102+
// from the `cookie` session BEFORE the Authorization header, so replaying a
103+
// login session cookie would silently override the bearer token a caller
104+
// passed explicitly. The MFA session is bound to one user id and consumed
105+
// on use, so it cannot be traded for another user's token.
106+
private mfaSessionCookies = new Map<string, string>();
85107

86108
// constructor
87109
constructor(config: Types.ConfigType) {
@@ -1308,15 +1330,14 @@ export class Authorizer {
13081330
graphqlQuery = async (
13091331
data: Types.GraphqlQueryRequest,
13101332
): Promise<Types.GrapQlResponseType> => {
1311-
const fetcher = getFetcher();
13121333
const body: Record<string, unknown> = {
13131334
query: data.query,
13141335
variables: data.variables || {},
13151336
};
13161337
if (data.operationName) {
13171338
body.operationName = data.operationName;
13181339
}
1319-
const res = await fetcher(`${this.config.authorizerURL}/graphql`, {
1340+
const res = await this.fetchWithCookies(`${this.config.authorizerURL}/graphql`, {
13201341
method: 'POST',
13211342
body: JSON.stringify(body),
13221343
headers: {
@@ -1426,8 +1447,7 @@ export class Authorizer {
14261447
body?: Record<string, unknown>,
14271448
headers?: Types.Headers,
14281449
): Promise<Types.GrapQlResponseType> => {
1429-
const fetcher = getFetcher();
1430-
const res = await fetcher(`${this.config.authorizerURL}${path}`, {
1450+
const res = await this.fetchWithCookies(`${this.config.authorizerURL}${path}`, {
14311451
method,
14321452
...(method === 'POST' ? { body: JSON.stringify(body || {}) } : {}),
14331453
headers: {
@@ -1472,6 +1492,68 @@ export class Authorizer {
14721492
return { data: coerceInt64Fields(json), errors: [] };
14731493
};
14741494

1495+
// fetchWithCookies is the single choke point every graphql/rest call goes
1496+
// through. In a browser it is a plain fetch (the browser owns the cookies
1497+
// and `Cookie` is a forbidden request header anyway); elsewhere it replays
1498+
// the stored MFA session and records the one the response sets.
1499+
private fetchWithCookies = async (
1500+
url: string,
1501+
init: Record<string, any>,
1502+
): Promise<any> => {
1503+
const fetcher = getFetcher();
1504+
if (hasWindow()) return fetcher(url, init as any);
1505+
1506+
const cookie = [...this.mfaSessionCookies]
1507+
.map(([k, v]) => `${k}=${v}`)
1508+
.join('; ');
1509+
const res = await fetcher(url, {
1510+
...init,
1511+
headers: {
1512+
// Caller-supplied headers still win, so an explicit Cookie header
1513+
// overrides the stored one.
1514+
...(cookie ? { Cookie: cookie } : {}),
1515+
...init.headers,
1516+
},
1517+
} as any);
1518+
this.storeMfaSessionCookies(res);
1519+
return res;
1520+
};
1521+
1522+
// storeMfaSessionCookies records the MFA-gate cookies from a response.
1523+
// ponytail: name=value only - no domain/path/Secure matching, because every
1524+
// request from this instance goes to the one origin in config.authorizerURL.
1525+
private storeMfaSessionCookies = (res: any): void => {
1526+
const h = res?.headers;
1527+
// undici/whatwg expose getSetCookie(); cross-fetch on node (node-fetch v2)
1528+
// exposes raw(). `get('set-cookie')` is the last-resort single-value read.
1529+
const raw: string[] =
1530+
typeof h?.getSetCookie === 'function'
1531+
? h.getSetCookie()
1532+
: typeof h?.raw === 'function'
1533+
? h.raw()['set-cookie'] || []
1534+
: h?.get?.('set-cookie')
1535+
? [h.get('set-cookie')]
1536+
: [];
1537+
1538+
for (const entry of raw) {
1539+
const [pair, ...attrs] = entry.split(';');
1540+
const eq = pair.indexOf('=');
1541+
if (eq < 1) continue;
1542+
const name = pair.slice(0, eq).trim();
1543+
if (!name.startsWith(MFA_COOKIE_PREFIX)) continue;
1544+
const value = pair.slice(eq + 1).trim();
1545+
// The server expires a cookie by resending it empty with Max-Age<=0
1546+
// (consuming or abandoning the mfa session); drop it rather than
1547+
// replaying a dead session id.
1548+
const expired = attrs.some((a) => {
1549+
const [k, v] = a.split('=');
1550+
return k.trim().toLowerCase() === 'max-age' && Number(v) <= 0;
1551+
});
1552+
if (!value || expired) this.mfaSessionCookies.delete(name);
1553+
else this.mfaSessionCookies.set(name, value);
1554+
}
1555+
};
1556+
14751557
errorResponse = (errors: unknown): Types.ApiResponse<any> => {
14761558
return {
14771559
data: undefined,

0 commit comments

Comments
 (0)