Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions database/schema_diagram.puml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ entity "session" as session {
identity_proofing_level : varchar(10)
email : varchar(320)
email_verified : boolean
phone_number: varchar(20)
Comment thread
billwirz1 marked this conversation as resolved.
phone_number_verified : boolean
birth_date : date
nhs_number : varchar(10)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- +goose Up
ALTER TABLE session
ADD COLUMN IF NOT EXISTS phone_number varchar(20) NOT NULL;


-- +goose Down
ALTER TABLE session
DROP COLUMN IF EXISTS phone_number;
5 changes: 5 additions & 0 deletions lambdas/src/lib/db/db-retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ const TRANSIENT_NETWORK_ERROR_CODES = new Set([
// Source: https://www.postgresql.org/docs/current/errcodes-appendix.html
const TRANSIENT_POSTGRES_ERROR_CODES = new Set(["57P01", "57P02", "57P03"]);

// Note: unique-violation conflicts (for example PostgreSQL 23505) are intentionally not treated
// as retryable here. The generic retry layer cannot distinguish a replay of the same successful
// write after a transient connection failure from a genuine duplicate-key conflict. Session create
// recovery for that case is deferred to a dedicated follow-up change.

const TRANSIENT_ERROR_PATTERNS = [
/cannot connect now/i,
/connection terminated unexpectedly/i,
Expand Down
9 changes: 7 additions & 2 deletions lambdas/src/lib/db/session-db-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ function buildCreateSessionInput(overrides: Partial<CreateSessionInput> = {}): C
identityProofingLevel: "P9",
email: "jane@example.com",
emailVerified: true,
phoneNumber: "+447700900123",
phoneNumberVerified: false,
birthDate: "1990-01-01",
nhsNumber: "1234567890",
Expand Down Expand Up @@ -64,6 +65,7 @@ function buildSessionRow(overrides: Record<string, unknown> = {}): Record<string
identity_proofing_level: session.userInfo.identityProofingLevel,
email: session.userInfo.email,
email_verified: session.userInfo.emailVerified,
phone_number: session.userInfo.phoneNumber,
phone_number_verified: session.userInfo.phoneNumberVerified,
birth_date: session.userInfo.birthDate,
nhs_number: session.userInfo.nhsNumber,
Expand Down Expand Up @@ -123,6 +125,7 @@ describe("SessionDbClient", () => {
session.userInfo.identityProofingLevel,
session.userInfo.email,
session.userInfo.emailVerified,
session.userInfo.phoneNumber,
session.userInfo.phoneNumberVerified,
session.userInfo.birthDate,
session.userInfo.nhsNumber,
Expand All @@ -134,8 +137,8 @@ describe("SessionDbClient", () => {
);

const [query] = mockDbClient.query.mock.calls[0];
expect(query).toContain("$13::date");
expect(query).toContain("ON CONFLICT (session_id) DO UPDATE");
expect(query).toContain("$14::date");
Comment thread
billwirz1 marked this conversation as resolved.
expect(query).not.toContain("ON CONFLICT (session_id) DO UPDATE");
});

it("should retry transient insert failures", async () => {
Expand Down Expand Up @@ -224,6 +227,7 @@ describe("SessionDbClient", () => {
identity_proofing_level: "P5",
email: "jo@example.com",
email_verified: false,
phone_number: "+447700900124",
phone_number_verified: true,
birth_date: "1985-05-05",
nhs_number: "9999999999",
Expand All @@ -244,6 +248,7 @@ describe("SessionDbClient", () => {
identityProofingLevel: "P5",
email: "jo@example.com",
emailVerified: false,
phoneNumber: "+447700900124",
phoneNumberVerified: true,
birthDate: "1985-05-05",
nhsNumber: "9999999999",
Expand Down
21 changes: 15 additions & 6 deletions lambdas/src/lib/db/session-db-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface SessionRow {
identity_proofing_level: string;
email: string;
email_verified: boolean;
phone_number: string;
phone_number_verified: boolean;
birth_date: string;
nhs_number: string;
Expand Down Expand Up @@ -61,6 +62,7 @@ const SESSION_COLUMNS = `
identity_proofing_level,
email,
email_verified,
phone_number,
phone_number_verified,
birth_date,
nhs_number,
Expand Down Expand Up @@ -90,6 +92,7 @@ export class SessionDbClient {
identity_proofing_level,
email,
email_verified,
phone_number,
phone_number_verified,
birth_date,
nhs_number,
Expand All @@ -111,15 +114,19 @@ export class SessionDbClient {
$10,
$11,
$12,
$13::date,
$14,
$13,
$14::date,
$15,
$16::timestamptz,
$16,
Comment thread
billwirz1 marked this conversation as resolved.
$17::timestamptz,
$18::timestamptz
$18::timestamptz,
$19::timestamptz
)
ON CONFLICT (session_id) DO UPDATE
SET session_id = EXCLUDED.session_id
-- Intentionally use a plain INSERT here.
-- We do not use ON CONFLICT ... DO UPDATE for session creation because a true
-- session_id or refresh_token_id collision must never overwrite another user's session.
-- A future ticket will add safe recovery for ambiguous duplicate-key failures that can
-- happen after a transient connection error during session creation.
RETURNING ${SESSION_COLUMNS};
`;

Expand All @@ -135,6 +142,7 @@ export class SessionDbClient {
session.userInfo.identityProofingLevel,
session.userInfo.email,
session.userInfo.emailVerified,
session.userInfo.phoneNumber,
session.userInfo.phoneNumberVerified,
session.userInfo.birthDate,
session.userInfo.nhsNumber,
Expand Down Expand Up @@ -290,6 +298,7 @@ export class SessionDbClient {
identityProofingLevel: row.identity_proofing_level,
email: row.email,
emailVerified: row.email_verified,
phoneNumber: row.phone_number,
phoneNumberVerified: row.phone_number_verified,
birthDate: row.birth_date,
nhsNumber: row.nhs_number,
Expand Down
7 changes: 6 additions & 1 deletion lambdas/src/lib/login/nhs-login-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as testUserMapping from "./test-user-mapping";
function createUserInfo(
overrides: Partial<INhsUserInfoResponseModel> = {},
): INhsUserInfoResponseModel {
return {
const defaults: INhsUserInfoResponseModel = {
iss: "https://issuer.example",
aud: "hometest",
sub: "user-123",
Expand All @@ -20,12 +20,17 @@ function createUserInfo(
identity_proofing_level: "P9",
email: "test.user@example.com",
email_verified: "true",
phone_number: "+447700900123",
phone_number_verified: "true",
birthdate: "1990-01-01",
nhs_number: "9999999999",
gp_registration_details: {
gp_ods_code: "A12345",
},
};

return {
...defaults,
...overrides,
};
}
Expand Down
12 changes: 11 additions & 1 deletion lambdas/src/lib/login/nhs-login-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
function createUserInfo(
overrides: Partial<INhsUserInfoResponseModel> = {},
): INhsUserInfoResponseModel {
return {
const defaults: INhsUserInfoResponseModel = {
iss: "https://issuer.example",
aud: "hometest",
sub: "user-123",
Expand All @@ -21,10 +21,15 @@ function createUserInfo(
identity_proofing_level: "P9",
email: "test.user@example.com",
email_verified: "true",
phone_number: "+447700900123",
phone_number_verified: "true",
birthdate: "1990-01-01",
nhs_number: "9999999999",
gp_registration_details: { gp_ods_code: "A12345" },
};

return {
...defaults,
...overrides,
};
}
Expand All @@ -49,6 +54,7 @@ function verificationSuccess(sub: string): NhsTokenVerificationResult {
payload: {
sub,
iss: "https://issuer.example",
aud: "hometest",
iat: 1234567890,
exp: 1234571490,
} as JwtPayload,
Expand Down Expand Up @@ -108,6 +114,8 @@ describe("NhsLoginService.executeCallback", () => {
nhsAccessToken: "nhs-access-token",
nhsRefreshToken: "nhs-refresh-token",
idTokenSubject: "user-123",
idTokenIssuer: "https://issuer.example",
idTokenAudience: "hometest",
},
});
});
Expand Down Expand Up @@ -276,6 +284,8 @@ describe("NhsLoginService.executeCallback", () => {
nhsAccessToken: "nhs-access-token",
nhsRefreshToken: undefined,
idTokenSubject: "user-123",
idTokenIssuer: "https://issuer.example",
idTokenAudience: "hometest",
},
});
});
Expand Down
4 changes: 4 additions & 0 deletions lambdas/src/lib/login/nhs-login-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export interface INhsLoginResult {
nhsAccessToken: string;
nhsRefreshToken?: string;
idTokenSubject: string;
idTokenIssuer?: string;
idTokenAudience?: string | string[];
}

export type NhsLoginErrorCode =
Expand Down Expand Up @@ -107,6 +109,8 @@ export class NhsLoginService implements INhsLoginService {
nhsAccessToken: tokenResponse.access_token,
nhsRefreshToken: this.normalizeRefreshToken(tokenResponse.refresh_token),
idTokenSubject,
idTokenIssuer: idTokenResult.payload.iss,
idTokenAudience: idTokenResult.payload.aud,
},
};
}
Expand Down
7 changes: 6 additions & 1 deletion lambdas/src/lib/login/test-user-mapping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { enrichUserInfoWithTestFirstName } from "./test-user-mapping";
function createUserInfo(
overrides: Partial<INhsUserInfoResponseModel> = {},
): INhsUserInfoResponseModel {
return {
const defaults: INhsUserInfoResponseModel = {
iss: "https://issuer.example",
aud: "hometest",
sub: "user-123",
Expand All @@ -13,12 +13,17 @@ function createUserInfo(
identity_proofing_level: "P9",
email: "test.user@example.com",
email_verified: "true",
phone_number: "+447700900123",
phone_number_verified: "true",
birthdate: "1990-01-01",
nhs_number: "9999999999",
gp_registration_details: {
gp_ods_code: "A12345",
},
};

return {
...defaults,
...overrides,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface INhsUserInfoResponseModel {
identity_proofing_level: string;
email: string;
email_verified: string;
phone_number: string;
phone_number_verified: string;
birthdate: string;
nhs_number: string;
Expand Down
10 changes: 9 additions & 1 deletion lambdas/src/lib/models/session/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
function createNhsUserInfo(
overrides: Partial<INhsUserInfoResponseModel> = {},
): INhsUserInfoResponseModel {
return {
const defaults: INhsUserInfoResponseModel = {
iss: "https://issuer.example",
aud: "hometest",
sub: "user-123",
Expand All @@ -17,12 +17,17 @@ function createNhsUserInfo(
identity_proofing_level: "P9",
email: "test.user@example.com",
email_verified: "true",
phone_number: "+447700900123",
phone_number_verified: "false",
birthdate: "1990-01-01",
nhs_number: "9999999999",
gp_registration_details: {
gp_ods_code: "A12345",
},
};

return {
...defaults,
...overrides,
};
}
Expand All @@ -37,6 +42,7 @@ function createSessionUserInfo(overrides: Partial<ISessionUserInfo> = {}): ISess
identityProofingLevel: "P9",
email: "test.user@example.com",
emailVerified: true,
phoneNumber: "+447700900123",
phoneNumberVerified: false,
birthDate: "1990-01-01",
nhsNumber: "9999999999",
Expand All @@ -60,6 +66,7 @@ describe("mapNhsUserInfoToSessionUserInfo", () => {
identityProofingLevel: "P9",
email: "test.user@example.com",
emailVerified: true,
phoneNumber: "+447700900123",
phoneNumberVerified: false,
birthDate: "1990-01-01",
nhsNumber: "9999999999",
Expand Down Expand Up @@ -91,6 +98,7 @@ describe("mapSessionUserInfoToNhsUserInfo", () => {
identity_proofing_level: "P9",
email: "test.user@example.com",
email_verified: "true",
phone_number: "+447700900123",
phone_number_verified: "false",
birthdate: "1990-01-01",
nhs_number: "9999999999",
Expand Down
3 changes: 3 additions & 0 deletions lambdas/src/lib/models/session/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export interface ISessionUserInfo {
identityProofingLevel: string;
email: string;
emailVerified: boolean;
phoneNumber: string;
phoneNumberVerified: boolean;
birthDate: string;
nhsNumber: string;
Expand Down Expand Up @@ -43,6 +44,7 @@ export function mapNhsUserInfoToSessionUserInfo(
identityProofingLevel: userInfo.identity_proofing_level,
email: userInfo.email,
emailVerified: parseVerificationFlag(userInfo.email_verified),
phoneNumber: userInfo.phone_number,
phoneNumberVerified: parseVerificationFlag(userInfo.phone_number_verified),
birthDate: userInfo.birthdate,
nhsNumber: userInfo.nhs_number,
Expand All @@ -62,6 +64,7 @@ export function mapSessionUserInfoToNhsUserInfo(
identity_proofing_level: userInfo.identityProofingLevel,
email: userInfo.email,
email_verified: toVerificationFlag(userInfo.emailVerified),
phone_number: userInfo.phoneNumber,
phone_number_verified: toVerificationFlag(userInfo.phoneNumberVerified),
birthdate: userInfo.birthDate,
nhs_number: userInfo.nhsNumber,
Expand Down
11 changes: 8 additions & 3 deletions lambdas/src/login-lambda/login-service.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { LoginService } from "./login-service";
import { type ITokenService } from "../lib/login/token-service";
import { type INhsLoginClient } from "../lib/login/nhs-login-client";
import { type ITokenService } from "../lib/login/token-service";
import { type INhsUserInfoResponseModel } from "../lib/models/nhs-login/nhs-login-user-info-response-model";
import { LoginService } from "./login-service";

function createUserInfo(
overrides: Partial<INhsUserInfoResponseModel> = {},
): INhsUserInfoResponseModel {
return {
const defaults: INhsUserInfoResponseModel = {
iss: "https://issuer.example",
aud: "hometest",
sub: "user-123",
Expand All @@ -15,12 +15,17 @@ function createUserInfo(
identity_proofing_level: "P9",
email: "test.user@example.com",
email_verified: "true",
phone_number: "+447700900123",
phone_number_verified: "true",
birthdate: "1990-01-01",
nhs_number: "9999999999",
gp_registration_details: {
gp_ods_code: "A12345",
},
};

return {
...defaults,
...overrides,
};
}
Expand Down
Loading
Loading