77343e3113
## Summary PR #129 (`feat(portal-bff): distinct admin session + /api/admin/auth flow`) baked `4201` into a handful of comments, test fixtures, and the `.env.example` as the portal-admin dev port. The actual port wired in [apps/portal-admin/project.json](apps/portal-admin/project.json#L87) `serve.options.port` is **4300** — that's what `pnpm nx serve portal-admin` listens on. This PR aligns the references so a contributor copying values from `.env.example` (or reading the test fixtures) sees the same port their browser is going to hit. It also drops `http://localhost:4300` into `CORS_ALLOWED_ORIGINS` — the portal-admin SPA will hit the BFF with credentials as soon as the admin auth flow is exercised end-to-end, and without the origin in the allowlist the browser blocks the call. Better to set the right example now than have the next contributor chase a CORS error. ## Touched - [apps/portal-bff/.env.example](apps/portal-bff/.env.example): - `ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI` default + the surrounding comment now point at `http://localhost:4300/`. - `CORS_ALLOWED_ORIGINS` example lists both `:4200` (portal-shell) and `:4300` (portal-admin). - Both sections cite `apps/<app>/project.json` `serve.options.port` as the source of truth so a future reader doesn't have to grep. - [apps/portal-bff/src/config/check-cors-allowlist.ts](apps/portal-bff/src/config/check-cors-allowlist.ts) — stale doc-comment that pre-dated the portal-admin scaffolding, now matches reality. - Test-fixture `adminPostLogoutRedirectUri` values in `auth.module.spec.ts`, `auth.controller.spec.ts`, `auth.service.spec.ts`, `admin-auth.controller.spec.ts`, `check-entra-config.spec.ts` — tests don't depend on the port; aligned for clarity only. ## Test plan - [x] `grep -rn 4201 apps/ libs/` → empty. - [x] `pnpm nx test portal-bff` — **278 specs pass** (unchanged from #129; this PR only touches strings). - [x] No behaviour change in the BFF; only the example values shift. Developers must update their local `.env` to pick up the new port + origin. ## Notes for the reviewer The two new env vars from #129 (`ENTRA_ADMIN_REDIRECT_URI`, `ENTRA_ADMIN_POST_LOGOUT_REDIRECT_URI`) plus the existing `CORS_ALLOWED_ORIGINS` are mandatory at boot. If your local `apps/portal-bff/.env` still has the `4201` value, the BFF will still start (any valid URL passes the validators) — but admin logout will 302 you to a port nothing is listening on, and the admin SPA's BFF calls will fail CORS. Update to `4300` to match the actual portal-admin dev server. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #131
292 lines
11 KiB
TypeScript
292 lines
11 KiB
TypeScript
import type { AuthenticationResult, ConfidentialClientApplication } from '@azure/msal-node';
|
|
import { PRE_AUTH_COOKIE_TTL_MS } from './auth.cookie';
|
|
import { AuthCodeFlowException } from './auth.errors';
|
|
import { AuthService, type PreAuthPayload } from './auth.service';
|
|
import type { EntraConfig } from './entra-config.token';
|
|
|
|
const ENTRA: EntraConfig = {
|
|
instanceUrl: 'https://login.microsoftonline.com/',
|
|
tenantId: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee',
|
|
clientId: '11111111-2222-3333-4444-555555555555',
|
|
clientSecret: 's3cret',
|
|
redirectUri: 'http://localhost:3000/api/auth/callback',
|
|
postLogoutRedirectUri: 'http://localhost:4200/',
|
|
adminRedirectUri: 'http://localhost:3000/api/admin/auth/callback',
|
|
adminPostLogoutRedirectUri: 'http://localhost:4300/',
|
|
authority: 'https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee',
|
|
};
|
|
|
|
interface ServiceFixture {
|
|
service: AuthService;
|
|
getAuthCodeUrl: jest.Mock;
|
|
acquireTokenByCode: jest.Mock;
|
|
}
|
|
|
|
function makeService(overrides?: { acquireTokenByCode?: jest.Mock }): ServiceFixture {
|
|
const getAuthCodeUrl = jest.fn().mockResolvedValue('https://entra.example/authorize?…');
|
|
const acquireTokenByCode =
|
|
overrides?.acquireTokenByCode ??
|
|
jest.fn().mockResolvedValue(makeAuthResult({ amr: ['pwd', 'mfa'] }));
|
|
const msalStub = {
|
|
getAuthCodeUrl,
|
|
acquireTokenByCode,
|
|
} as unknown as ConfidentialClientApplication;
|
|
return { service: new AuthService(msalStub, ENTRA), getAuthCodeUrl, acquireTokenByCode };
|
|
}
|
|
|
|
function makeAuthResult(
|
|
claims: Partial<{
|
|
amr: string[];
|
|
roles: unknown;
|
|
oid: string;
|
|
tid: string;
|
|
name: string;
|
|
preferred_username: string;
|
|
}>,
|
|
): AuthenticationResult {
|
|
const idTokenClaims: Record<string, unknown> = {
|
|
oid: claims.oid ?? 'user-oid',
|
|
tid: claims.tid ?? ENTRA.tenantId,
|
|
name: claims.name ?? 'Jane Doe',
|
|
preferred_username: claims.preferred_username ?? 'jane.doe@apf.example',
|
|
amr: claims.amr ?? ['pwd', 'mfa'],
|
|
};
|
|
// `roles` is optional in Entra ID tokens — only present when the
|
|
// user has at least one app role assigned. The fixture omits it
|
|
// unless the caller explicitly opts in (including with falsy
|
|
// values to exercise the empty-array fallback).
|
|
if (Object.prototype.hasOwnProperty.call(claims, 'roles')) {
|
|
idTokenClaims['roles'] = claims.roles;
|
|
}
|
|
return {
|
|
idTokenClaims,
|
|
account: { username: 'jane.doe@apf.example', name: 'Jane Doe' },
|
|
} as unknown as AuthenticationResult;
|
|
}
|
|
|
|
const PRE_AUTH_OK: PreAuthPayload = {
|
|
state: 'state-nonce',
|
|
codeVerifier: 'verifier-secret',
|
|
createdAt: 1_000_000,
|
|
};
|
|
|
|
describe('AuthService.beginAuthCodeFlow', () => {
|
|
it('builds the auth URL with the configured redirect, OIDC scopes, S256 challenge', async () => {
|
|
const { service, getAuthCodeUrl } = makeService();
|
|
await service.beginAuthCodeFlow(ENTRA.redirectUri);
|
|
expect(getAuthCodeUrl).toHaveBeenCalledTimes(1);
|
|
const arg = getAuthCodeUrl.mock.calls[0]?.[0] as Record<string, unknown>;
|
|
expect(arg['redirectUri']).toBe(ENTRA.redirectUri);
|
|
expect(arg['scopes']).toEqual(['openid', 'profile', 'email']);
|
|
expect(arg['codeChallengeMethod']).toBe('S256');
|
|
expect(typeof arg['codeChallenge']).toBe('string');
|
|
expect(typeof arg['state']).toBe('string');
|
|
});
|
|
|
|
it('returns the pre-auth payload with state + codeVerifier matching what MSAL was called with', async () => {
|
|
const { service, getAuthCodeUrl } = makeService();
|
|
const { preAuthPayload } = await service.beginAuthCodeFlow(ENTRA.redirectUri);
|
|
const arg = getAuthCodeUrl.mock.calls[0]?.[0] as Record<string, unknown>;
|
|
expect(preAuthPayload.state).toBe(arg['state']);
|
|
expect(preAuthPayload.codeVerifier).not.toBe(arg['codeChallenge']);
|
|
expect(preAuthPayload.createdAt).toBeLessThanOrEqual(Date.now());
|
|
});
|
|
|
|
it('generates a fresh state + verifier on every call', async () => {
|
|
const { service } = makeService();
|
|
const a = await service.beginAuthCodeFlow(ENTRA.redirectUri);
|
|
const b = await service.beginAuthCodeFlow(ENTRA.redirectUri);
|
|
expect(a.preAuthPayload.state).not.toBe(b.preAuthPayload.state);
|
|
expect(a.preAuthPayload.codeVerifier).not.toBe(b.preAuthPayload.codeVerifier);
|
|
});
|
|
});
|
|
|
|
describe('AuthService.completeAuthCodeFlow', () => {
|
|
it('returns the authenticated user on the happy path', async () => {
|
|
const { service, acquireTokenByCode } = makeService();
|
|
const user = await service.completeAuthCodeFlow(
|
|
'auth-code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt + 1_000,
|
|
);
|
|
expect(acquireTokenByCode).toHaveBeenCalledWith({
|
|
code: 'auth-code',
|
|
codeVerifier: PRE_AUTH_OK.codeVerifier,
|
|
redirectUri: ENTRA.redirectUri,
|
|
scopes: ['openid', 'profile', 'email'],
|
|
});
|
|
expect(user).toEqual({
|
|
oid: 'user-oid',
|
|
tid: ENTRA.tenantId,
|
|
username: 'jane.doe@apf.example',
|
|
displayName: 'Jane Doe',
|
|
amr: ['pwd', 'mfa'],
|
|
roles: [],
|
|
});
|
|
});
|
|
|
|
it('throws state-mismatch when the query state differs from the cookie state', async () => {
|
|
const { service } = makeService();
|
|
await expect(
|
|
service.completeAuthCodeFlow(
|
|
'code',
|
|
'other-state',
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
),
|
|
).rejects.toMatchObject({ failure: { kind: 'state-mismatch' } });
|
|
});
|
|
|
|
it('throws flow-expired when the cookie is older than the TTL', async () => {
|
|
const { service } = makeService();
|
|
const now = PRE_AUTH_OK.createdAt + PRE_AUTH_COOKIE_TTL_MS + 1;
|
|
await expect(
|
|
service.completeAuthCodeFlow('code', PRE_AUTH_OK.state, PRE_AUTH_OK, ENTRA.redirectUri, now),
|
|
).rejects.toMatchObject({ failure: { kind: 'flow-expired' } });
|
|
});
|
|
|
|
it('returns the user even when the ID token has no `amr` claim (Entra optional)', async () => {
|
|
const acquireTokenByCode = jest.fn().mockResolvedValue(makeAuthResult({ amr: [] }));
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
const user = await service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
);
|
|
// `amr` flows through as an empty array; MFA enforcement is
|
|
// Conditional Access's job per ADR-0011, not this code path.
|
|
expect(user.amr).toEqual([]);
|
|
expect(user.oid).toBe('user-oid');
|
|
});
|
|
|
|
it('surfaces the `roles` claim when Entra includes it (app-role-assigned user)', async () => {
|
|
const acquireTokenByCode = jest
|
|
.fn()
|
|
.mockResolvedValue(makeAuthResult({ roles: ['admin', 'editor'] }));
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
const user = await service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
);
|
|
expect(user.roles).toEqual(['admin', 'editor']);
|
|
});
|
|
|
|
it('returns an empty `roles` array when the claim is absent (no app role assigned)', async () => {
|
|
const acquireTokenByCode = jest.fn().mockResolvedValue(makeAuthResult({}));
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
const user = await service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
);
|
|
expect(user.roles).toEqual([]);
|
|
});
|
|
|
|
it('returns an empty `roles` array when the claim is non-array (defensive)', async () => {
|
|
const acquireTokenByCode = jest.fn().mockResolvedValue(makeAuthResult({ roles: 'admin' }));
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
const user = await service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
);
|
|
expect(user.roles).toEqual([]);
|
|
});
|
|
|
|
it('drops non-string entries from `roles` (defensive against unexpected claim shapes)', async () => {
|
|
const acquireTokenByCode = jest
|
|
.fn()
|
|
.mockResolvedValue(makeAuthResult({ roles: ['admin', 42, null, 'editor'] }));
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
const user = await service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
);
|
|
expect(user.roles).toEqual(['admin', 'editor']);
|
|
});
|
|
|
|
it('throws token-exchange-failed when MSAL throws', async () => {
|
|
const acquireTokenByCode = jest.fn().mockRejectedValue(new Error('AADSTS70008'));
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
await expect(
|
|
service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
),
|
|
).rejects.toMatchObject({
|
|
failure: { kind: 'token-exchange-failed', cause: 'AADSTS70008' },
|
|
});
|
|
});
|
|
|
|
it('throws token-exchange-failed when MSAL returns null', async () => {
|
|
const acquireTokenByCode = jest.fn().mockResolvedValue(null);
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
await expect(
|
|
service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
),
|
|
).rejects.toBeInstanceOf(AuthCodeFlowException);
|
|
});
|
|
|
|
it('throws token-exchange-failed when oid claim is missing', async () => {
|
|
const acquireTokenByCode = jest.fn().mockResolvedValue({
|
|
idTokenClaims: { tid: ENTRA.tenantId, amr: ['mfa'] },
|
|
account: { username: '', name: '' },
|
|
} as unknown as AuthenticationResult);
|
|
const { service } = makeService({ acquireTokenByCode });
|
|
await expect(
|
|
service.completeAuthCodeFlow(
|
|
'code',
|
|
PRE_AUTH_OK.state,
|
|
PRE_AUTH_OK,
|
|
ENTRA.redirectUri,
|
|
PRE_AUTH_OK.createdAt,
|
|
),
|
|
).rejects.toMatchObject({
|
|
failure: { kind: 'token-exchange-failed', cause: /oid/ },
|
|
});
|
|
});
|
|
});
|
|
|
|
describe('AuthService.buildLogoutUrl', () => {
|
|
it('targets the v2.0 logout endpoint on the configured authority', () => {
|
|
const { service } = makeService();
|
|
const url = new URL(service.buildLogoutUrl(ENTRA.postLogoutRedirectUri));
|
|
expect(url.origin + url.pathname).toBe(`${ENTRA.authority}/oauth2/v2.0/logout`);
|
|
});
|
|
|
|
it('includes the post_logout_redirect_uri query param', () => {
|
|
const { service } = makeService();
|
|
const url = new URL(service.buildLogoutUrl(ENTRA.postLogoutRedirectUri));
|
|
expect(url.searchParams.get('post_logout_redirect_uri')).toBe(ENTRA.postLogoutRedirectUri);
|
|
});
|
|
|
|
it('does not pass id_token_hint (v1 does not persist the id_token yet)', () => {
|
|
const { service } = makeService();
|
|
const url = new URL(service.buildLogoutUrl(ENTRA.postLogoutRedirectUri));
|
|
expect(url.searchParams.has('id_token_hint')).toBe(false);
|
|
});
|
|
});
|