Skip to content

Commit 5e0922b

Browse files
anoop-surej-nhsanoop-surej-nhs
committed
VIA-172 AS Extract auth session callback functionality into a new file, remove logger injection
1 parent 6d714a9 commit 5e0922b

File tree

7 files changed

+105
-120
lines changed

7 files changed

+105
-120
lines changed

‎auth.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,11 @@ import NHSLoginAuthProvider from "@src/app/api/auth/[...nextauth]/provider";
22
import { SESSION_LOGOUT_ROUTE } from "@src/app/session-logout/constants";
33
import { SSO_FAILURE_ROUTE } from "@src/app/sso-failure/constants";
44
import { AppConfig, configProvider } from "@src/utils/config";
5-
import { logger } from "@src/utils/logger";
65
import NextAuth from "next-auth";
76
import "next-auth/jwt";
8-
import { Logger } from "pino";
97
import { isValidSignIn } from "@src/utils/auth/callbacks/is-valid-signin";
108
import { getToken } from "@src/utils/auth/callbacks/get-token";
11-
12-
const log: Logger = logger.child({ module: "auth" });
9+
import { getUpdatedSession } from "@src/utils/auth/callbacks/get-updated-session";
1310

1411
const MAX_SESSION_AGE_SECONDS: number = 12 * 60 * 60; // 12 hours of continuous usage
1512

