Skip to content

Commit

Permalink
DCMAW-9802: Refactor session creation (#44)
Browse files Browse the repository at this point in the history 
​DCMAW-9802

### What changed

- Refactors creating a session by moving logic to the Session Service to reduce the length of the /credential
Lambda.

## Checklists
<!-- Merging this PR is effectively deploying to production. Be mindful
to answer accurately. -->

- [x] There is a ticket raised for this PR that is present in the branch
name
- [x] No PII data logged. [See guidance
here](https://govukverify.atlassian.net/wiki/spaces/DCMAW/pages/3502407722/PII+Logging+Considerations)
- [ ] Demo to a BA, TA, and the team.
- [ ] Update [README](./blob/main/README.md) with any new instructions
or tasks
  • Loading branch information
@J-son1
J-son1 authored Jul 25, 2024
1 parent 4e646f5 commit 4ee7bf3
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 102 deletions.
20 changes: 10 additions & 10 deletions backend-api/src/functions/asyncCredential/asyncCredentialHandler.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ describe("Async Credential", () => {
);

expect(mockLogger.getLogMessages()[0].logMessage.sessionId).toEqual(
expect.any(String),
"mockSessionId",
);

expect(result).toStrictEqual({
Expand Down Expand Up @@ -1164,8 +1164,8 @@ class MockSessionServiceGetSessionBySubFailure
return errorResponse("Mock failing DB call");
};

createSession = async (): Promise<ErrorOrSuccess<null>> => {
return successResponse(null);
createSession = async (): Promise<ErrorOrSuccess<string>> => {
return successResponse("mockSessionId");
};
}

Expand All @@ -1183,8 +1183,8 @@ class MockSessionServiceNoActiveSession
return successResponse(null);
};

createSession = async (): Promise<ErrorOrSuccess<null>> => {
return successResponse(null);
createSession = async (): Promise<ErrorOrSuccess<string>> => {
return successResponse("mockSessionId");
};
}

Expand All @@ -1203,8 +1203,8 @@ class MockSessionServiceActiveSessionFound
return successResponse("mockSessionId");
};

createSession = async (): Promise<ErrorOrSuccess<null>> => {
return successResponse(null);
createSession = async (): Promise<ErrorOrSuccess<string>> => {
return successResponse("mockSessionId");
};
}

Expand All @@ -1223,7 +1223,7 @@ class MockSessionServiceFailToCreateSession
return successResponse(null);
};

createSession = async (): Promise<ErrorOrSuccess<null>> => {
createSession = async (): Promise<ErrorOrSuccess<string>> => {
return errorResponse("Mock error");
};
}
Expand All @@ -1243,7 +1243,7 @@ class MockSessionServiceSessionCreated
return successResponse(null);
};

createSession = async (): Promise<ErrorOrSuccess<null>> => {
return successResponse(null);
createSession = async (): Promise<ErrorOrSuccess<string>> => {
return successResponse("mockSessionId");
};
}
29 changes: 7 additions & 22 deletions backend-api/src/functions/asyncCredential/asyncCredentialHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
ICreateSession,
IGetActiveSession,
} from "./sessionService/sessionService";
import { randomUUID } from "crypto";
import { Logger } from "../services/logging/logger";
import { MessageName } from "./registeredLogs";
import { IGetClientCredentials } from "../asyncToken/ssmService/ssmService";
Expand Down Expand Up @@ -185,26 +184,12 @@ export async function lambdaHandler(
return activeSessionFoundResponse(requestBody.sub);
}

const { sub, client_id, govuk_signin_journey_id, redirect_uri, state } =
requestBody;
const { iss } = jwtPayload;

const sessionId = randomUUID();

const sessionConfig = {
sessionId,
state,
sub,
clientId: client_id,
govukSigninJourneyId: govuk_signin_journey_id,
redirectUri: redirect_uri,
issuer: iss,
issuedOn: Date.now().toString(),
sessionState: "ASYNC_AUTH_SESSION_CREATED",
};
const sessionServiceCreateSessionResult = await sessionService.createSession({
...requestBody,
issuer: jwtPayload.iss,
});

const sessionServiceCreateSessionResult =
await sessionService.createSession(sessionConfig);
const sessionId = sessionServiceCreateSessionResult.value;

const eventService = dependencies.eventService(config.SQS_QUEUE);

