[HOTE-1077] feat: session token service + interfaces#353
[HOTE-1077] feat: session token service + interfaces#353Cormac-F-NHS wants to merge 1 commit intomainfrom
Conversation
UI Coverage Report
|
There was a problem hiding this comment.
Pull request overview
This PR adds a new session token signing service to the lambdas/src/lib/auth shared library along with typed JWT payload interfaces, enabling consistent generation of access/refresh session JWTs.
Changes:
- Added
SessionTokenServicefor signing access and refresh session tokens usingjsonwebtoken(RS512). - Introduced typed payload interfaces for session access/refresh token claims.
- Added unit tests covering token signing and constructor validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lambdas/src/lib/models/auth/session-token-payload.ts |
Adds interfaces describing the JWT payload shapes for session access/refresh tokens. |
lambdas/src/lib/auth/session-token-service.ts |
Implements a token-signing service with configurable expiries and private key handling. |
lambdas/src/lib/auth/session-token-service.test.ts |
Adds unit tests for signing behavior and private key validation. |
| import { SessionTokenService } from "./session-token-service"; | ||
|
|
||
| const mockSign = jest.fn(); | ||
| const mockCleanupKey = jest.fn(); | ||
|
|
||
| jest.mock("jsonwebtoken", () => ({ | ||
| __esModule: true, | ||
| default: { | ||
| sign: mockSign, | ||
| }, | ||
| })); | ||
|
|
||
| jest.mock("./auth-utils", () => ({ | ||
| cleanupKey: mockCleanupKey, | ||
| })); |
There was a problem hiding this comment.
The Jest mocks here are defined after the SessionTokenService import, so the module will be loaded before jsonwebtoken/auth-utils are mocked (unlike the existing auth-token tests), which is likely to call the real jwt.sign and make this test fail.
Move the import { SessionTokenService ... } to after the jest.mock(...) calls (or convert to jest.unstable_mockModule if you switch these tests to ESM mocking).
| export interface ISessionTokenServiceConfig { | ||
| privateKey: string; | ||
| accessTokenExpiryDurationMinutes: number; | ||
| refreshTokenExpiryDurationMinutes: number; | ||
| } |
There was a problem hiding this comment.
ISessionTokenServiceConfig introduces an I*-prefixed config interface, but the existing config type in this folder is AuthTokenVerifierConfig (no I prefix), which makes the auth module’s naming inconsistent.
Consider renaming this to SessionTokenServiceConfig (or aligning all config interfaces in src/lib/auth to the same convention).
| export interface IAccessTokenPayload { | ||
| sessionId: string; | ||
| sessionCreatedAt: string; | ||
| } | ||
|
|
||
| export interface IRefreshTokenPayload { |
There was a problem hiding this comment.
The payload interface names IAccessTokenPayload / IRefreshTokenPayload are very generic in a codebase that already has multiple “access tokens” (e.g. NHS Login access tokens, auth tokens), which makes imports and usage harder to understand.
Consider renaming them to be domain-specific (e.g. ISessionAccessTokenPayload / ISessionRefreshTokenPayload) and matching the file name (session-token-payload.ts).
| export interface IAccessTokenPayload { | |
| sessionId: string; | |
| sessionCreatedAt: string; | |
| } | |
| export interface IRefreshTokenPayload { | |
| export interface ISessionAccessTokenPayload { | |
| sessionId: string; | |
| sessionCreatedAt: string; | |
| } | |
| export interface ISessionRefreshTokenPayload { |
Description
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.