Skip to content

Commit 29395fa

Browse files
peppescgpeppescg
committed
refactor: suggestions from copilot
1 parent 60769cb commit 29395fa

File tree

2 files changed

+60
-53
lines changed

2 files changed

+60
-53
lines changed

‎src/lib/__tests__/auth.test.ts‎

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
import crypto from "crypto";
21
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
32
import type { OidcTokenData } from "../auth";
4-
import { clearOidcProviderToken, getOidcProviderAccessToken } from "../auth";
3+
import {
4+
clearOidcProviderToken,
5+
encrypt,
6+
getOidcProviderAccessToken,
7+
} from "../auth";
58

69
// Mock next/headers
710
const mockCookies = vi.hoisted(() => ({
@@ -27,36 +30,6 @@ vi.mock("better-auth/plugins", () => ({
2730
genericOAuth: vi.fn(() => ({})),
2831
}));
2932

30-
// Encryption helper using same logic as auth.ts
31-
const ENCRYPTION_SALT = "oidc_token_salt";
32-
const KEY_LENGTH = 32;
33-
const IV_LENGTH = 12;
34-
35-
// Cache derived key to match auth.ts behavior
36-
const TEST_DERIVED_KEY = crypto.scryptSync(
37-
process.env.BETTER_AUTH_SECRET as string,
38-
ENCRYPTION_SALT,
39-
KEY_LENGTH,
40-
);
41-
42-
function encryptTestData(text: string): string {
43-
const iv = crypto.randomBytes(IV_LENGTH);
44-
const cipher = crypto.createCipheriv("aes-256-gcm", TEST_DERIVED_KEY, iv);
45-
46-
const encrypted = Buffer.concat([
47-
cipher.update(text, "utf8"),
48-
cipher.final(),
49-
]);
50-
51-
const authTag = cipher.getAuthTag();
52-
53-
return [
54-
iv.toString("hex"),
55-
authTag.toString("hex"),
56-
encrypted.toString("hex"),
57-
].join(":");
58-
}
59-
6033
describe("auth.ts", () => {
6134
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
6235

@@ -78,8 +51,8 @@ describe("auth.ts", () => {
7851
expiresAt: Date.now() + 3600000,
7952
});
8053

81-
// Encrypt using helper
82-
const encryptedPayload = encryptTestData(originalData);
54+
// Encrypt using exported function
55+
const encryptedPayload = encrypt(originalData);
8356

8457
// Verify format (iv:authTag:encrypted)
8558
const parts = encryptedPayload.split(":");
@@ -91,8 +64,8 @@ describe("auth.ts", () => {
9164
it("should create different ciphertext for same plaintext", () => {
9265
const data = "same plaintext";
9366

94-
const encrypted1 = encryptTestData(data);
95-
const encrypted2 = encryptTestData(data);
67+
const encrypted1 = encrypt(data);
68+
const encrypted2 = encrypt(data);
9669

9770
// Different IV should produce different ciphertext
9871
expect(encrypted1).not.toBe(encrypted2);
@@ -124,9 +97,7 @@ describe("auth.ts", () => {
12497
expiresAt: Date.now() - 1000, // Expired 1 second ago
12598
};
12699

127-
const encryptedPayload = encryptTestData(
128-
JSON.stringify(expiredTokenData),
129-
);
100+
const encryptedPayload = encrypt(JSON.stringify(expiredTokenData));
130101
mockCookies.get.mockReturnValue({ value: encryptedPayload });
131102

132103
const token = await getOidcProviderAccessToken("user-123");
@@ -142,7 +113,7 @@ describe("auth.ts", () => {
142113
expiresAt: Date.now() + 3600000,
143114
};
144115

145-
const encryptedPayload = encryptTestData(JSON.stringify(tokenData));
116+
const encryptedPayload = encrypt(JSON.stringify(tokenData));
146117
mockCookies.get.mockReturnValue({ value: encryptedPayload });
147118

148119
const token = await getOidcProviderAccessToken("user-123");
@@ -157,7 +128,7 @@ describe("auth.ts", () => {
157128
expiresAt: Date.now() + 3600000, // Valid for 1 hour
158129
};
159130

160-
const encryptedPayload = encryptTestData(JSON.stringify(tokenData));
131+
const encryptedPayload = encrypt(JSON.stringify(tokenData));
161132
mockCookies.get.mockReturnValue({ value: encryptedPayload });
162133

163134
const token = await getOidcProviderAccessToken("user-123");
@@ -172,7 +143,7 @@ describe("auth.ts", () => {
172143
// No userId, no expiresAt
173144
};
174145

175-
const encryptedPayload = encryptTestData(JSON.stringify(invalidData));
146+
const encryptedPayload = encrypt(JSON.stringify(invalidData));
176147
mockCookies.get.mockReturnValue({ value: encryptedPayload });
177148

178149
const token = await getOidcProviderAccessToken("user-123");
@@ -190,8 +161,9 @@ describe("auth.ts", () => {
190161
const token = await getOidcProviderAccessToken("user-123");
191162

192163
expect(token).toBeNull();
164+
expect(mockCookies.delete).toHaveBeenCalledWith("oidc_token");
193165
expect(consoleErrorSpy).toHaveBeenCalledWith(
194-
"[Auth] Error reading OIDC token from cookie:",
166+
"[Auth] Token decryption failed - possible tampering or key mismatch:",
195167
expect.any(Error),
196168
);
197169
});

‎src/lib/auth.ts‎

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,27 @@ import { cookies } from "next/headers";
88
const OIDC_PROVIDER_ID = process.env.OIDC_PROVIDER_ID || "oidc";
99
const OIDC_ISSUER = process.env.OIDC_ISSUER || "";
1010
const BASE_URL = process.env.BETTER_AUTH_URL || "http://localhost:3000";
11-
const ENCRYPTION_KEY =
12-
process.env.BETTER_AUTH_SECRET || "build-time-placeholder";
11+
const ENCRYPTION_KEY = process.env.BETTER_AUTH_SECRET;
1312
const IS_PRODUCTION = process.env.NODE_ENV === "production";
1413

1514
// Encryption constants
16-
const ENCRYPTION_SALT = "oidc_token_salt";
1715
const KEY_LENGTH = 32;
1816
const IV_LENGTH = 12;
1917

18+
/**
19+
* Static salt for key derivation.
20+
*
21+
* NOTE: Using a constant salt is intentional and secure in this context.
22+
* We're deriving an encryption key from a secret (BETTER_AUTH_SECRET), not hashing passwords.
23+
* The salt ensures consistent key derivation from the same secret across restarts.
24+
* Security comes from the secret itself, not from the salt.
25+
* Each encryption operation uses a unique random IV for semantic security.
26+
*/
27+
const ENCRYPTION_SALT = "oidc_token_salt";
28+
2029
// Derive encryption key once at module initialization to avoid blocking event loop
2130
const DERIVED_KEY = crypto.scryptSync(
22-
ENCRYPTION_KEY,
31+
ENCRYPTION_KEY as string,
2332
ENCRYPTION_SALT,
2433
KEY_LENGTH,
2534
);
@@ -71,8 +80,9 @@ function isOidcTokenData(data: unknown): data is OidcTokenData {
7180
* Encrypts data using AES-256-GCM (AEAD).
7281
* Provides both confidentiality and integrity/authentication.
7382
* Returns encrypted data in format: iv:authTag:encrypted (all hex-encoded).
83+
* Exported for testing purposes.
7484
*/
75-
function encrypt(text: string): string {
85+
export function encrypt(text: string): string {
7686
const iv = crypto.randomBytes(IV_LENGTH);
7787
const cipher = crypto.createCipheriv("aes-256-gcm", DERIVED_KEY, iv);
7888

@@ -94,8 +104,9 @@ function encrypt(text: string): string {
94104
* Decrypts data encrypted with AES-256-GCM.
95105
* Verifies authenticity before decryption.
96106
* Expects data in format: iv:authTag:encrypted (all hex-encoded).
107+
* Exported for testing purposes.
97108
*/
98-
function decrypt(payload: string): string {
109+
export function decrypt(payload: string): string {
99110
const [ivHex, tagHex, encryptedHex] = payload.split(":");
100111

101112
if (!ivHex || !tagHex || !encryptedHex) {
@@ -190,8 +201,31 @@ export async function getOidcProviderAccessToken(
190201
return null;
191202
}
192203

193-
const decrypted = decrypt(encryptedCookie.value);
194-
const parsedData: unknown = JSON.parse(decrypted);
204+
let decrypted: string;
205+
try {
206+
decrypted = decrypt(encryptedCookie.value);
207+
} catch (error) {
208+
// Decryption failure indicates tampering, corruption, or wrong secret
209+
console.error(
210+
"[Auth] Token decryption failed - possible tampering or key mismatch:",
211+
error,
212+
);
213+
cookieStore.delete(COOKIE_NAME);
214+
return null;
215+
}
216+
217+
let parsedData: unknown;
218+
try {
219+
parsedData = JSON.parse(decrypted);
220+
} catch (error) {
221+
// JSON parsing failure indicates malformed data
222+
console.error(
223+
"[Auth] Token JSON parsing failed - malformed data:",
224+
error,
225+
);
226+
cookieStore.delete(COOKIE_NAME);
227+
return null;
228+
}
195229

196230
// Runtime validation using type guard
197231
if (!isOidcTokenData(parsedData)) {
@@ -208,14 +242,15 @@ export async function getOidcProviderAccessToken(
208242

209243
// Check if token is expired
210244
const now = Date.now();
211-
if (parsedData.expiresAt < now) {
245+
if (parsedData.expiresAt <= now) {
212246
cookieStore.delete(COOKIE_NAME);
213247
return null;
214248
}
215249

216250
return parsedData.accessToken;
217251
} catch (error) {
218-
console.error("[Auth] Error reading OIDC token from cookie:", error);
252+
// Unexpected error (e.g., cookie operations failure)
253+
console.error("[Auth] Unexpected error reading OIDC token:", error);
219254
return null;
220255
}
221256
}

0 commit comments

Comments
 (0)