Expand All @@ -217,9 +202,9 @@ export async function lambdaHandler(

const writeEventResult = await eventService.writeGenericEvent({
eventName: "DCMAW_ASYNC_CRI_START",
sub,
sub: requestBody.sub,
sessionId,
govukSigninJourneyId: govuk_signin_journey_id,
govukSigninJourneyId: requestBody.govuk_signin_journey_id,
getNowInMilliseconds: Date.now,
componentId: config.ISSUER,
});
Expand Down
47 changes: 16 additions & 31 deletions backend-api/src/functions/asyncCredential/sessionService/sessionService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,12 @@ describe("Session Service", () => {
dbMock.on(GetItemCommand).rejects("Mock DB Error");

const result = await service.createSession({
sessionId: "137d5a4b-3046-456d-986a-147e0469cf62",
state: "mockValidState",
sub: "mockSub",
clientId: "mockClientId",
govukSigninJourneyId: "mockJourneyId",
redirectUri: "https://mockRedirectUri.com",
client_id: "mockClientId",
govuk_signin_journey_id: "mockJourneyId",
redirect_uri: "https://mockRedirectUri.com",
issuer: "mockIssuer",
sessionState: "mockSessionState",
issuedOn: "mockIssuedOn",
});

expect(result.isError).toBe(true);
Expand All @@ -138,15 +135,12 @@ describe("Session Service", () => {
});

const result = await service.createSession({
sessionId: "137d5a4b-3046-456d-986a-147e0469cf62",
state: "mockValidState",
sub: "mockSub",
clientId: "mockClientId",
govukSigninJourneyId: "mockJourneyId",
redirectUri: "https://mockRedirectUri.com",
client_id: "mockClientId",
govuk_signin_journey_id: "mockJourneyId",
redirect_uri: "https://mockRedirectUri.com",
issuer: "mockIssuer",
sessionState: "mockSessionState",
issuedOn: "mockIssuedOn",
});

expect(result.isError).toBe(true);
Expand All @@ -163,15 +157,12 @@ describe("Session Service", () => {
dbMock.on(PutItemCommand).rejects("Mock DB Error");

const result = await service.createSession({
sessionId: "137d5a4b-3046-456d-986a-147e0469cf62",
state: "mockValidState",
sub: "mockSub",
clientId: "mockClientId",
govukSigninJourneyId: "mockJourneyId",
redirectUri: "https://mockRedirectUri.com",
client_id: "mockClientId",
govuk_signin_journey_id: "mockJourneyId",
redirect_uri: "https://mockRedirectUri.com",
issuer: "mockIssuer",
sessionState: "mockSessionState",
issuedOn: "mockIssuedOn",
});

expect(result.isError).toBe(true);
Expand All @@ -189,18 +180,15 @@ describe("Session Service", () => {
dbMock.on(PutItemCommand).resolves({});

const result = await service.createSession({
sessionId: "137d5a4b-3046-456d-986a-147e0469cf62",
state: "mockValidState",
sub: "mockSub",
clientId: "mockClientId",
govukSigninJourneyId: "mockJourneyId",
client_id: "mockClientId",
govuk_signin_journey_id: "mockJourneyId",
issuer: "mockIssuer",
sessionState: "mockSessionState",
issuedOn: "mockIssuedOn",
});

expect(result.isError).toBe(false);
expect(result.value).toEqual(null);
expect(typeof result.value).toBe("string");
});
});

Expand All @@ -211,19 +199,16 @@ describe("Session Service", () => {
dbMock.on(PutItemCommand).resolves({});

const result = await service.createSession({
sessionId: "137d5a4b-3046-456d-986a-147e0469cf62",
state: "mockValidState",
sub: "mockSub",
clientId: "mockClientId",
govukSigninJourneyId: "mockJourneyId",
redirectUri: "https://mockRedirectUri.com",
client_id: "mockClientId",
govuk_signin_journey_id: "mockJourneyId",
redirect_uri: "https://mockRedirectUri.com",
issuer: "mockIssuer",
sessionState: "mockSessionState",
issuedOn: "mockIssuedOn",
});

expect(result.isError).toBe(false);
expect(result.value).toEqual(null);
expect(typeof result.value).toBe("string");
});
});
});
Expand Down
95 changes: 56 additions & 39 deletions backend-api/src/functions/asyncCredential/sessionService/sessionService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
errorResponse,
successResponse,
} from "../../types/errorOrValue";
import { randomUUID } from "crypto";