@@ -32,21 +29,15 @@ export const { handlers, signIn, signOut, auth } = NextAuth(async () => {
3229
trustHost: true,
3330
callbacks: {
3431
async signIn({ account }) {
35-
return isValidSignIn(account, config, log);
32+
return isValidSignIn(account, config);
3633
},
3734

3835
async jwt({ token, account, profile}) {
39-
return getToken(token, account, profile, config, MAX_SESSION_AGE_SECONDS, log);
36+
return getToken(token, account, profile, config, MAX_SESSION_AGE_SECONDS);
4037
},
4138

4239
async session({ session, token }) {
43-
if(token?.user && session.user) {
44-
session.user.nhs_number = token.user.nhs_number;
45-
session.user.birthdate = token.user.birthdate;
46-
session.user.access_token = token.access_token;
47-
}
48-
49-
return session;
40+
return getUpdatedSession(session, token);
5041
}
5142
},
5243
};

‎src/utils/auth/callbacks/get-token.test.ts‎

Lines changed: 6 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { getToken } from "@src/utils/auth/callbacks/get-token";
22
import { generateClientAssertion } from "@src/utils/auth/generate-refresh-client-assertion";
3-
import { Logger } from "pino";
43
import { Account, Profile } from "next-auth";
54
import { JWT } from "next-auth/jwt";
65
import { AppConfig } from "@src/utils/config";
@@ -19,11 +18,6 @@ describe("getToken", () => {
1918
NHS_LOGIN_PRIVATE_KEY: "mock-private-key",
2019
} as AppConfig;
2120

22-
const mockLog = {
23-
info: jest.fn(),
24-
error: jest.fn(),
25-
} as unknown as Logger;
26-
2721
const nowInSeconds = Math.floor(Date.now() / 1000);
2822

2923
beforeEach(() => {
@@ -42,12 +36,8 @@ describe("getToken", () => {
4236
undefined,
4337
mockConfig,
4438
300,
45-
mockLog,
4639
);
4740
expect(result).toBeNull();
48-
expect(mockLog.error).toHaveBeenCalledWith(
49-
"No token available in jwt callback.",
50-
);
5141
});
5242

5343
it("should return updated token on initial login with account and profile", async () => {
@@ -72,7 +62,6 @@ describe("getToken", () => {
7262
profile,
7363
mockConfig,
7464
maxAgeInSeconds,
75-
mockLog,
7665
);
7766

7867
expect(result).toMatchObject({
@@ -85,8 +74,6 @@ describe("getToken", () => {
8574
},
8675
fixedExpiry: nowInSeconds + maxAgeInSeconds,
8776
});
88-
89-
expect(mockLog.info).not.toHaveBeenCalled();
9077
});
9178

9279
it("should return token with empty values on initial login if account and profile are undefined", async () => {
@@ -104,7 +91,6 @@ describe("getToken", () => {
10491
profile,
10592
mockConfig,
10693
maxAgeInSeconds,
107-
mockLog,
10894
);
10995

11096
expect(result).toMatchObject({
@@ -117,8 +103,6 @@ describe("getToken", () => {
117103
},
118104
fixedExpiry: nowInSeconds + maxAgeInSeconds,
119105
});
120-
121-
expect(mockLog.info).not.toHaveBeenCalled();
122106
});
123107

124108
it("should return null if fixedExpiry reached", async () => {
@@ -128,19 +112,9 @@ describe("getToken", () => {
128112
expires_at: nowInSeconds + 1000,
129113
} as JWT;
130114

131-
const result = await getToken(
132-
token,
133-
null,
134-
undefined,
135-
mockConfig,
136-
300,
137-
mockLog,
138-
);
115+
const result = await getToken(token, null, undefined, mockConfig, 300);
139116

140117
expect(result).toBeNull();
141-
expect(mockLog.info).toHaveBeenCalledWith(
142-
"Session has reached the max age",
143-
);
144118
});
145119

146120
it("should refresh access token if expired, and refresh_token exists; new expires_in and refresh_token are received", async () => {
@@ -172,16 +146,8 @@ describe("getToken", () => {
172146
}),
173147
});
174148

175-
const result = await getToken(
176-
token,
177-
null,
178-
undefined,
179-
mockConfig,
180-
300,
181-
mockLog,
182-
);
149+
const result = await getToken(token, null, undefined, mockConfig, 300);
183150

184-
expect(mockLog.info).toHaveBeenCalledWith("Attempting to refresh token");
185151
expect(mockGenerateClientAssertion).toHaveBeenCalledWith(mockConfig);
186152

187153
expect(global.fetch).toHaveBeenCalledWith(
@@ -227,16 +193,8 @@ describe("getToken", () => {
227193
}),
228194
});
229195

230-
const result = await getToken(
231-
token,
232-
null,
233-
undefined,
234-
mockConfig,
235-
300,
236-
mockLog,
237-
);
196+
const result = await getToken(token, null, undefined, mockConfig, 300);
238197

239-
expect(mockLog.info).toHaveBeenCalledWith("Attempting to refresh token");
240198
expect(mockGenerateClientAssertion).toHaveBeenCalledWith(mockConfig);
241199

242200
expect(global.fetch).toHaveBeenCalledWith(
@@ -261,18 +219,9 @@ describe("getToken", () => {
261219
refresh_token: "",
262220
} as JWT;
263221

264-
const result = await getToken(
265-
token,
266-
null,
267-
undefined,
268-
mockConfig,
269-
300,
270-
mockLog,
271-
);
222+
const result = await getToken(token, null, undefined, mockConfig, 300);
272223

273224
expect(result).toBeNull();
274-
expect(mockLog.info).toHaveBeenCalledWith("Attempting to refresh token");
275-
expect(mockLog.error).toHaveBeenCalledWith("Refresh token missing");
276225
});
277226

278227
it("should return null and logs error if fetch response not ok", async () => {
@@ -288,20 +237,9 @@ describe("getToken", () => {
288237
json: jest.fn().mockResolvedValue({ error: "Error" }),
289238
});
290239

291-
const result = await getToken(
292-
token,
293-
null,
294-
undefined,
295-
mockConfig,
296-
300,
297-
mockLog,
298-
);
240+
const result = await getToken(token, null, undefined, mockConfig, 300);
299241

300242
expect(result).toBeNull();
301-
expect(mockLog.error).toHaveBeenCalledWith(
302-
{ error: "Error" },
303-
"Error in jwt callback",
304-
);
305243
});
306244

307245
it("should return the token if no refresh needed", async () => {
@@ -315,16 +253,8 @@ describe("getToken", () => {
315253
},
316254
} as JWT;
317255

318-
const result = await getToken(
319-
token,
320-
null,
321-
undefined,
322-
mockConfig,
323-
300,
324-
mockLog,
325-
);
256+
const result = await getToken(token, null, undefined, mockConfig, 300);
326257

327258
expect(result).toEqual(token);
328-
expect(mockLog.info).not.toHaveBeenCalled();
329259
});
330260
});

‎src/utils/auth/callbacks/get-token.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@ import { JWT } from "next-auth/jwt";
33
import { Account, Profile } from "next-auth";
44
import { Logger } from "pino";
55
import { AppConfig } from "@src/utils/config";
6+
import { logger } from "@src/utils/logger";
67

78
const DEFAULT_ACCESS_TOKEN_EXPIRY: number = 5 * 60;
9+
const log: Logger = logger.child({ module: "utils-auth-callbacks-get-token" });
810

911
const getToken = async (
1012
token: JWT,
1113
account: Account | null | undefined,
1214
profile: Profile | undefined,
1315
config: AppConfig,
1416
maxAgeInSeconds: number,
15-
log: Logger,
1617
) => {
1718
if (!token) {
1819
log.error("No token available in jwt callback.");

‎src/utils/auth/callbacks/get-updated-session.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { getUpdatedSession } from "@src/utils/auth/callbacks/get-updated-session";
2+
import { Session } from "next-auth";
3+
import { JWT } from "next-auth/jwt";
4+
5+
describe("getSession", () => {
6+
it("updates session user fields from token when both defined", () => {
7+
const session: Session = {
8+
user: {
9+
nhs_number: "",
10+
birthdate: "",
11+
access_token: "",
12+
},
13+
expires: "some-date",
14+
};
15+
16+
const token = {
17+
user: {
18+
nhs_number: "test-nhs-number",
19+
birthdate: "test-birthdate",
20+
},
21+
access_token: "access-token",
22+
} as JWT;
23+
24+
const result: Session = getUpdatedSession(session, token);
25+
26+
expect(result.user.nhs_number).toBe("test-nhs-number");
27+
expect(result.user.birthdate).toBe("test-birthdate");
28+
expect(result.user.access_token).toBe("access-token");
29+
});
30+
31+
it("does not update session if token.user is missing", () => {
32+
const session: Session = {
33+
user: {
34+
nhs_number: "old-nhs-number",
35+
birthdate: "old-birthdate",
36+
access_token: "old-access-token",
37+
},
38+
expires: "some-date",
39+
};
40+
41+
const token = {
42+
access_token: "new-access-token",
43+
} as JWT;
44+
45+
const result: Session = getUpdatedSession(session, token);
46+
47+
expect(result.user.nhs_number).toBe("old-nhs-number");
48+
expect(result.user.birthdate).toBe("old-birthdate");
49+
expect(result.user.access_token).toBe("old-access-token");
50+
});
51+
52+
it("does not update session if session.user is missing", () => {
53+
const session = {
54+
expires: "some-date",
55+
} as Session;
56+
57+
const token = {
58+
user: {
59+
nhs_number: "test-nhs-number",
60+
birthdate: "test-birthdate",
61+
},
62+
access_token: "token-access",
63+
} as JWT;
64+
65+
const result: Session = getUpdatedSession(session, token);
66+
67+
expect(result.user).toBeUndefined();
68+
});
69+
});

‎src/utils/auth/callbacks/get-updated-session.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { Session } from "next-auth";
2+
import { JWT } from "next-auth/jwt";
3+
4+
const getUpdatedSession = (session: Session, token: JWT) => {
5+
if (token?.user && session.user) {
6+
session.user.nhs_number = token.user.nhs_number;
7+
session.user.birthdate = token.user.birthdate;
8+
session.user.access_token = token.access_token;
9+
}
10+
11+
return session;
12+
};
13+
14+
export { getUpdatedSession };

0 commit comments

Comments
 (0)