feat(portal-bff): close the auth loop — callback persists session, /me, RP-initiated /logout
callback now writes the resolved AuthenticatedUser into
req.session.user and waits for req.session.save() before the 302 so
the spa lands on the post-login page with a populated session
already on redis. without the awaited save, connect-redis can race
the spa's subsequent /me call.
GET /auth/me returns the curated public subset of the session user
(oid / tid / username / displayName) or 401 with
{"error":"unauthenticated"}. amr and other internal claims stay
server-side — the @requireMfa() guard (adr-0011) and the audit
pipeline (adr-0013) consume them directly off the session.
GET /auth/logout destroys the redis-side session, clears the
session cookie (sessionCookieName() so it matches what the middleware
set — __Host-portal_session in prod, portal_session in dev), then
302s to entra's /oauth2/v2.0/logout?post_logout_redirect_uri=… so
the idp-side session is killed too (rp-initiated logout per
adr-0009). id_token_hint is deliberately omitted in v1: the
id_token isn't persisted in the session yet (adr-0014 dependency),
and the account-picker is a safer default in the interim.
logout is idempotent: anonymous users get the same redirect, the
session.destroy() is a no-op. a redis-side destroy failure is
logged via pino as event=session.destroy_failed and the cookie is
cleared anyway — orphan redis keys hit their idle ttl on their own.
a small typescript module augmentation in session/session.types.ts
declares req.session.user so every consumer sees a typed payload
instead of `unknown`. side-effect imported by session.module.ts to
keep it in the compile graph.
out of scope, landing in follow-ups:
- absolute-timeout interceptor (12 h hard ceiling, adr-0010)
- user_sessions:{userId} secondary index (admin "logout everywhere")
- encrypted tokens blob in the session (id_token, access_token,
refresh_token — adr-0014)
- csrf middleware (phase-2 security)
This commit is contained in:
@@ -31,19 +31,50 @@ const USER: AuthenticatedUser = {
|
||||
};
|
||||
|
||||
function makeResStub() {
|
||||
const res = {
|
||||
cookie: jest.fn(),
|
||||
clearCookie: jest.fn(),
|
||||
redirect: jest.fn(),
|
||||
status: jest.fn(),
|
||||
json: jest.fn(),
|
||||
};
|
||||
// status/json/cookie/clearCookie/redirect all return the response
|
||||
// in Express for chaining. Wire `mockReturnThis` after the object
|
||||
// exists so `this` resolves to the same stub.
|
||||
res.cookie.mockReturnValue(res);
|
||||
res.clearCookie.mockReturnValue(res);
|
||||
res.redirect.mockReturnValue(res);
|
||||
res.status.mockReturnValue(res);
|
||||
res.json.mockReturnValue(res);
|
||||
return res as unknown as Response & typeof res;
|
||||
}
|
||||
|
||||
interface SessionStub {
|
||||
user?: AuthenticatedUser;
|
||||
save: jest.Mock;
|
||||
destroy: jest.Mock;
|
||||
}
|
||||
|
||||
function makeSessionStub(opts?: {
|
||||
user?: AuthenticatedUser;
|
||||
saveError?: Error;
|
||||
destroyError?: Error;
|
||||
}): SessionStub {
|
||||
return {
|
||||
cookie: jest.fn().mockReturnThis(),
|
||||
clearCookie: jest.fn().mockReturnThis(),
|
||||
redirect: jest.fn().mockReturnThis(),
|
||||
} as unknown as Response & {
|
||||
cookie: jest.Mock;
|
||||
clearCookie: jest.Mock;
|
||||
redirect: jest.Mock;
|
||||
...(opts?.user !== undefined ? { user: opts.user } : {}),
|
||||
save: jest.fn((cb: (err?: Error) => void) => cb(opts?.saveError)),
|
||||
destroy: jest.fn((cb: (err?: Error) => void) => cb(opts?.destroyError)),
|
||||
};
|
||||
}
|
||||
|
||||
function makeReqStub(signedCookies: Record<string, unknown> = {}): Request {
|
||||
return { signedCookies } as unknown as Request;
|
||||
function makeReqStub(opts?: {
|
||||
signedCookies?: Record<string, unknown>;
|
||||
session?: SessionStub;
|
||||
}): Request & { session: SessionStub } {
|
||||
return {
|
||||
signedCookies: opts?.signedCookies ?? {},
|
||||
session: opts?.session ?? makeSessionStub(),
|
||||
} as unknown as Request & { session: SessionStub };
|
||||
}
|
||||
|
||||
function makeLoggerStub() {
|
||||
@@ -52,7 +83,12 @@ function makeLoggerStub() {
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
debug: jest.fn(),
|
||||
} as unknown as Logger & { log: jest.Mock; warn: jest.Mock };
|
||||
} as unknown as Logger & {
|
||||
log: jest.Mock;
|
||||
warn: jest.Mock;
|
||||
error: jest.Mock;
|
||||
debug: jest.Mock;
|
||||
};
|
||||
}
|
||||
|
||||
interface ControllerFixture {
|
||||
@@ -62,13 +98,20 @@ interface ControllerFixture {
|
||||
logger: ReturnType<typeof makeLoggerStub>;
|
||||
}
|
||||
|
||||
const LOGOUT_URL = `${ENTRA.authority}/oauth2/v2.0/logout?post_logout_redirect_uri=${encodeURIComponent(ENTRA.postLogoutRedirectUri)}`;
|
||||
|
||||
function makeController(opts?: { completeAuthCodeFlow?: jest.Mock }): ControllerFixture {
|
||||
const beginAuthCodeFlow = jest.fn().mockResolvedValue({
|
||||
authUrl: 'https://entra.example/authorize?state=state-nonce',
|
||||
preAuthPayload: PRE_AUTH,
|
||||
});
|
||||
const completeAuthCodeFlow = opts?.completeAuthCodeFlow ?? jest.fn().mockResolvedValue(USER);
|
||||
const service = { beginAuthCodeFlow, completeAuthCodeFlow } as unknown as AuthService;
|
||||
const buildLogoutUrl = jest.fn().mockReturnValue(LOGOUT_URL);
|
||||
const service = {
|
||||
beginAuthCodeFlow,
|
||||
completeAuthCodeFlow,
|
||||
buildLogoutUrl,
|
||||
} as unknown as AuthService;
|
||||
const logger = makeLoggerStub();
|
||||
return {
|
||||
controller: new AuthController(service, logger as unknown as Logger, ENTRA),
|
||||
@@ -124,15 +167,23 @@ describe('AuthController.login', () => {
|
||||
});
|
||||
|
||||
describe('AuthController.callback', () => {
|
||||
it('clears the pre-auth cookie, logs success, redirects to the SPA on the happy path', async () => {
|
||||
it('clears the pre-auth cookie, persists the user in the session, logs success, redirects to the SPA on the happy path', async () => {
|
||||
const { controller, completeAuthCodeFlow, logger } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) });
|
||||
const session = makeSessionStub();
|
||||
const req = makeReqStub({
|
||||
signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) },
|
||||
session,
|
||||
});
|
||||
|
||||
await controller.callback(req, res, 'auth-code', PRE_AUTH.state);
|
||||
|
||||
expect(res.clearCookie).toHaveBeenCalledWith(PRE_AUTH_COOKIE_NAME, expect.any(Object));
|
||||
expect(completeAuthCodeFlow).toHaveBeenCalledWith('auth-code', PRE_AUTH.state, PRE_AUTH);
|
||||
// The resolved user must be on the session before the redirect
|
||||
// so the subsequent /me call sees a populated payload.
|
||||
expect(session.user).toEqual(USER);
|
||||
expect(session.save).toHaveBeenCalledTimes(1);
|
||||
expect(logger.log).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
event: 'auth.signed_in',
|
||||
@@ -145,10 +196,27 @@ describe('AuthController.callback', () => {
|
||||
expect(res.redirect).toHaveBeenCalledWith(302, ENTRA.postLogoutRedirectUri);
|
||||
});
|
||||
|
||||
it('propagates a session.save() error as an exception (so the SPA never sees a successful redirect with no session)', async () => {
|
||||
const { controller } = makeController();
|
||||
const res = makeResStub();
|
||||
const session = makeSessionStub({ saveError: new Error('redis down') });
|
||||
const req = makeReqStub({
|
||||
signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) },
|
||||
session,
|
||||
});
|
||||
|
||||
await expect(controller.callback(req, res, 'auth-code', PRE_AUTH.state)).rejects.toThrow(
|
||||
/redis down/,
|
||||
);
|
||||
expect(res.redirect).not.toHaveBeenCalledWith(302, ENTRA.postLogoutRedirectUri);
|
||||
});
|
||||
|
||||
it('redirects with ?auth_error=token-exchange-failed when Entra returns error', async () => {
|
||||
const { controller, completeAuthCodeFlow } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) });
|
||||
const req = makeReqStub({
|
||||
signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) },
|
||||
});
|
||||
|
||||
await controller.callback(req, res, undefined, undefined, 'access_denied', 'user cancelled');
|
||||
|
||||
@@ -163,7 +231,7 @@ describe('AuthController.callback', () => {
|
||||
it('redirects with ?auth_error=flow-expired when the pre-auth cookie is missing', async () => {
|
||||
const { controller, completeAuthCodeFlow, logger } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({});
|
||||
const req = makeReqStub();
|
||||
|
||||
await controller.callback(req, res, 'code', 'state');
|
||||
|
||||
@@ -184,7 +252,9 @@ describe('AuthController.callback', () => {
|
||||
.mockRejectedValue(new AuthCodeFlowException({ kind: 'state-mismatch' }));
|
||||
const { controller, logger } = makeController({ completeAuthCodeFlow });
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) });
|
||||
const req = makeReqStub({
|
||||
signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) },
|
||||
});
|
||||
|
||||
await controller.callback(req, res, 'code', 'state');
|
||||
|
||||
@@ -204,7 +274,9 @@ describe('AuthController.callback', () => {
|
||||
it('redirects with ?auth_error=token-exchange-failed when query is missing code/state', async () => {
|
||||
const { controller } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) });
|
||||
const req = makeReqStub({
|
||||
signedCookies: { [PRE_AUTH_COOKIE_NAME]: JSON.stringify(PRE_AUTH) },
|
||||
});
|
||||
|
||||
await controller.callback(req, res); // no code, no state
|
||||
|
||||
@@ -217,7 +289,7 @@ describe('AuthController.callback', () => {
|
||||
it('rejects a malformed pre-auth cookie as flow-expired', async () => {
|
||||
const { controller, completeAuthCodeFlow } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ [PRE_AUTH_COOKIE_NAME]: 'not-json' });
|
||||
const req = makeReqStub({ signedCookies: { [PRE_AUTH_COOKIE_NAME]: 'not-json' } });
|
||||
|
||||
await controller.callback(req, res, 'code', 'state');
|
||||
|
||||
@@ -228,3 +300,127 @@ describe('AuthController.callback', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AuthController.me', () => {
|
||||
it('returns the curated public user when the session is populated', () => {
|
||||
const { controller } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ session: makeSessionStub({ user: USER }) });
|
||||
|
||||
controller.me(req, res);
|
||||
|
||||
expect(res.status).not.toHaveBeenCalledWith(401);
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
oid: USER.oid,
|
||||
tid: USER.tid,
|
||||
username: USER.username,
|
||||
displayName: USER.displayName,
|
||||
});
|
||||
});
|
||||
|
||||
it('strips amr (internal claim) from the response payload', () => {
|
||||
const { controller } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ session: makeSessionStub({ user: USER }) });
|
||||
|
||||
controller.me(req, res);
|
||||
|
||||
const payload = res.json.mock.calls[0]?.[0] as Record<string, unknown>;
|
||||
expect(payload).not.toHaveProperty('amr');
|
||||
});
|
||||
|
||||
it('returns 401 when no user is on the session', () => {
|
||||
const { controller } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub();
|
||||
|
||||
controller.me(req, res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(401);
|
||||
expect(res.json).toHaveBeenCalledWith({ error: 'unauthenticated' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('AuthController.logout', () => {
|
||||
it('destroys the session, clears the session cookie, logs success, redirects to Entra logout', async () => {
|
||||
const { controller, logger } = makeController();
|
||||
const res = makeResStub();
|
||||
const session = makeSessionStub({ user: USER });
|
||||
const req = makeReqStub({ session });
|
||||
|
||||
await controller.logout(req, res);
|
||||
|
||||
expect(session.destroy).toHaveBeenCalledTimes(1);
|
||||
expect(res.clearCookie).toHaveBeenCalledWith('portal_session', { path: '/' });
|
||||
expect(logger.log).toHaveBeenCalledWith(
|
||||
{ event: 'auth.signed_out', wasAuthenticated: true },
|
||||
'AuthLogout',
|
||||
);
|
||||
// Authority + /oauth2/v2.0/logout?post_logout_redirect_uri=…
|
||||
expect(res.redirect).toHaveBeenCalledWith(
|
||||
302,
|
||||
expect.stringContaining(`${ENTRA.authority}/oauth2/v2.0/logout`),
|
||||
);
|
||||
expect(res.redirect).toHaveBeenCalledWith(
|
||||
302,
|
||||
expect.stringContaining(
|
||||
`post_logout_redirect_uri=${encodeURIComponent(ENTRA.postLogoutRedirectUri)}`,
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it('uses the __Host- prefixed cookie name in production', async () => {
|
||||
const originalNodeEnv = process.env['NODE_ENV'];
|
||||
try {
|
||||
process.env['NODE_ENV'] = 'production';
|
||||
const { controller } = makeController();
|
||||
const res = makeResStub();
|
||||
const req = makeReqStub({ session: makeSessionStub({ user: USER }) });
|
||||
|
||||
await controller.logout(req, res);
|
||||
|
||||
expect(res.clearCookie).toHaveBeenCalledWith('__Host-portal_session', { path: '/' });
|
||||
} finally {
|
||||
if (originalNodeEnv === undefined) {
|
||||
delete process.env['NODE_ENV'];
|
||||
} else {
|
||||
process.env['NODE_ENV'] = originalNodeEnv;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('still clears the cookie and redirects when the user was already anonymous', async () => {
|
||||
const { controller, logger } = makeController();
|
||||
const res = makeResStub();
|
||||
const session = makeSessionStub();
|
||||
const req = makeReqStub({ session });
|
||||
|
||||
await controller.logout(req, res);
|
||||
|
||||
expect(session.destroy).toHaveBeenCalledTimes(1);
|
||||
expect(logger.log).toHaveBeenCalledWith(
|
||||
{ event: 'auth.signed_out', wasAuthenticated: false },
|
||||
'AuthLogout',
|
||||
);
|
||||
expect(res.redirect).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('logs but does not throw when session.destroy fails — still clears cookie and redirects', async () => {
|
||||
const { controller, logger } = makeController();
|
||||
const res = makeResStub();
|
||||
const session = makeSessionStub({ user: USER, destroyError: new Error('redis down') });
|
||||
const req = makeReqStub({ session });
|
||||
|
||||
await controller.logout(req, res);
|
||||
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
event: 'session.destroy_failed',
|
||||
message: 'redis down',
|
||||
}),
|
||||
'AuthLogout',
|
||||
);
|
||||
expect(res.clearCookie).toHaveBeenCalled();
|
||||
expect(res.redirect).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user