export class SessionService implements IGetActiveSession, ICreateSession {
readonly tableName: string;
Expand Down Expand Up @@ -72,31 +73,14 @@ export class SessionService implements IGetActiveSession, ICreateSession {
}

async createSession(
sessionConfig: IAuthSession,
): Promise<ErrorOrSuccess<null>> {
const config: IPutAuthSessionConfig = {
TableName: this.tableName,
Item: {
sessionId: { S: sessionConfig.sessionId },
state: { S: sessionConfig.state },
sub: { S: sessionConfig.sub },
clientId: { S: sessionConfig.clientId },
govukSigninJourneyId: { S: sessionConfig.govukSigninJourneyId },
issuer: { S: sessionConfig.issuer },
sessionState: { S: sessionConfig.sessionState },
issuedOn: { S: sessionConfig.issuedOn },
},
};

if (sessionConfig.redirectUri) {
config.Item.redirectUri = { S: sessionConfig.redirectUri };
}
config: ICreateSessionConfig,
): Promise<ErrorOrSuccess<string>> {
const sessionId = randomUUID();
const putSessionConfig = this.buildPutItemCommandInput(sessionId, config);

let doesSessionExist;
try {
doesSessionExist = await this.checkSessionsExists(
sessionConfig.sessionId,
);
doesSessionExist = await this.checkSessionsExists(sessionId);
} catch (error) {
return errorResponse(
"Unexpected error when querying session table to check if sessionId exists",
Expand All @@ -108,14 +92,14 @@ export class SessionService implements IGetActiveSession, ICreateSession {
}

try {
await this.putSessionInDb(config);
await this.putSessionInDb(putSessionConfig);
} catch (error) {
return errorResponse(
"Unexpected error when querying session table whilst creating a session",
);
}

return successResponse(null);
return successResponse(sessionId);
}

private hasValidSession(
Expand All @@ -129,6 +113,40 @@ export class SessionService implements IGetActiveSession, ICreateSession {
);
}

private buildPutItemCommandInput(
sessionId: string,
config: ICreateSessionConfig,
) {
const {
state,
sub,
client_id,
govuk_signin_journey_id,
redirect_uri,
issuer,
} = config;

const putSessionConfig: IPutSessionConfig = {
TableName: this.tableName,
Item: {
sessionId: { S: sessionId },
state: { S: state },
sub: { S: sub },
clientId: { S: client_id },
govukSigninJourneyId: { S: govuk_signin_journey_id },
issuer: { S: issuer },
sessionState: { S: "ASYNC_AUTH_SESSION_CREATED" },
issuedOn: { S: Date.now().toString() },
},
};

if (redirect_uri) {
putSessionConfig.Item.redirectUri = { S: redirect_uri };
}

return putSessionConfig;
}

private async checkSessionsExists(sessionId: string): Promise<boolean> {
const output = await dbClient.send(
new GetItemCommand({
Expand All @@ -142,24 +160,12 @@ export class SessionService implements IGetActiveSession, ICreateSession {
return output.Item != null;
}

private async putSessionInDb(config: IPutAuthSessionConfig) {
private async putSessionInDb(config: IPutSessionConfig) {
await dbClient.send(new PutItemCommand(config));
}
}

interface IAuthSession {
sessionId: string;
state: string;
sub: string;
clientId: string;
govukSigninJourneyId: string;
issuer: string;
sessionState: string;
issuedOn: string;
redirectUri?: string;
}

interface IPutAuthSessionConfig {
interface IPutSessionConfig {
TableName: string;
Item: {
sessionId: { S: string };
Expand All @@ -181,8 +187,19 @@ export interface IGetActiveSession {
) => Promise<ErrorOrSuccess<string | null>>;
}

interface ICreateSessionConfig {
state: string;
sub: string;
client_id: string;
govuk_signin_journey_id: string;
issuer: string;
redirect_uri?: string;
}

export interface ICreateSession {
createSession: (sessionConfig: IAuthSession) => Promise<ErrorOrSuccess<null>>;
createSession: (
config: ICreateSessionConfig,
) => Promise<ErrorOrSuccess<string>>;
}

type IQueryCommandOutputType = {
Expand Down

0 comments on commit 4ee7bf3

Please sign in to comment.