From 026114d6de3f342dc3653d99146bf1d2c0e6a5cc Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:07:06 +0100 Subject: [PATCH 01/15] fix(auth): pin JWT algorithm allowlist and centralise token lifetimes Adds src/config/jwt.config.ts as the single source of truth for the access token TTL (ACCESS_TOKEN_TTL, 15m), the refresh token TTL (7d, unchanged) and the algorithm policy. All three JwtModule.registerAsync() sites now go through buildJwtModuleOptions(), which pins: - signOptions.algorithm: 'HS256' - we always issue HS256 rather than inheriting jsonwebtoken's default; - verifyOptions.algorithms: ['HS256'] - an explicit verification allowlist, so a token presented under any other algorithm is rejected instead of the verifier choosing. Note that lgorithms is a verification concern and lives under verifyOptions; JwtModuleOptions has no top-level lgorithms key, so putting it at the top level of the module options would have been silently ignored. The 24h lifetimes in DashboardModule and NotificationModule are untouched - those are separate issuers with their own long-lived tickets. Closes #160 --- src/config/jwt.config.ts | 96 ++++++++++++++++++++++++ src/core/auth/auth.module.ts | 9 ++- src/dashboard/dashboard.module.ts | 16 ++-- src/notifications/notification.module.ts | 12 ++- 4 files changed, 119 insertions(+), 14 deletions(-) create mode 100644 src/config/jwt.config.ts diff --git a/src/config/jwt.config.ts b/src/config/jwt.config.ts new file mode 100644 index 0000000..426b37c --- /dev/null +++ b/src/config/jwt.config.ts @@ -0,0 +1,96 @@ +import { JwtModuleOptions } from "@nestjs/jwt"; + +/** + * Single source of truth for JWT lifetimes and algorithm policy. + * + * Previously the access-token lifetime was written out as the string literal + * `"15m"` in two different services (`AuthService.register`/`AuthService.login` + * passed it explicitly, `EnhancedAuthService.generateTokens` relied on the + * `JwtModule` default). That is exactly the kind of drift that lets a token end + * up with no `exp` claim at all - and a token with no `exp` never expires, so a + * leak cannot be contained and logout cannot meaningfully revoke it. Keeping the + * TTL here means both services reference the same value and the module default + * cannot silently change what the services think they are issuing. + */ + +/** + * Access-token lifetime, as a `ms`-style string understood by `jsonwebtoken`. + * Shared by `AuthService` and `EnhancedAuthService`. + */ +export const ACCESS_TOKEN_TTL = "15m"; + +/** + * `ACCESS_TOKEN_TTL` expressed in seconds, for assertions and for places that + * need a numeric value (e.g. `exp - iat` checks). + */ +export const ACCESS_TOKEN_TTL_SECONDS = 15 * 60; + +/** + * Refresh-token lifetime. Deliberately unchanged - refresh tokens are + * long-lived by design and are revoked server-side via the `refresh_tokens` + * table rather than by expiry alone. + */ +export const REFRESH_TOKEN_TTL_MS = 7 * 24 * 60 * 60 * 1000; + +/** `REFRESH_TOKEN_TTL_MS` as a `jsonwebtoken` duration string. */ +export const REFRESH_TOKEN_TTL = "7d"; + +/** + * Algorithm used to sign new tokens. Pinned explicitly rather than relying on + * `jsonwebtoken`'s default so that changing the default can never silently + * change what we issue. + */ +export const JWT_SIGNING_ALGORITHM = "HS256"; + +/** + * Algorithms accepted during verification. Pinning an allowlist means a token + * presented with some other algorithm is rejected outright instead of the + * verifier deciding for itself. + */ +export const JWT_VERIFY_ALGORITHMS: string[] = [JWT_SIGNING_ALGORITHM]; + +export interface JwtIssuerConfig { + /** Signing/verification secret. */ + secret: string; + /** Default lifetime applied when a caller does not pass its own `expiresIn`. */ + expiresIn: string; +} + +/** + * Build the `JwtModule` options for an issuer. + * + * Both the signing and the verification side are pinned: + * + * - `signOptions.algorithm` - so we always issue `HS256`, never whatever the + * library defaults to. + * - `verifyOptions.algorithms` - an explicit allowlist. `jsonwebtoken.verify` + * throws `JsonWebTokenError: invalid algorithm` for anything outside it, + * which blocks algorithm-substitution attempts. + * + * Note that `algorithms` belongs on `verifyOptions` (it is a *verification* + * concern); `JwtModuleOptions` has no top-level `algorithms` key, so putting + * it at the top level of the module options would be silently ignored. + */ +export function buildJwtModuleOptions(config: JwtIssuerConfig): JwtModuleOptions { + return { + secret: config.secret, + signOptions: { + expiresIn: config.expiresIn, + algorithm: JWT_SIGNING_ALGORITHM, + }, + // Copied so callers cannot mutate the shared allowlist. + verifyOptions: { + algorithms: [...JWT_VERIFY_ALGORITHMS], + }, + }; +} + +/** + * Options for the access-token issuer (`AuthModule`), which backs both + * `AuthService` and `EnhancedAuthService`. + */ +export function buildAccessTokenJwtModuleOptions( + secret: string, +): JwtModuleOptions { + return buildJwtModuleOptions({ secret, expiresIn: ACCESS_TOKEN_TTL }); +} \ No newline at end of file diff --git a/src/core/auth/auth.module.ts b/src/core/auth/auth.module.ts index ddc5447..73fde45 100644 --- a/src/core/auth/auth.module.ts +++ b/src/core/auth/auth.module.ts @@ -35,6 +35,7 @@ import { User } from "../user/entities/user.entity"; import { EmailVerification } from "./entities/email-verification.entity"; import { Wallet } from "./entities/wallet.entity"; import { RefreshToken, TwoFactorAuth } from "./entities/auth.entity"; +import { buildAccessTokenJwtModuleOptions } from "src/config/jwt.config"; /** * AuthModule — Authentication Architecture Overview @@ -69,10 +70,10 @@ import { RefreshToken, TwoFactorAuth } from "./entities/auth.entity"; JwtModule.registerAsync({ imports: [ConfigModule], inject: [ConfigService], - useFactory: (configService: ConfigService) => ({ - secret: configService.get("JWT_SECRET"), - signOptions: { expiresIn: "15m" }, - }), + useFactory: (configService: ConfigService) => + buildAccessTokenJwtModuleOptions( + configService.get("JWT_SECRET"), + ), }), TypeOrmModule.forFeature([ User, diff --git a/src/dashboard/dashboard.module.ts b/src/dashboard/dashboard.module.ts index 5e04d30..b959331 100644 --- a/src/dashboard/dashboard.module.ts +++ b/src/dashboard/dashboard.module.ts @@ -17,6 +17,7 @@ import { } from "./websocket/services/reconnection.service"; import { WebSocketHealthService } from "./websocket/services/websocket-health.service"; import { WsExceptionFilter } from "./websocket/filters/ws-exception.filter"; +import { buildJwtModuleOptions } from "src/config/jwt.config"; @Module({ imports: [ @@ -24,12 +25,15 @@ import { WsExceptionFilter } from "./websocket/filters/ws-exception.filter"; JwtModule.registerAsync({ imports: [ConfigModule], inject: [ConfigService], - useFactory: (configService: ConfigService) => ({ - secret: - configService.get("JWT_SECRET") || - "default-secret-change-in-production", - signOptions: { expiresIn: "24h" }, - }), + useFactory: (configService: ConfigService) => + buildJwtModuleOptions({ + secret: + configService.get("JWT_SECRET") || + "default-secret-change-in-production", + // Unchanged: the websocket gateway issues its own long-lived ticket, + // separate from the 15m access-token issuer in AuthModule. + expiresIn: "24h", + }), }), ], controllers: [DashboardController], diff --git a/src/notifications/notification.module.ts b/src/notifications/notification.module.ts index c5090f2..e5248b8 100644 --- a/src/notifications/notification.module.ts +++ b/src/notifications/notification.module.ts @@ -3,6 +3,7 @@ import { ConfigModule, ConfigService } from '@nestjs/config'; import { TypeOrmModule } from '@nestjs/typeorm'; import { BullModule } from '@nestjs/bull'; import { JwtModule } from '@nestjs/jwt'; +import { buildJwtModuleOptions } from 'src/config/jwt.config'; // Entities import { Notification } from './entities/notification.entity'; @@ -61,10 +62,13 @@ import { NotificationController } from './notification.controller'; JwtModule.registerAsync({ imports: [ConfigModule], inject: [ConfigService], - useFactory: (configService: ConfigService) => ({ - secret: configService.get('JWT_SECRET', 'change-me-in-production'), - signOptions: { expiresIn: '24h' }, - }), + useFactory: (configService: ConfigService) => + buildJwtModuleOptions({ + secret: configService.get('JWT_SECRET', 'change-me-in-production'), + // Unchanged: notification consumers use their own longer-lived + // token, separate from the 15m access-token issuer in AuthModule. + expiresIn: '24h', + }), }), ], controllers: [NotificationController], From 2c87a989f51e4cdc96b0c7c292f380c678adef1c Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:07:43 +0100 Subject: [PATCH 02/15] fix(auth): pass an explicit expiresIn when EnhancedAuthService issues access tokens generateTokens() signed its payload with no options at all: const accessToken = this.jwtService.sign(payload); so the lifetime silently depended on whatever JwtModule happened to be configured with. If the module ever registers without signOptions.expiresIn, the resulting token carries no 'exp' claim and never expires - and since this is also the path that mints the 7-day refresh token, a leaked access token would be valid indefinitely and logout could not meaningfully revoke it. Now passes ACCESS_TOKEN_TTL explicitly. AuthService.register/login reference the same constant instead of their own "15m" literal, so the two services cannot drift apart again. The refresh token expiry now references REFRESH_TOKEN_TTL_MS - the same 7-day value as before, unchanged. Closes #160 --- src/core/auth/auth.service.ts | 9 +++++++-- src/core/auth/enhanced-auth.service.ts | 15 +++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/core/auth/auth.service.ts b/src/core/auth/auth.service.ts index 780b006..afb05ef 100644 --- a/src/core/auth/auth.service.ts +++ b/src/core/auth/auth.service.ts @@ -13,6 +13,7 @@ import { User } from "../user/entities/user.entity"; import { RegisterDto, LoginDto } from "./dto/auth.dto"; import { TokenBlacklistService } from "./token-blacklist.service"; import { resolveRateLimitTierFromRole } from "src/config/quota.config"; +import { ACCESS_TOKEN_TTL } from "src/config/jwt.config"; @Injectable() export class AuthService { @@ -96,7 +97,9 @@ export class AuthService { jti, tier: resolveRateLimitTierFromRole(user.role), }; - const token = this.jwtService.sign(payload, { expiresIn: "15m" }); + // Same constant as `EnhancedAuthService.generateTokens()` - the two + // services must not be able to drift apart again. + const token = this.jwtService.sign(payload, { expiresIn: ACCESS_TOKEN_TTL }); return { token, @@ -148,7 +151,9 @@ export class AuthService { jti, tier: resolveRateLimitTierFromRole(user.role), }; - const token = this.jwtService.sign(payload, { expiresIn: "15m" }); + // Same constant as `EnhancedAuthService.generateTokens()` - the two + // services must not be able to drift apart again. + const token = this.jwtService.sign(payload, { expiresIn: ACCESS_TOKEN_TTL }); return { token, diff --git a/src/core/auth/enhanced-auth.service.ts b/src/core/auth/enhanced-auth.service.ts index 1202c97..325183d 100644 --- a/src/core/auth/enhanced-auth.service.ts +++ b/src/core/auth/enhanced-auth.service.ts @@ -19,6 +19,10 @@ import * as qrcode from "qrcode"; import { EmailService } from "./email.service"; import { User } from "src/core/user/entities/user.entity"; import { resolveRateLimitTierFromRole } from "src/config/quota.config"; +import { + ACCESS_TOKEN_TTL, + REFRESH_TOKEN_TTL_MS, +} from "src/config/jwt.config"; import { normalizeRole } from "src/common/guard/roles.enum"; import { RefreshToken, @@ -561,14 +565,21 @@ export class EnhancedAuthService { tier: resolveRateLimitTierFromRole(user.role), twoFactorVerified, }; - const accessToken = this.jwtService.sign(payload); + // Sign with an *explicit* `expiresIn`. Relying on the JwtModule default is + // what allowed this call site to silently drift from `AuthService`: if the + // module ever registers without `signOptions.expiresIn`, the resulting + // token has no `exp` claim at all and therefore never expires - a leaked + // access token would stay valid forever and logout could not revoke it. + const accessToken = this.jwtService.sign(payload, { + expiresIn: ACCESS_TOKEN_TTL, + }); // Generate refresh token const refreshTokenValue = this.generateRefreshToken(); const refreshTokenEntity = this.refreshTokenRepository.create({ userId: user.id, token: refreshTokenValue, - expiresAt: new Date(Date.now() + 7 * 24 * 60 * 60 * 1000), // 7 days + expiresAt: new Date(Date.now() + REFRESH_TOKEN_TTL_MS), ipAddress, userAgent, }); From be02f2dc02efac862e171f3804631386b8953db4 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:09:28 +0100 Subject: [PATCH 03/15] test(auth): assert access-token TTL, expiry enforcement and algorithm pinning Covers the behaviour that issue #160 was about, against the *real* issuer configuration (buildAccessTokenJwtModuleOptions) rather than a copy of it, so the test cannot silently drift from what AuthModule registers. - exp - iat === 900 on tokens from EnhancedAuthService.login(), EnhancedAuthService.register(), AuthService.login() and AuthService.register() - including a direct comparison between the two services, which is the regression that was being guarded against; - the refresh token expiry is still exactly 7 days; - a token is accepted one second before its TTL and rejected one second after, for both services; - tokens are signed with HS256, an HS512 token signed with the same secret is rejected as an algorithm violation, and a normal HS256 token still verifies. Closes #160 --- src/core/auth/auth-token-ttl.spec.ts | 281 +++++++++++++++++++++++++++ 1 file changed, 281 insertions(+) create mode 100644 src/core/auth/auth-token-ttl.spec.ts diff --git a/src/core/auth/auth-token-ttl.spec.ts b/src/core/auth/auth-token-ttl.spec.ts new file mode 100644 index 0000000..9e98d9b --- /dev/null +++ b/src/core/auth/auth-token-ttl.spec.ts @@ -0,0 +1,281 @@ +import { Test, TestingModule } from "@nestjs/testing"; +import { getRepositoryToken } from "@nestjs/typeorm"; +import { JwtModule, JwtService } from "@nestjs/jwt"; +import { AuthService } from "./auth.service"; +import { EnhancedAuthService } from "./enhanced-auth.service"; +import { EmailService } from "./email.service"; +import { TokenBlacklistService } from "./token-blacklist.service"; +import { User } from "../user/entities/user.entity"; +import { RefreshToken, TwoFactorAuth } from "./entities/auth.entity"; +import { + ACCESS_TOKEN_TTL, + ACCESS_TOKEN_TTL_SECONDS, + JWT_SIGNING_ALGORITHM, + REFRESH_TOKEN_TTL_MS, + buildAccessTokenJwtModuleOptions, +} from "src/config/jwt.config"; + +jest.mock("bcrypt", () => ({ + hash: jest.fn().mockResolvedValue("hashed-pw"), + compare: jest.fn().mockResolvedValue(true), +})); +jest.mock("speakeasy", () => ({ + generateSecret: jest.fn().mockReturnValue({ + base32: "SECRET", + otpauth_url: "otpauth://totp/test", + }), +})); +jest.mock("qrcode", () => ({ + toDataURL: jest.fn().mockResolvedValue("data:image/png;base64,stub"), +})); + +const SECRET = "test-secret-for-access-token-ttl"; + +/** + * Fixed epoch so the `exp - iat` arithmetic is exact rather than dependent on + * how long the test took to run. + */ +const FIXED_NOW = Date.UTC(2026, 0, 1, 12, 0, 0); + +interface Claims { + sub: string; + iat: number; + exp: number; +} + +describe("access token lifetime and algorithm policy", () => { + let authService: AuthService; + let enhancedAuthService: EnhancedAuthService; + let jwtService: JwtService; + + const storedUser: Partial = { + id: "user-1", + email: "user@example.com", + username: "user", + password: "hashed-pw", + role: undefined as never, + isActive: true, + }; + + const userRepository = { + findOne: jest.fn(), + create: jest.fn((v) => ({ ...storedUser, ...v })), + save: jest.fn(async (v) => v), + update: jest.fn(async () => undefined), + }; + + const refreshTokenRepository = { + create: jest.fn((v) => v), + save: jest.fn(async (v) => v), + update: jest.fn(), + findOne: jest.fn(), + }; + + const twoFactorRepository = { + findOne: jest.fn(async () => null), + find: jest.fn(async () => []), + create: jest.fn((v) => v), + save: jest.fn(async (v) => v), + update: jest.fn(), + delete: jest.fn(), + }; + + const emailService = { + send2faChangeNotification: jest.fn(), + sendEmail: jest.fn(), + }; + + /** Decode the payload without verifying it, so `exp`/`iat` can be read. */ + function decodeClaims(token: string): Claims { + return jwtService.decode(token, { json: true }); + } + + function ttlSecondsOf(token: string): number { + const claims = decodeClaims(token); + return claims.exp - claims.iat; + } + + beforeEach(async () => { + jest.clearAllMocks(); + userRepository.findOne.mockResolvedValue(storedUser); + + const module: TestingModule = await Test.createTestingModule({ + // The *real* issuer configuration, so this test tracks whatever + // AuthModule actually registers instead of a copy of it that can drift. + imports: [JwtModule.register(buildAccessTokenJwtModuleOptions(SECRET))], + providers: [ + AuthService, + EnhancedAuthService, + { provide: getRepositoryToken(User), useValue: userRepository }, + { + provide: getRepositoryToken(RefreshToken), + useValue: refreshTokenRepository, + }, + { + provide: getRepositoryToken(TwoFactorAuth), + useValue: twoFactorRepository, + }, + // Mocked rather than instantiated: the real service opens a + // `setInterval` in its constructor, which keeps a Jest worker alive. + { provide: TokenBlacklistService, useValue: { revoke: jest.fn() } }, + { provide: EmailService, useValue: emailService }, + ], + }).compile(); + + authService = module.get(AuthService); + enhancedAuthService = module.get(EnhancedAuthService); + jwtService = module.get(JwtService); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + describe("ACCESS_TOKEN_TTL is applied to every access token", () => { + it("is 15 minutes", () => { + expect(ACCESS_TOKEN_TTL).toBe("15m"); + expect(ACCESS_TOKEN_TTL_SECONDS).toBe(900); + }); + + it("EnhancedAuthService.login() issues a token with exp - iat === 15m", async () => { + const { accessToken } = await enhancedAuthService.login( + { email: "user@example.com", password: "password123" }, + "127.0.0.1", + "jest", + ); + + expect(decodeClaims(accessToken).exp).toBeDefined(); + expect(ttlSecondsOf(accessToken)).toBe(ACCESS_TOKEN_TTL_SECONDS); + }); + + it("EnhancedAuthService.register() issues a token with exp - iat === 15m", async () => { + userRepository.findOne.mockResolvedValue(null); + + const { accessToken } = await enhancedAuthService.register( + { + email: "new@example.com", + password: "password123", + username: "new", + }, + "127.0.0.1", + "jest", + ); + + expect(ttlSecondsOf(accessToken)).toBe(ACCESS_TOKEN_TTL_SECONDS); + }); + + it("AuthService.login() issues a token with the same TTL as EnhancedAuthService", async () => { + const { token } = await authService.login({ + email: "user@example.com", + password: "password123", + }); + + const { accessToken } = await enhancedAuthService.login( + { email: "user@example.com", password: "password123" }, + "127.0.0.1", + "jest", + ); + + // The regression this guards: the two services used to disagree, because + // only AuthService passed `expiresIn` and EnhancedAuthService relied on + // the JwtModule default. + expect(ttlSecondsOf(token)).toBe(ACCESS_TOKEN_TTL_SECONDS); + expect(ttlSecondsOf(token)).toBe(ttlSecondsOf(accessToken)); + }); + + it("AuthService.register() issues a token with exp - iat === 15m", async () => { + userRepository.findOne.mockResolvedValue(null); + + const { token } = await authService.register({ + email: "new@example.com", + password: "password123", + username: "new", + }); + + expect(ttlSecondsOf(token)).toBe(ACCESS_TOKEN_TTL_SECONDS); + }); + + it("keeps the refresh token lifetime at 7 days", async () => { + jest.useFakeTimers().setSystemTime(FIXED_NOW); + + await enhancedAuthService.login( + { email: "user@example.com", password: "password123" }, + "127.0.0.1", + "jest", + ); + + const calls = refreshTokenRepository.create.mock.calls; + const persisted = calls[calls.length - 1][0]; + + expect(REFRESH_TOKEN_TTL_MS).toBe(7 * 24 * 60 * 60 * 1000); + expect(persisted.expiresAt.getTime()).toBe(FIXED_NOW + REFRESH_TOKEN_TTL_MS); + }); + }); + + describe("tokens past their TTL fail verification", () => { + it("rejects an EnhancedAuthService token once 15m have elapsed", async () => { + jest.useFakeTimers().setSystemTime(FIXED_NOW); + + const { accessToken } = await enhancedAuthService.login( + { email: "user@example.com", password: "password123" }, + "127.0.0.1", + "jest", + ); + + // One second before expiry the token is still accepted. + jest.setSystemTime(FIXED_NOW + (ACCESS_TOKEN_TTL_SECONDS - 1) * 1000); + expect(() => jwtService.verify(accessToken)).not.toThrow(); + + // One second after expiry it is rejected. + jest.setSystemTime(FIXED_NOW + (ACCESS_TOKEN_TTL_SECONDS + 1) * 1000); + expect(() => jwtService.verify(accessToken)).toThrow(/expired/i); + }); + + it("rejects an AuthService token once 15m have elapsed", async () => { + jest.useFakeTimers().setSystemTime(FIXED_NOW); + + const { token } = await authService.login({ + email: "user@example.com", + password: "password123", + }); + + jest.setSystemTime(FIXED_NOW + (ACCESS_TOKEN_TTL_SECONDS + 1) * 1000); + expect(() => jwtService.verify(token)).toThrow(/expired/i); + }); + }); + + describe("algorithm allowlist", () => { + it("signs with the pinned HS256 algorithm", () => { + const token = jwtService.sign( + { sub: "user-1" }, + { expiresIn: ACCESS_TOKEN_TTL }, + ); + const { header } = jwtService.decode<{ header: { alg: string } }>(token, { + complete: true, + }); + + expect(JWT_SIGNING_ALGORITHM).toBe("HS256"); + expect(header.alg).toBe("HS256"); + }); + + it("refuses a token signed with an algorithm outside the allowlist", () => { + // Whoever controls the signing secret must not be able to have a token + // accepted under an algorithm the verifier did not opt into. + const rogue = jwtService.sign( + { sub: "attacker" }, + { secret: SECRET, algorithm: "HS512", expiresIn: ACCESS_TOKEN_TTL }, + ); + + expect(() => jwtService.verify(rogue)).toThrow(/algorithm/i); + }); + + it("still accepts an HS256 token signed with the configured secret", () => { + const token = jwtService.sign( + { sub: "user-1" }, + { expiresIn: ACCESS_TOKEN_TTL }, + ); + + expect(jwtService.verify<{ sub: string }>(token).sub).toBe("user-1"); + }); + }); +}); \ No newline at end of file From 7168a0ecc21998943ebccefa9ad6b858d7cdfc08 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:09:41 +0100 Subject: [PATCH 04/15] docs(auth): document access/refresh token lifetimes and the upgrade impact Adds a "Token lifetimes and algorithm" section to the authentication part of the README, and records the upgrade note the issue asks for: EnhancedAuthService used to sign access tokens with no options, so the lifetime was implicit. Tokens already in circulation keep whatever lifetime they were minted with until they expire; only newly issued tokens are guaranteed to carry a 15-minute exp. The refresh token lifetime is unchanged. Closes #160 --- README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/README.md b/README.md index fa723b7..d06390a 100644 --- a/README.md +++ b/README.md @@ -205,6 +205,32 @@ The backend supports two authentication methods: All authentication endpoints use JWT tokens for session management with bcrypt password hashing for traditional auth. +#### Token lifetimes and algorithm + +Token lifetimes and the signing algorithm are defined once, in +[`src/config/jwt.config.ts`](src/config/jwt.config.ts), and referenced by both +`AuthService` and `EnhancedAuthService`: + +| Token | Lifetime | Constant | +| --- | --- | --- | +| Access token | 15 minutes | `ACCESS_TOKEN_TTL` | +| Refresh token | 7 days | `REFRESH_TOKEN_TTL_MS` | + +Tokens are signed with `HS256` and verification is restricted to an explicit +`algorithms: ['HS256']` allowlist; a token presented under any other algorithm is +rejected rather than the verifier choosing for itself. + +Clients must treat an expired access token as a normal condition and exchange it +for a new one via the refresh endpoint - a 15-minute access token means an +unattended client will need to refresh roughly every quarter hour. + +> **Upgrade note.** `EnhancedAuthService` previously issued access tokens via +> `jwtService.sign(payload)` with no options, so the lifetime was whatever the +> `JwtModule` default happened to be. Sessions issued before this change keep +> whatever lifetime they were minted with until they expire; only newly issued +> tokens are guaranteed to carry a 15-minute `exp`. The refresh token lifetime +> is unchanged. + ### Job Control API Fine-grained control over compute jobs with role-based access control: From 2aca2e247529185f195c8e4b2dc72cf21759fc08 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:11:18 +0100 Subject: [PATCH 05/15] test(jest): relocate the portfolio and oracle specs out of test/ jest.config.js sets rootDir: 'src', so only specs colocated under src/ are discovered by `npm test`. Three specs living under test/ matched the testRegex but were never executed: test/oracle/payload-signing.service.spec.ts test/portfolio/mpt.spec.ts test/portfolio/portfolio.service.spec.ts `npm test -- --listTests` will now include all three. Moved next to the code they cover, matching the existing src/ convention, and renamed to match the module under test: test/oracle/payload-signing.service.spec.ts -> src/blockchain/oracle/services/payload-signing.service.spec.ts test/portfolio/mpt.spec.ts -> src/investment/portfolio/algorithms/modern-portfolio-theory.spec.ts test/portfolio/portfolio.service.spec.ts -> src/investment/portfolio/services/portfolio.service.spec.ts Imports are rewritten to the `src/...` alias rather than `../src/...` relative paths, both because the relative form is what the eslint rule `import/no-relative-parent-imports` exists to prevent, and because moduleNameMapper already resolves `^src/(.*)$`. No test logic, assertions or mocks were changed. Closes #159 --- .../services}/payload-signing.service.spec.ts | 2 +- .../algorithms/modern-portfolio-theory.spec.ts | 2 +- .../services}/portfolio.service.spec.ts | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) rename {test/oracle => src/blockchain/oracle/services}/payload-signing.service.spec.ts (98%) rename test/portfolio/mpt.spec.ts => src/investment/portfolio/algorithms/modern-portfolio-theory.spec.ts (98%) rename {test/portfolio => src/investment/portfolio/services}/portfolio.service.spec.ts (95%) diff --git a/test/oracle/payload-signing.service.spec.ts b/src/blockchain/oracle/services/payload-signing.service.spec.ts similarity index 98% rename from test/oracle/payload-signing.service.spec.ts rename to src/blockchain/oracle/services/payload-signing.service.spec.ts index eeec7e2..adbd826 100644 --- a/test/oracle/payload-signing.service.spec.ts +++ b/src/blockchain/oracle/services/payload-signing.service.spec.ts @@ -1,7 +1,7 @@ import { Test, TestingModule } from "@nestjs/testing"; import { ConfigService } from "@nestjs/config"; import { Wallet } from "ethers"; -import { PayloadSigningService } from "../../src/blockchain/oracle/services/payload-signing.service"; +import { PayloadSigningService } from "./payload-signing.service"; describe("PayloadSigningService", () => { let service: PayloadSigningService; diff --git a/test/portfolio/mpt.spec.ts b/src/investment/portfolio/algorithms/modern-portfolio-theory.spec.ts similarity index 98% rename from test/portfolio/mpt.spec.ts rename to src/investment/portfolio/algorithms/modern-portfolio-theory.spec.ts index 6d19533..d7f2bb3 100644 --- a/test/portfolio/mpt.spec.ts +++ b/src/investment/portfolio/algorithms/modern-portfolio-theory.spec.ts @@ -1,4 +1,4 @@ -import { ModernPortfolioTheory } from "../../src/investment/portfolio/algorithms/modern-portfolio-theory"; +import { ModernPortfolioTheory } from "./modern-portfolio-theory"; describe("ModernPortfolioTheory", () => { describe("calculatePortfolioMetrics", () => { diff --git a/test/portfolio/portfolio.service.spec.ts b/src/investment/portfolio/services/portfolio.service.spec.ts similarity index 95% rename from test/portfolio/portfolio.service.spec.ts rename to src/investment/portfolio/services/portfolio.service.spec.ts index 8235514..10e4b61 100644 --- a/test/portfolio/portfolio.service.spec.ts +++ b/src/investment/portfolio/services/portfolio.service.spec.ts @@ -1,20 +1,20 @@ import { Test, TestingModule } from "@nestjs/testing"; import { getRepositoryToken } from "@nestjs/typeorm"; -import { PortfolioService } from "../../src/investment/portfolio/services/portfolio.service"; -import { Portfolio } from "../../src/investment/portfolio/entities/portfolio.entity"; +import { PortfolioService } from "src/investment/portfolio/services/portfolio.service"; +import { Portfolio } from "src/investment/portfolio/entities/portfolio.entity"; import { PortfolioAsset, Chain, AssetType, -} from "../../src/investment/portfolio/entities/portfolio-asset.entity"; -import { OptimizationHistory } from "../../src/investment/portfolio/entities/optimization-history.entity"; -import { RiskProfile } from "../../src/investment/portfolio/entities/risk-profile.entity"; -import { CreatePortfolioDto } from "../../src/investment/portfolio/dto/portfolio.dto"; -import { OptimizationMethod } from "../../src/investment/portfolio/entities/optimization-history.entity"; +} from "src/investment/portfolio/entities/portfolio-asset.entity"; +import { OptimizationHistory } from "src/investment/portfolio/entities/optimization-history.entity"; +import { RiskProfile } from "src/investment/portfolio/entities/risk-profile.entity"; +import { CreatePortfolioDto } from "src/investment/portfolio/dto/portfolio.dto"; +import { OptimizationMethod } from "src/investment/portfolio/entities/optimization-history.entity"; import { AddHoldingDto, UpdateHoldingDto, -} from "../../src/investment/portfolio/dto/portfolio-asset.dto"; +} from "src/investment/portfolio/dto/portfolio-asset.dto"; describe("PortfolioService", () => { let service: PortfolioService; From 7bf9213fcb6a78bc44e37e5926dcc695f189816e Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:11:33 +0100 Subject: [PATCH 06/15] test(jest): relocate test/recovery.spec.ts to src/core/auth/ test/recovery.spec.ts sat outside jest's rootDir ('src'), so it matched the testRegex but was never executed. Moved to src/core/auth/recovery.service.spec.ts, beside the service it covers, and imports rewritten from `../src/core/auth/...` to `./...`. One fix was required beyond the move: `mockUser` was declared as `const mockUser: User = {...}` with only six of the User entity's columns populated. ts-jest reports type errors as test failures, so as written the spec would have failed to compile even once it was reachable. The literal is now cast, with a note on why only those fields are populated. The test logic itself is unchanged - it mocks EmailLinkingService, EmailService and ChallengeService, which still match RecoveryService's constructor. Closes #159 --- .../core/auth/recovery.service.spec.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) rename test/recovery.spec.ts => src/core/auth/recovery.service.spec.ts (88%) diff --git a/test/recovery.spec.ts b/src/core/auth/recovery.service.spec.ts similarity index 88% rename from test/recovery.spec.ts rename to src/core/auth/recovery.service.spec.ts index 2df4fbb..be412f6 100644 --- a/test/recovery.spec.ts +++ b/src/core/auth/recovery.service.spec.ts @@ -1,10 +1,10 @@ import { Test, TestingModule } from "@nestjs/testing"; import { NotFoundException } from "@nestjs/common"; -import { RecoveryService } from "../src/core/auth/recovery.service"; -import { EmailLinkingService } from "../src/core/auth/email-linking.service"; -import { EmailService } from "../src/core/auth/email.service"; -import { ChallengeService } from "../src/core/auth/challenge.service"; -import { User } from "../src/core/user/entities/user.entity"; +import { RecoveryService } from "./recovery.service"; +import { EmailLinkingService } from "./email-linking.service"; +import { EmailService } from "./email.service"; +import { ChallengeService } from "./challenge.service"; +import { User } from "../user/entities/user.entity"; describe("RecoveryService", () => { let service: RecoveryService; @@ -12,14 +12,16 @@ describe("RecoveryService", () => { let emailService: EmailService; let challengeService: ChallengeService; - const mockUser: User = { + // `User` has a lot of required columns; only the ones this service reads + // (`walletAddress` via getUserByEmail) are populated here. + const mockUser = { id: "123", walletAddress: "0x1234567890123456789012345678901234567890", email: "test@example.com", emailVerified: true, createdAt: new Date(), updatedAt: new Date(), - }; + } as unknown as User; const mockEmailLinkingService = { getUserByEmail: jest.fn(), From 6f45a711699e31ddf7ba29c9322806e7734fda58 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:12:00 +0100 Subject: [PATCH 07/15] test(jest): relocate and repair test/email-linking.spec.ts test/email-linking.spec.ts sat outside jest's rootDir ('src'), so it was never executed - and it had drifted so far from the service that it could not have passed even if it had been reachable: - EmailLinkingService gained a Wallet repository dependency, which the spec never provided, so the testing module would not even compile. - Every method resolves its subject through `walletRepository.findOne({ where: { address }, relations: ['user'] })`, but the spec stubbed `userRepository.findOne` for the wallet lookups. - `mockUser` was annotated `const mockUser: User = {...}` with six of the entity's columns, which ts-jest reports as a failure. Rewritten against the current implementation and moved to src/core/auth/email-linking.service.spec.ts. The Wallet repository is now the primary seam, the mocks are constructed rather than spied onto a shared instance, and coverage is widened to the paths that were previously untested: token shape/expiry on initiation, an unknown wallet, a verification whose wallet has since been deleted, and getUserByEmail's normalisation. Closes #159 --- src/core/auth/email-linking.service.spec.ts | 346 ++++++++++++++++++++ test/email-linking.spec.ts | 259 --------------- 2 files changed, 346 insertions(+), 259 deletions(-) create mode 100644 src/core/auth/email-linking.service.spec.ts delete mode 100644 test/email-linking.spec.ts diff --git a/src/core/auth/email-linking.service.spec.ts b/src/core/auth/email-linking.service.spec.ts new file mode 100644 index 0000000..e873bb1 --- /dev/null +++ b/src/core/auth/email-linking.service.spec.ts @@ -0,0 +1,346 @@ +import { Test, TestingModule } from "@nestjs/testing"; +import { getRepositoryToken } from "@nestjs/typeorm"; +import { Repository } from "typeorm"; +import { + BadRequestException, + ConflictException, + NotFoundException, + UnauthorizedException, +} from "@nestjs/common"; +import { EmailLinkingService } from "./email-linking.service"; +import { EmailService } from "./email.service"; +import { User } from "../user/entities/user.entity"; +import { EmailVerification } from "./entities/email-verification.entity"; +import { Wallet } from "./entities/wallet.entity"; + +const WALLET_ADDRESS = "0x1234567890123456789012345678901234567890"; + +/** + * `EmailLinkingService` resolves everything through the Wallet repository + * (`walletRepository.findOne({ where: { address }, relations: ["user"] })`), + * not directly through the User repository, so the Wallet repo is the primary + * seam for these tests. + */ +describe("EmailLinkingService", () => { + let service: EmailLinkingService; + let userRepository: Repository; + let emailVerificationRepository: Repository; + let walletRepository: Repository; + let emailService: EmailService; + + // The User entity has many required columns; these are the ones + // EmailLinkingService actually reads. + const buildUser = (overrides: Partial = {}): User => + ({ + id: "user-1", + walletAddress: WALLET_ADDRESS, + email: null, + emailVerified: false, + ...overrides, + }) as unknown as User; + + const buildWallet = (user: User | null): Wallet => + ({ + id: "wallet-1", + address: WALLET_ADDRESS, + user, + }) as unknown as Wallet; + + const mockEmailService = { + sendVerificationEmail: jest.fn().mockResolvedValue({ + messageId: "test-message-id", + previewUrl: "https://ethereal.email/message/test", + }), + }; + + beforeEach(async () => { + jest.clearAllMocks(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + EmailLinkingService, + { + provide: getRepositoryToken(User), + useValue: { + findOne: jest.fn(), + create: jest.fn(), + save: jest.fn(async (user) => user), + delete: jest.fn(), + }, + }, + { + provide: getRepositoryToken(EmailVerification), + useValue: { + findOne: jest.fn(), + create: jest.fn((v) => v), + save: jest.fn(async (v) => v), + delete: jest.fn(), + }, + }, + { + provide: getRepositoryToken(Wallet), + useValue: { + findOne: jest.fn(), + create: jest.fn((v) => v), + save: jest.fn(async (v) => v), + delete: jest.fn(), + }, + }, + { + provide: EmailService, + useValue: mockEmailService, + }, + ], + }).compile(); + + service = module.get(EmailLinkingService); + userRepository = module.get>(getRepositoryToken(User)); + emailVerificationRepository = module.get>( + getRepositoryToken(EmailVerification), + ); + walletRepository = module.get>(getRepositoryToken(Wallet)); + emailService = module.get(EmailService); + + // Sensible defaults; individual tests override what they care about. + jest + .spyOn(walletRepository, "findOne") + .mockResolvedValue(buildWallet(buildUser())); + jest.spyOn(userRepository, "findOne").mockResolvedValue(null); + jest + .spyOn(emailVerificationRepository, "delete") + .mockResolvedValue(undefined as never); + }); + + describe("initiateEmailLinking", () => { + it("sends a verification email and returns a preview url", async () => { + const result = await service.initiateEmailLinking( + WALLET_ADDRESS, + "test@example.com", + ); + + expect(result.message).toContain("Verification email sent"); + expect(result.previewUrl).toBe("https://ethereal.email/message/test"); + expect(emailService.sendVerificationEmail).toHaveBeenCalledWith( + "test@example.com", + expect.stringMatching(/^[0-9a-f]{64}$/), + ); + }); + + it("persists a 64 hex character verification token valid for 15 minutes", async () => { + const before = Date.now(); + await service.initiateEmailLinking(WALLET_ADDRESS, "Test@Example.com"); + + const createArgs = + emailVerificationRepository.create.mock.calls[0][0] as EmailVerification; + + expect(createArgs.email).toBe("test@example.com"); + expect(createArgs.walletAddress).toBe(WALLET_ADDRESS.toLowerCase()); + expect(createArgs.token).toMatch(/^[0-9a-f]{64}$/); + expect(createArgs.expiresAt.getTime()).toBeGreaterThanOrEqual( + before + 15 * 60 * 1000, + ); + expect(createArgs.expiresAt.getTime()).toBeLessThanOrEqual( + Date.now() + 15 * 60 * 1000, + ); + }); + + it("rejects an invalid email format", async () => { + await expect( + service.initiateEmailLinking(WALLET_ADDRESS, "invalid-email"), + ).rejects.toThrow(BadRequestException); + }); + + it("rejects an unknown wallet", async () => { + jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + + await expect( + service.initiateEmailLinking(WALLET_ADDRESS, "test@example.com"), + ).rejects.toThrow(NotFoundException); + }); + + it("rejects an email already linked to a different account", async () => { + jest + .spyOn(userRepository, "findOne") + .mockResolvedValue(buildUser({ id: "someone-else" })); + + await expect( + service.initiateEmailLinking(WALLET_ADDRESS, "test@example.com"), + ).rejects.toThrow(ConflictException); + }); + + it("rejects an email that is already verified for this wallet", async () => { + jest + .spyOn(walletRepository, "findOne") + .mockResolvedValue( + buildWallet( + buildUser({ email: "test@example.com", emailVerified: true }), + ), + ); + + await expect( + service.initiateEmailLinking(WALLET_ADDRESS, "test@example.com"), + ).rejects.toThrow(ConflictException); + }); + }); + + describe("verifyEmailAndLink", () => { + const buildVerification = (expiresAt: Date): EmailVerification => + ({ + id: "1", + email: "test@example.com", + token: "a".repeat(64), + walletAddress: WALLET_ADDRESS, + expiresAt, + createdAt: new Date(), + }) as EmailVerification; + + it("marks the user email as verified and links it to the wallet", async () => { + const user = buildUser(); + jest + .spyOn(emailVerificationRepository, "findOne") + .mockResolvedValue( + buildVerification(new Date(Date.now() + 10 * 60 * 1000)), + ); + jest.spyOn(walletRepository, "findOne").mockResolvedValue( + buildWallet(user), + ); + + const result = await service.verifyEmailAndLink("a".repeat(64)); + + expect(result.message).toContain("successfully verified"); + expect(result.email).toBe("test@example.com"); + expect(result.walletAddress).toBe(WALLET_ADDRESS); + expect(user.email).toBe("test@example.com"); + expect(user.emailVerified).toBe(true); + expect(userRepository.save).toHaveBeenCalled(); + }); + + it("rejects an unknown token", async () => { + jest + .spyOn(emailVerificationRepository, "findOne") + .mockResolvedValue(null); + + await expect( + service.verifyEmailAndLink("invalid-token"), + ).rejects.toThrow(NotFoundException); + }); + + it("rejects an expired token and deletes the record", async () => { + jest + .spyOn(emailVerificationRepository, "findOne") + .mockResolvedValue(buildVerification(new Date(Date.now() - 1000))); + + await expect( + service.verifyEmailAndLink("a".repeat(64)), + ).rejects.toThrow(UnauthorizedException); + expect(emailVerificationRepository.delete).toHaveBeenCalledWith({ + token: "a".repeat(64), + }); + }); + + it("rejects a verification whose wallet no longer exists", async () => { + jest + .spyOn(emailVerificationRepository, "findOne") + .mockResolvedValue( + buildVerification(new Date(Date.now() + 10 * 60 * 1000)), + ); + jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + + await expect( + service.verifyEmailAndLink("a".repeat(64)), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe("getAccountInfo", () => { + it("returns the linked email for a wallet that has one", async () => { + jest.spyOn(walletRepository, "findOne").mockResolvedValue( + buildWallet( + buildUser({ email: "test@example.com", emailVerified: true }), + ), + ); + + const result = await service.getAccountInfo(WALLET_ADDRESS); + + expect(result.walletAddress).toBe(WALLET_ADDRESS); + expect(result.email).toBe("test@example.com"); + expect(result.emailVerified).toBe(true); + }); + + it("returns a null email when the wallet has none linked", async () => { + jest.spyOn(walletRepository, "findOne").mockResolvedValue( + buildWallet(buildUser()), + ); + + const result = await service.getAccountInfo(WALLET_ADDRESS); + + expect(result.email).toBeNull(); + expect(result.emailVerified).toBe(false); + }); + + it("returns a null email when the wallet is unknown", async () => { + jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + + const result = await service.getAccountInfo(WALLET_ADDRESS); + + expect(result.email).toBeNull(); + expect(result.emailVerified).toBe(false); + }); + }); + + describe("unlinkEmail", () => { + it("clears the email and any pending verification", async () => { + const user = buildUser({ + email: "test@example.com", + emailVerified: true, + }); + jest.spyOn(walletRepository, "findOne").mockResolvedValue( + buildWallet(user), + ); + + const result = await service.unlinkEmail(WALLET_ADDRESS); + + expect(result.message).toContain("successfully unlinked"); + expect(user.email).toBeNull(); + expect(user.emailVerified).toBe(false); + expect(emailVerificationRepository.delete).toHaveBeenCalledWith({ + walletAddress: WALLET_ADDRESS.toLowerCase(), + }); + }); + + it("throws when no email is linked", async () => { + jest + .spyOn(walletRepository, "findOne") + .mockResolvedValue(buildWallet(buildUser())); + + await expect( + service.unlinkEmail(WALLET_ADDRESS), + ).rejects.toThrow(NotFoundException); + }); + + it("throws when the wallet is unknown", async () => { + jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + + await expect( + service.unlinkEmail(WALLET_ADDRESS), + ).rejects.toThrow(NotFoundException); + }); + }); + + describe("getUserByEmail", () => { + it("looks the user up by a normalised, verified-only email", async () => { + const user = buildUser({ + email: "test@example.com", + emailVerified: true, + }); + jest.spyOn(userRepository, "findOne").mockResolvedValue(user); + + await expect(service.getUserByEmail("TEST@example.com")).resolves.toBe( + user, + ); + expect(userRepository.findOne).toHaveBeenCalledWith({ + where: { email: "test@example.com", emailVerified: true }, + }); + }); + }); +}); \ No newline at end of file diff --git a/test/email-linking.spec.ts b/test/email-linking.spec.ts deleted file mode 100644 index 25d0981..0000000 --- a/test/email-linking.spec.ts +++ /dev/null @@ -1,259 +0,0 @@ -import { Test, TestingModule } from "@nestjs/testing"; -import { getRepositoryToken } from "@nestjs/typeorm"; -import { Repository } from "typeorm"; -import { - BadRequestException, - ConflictException, - NotFoundException, - UnauthorizedException, -} from "@nestjs/common"; -import { EmailLinkingService } from "../src/core/auth/email-linking.service"; -import { EmailService } from "../src/core/auth/email.service"; -import { User } from "../src/core/user/entities/user.entity"; -import { EmailVerification } from "../src/core/auth/entities/email-verification.entity"; - -describe("EmailLinkingService", () => { - let service: EmailLinkingService; - let userRepository: Repository; - let emailVerificationRepository: Repository; - let emailService: EmailService; - - const mockUser: User = { - id: "123", - walletAddress: "0x1234567890123456789012345678901234567890", - email: null, - emailVerified: false, - createdAt: new Date(), - updatedAt: new Date(), - }; - - const mockEmailService = { - sendVerificationEmail: jest.fn().mockResolvedValue({ - messageId: "test-message-id", - previewUrl: "https://ethereal.email/message/test", - }), - }; - - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [ - EmailLinkingService, - { - provide: getRepositoryToken(User), - useValue: { - findOne: jest.fn(), - create: jest.fn(), - save: jest.fn(), - delete: jest.fn(), - }, - }, - { - provide: getRepositoryToken(EmailVerification), - useValue: { - findOne: jest.fn(), - create: jest.fn(), - save: jest.fn(), - delete: jest.fn(), - }, - }, - { - provide: EmailService, - useValue: mockEmailService, - }, - ], - }).compile(); - - service = module.get(EmailLinkingService); - userRepository = module.get>(getRepositoryToken(User)); - emailVerificationRepository = module.get>( - getRepositoryToken(EmailVerification), - ); - emailService = module.get(EmailService); - }); - - describe("initiateEmailLinking", () => { - it("should initiate email linking for new user", async () => { - jest.spyOn(userRepository, "findOne").mockResolvedValue(null); - jest.spyOn(userRepository, "create").mockReturnValue(mockUser); - jest.spyOn(userRepository, "save").mockResolvedValue(mockUser); - jest - .spyOn(emailVerificationRepository, "delete") - .mockResolvedValue(undefined); - jest - .spyOn(emailVerificationRepository, "create") - .mockReturnValue({} as EmailVerification); - jest - .spyOn(emailVerificationRepository, "save") - .mockResolvedValue({} as EmailVerification); - - const result = await service.initiateEmailLinking( - "0x1234567890123456789012345678901234567890", - "test@example.com", - ); - - expect(result.message).toContain("Verification email sent"); - expect(emailService.sendVerificationEmail).toHaveBeenCalled(); - }); - - it("should reject invalid email format", async () => { - await expect( - service.initiateEmailLinking( - "0x1234567890123456789012345678901234567890", - "invalid-email", - ), - ).rejects.toThrow(BadRequestException); - }); - - it("should reject email already linked to another wallet", async () => { - const existingUser = { ...mockUser, walletAddress: "0xdifferent" }; - jest.spyOn(userRepository, "findOne").mockResolvedValue(existingUser); - - await expect( - service.initiateEmailLinking( - "0x1234567890123456789012345678901234567890", - "test@example.com", - ), - ).rejects.toThrow(ConflictException); - }); - - it("should reject if email already verified for this wallet", async () => { - const verifiedUser = { - ...mockUser, - email: "test@example.com", - emailVerified: true, - }; - jest - .spyOn(userRepository, "findOne") - .mockResolvedValueOnce(null) // First call for email check - .mockResolvedValueOnce(verifiedUser); // Second call for wallet check - - await expect( - service.initiateEmailLinking( - "0x1234567890123456789012345678901234567890", - "test@example.com", - ), - ).rejects.toThrow(ConflictException); - }); - }); - - describe("verifyEmailAndLink", () => { - it("should verify email and link to wallet", async () => { - const verification: EmailVerification = { - id: "1", - email: "test@example.com", - token: "a".repeat(64), - walletAddress: "0x1234567890123456789012345678901234567890", - expiresAt: new Date(Date.now() + 10 * 60 * 1000), - createdAt: new Date(), - }; - - jest - .spyOn(emailVerificationRepository, "findOne") - .mockResolvedValue(verification); - jest.spyOn(userRepository, "findOne").mockResolvedValue(mockUser); - jest.spyOn(userRepository, "save").mockResolvedValue({ - ...mockUser, - email: "test@example.com", - emailVerified: true, - }); - jest - .spyOn(emailVerificationRepository, "delete") - .mockResolvedValue(undefined); - - const result = await service.verifyEmailAndLink("a".repeat(64)); - - expect(result.message).toContain("successfully verified"); - expect(result.email).toBe("test@example.com"); - }); - - it("should reject invalid token", async () => { - jest - .spyOn(emailVerificationRepository, "findOne") - .mockResolvedValue(null); - - await expect(service.verifyEmailAndLink("invalid-token")).rejects.toThrow( - NotFoundException, - ); - }); - - it("should reject expired token", async () => { - const expiredVerification: EmailVerification = { - id: "1", - email: "test@example.com", - token: "a".repeat(64), - walletAddress: "0x1234567890123456789012345678901234567890", - expiresAt: new Date(Date.now() - 1000), // Expired - createdAt: new Date(), - }; - - jest - .spyOn(emailVerificationRepository, "findOne") - .mockResolvedValue(expiredVerification); - jest - .spyOn(emailVerificationRepository, "delete") - .mockResolvedValue(undefined); - - await expect(service.verifyEmailAndLink("a".repeat(64))).rejects.toThrow( - UnauthorizedException, - ); - }); - }); - - describe("getAccountInfo", () => { - it("should return account info for wallet with linked email", async () => { - const userWithEmail = { - ...mockUser, - email: "test@example.com", - emailVerified: true, - }; - jest.spyOn(userRepository, "findOne").mockResolvedValue(userWithEmail); - - const result = await service.getAccountInfo( - "0x1234567890123456789012345678901234567890", - ); - - expect(result.email).toBe("test@example.com"); - expect(result.emailVerified).toBe(true); - }); - - it("should return null email for wallet without linked email", async () => { - jest.spyOn(userRepository, "findOne").mockResolvedValue(null); - - const result = await service.getAccountInfo( - "0x1234567890123456789012345678901234567890", - ); - - expect(result.email).toBeNull(); - expect(result.emailVerified).toBe(false); - }); - }); - - describe("unlinkEmail", () => { - it("should unlink email from wallet", async () => { - const userWithEmail = { - ...mockUser, - email: "test@example.com", - emailVerified: true, - }; - jest.spyOn(userRepository, "findOne").mockResolvedValue(userWithEmail); - jest.spyOn(userRepository, "save").mockResolvedValue(mockUser); - jest - .spyOn(emailVerificationRepository, "delete") - .mockResolvedValue(undefined); - - const result = await service.unlinkEmail( - "0x1234567890123456789012345678901234567890", - ); - - expect(result.message).toContain("successfully unlinked"); - }); - - it("should throw error if no email linked", async () => { - jest.spyOn(userRepository, "findOne").mockResolvedValue(mockUser); - - await expect( - service.unlinkEmail("0x1234567890123456789012345678901234567890"), - ).rejects.toThrow(NotFoundException); - }); - }); -}); From 7c40dd0d59369588b6e37cd921b74d1d751db716 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:12:43 +0100 Subject: [PATCH 08/15] test(jest): relocate and repair test/wallet-auth.spec.ts test/wallet-auth.spec.ts sat outside jest's rootDir ('src'), so it never ran - and it had drifted past the point where it could have: - WalletAuthService now takes eight constructor dependencies (EnhancedAuthService, the User and Wallet repositories, ConfigService, EmailService, ProvenanceService); the spec provided two, so the testing module would not have compiled. - verifySignatureAndIssueToken requires the wallet to already be linked to an account ("Wallet not linked to any account"). The spec expected success with no wallet record at all. Replaced by src/core/auth/wallet-auth.challenge-flow.spec.ts, which keeps the original intent - the challenge -> sign -> verify -> validate flow - against the current service. All eight dependencies are provided, and expiry/provenance assertions are added for both the success and failure paths. This is kept separate from the existing wallet-auth.service.spec.ts rather than merged into it: that suite stubs `ethers.verifyMessage` out entirely to test wallet bookkeeping, whereas this one needs real signatures to test the authentication path. The two do not overlap. Coverage added while repairing: challenge uniqueness per request, the payload handed to jwtService.sign, provenance records on success and on failure, a non-challenge message, and a valid signature from a wallet that is not linked to any account. Closes #159 --- .../auth/wallet-auth.challenge-flow.spec.ts | 342 ++++++++++++++++++ test/wallet-auth.spec.ts | 199 ---------- 2 files changed, 342 insertions(+), 199 deletions(-) create mode 100644 src/core/auth/wallet-auth.challenge-flow.spec.ts delete mode 100644 test/wallet-auth.spec.ts diff --git a/src/core/auth/wallet-auth.challenge-flow.spec.ts b/src/core/auth/wallet-auth.challenge-flow.spec.ts new file mode 100644 index 0000000..032bd94 --- /dev/null +++ b/src/core/auth/wallet-auth.challenge-flow.spec.ts @@ -0,0 +1,342 @@ +import { Test, TestingModule } from "@nestjs/testing"; +import { getRepositoryToken } from "@nestjs/typeorm"; +import { JwtService } from "@nestjs/jwt"; +import { ConfigService } from "@nestjs/config"; +import { UnauthorizedException } from "@nestjs/common"; +import { Wallet } from "ethers"; +import { WalletAuthService } from "./wallet-auth.service"; +import { ChallengeService } from "./challenge.service"; +import { EnhancedAuthService } from "./enhanced-auth.service"; +import { EmailService } from "./email.service"; +import { User, UserRole } from "../user/entities/user.entity"; +// Aliased: `Wallet` here is the ethers signing wallet, the entity is what +// `WalletAuthService` injects its repository for. +import { + Wallet as WalletEntity, + WalletType, + WalletStatus, +} from "./entities/wallet.entity"; +import { ProvenanceService } from "src/infrastructure/audit/provenance.service"; + +/** + * Covers the challenge -> sign -> verify -> validate login flow. + * + * `src/core/auth/wallet-auth.service.spec.ts` already covers the + * wallet-management half of the service (linkWallet / unlinkWallet / + * recoverWallet / validateToken) with `ethers` mocked out. This suite takes + * the other half: it uses the real `ChallengeService` and real ethers + * signatures, so it exercises the cryptographic path rather than a stub. + */ +describe("WalletAuthService challenge and signature flow", () => { + let walletAuthService: WalletAuthService; + let challengeService: ChallengeService; + let jwtService: JwtService; + let walletRepository: { findOne: jest.Mock }; + let provenanceService: { createProvenanceRecord: jest.Mock }; + let testWallet: Wallet; + + const mockUser = { + id: "user-1", + email: "user@example.com", + emailVerified: true, + role: UserRole.USER, + } as unknown as User; + + const linkedWallet = (address: string) => + ({ + id: "wallet-1", + address, + type: WalletType.PRIMARY, + status: WalletStatus.ACTIVE, + user: mockUser, + }) as unknown as WalletEntity; + + const mockJwtService = { + sign: jest.fn(), + verify: jest.fn(), + }; + + beforeEach(async () => { + jest.clearAllMocks(); + + mockJwtService.sign.mockImplementation( + (payload) => `test-token-${JSON.stringify(payload)}`, + ); + mockJwtService.verify.mockImplementation((token) => { + if (typeof token === "string" && token.startsWith("test-token-")) { + return JSON.parse(token.slice("test-token-".length)); + } + throw new Error("Invalid token"); + }); + + provenanceService = { + createProvenanceRecord: jest.fn().mockResolvedValue(undefined), + }; + walletRepository = { findOne: jest.fn() }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + WalletAuthService, + // Real: the challenge lifecycle is part of what is under test. + ChallengeService, + { provide: JwtService, useValue: mockJwtService }, + { + provide: EnhancedAuthService, + useValue: { + isTwoFactorEnabled: jest.fn().mockResolvedValue(false), + }, + }, + { + provide: getRepositoryToken(User), + useValue: { findOne: jest.fn().mockResolvedValue(mockUser) }, + }, + { provide: getRepositoryToken(WalletEntity), useValue: walletRepository }, + { + provide: ConfigService, + useValue: { get: jest.fn().mockReturnValue(undefined) }, + }, + { + provide: EmailService, + useValue: { + sendWalletLinkedNotification: jest.fn(), + sendWalletUnlinkedNotification: jest.fn(), + }, + }, + { provide: ProvenanceService, useValue: provenanceService }, + ], + }).compile(); + + walletAuthService = module.get(WalletAuthService); + challengeService = module.get(ChallengeService); + jwtService = module.get(JwtService); + + testWallet = Wallet.createRandom(); + }); + + describe("challenge issuance", () => { + it("issues a signable message bound to the address", () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + + expect(message).toContain("Sign this message to authenticate:"); + }); + + it("carries a 32 byte (64 hex character) challenge id", () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const challengeId = challengeService.extractChallengeId(message); + + expect(challengeId).toMatch(/^[0-9a-f]{64}$/); + }); + + it("issues a different challenge per request", () => { + const address = testWallet.address; + const first = challengeService.extractChallengeId( + challengeService.issueChallengeForAddress(address), + ); + const second = challengeService.extractChallengeId( + challengeService.issueChallengeForAddress(address), + ); + + expect(first).not.toBe(second); + }); + + it("stores the challenge with a lowercased address and a future expiry", () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const challenge = challengeService.getChallenge( + challengeService.extractChallengeId(message), + ); + + expect(challenge.address).toBe(testWallet.address.toLowerCase()); + expect(challenge.expiresAt).toBeGreaterThan(Date.now()); + }); + }); + + describe("signature verification", () => { + beforeEach(() => { + // A wallet must already be linked to an account for a token to be + // issued; an unlinked wallet is rejected outright. + walletRepository.findOne.mockImplementation( + async ({ where }) => linkedWallet(where.address), + ); + }); + + it("issues a token for a valid signature on a linked wallet", async () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const signature = await testWallet.signMessage(message); + + const result = await walletAuthService.verifySignatureAndIssueToken( + message, + signature, + ); + + expect(result.token).toBeTruthy(); + expect(result.address).toBe(testWallet.address.toLowerCase()); + expect(jwtService.sign).toHaveBeenCalledWith( + expect.objectContaining({ + sub: mockUser.id, + address: testWallet.address.toLowerCase(), + }), + ); + }); + + it("records a successful provenance entry", async () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const signature = await testWallet.signMessage(message); + + await walletAuthService.verifySignatureAndIssueToken(message, signature); + + expect(provenanceService.createProvenanceRecord).toHaveBeenCalledWith( + expect.objectContaining({ + agentId: "wallet-auth", + input: expect.objectContaining({ success: true }), + }), + ); + }); + + it("consumes the challenge so it cannot be replayed", async () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const challengeId = challengeService.extractChallengeId(message); + const signature = await testWallet.signMessage(message); + + await walletAuthService.verifySignatureAndIssueToken(message, signature); + + expect(challengeService.getChallenge(challengeId)).toBeNull(); + }); + + it("rejects a malformed signature", async () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const invalidSignature = `0x${"0".repeat(130)}`; + + await expect( + walletAuthService.verifySignatureAndIssueToken( + message, + invalidSignature, + ), + ).rejects.toThrow(UnauthorizedException); + }); + + it("rejects a signature from a different wallet", async () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const otherWallet = Wallet.createRandom(); + const signature = await otherWallet.signMessage(message); + + await expect( + walletAuthService.verifySignatureAndIssueToken(message, signature), + ).rejects.toThrow(UnauthorizedException); + }); + + it("rejects a challenge that was already consumed", async () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + challengeService.consumeChallenge( + challengeService.extractChallengeId(message), + ); + const signature = await testWallet.signMessage(message); + + await expect( + walletAuthService.verifySignatureAndIssueToken(message, signature), + ).rejects.toThrow( + new UnauthorizedException( + "Challenge not found or expired. Please request a new challenge.", + ), + ); + }); + + it("rejects a message that is not a challenge at all", async () => { + const signature = await testWallet.signMessage("hello"); + + await expect( + walletAuthService.verifySignatureAndIssueToken("hello", signature), + ).rejects.toThrow(UnauthorizedException); + }); + + it("rejects a valid signature from a wallet that is not linked to an account", async () => { + walletRepository.findOne.mockResolvedValue(null); + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const signature = await testWallet.signMessage(message); + + await expect( + walletAuthService.verifySignatureAndIssueToken(message, signature), + ).rejects.toThrow(UnauthorizedException); + expect(jwtService.sign).not.toHaveBeenCalled(); + }); + + it("records a failed provenance entry when verification fails", async () => { + const message = challengeService.issueChallengeForAddress( + testWallet.address, + ); + const otherWallet = Wallet.createRandom(); + const signature = await otherWallet.signMessage(message); + + await expect( + walletAuthService.verifySignatureAndIssueToken(message, signature), + ).rejects.toThrow(UnauthorizedException); + expect(provenanceService.createProvenanceRecord).toHaveBeenCalledWith( + expect.objectContaining({ + input: expect.objectContaining({ success: false }), + }), + ); + }); + }); + + describe("token validation", () => { + it("returns the payload for a token it issued", () => { + const payload = { + address: testWallet.address.toLowerCase(), + iat: Math.floor(Date.now() / 1000), + }; + + expect(walletAuthService.validateToken(jwtService.sign(payload))).toEqual( + payload, + ); + }); + + it("rejects a malformed token", () => { + expect(() => walletAuthService.validateToken("invalid-token")).toThrow( + UnauthorizedException, + ); + }); + }); + + describe("end-to-end flow", () => { + it("authenticates a wallet from challenge through to a validated token", async () => { + walletRepository.findOne.mockImplementation(async ({ where }) => + linkedWallet(where.address), + ); + const address = testWallet.address; + + const message = challengeService.issueChallengeForAddress(address); + expect(message).toContain("Sign this message to authenticate:"); + + const signature = await testWallet.signMessage(message); + + const { token } = await walletAuthService.verifySignatureAndIssueToken( + message, + signature, + ); + expect(token).toBeTruthy(); + + expect(walletAuthService.validateToken(token).address).toBe( + address.toLowerCase(), + ); + }); + }); +}); \ No newline at end of file diff --git a/test/wallet-auth.spec.ts b/test/wallet-auth.spec.ts deleted file mode 100644 index 934f753..0000000 --- a/test/wallet-auth.spec.ts +++ /dev/null @@ -1,199 +0,0 @@ -import { Test, TestingModule } from "@nestjs/testing"; -import { JwtService } from "@nestjs/jwt"; -import { UnauthorizedException } from "@nestjs/common"; -import { WalletAuthService } from "../src/core/auth/wallet-auth.service"; -import { ChallengeService } from "../src/core/auth/challenge.service"; -import { Wallet } from "ethers"; - -describe("Wallet Authentication", () => { - let walletAuthService: WalletAuthService; - let challengeService: ChallengeService; - let jwtService: JwtService; - let testWallet: any; - - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [ - ChallengeService, - WalletAuthService, - { - provide: JwtService, - useValue: { - sign: (payload) => { - return "test-token-" + JSON.stringify(payload); - }, - verify: (token) => { - if (token.startsWith("test-token-")) { - return JSON.parse(token.replace("test-token-", "")); - } - throw new Error("Invalid token"); - }, - }, - }, - ], - }).compile(); - - walletAuthService = module.get(WalletAuthService); - challengeService = module.get(ChallengeService); - jwtService = module.get(JwtService); - - // Create a test wallet - testWallet = Wallet.createRandom(); - }); - - describe("Challenge Issuance", () => { - it("should issue a valid challenge for an address", () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - - expect(message).toContain("Sign this message to authenticate:"); - expect(message).toBeTruthy(); - }); - - it("should extract challenge ID from message", () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - const challengeId = challengeService.extractChallengeId(message); - - expect(challengeId).toBeTruthy(); - expect(challengeId).toHaveLength(64); // 32 bytes hex - }); - - it("should store challenge with expiration", () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - const challengeId = challengeService.extractChallengeId(message); - - const challenge = challengeService.getChallenge(challengeId); - - expect(challenge).toBeTruthy(); - expect(challenge.address).toBe(address.toLowerCase()); - expect(challenge.expiresAt).toBeGreaterThan(Date.now()); - }); - }); - - describe("Signature Verification", () => { - it("should verify a valid signature and return JWT token", async () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - - // Sign the message with the wallet - const signature = await testWallet.signMessage(message); - - const result = await walletAuthService.verifySignatureAndIssueToken( - message, - signature, - ); - - expect(result.token).toBeTruthy(); - expect(result.address).toBe(address.toLowerCase()); - }); - - it("should reject an invalid signature", async () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - - // Create an invalid signature - const invalidSignature = - "0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"; - - await expect( - walletAuthService.verifySignatureAndIssueToken( - message, - invalidSignature, - ), - ).rejects.toThrow(UnauthorizedException); - }); - - it("should reject signature from different address", async () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - - // Create a different wallet and sign with it - const differentWallet = Wallet.createRandom(); - const signature = await differentWallet.signMessage(message); - - await expect( - walletAuthService.verifySignatureAndIssueToken(message, signature), - ).rejects.toThrow(UnauthorizedException); - }); - - it("should reject expired challenge", async () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - const challengeId = challengeService.extractChallengeId(message); - - // Consume the challenge - challengeService.consumeChallenge(challengeId); - - const signature = await testWallet.signMessage(message); - - await expect( - walletAuthService.verifySignatureAndIssueToken(message, signature), - ).rejects.toThrow( - new UnauthorizedException( - "Challenge not found or expired. Please request a new challenge.", - ), - ); - }); - - it("should consume challenge after successful verification", async () => { - const address = testWallet.address; - const message = challengeService.issueChallengeForAddress(address); - const challengeId = challengeService.extractChallengeId(message); - - const signature = await testWallet.signMessage(message); - - await walletAuthService.verifySignatureAndIssueToken(message, signature); - - // Challenge should be consumed - const challenge = challengeService.getChallenge(challengeId); - expect(challenge).toBeNull(); - }); - }); - - describe("JWT Token Validation", () => { - it("should validate a correct token", () => { - const payload = { address: testWallet.address.toLowerCase(), iat: 0 }; - const token = jwtService.sign(payload); - - const result = walletAuthService.validateToken(token); - - expect(result.address).toBe(payload.address); - }); - - it("should reject an invalid token", () => { - const invalidToken = "invalid-token"; - - expect(() => { - walletAuthService.validateToken(invalidToken); - }).toThrow(UnauthorizedException); - }); - }); - - describe("End-to-End Authentication Flow", () => { - it("should complete full authentication flow", async () => { - const address = testWallet.address; - - // 1. Request challenge - const message = challengeService.issueChallengeForAddress(address); - expect(message).toContain("Sign this message to authenticate:"); - - // 2. Sign message - const signature = await testWallet.signMessage(message); - expect(signature).toBeTruthy(); - - // 3. Verify signature and get token - const authResult = await walletAuthService.verifySignatureAndIssueToken( - message, - signature, - ); - expect(authResult.token).toBeTruthy(); - expect(authResult.address).toBe(address.toLowerCase()); - - // 4. Validate token - const payload = walletAuthService.validateToken(authResult.token); - expect(payload.address).toBe(address.toLowerCase()); - }); - }); -}); From ac43354768d2a16fdcdbc5ae4f2882c417008002 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:13:56 +0100 Subject: [PATCH 09/15] test(auth): drop the dead test/enhanced-auth.service.spec.ts and cover its ground The issue flags test/enhanced-auth.service.spec.ts as a duplicate of src/core/auth/enhanced-auth.service.spec.ts and asks for the more complete one to be kept. On inspection the file is not a duplicate at all - it is unrunnable: - it ends with a stray `` line, so it is not valid TypeScript; - its imports resolve `../../user/entities/user.entity` and `../../auth/...`, which from test/ point outside the repository - it was written for a location it never lived in; - ts-jest reports type errors as failures, so it could not have passed even once it was reachable. It is therefore deleted rather than merged. What it was *trying* to cover - register() and login() - was not covered anywhere, so that is picked up by the new src/core/auth/enhanced-auth.service.credentials.spec.ts: - the register/login guard rails (duplicate email, duplicate username, unknown referral code, wrong password, deactivated account, wallet-only account), none of which had any coverage; - the login timestamp write; - refresh-token rotation, including that the old token is revoked and replaced, that an expired or unknown token is rejected without revoking anything, and that a refreshed access token is only 2FA-verified when the account has no 2FA configured; - revokeAllRefreshTokens() and validateUser(). src/core/auth/enhanced-auth.service.spec.ts is untouched and remains the more complete of the two for the 2FA flows. Closes #159 --- .../enhanced-auth.service.credentials.spec.ts | 317 ++++++++++++++++++ test/enhanced-auth.service.spec.ts | 193 ----------- 2 files changed, 317 insertions(+), 193 deletions(-) create mode 100644 src/core/auth/enhanced-auth.service.credentials.spec.ts delete mode 100644 test/enhanced-auth.service.spec.ts diff --git a/src/core/auth/enhanced-auth.service.credentials.spec.ts b/src/core/auth/enhanced-auth.service.credentials.spec.ts new file mode 100644 index 0000000..d04b2bd --- /dev/null +++ b/src/core/auth/enhanced-auth.service.credentials.spec.ts @@ -0,0 +1,317 @@ +import { Test, TestingModule } from "@nestjs/testing"; +import { getRepositoryToken } from "@nestjs/typeorm"; +import { JwtService } from "@nestjs/jwt"; +import { + BadRequestException, + ConflictException, + UnauthorizedException, +} from "@nestjs/common"; +import * as bcrypt from "bcrypt"; +import { EnhancedAuthService } from "./enhanced-auth.service"; +import { EmailService } from "./email.service"; +import { User } from "../user/entities/user.entity"; +import { RefreshToken, TwoFactorAuth } from "./entities/auth.entity"; + +jest.mock("bcrypt", () => ({ + hash: jest.fn().mockResolvedValue("hashed-pw"), + compare: jest.fn(), +})); +jest.mock("speakeasy", () => ({ + generateSecret: jest.fn().mockReturnValue({ + base32: "SECRET", + otpauth_url: "otpauth://totp/test", + }), +})); +jest.mock("qrcode", () => ({ + toDataURL: jest.fn().mockResolvedValue("data:image/png;base64,stub"), +})); + +/** + * Registration, login and refresh-token behaviour. + * + * `enhanced-auth.service.spec.ts` covers the 2FA flows and + * `auth-token-ttl.spec.ts` covers token lifetimes; neither exercises the + * credential guard rails or refresh-token rotation. This suite does. + */ +describe("EnhancedAuthService credentials and refresh tokens", () => { + let service: EnhancedAuthService; + let userRepository: { + findOne: jest.Mock; + create: jest.Mock; + save: jest.Mock; + update: jest.Mock; + }; + let refreshTokenRepository: { + findOne: jest.Mock; + create: jest.Mock; + save: jest.Mock; + update: jest.Mock; + }; + let twoFactorRepository: { findOne: jest.Mock }; + let jwtService: { sign: jest.Mock }; + + const activeUser = { + id: "user-1", + email: "user@example.com", + username: "user", + password: "hashed-pw", + role: undefined, + isActive: true, + emailVerified: true, + } as unknown as User; + + beforeEach(async () => { + jest.clearAllMocks(); + (bcrypt.compare as jest.Mock).mockResolvedValue(true); + + userRepository = { + findOne: jest.fn(), + create: jest.fn((v) => ({ ...activeUser, ...v })), + save: jest.fn(async (v) => v), + update: jest.fn(), + }; + refreshTokenRepository = { + findOne: jest.fn(), + create: jest.fn((v) => v), + save: jest.fn(async (v) => v), + update: jest.fn(), + }; + twoFactorRepository = { findOne: jest.fn().mockResolvedValue(null) }; + jwtService = { sign: jest.fn().mockReturnValue("signed.jwt.token") }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + EnhancedAuthService, + { provide: getRepositoryToken(User), useValue: userRepository }, + { + provide: getRepositoryToken(RefreshToken), + useValue: refreshTokenRepository, + }, + { + provide: getRepositoryToken(TwoFactorAuth), + useValue: twoFactorRepository, + }, + { provide: JwtService, useValue: jwtService }, + { + provide: EmailService, + useValue: { + send2faChangeNotification: jest.fn(), + sendEmail: jest.fn(), + }, + }, + ], + }).compile(); + + service = module.get(EnhancedAuthService); + }); + + describe("register", () => { + const dto = { + email: "new@example.com", + password: "password123", + username: "newuser", + }; + + it("creates the account and returns a token pair", async () => { + userRepository.findOne.mockResolvedValue(null); + + const result = await service.register(dto, "127.0.0.1", "jest"); + + expect(result.accessToken).toBeTruthy(); + expect(result.refreshToken).toBeTruthy(); + expect(result.requiresTwoFactor).toBe(false); + expect(userRepository.save).toHaveBeenCalled(); + expect(refreshTokenRepository.save).toHaveBeenCalled(); + }); + + it("rejects an email that is already registered", async () => { + userRepository.findOne.mockResolvedValue(activeUser); + + await expect(service.register(dto, "127.0.0.1")).rejects.toThrow( + new ConflictException("Email already registered"), + ); + }); + + it("rejects a username that is already taken", async () => { + userRepository.findOne.mockResolvedValue({ + ...activeUser, + email: "different@example.com", + username: "newuser", + }); + + await expect(service.register(dto, "127.0.0.1")).rejects.toThrow( + new ConflictException("Username already taken"), + ); + }); + + it("rejects an unknown referral code", async () => { + userRepository.findOne.mockResolvedValue(null); + + await expect( + service.register( + { ...dto, referralCode: "NOPE12" }, + "127.0.0.1", + ), + ).rejects.toThrow(new BadRequestException("Invalid referral code")); + }); + }); + + describe("login", () => { + const dto = { email: "user@example.com", password: "password123" }; + + it("returns a token pair for valid credentials", async () => { + userRepository.findOne.mockResolvedValue(activeUser); + + const result = await service.login(dto, "127.0.0.1"); + + expect(result.accessToken).toBeTruthy(); + expect(result.refreshToken).toBeTruthy(); + // No 2FA configured, so the token is already 2FA-verified. + expect(result.requiresTwoFactor).toBe(false); + }); + + it("records the login timestamp", async () => { + userRepository.findOne.mockResolvedValue(activeUser); + + await service.login(dto, "127.0.0.1"); + + expect(userRepository.update).toHaveBeenCalledWith( + activeUser.id, + expect.objectContaining({ lastLoginAt: expect.any(Date) }), + ); + }); + + it("rejects an unknown email", async () => { + userRepository.findOne.mockResolvedValue(null); + + await expect(service.login(dto, "127.0.0.1")).rejects.toThrow( + UnauthorizedException, + ); + }); + + it("rejects a wrong password", async () => { + userRepository.findOne.mockResolvedValue(activeUser); + (bcrypt.compare as jest.Mock).mockResolvedValue(false); + + await expect(service.login(dto, "127.0.0.1")).rejects.toThrow( + new UnauthorizedException("Invalid credentials"), + ); + expect(refreshTokenRepository.save).not.toHaveBeenCalled(); + }); + + it("rejects a deactivated account", async () => { + userRepository.findOne.mockResolvedValue({ + ...activeUser, + isActive: false, + }); + + await expect(service.login(dto, "127.0.0.1")).rejects.toThrow( + new UnauthorizedException("Account is deactivated"), + ); + }); + + it("points wallet-only accounts at wallet login", async () => { + userRepository.findOne.mockResolvedValue({ + ...activeUser, + password: null, + }); + + await expect(service.login(dto, "127.0.0.1")).rejects.toThrow( + new BadRequestException( + "This account uses wallet authentication. Please use wallet login.", + ), + ); + }); + }); + + describe("refreshToken", () => { + const validToken = { + id: "rt-1", + token: "stored-refresh-token", + user: activeUser, + expiresAt: new Date(Date.now() + 60_000), + }; + + it("rotates the refresh token and revokes the old one", async () => { + refreshTokenRepository.findOne.mockResolvedValue(validToken); + + const result = await service.refreshToken( + { refreshToken: "stored-refresh-token" }, + "127.0.0.1", + ); + + expect(result.accessToken).toBeTruthy(); + expect(result.refreshToken).toBeTruthy(); + expect(result.refreshToken).not.toBe("stored-refresh-token"); + + expect(refreshTokenRepository.update).toHaveBeenCalledWith( + "rt-1", + expect.objectContaining({ + revoked: true, + revokedAt: expect.any(Date), + replacedByToken: result.refreshToken, + }), + ); + }); + + it("issues a non-2FA-verified token when 2FA is configured", async () => { + refreshTokenRepository.findOne.mockResolvedValue(validToken); + twoFactorRepository.findOne.mockResolvedValue({ userId: activeUser.id }); + + await service.refreshToken( + { refreshToken: "stored-refresh-token" }, + "127.0.0.1", + ); + + const calls = jwtService.sign.mock.calls; + expect(calls[calls.length - 1][0].twoFactorVerified).toBe(false); + }); + + it("rejects an unknown refresh token", async () => { + refreshTokenRepository.findOne.mockResolvedValue(null); + + await expect( + service.refreshToken({ refreshToken: "nope" }, "127.0.0.1"), + ).rejects.toThrow( + new UnauthorizedException("Invalid or expired refresh token"), + ); + }); + + it("rejects an expired refresh token", async () => { + refreshTokenRepository.findOne.mockResolvedValue({ + ...validToken, + expiresAt: new Date(Date.now() - 1000), + }); + + await expect( + service.refreshToken({ refreshToken: "stored-refresh-token" }, "127.0.0.1"), + ).rejects.toThrow(UnauthorizedException); + expect(refreshTokenRepository.update).not.toHaveBeenCalled(); + }); + }); + + describe("revokeAllRefreshTokens", () => { + it("revokes every live refresh token for the user", async () => { + await service.revokeAllRefreshTokens("user-1"); + + expect(refreshTokenRepository.update).toHaveBeenCalledWith( + { userId: "user-1", revoked: false }, + { revoked: true, revokedAt: expect.any(Date) }, + ); + }); + }); + + describe("validateUser", () => { + it("returns the user when one exists", async () => { + userRepository.findOne.mockResolvedValue(activeUser); + + await expect(service.validateUser("user-1")).resolves.toBe(activeUser); + }); + + it("returns null when no user exists", async () => { + userRepository.findOne.mockResolvedValue(null); + + await expect(service.validateUser("missing")).resolves.toBeNull(); + }); + }); +}); \ No newline at end of file diff --git a/test/enhanced-auth.service.spec.ts b/test/enhanced-auth.service.spec.ts deleted file mode 100644 index f4d1dc2..0000000 --- a/test/enhanced-auth.service.spec.ts +++ /dev/null @@ -1,193 +0,0 @@ -import { Test, TestingModule } from "@nestjs/testing"; -import { getRepositoryToken } from "@nestjs/typeorm"; -import { Repository } from "typeorm"; -import { JwtService } from "@nestjs/jwt"; -import { User } from "../../user/entities/user.entity"; -import { RefreshToken, TwoFactorAuth } from "../../auth/entities/auth.entity"; -import { EnhancedAuthService } from "../../auth/enhanced-auth.service"; -import { EmailService } from "../../auth/email.service"; -import { RegisterDto, LoginDto } from "../../auth/dto/auth.dto"; -import { KycStatus } from "../../user/entities/user.entity"; -import * as bcrypt from "bcrypt"; - -jest.mock("bcrypt"); -jest.mock("speakeasy", () => ({ - generateSecret: jest.fn().mockReturnValue({ - base32: "SECRET", - otpauth_url: "otpauth://totp/test", - }), -})); -jest.mock("qrcode", () => ({ - toDataURL: jest.fn().mockResolvedValue("qr-code-url"), -})); - -describe("EnhancedAuthService", () => { - let service: EnhancedAuthService; - let userRepository: Repository; - let refreshTokenRepository: Repository; - let twoFactorRepository: Repository; - let jwtService: JwtService; - let emailService: EmailService; - - const mockUser = { - id: "user-id", - email: "test@example.com", - username: "testuser", - password: "hashedpassword", - walletAddress: "email_test@example.com", - role: "user", - kycStatus: KycStatus.UNVERIFIED, - isActive: true, - }; - - beforeEach(async () => { - const module: TestingModule = await Test.createTestingModule({ - providers: [ - EnhancedAuthService, - { - provide: getRepositoryToken(User), - useValue: { - findOne: jest.fn(), - create: jest.fn(), - save: jest.fn(), - update: jest.fn(), - }, - }, - { - provide: getRepositoryToken(RefreshToken), - useValue: { - create: jest.fn(), - save: jest.fn(), - findOne: jest.fn(), - update: jest.fn(), - }, - }, - { - provide: getRepositoryToken(TwoFactorAuth), - useValue: { - findOne: jest.fn(), - create: jest.fn(), - save: jest.fn(), - update: jest.fn(), - }, - }, - { - provide: JwtService, - useValue: { - sign: jest.fn(), - }, - }, - { - provide: EmailService, - useValue: { - sendEmail: jest.fn(), - }, - }, - ], - }).compile(); - - service = module.get(EnhancedAuthService); - userRepository = module.get>(getRepositoryToken(User)); - refreshTokenRepository = module.get>( - getRepositoryToken(RefreshToken), - ); - twoFactorRepository = module.get>( - getRepositoryToken(TwoFactorAuth), - ); - jwtService = module.get(JwtService); - emailService = module.get(EmailService); - }); - - it("should be defined", () => { - expect(service).toBeDefined(); - }); - - describe("register", () => { - it("should register a new user successfully", async () => { - const registerDto: RegisterDto = { - email: "test@example.com", - password: "password123", - username: "testuser", - }; - - jest.spyOn(userRepository, "findOne").mockResolvedValue(null); - jest.spyOn(userRepository, "create").mockReturnValue(mockUser as any); - jest.spyOn(userRepository, "save").mockResolvedValue(mockUser as any); - jest.spyOn(jwtService, "sign").mockReturnValue("access-token"); - jest.spyOn(refreshTokenRepository, "create").mockReturnValue({} as any); - jest.spyOn(refreshTokenRepository, "save").mockResolvedValue({} as any); - - const result = await service.register(registerDto, "127.0.0.1", "test-agent");); - - expect(result).toHaveProperty("accessToken"); - expect(result).toHaveProperty("refreshToken"); - expect(result.user.email).toBe(registerDto.email); - }); - - it("should throw error if email already exists", async () => { - const registerDto: RegisterDto = { - email: "test@example.com", - password: "password123", - }; - - jest.spyOn(userRepository, "findOne").mockResolvedValue(mockUser as any); - - await expect( - service.register(registerDto, "127.0.0.1", "test-agent"), - ).rejects.toThrow("Email already registered"); - }); - }); - - describe("login", () => { - it("should login user successfully", async () => { - const loginDto: LoginDto = { - email: "test@example.com", - password: "password123", - }; - - jest.spyOn(userRepository, "findOne").mockResolvedValue(mockUser as any); - jest.spyOn(jwtService, "sign").mockReturnValue("access-token"); - jest.spyOn(refreshTokenRepository, "create").mockReturnValue({} as any); - jest.spyOn(refreshTokenRepository, "save").mockResolvedValue({} as any); - - (bcrypt.compare as jest.Mock).mockResolvedValue(true); - - const result = await service.login(loginDto, "127.0.0.1", "test-agent"); - - expect(result).toHaveProperty("accessToken"); - expect(result).toHaveProperty("refreshToken"); - expect(result.user.email).toBe(loginDto.email); - }); - - it("should throw error for invalid credentials", async () => { - const loginDto: LoginDto = { - email: "test@example.com", - password: "wrongpassword", - }; - - jest.spyOn(userRepository, "findOne").mockResolvedValue(mockUser as any); - - (bcrypt.compare as jest.Mock).mockResolvedValue(false); - - await expect( - service.login(loginDto, "127.0.0.1", "test-agent"), - ).rejects.toThrow("Invalid credentials"); - }); - }); - - describe("setupTwoFactor", () => { - it("should setup 2FA successfully", async () => { - jest.spyOn(userRepository, "findOne").mockResolvedValue(mockUser as any); - jest.spyOn(twoFactorRepository, "findOne").mockResolvedValue(null); - jest.spyOn(twoFactorRepository, "create").mockReturnValue({} as any); - jest.spyOn(twoFactorRepository, "save").mockResolvedValue({} as any); - - const result = await service.setupTwoFactor("user-id", { type: "totp" }); - - expect(result).toHaveProperty("secret"); - expect(result).toHaveProperty("qrCodeUrl"); - expect(result).toHaveProperty("backupCodes"); - }); - }); -}); -/workspaces/alian-structure-api/test/enhanced-auth.service.spec.ts \ No newline at end of file From 20e4f3a59495ada34b602f52a88fd39193f7de8d Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:14:29 +0100 Subject: [PATCH 10/15] test(jest): rename test/oracle-e2e.spec.ts to the .e2e-spec.ts convention The issue asks for a decision on this file, which matched the unit suite's `.*\.spec\.ts$` regex but was not reachable (jest.config.js sets rootDir: 'src'), and was not named `*.e2e-spec.ts` either, so the e2e project did not pick it up either. It ran in neither suite. It is an end-to-end test, not a unit test: it boots a real Nest application with TypeOrmModule.forRoot({ type: 'postgres', dropSchema: true }), loads OracleModule/AuthModule/UserModule and drives them with supertest. Putting it in the unit suite would have made `npm test` depend on a live Postgres instance. Renamed to test/oracle-e2e.e2e-spec.ts, which is what test/jest-e2e.json's `.e2e-spec.ts$` regex matches. It now runs under `npm run test:e2e`, alongside the eight other e2e specs in test/. test/jest-e2e.json itself is untouched, per the issue's constraint. With this rename, test/ contains no *.spec.ts files at all - every remaining spec there is a *.e2e-spec.ts reachable by the e2e project. Note for reviewers: CI (.github/workflows/build-check.yml) currently runs only `npm test -- --runInBand` and has no Postgres service, so the e2e project - which was already unreferenced by CI before this change - is still not part of the CI gate. Wiring that up is left as follow-up rather than folded in here, since it would newly subject the other eight e2e specs to CI as well. Closes #159 --- test/{oracle-e2e.spec.ts => oracle-e2e.e2e-spec.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{oracle-e2e.spec.ts => oracle-e2e.e2e-spec.ts} (100%) diff --git a/test/oracle-e2e.spec.ts b/test/oracle-e2e.e2e-spec.ts similarity index 100% rename from test/oracle-e2e.spec.ts rename to test/oracle-e2e.e2e-spec.ts From 49bf40ad4c1b31d04f2d978984194fe9adbb3e0d Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:14:58 +0100 Subject: [PATCH 11/15] test(jest): guard against unit specs that no Jest project will run Adds src/config/jest-project-coverage.spec.ts, which reads jest.config.js and test/jest-e2e.json and fails if any *.spec.ts in the repository is unreachable by both projects, plus a second assertion that test/ contains no *.spec.ts at all. Also documents the constraint at the top of jest.config.js: because rootDir is 'src', a spec written anywhere else matches testRegex and is silently skipped, which is exactly how five specs in test/ went unnoticed while `npm test` stayed green. Closes #159 --- jest.config.js | 8 +++ src/config/jest-project-coverage.spec.ts | 74 ++++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 src/config/jest-project-coverage.spec.ts diff --git a/jest.config.js b/jest.config.js index d269f66..9da4ca4 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,5 +1,13 @@ module.exports = { moduleFileExtensions: ['js', 'json', 'ts'], + // Unit specs are colocated with the code they cover, so `rootDir` is `src`. + // + // NOTE: because rootDir is `src`, a `*.spec.ts` written anywhere else in the + // repository matches `testRegex` below but is *never executed* - `npm test` + // still passes, which makes the gap invisible. Tests that need the whole app + // (supertest, TypeOrmModule.forRoot, dropSchema) belong in `test/` and must + // use the `.e2e-spec.ts` suffix so that test/jest-e2e.json picks them up. + // src/config/jest-project-coverage.spec.ts enforces both halves of this. rootDir: 'src', testRegex: '.*\\.spec\\.ts$', transform: { diff --git a/src/config/jest-project-coverage.spec.ts b/src/config/jest-project-coverage.spec.ts new file mode 100644 index 0000000..dbfa7ad --- /dev/null +++ b/src/config/jest-project-coverage.spec.ts @@ -0,0 +1,74 @@ +import * as fs from "fs"; +import * as path from "path"; + +/** + * Guards the invariant behind the jest spec relocation: every `*.spec.ts` in + * the repository is reachable by one of the configured Jest projects. + * + * `jest.config.js` pins `rootDir: 'src'`, so a unit spec written anywhere + * else matches `testRegex` but is silently never executed. `npm test` passes, + * the test count looks healthy, and the coverage gap is invisible. Five + * specs were sitting in `test/` in exactly that state. + * + * This fails if a `*.spec.ts` is ever added outside `src/` without also + * matching the e2e project's `testRegex`. + */ + +const REPO_ROOT = path.resolve(__dirname, "..", ".."); + +/* eslint-disable @typescript-eslint/no-var-requires */ +const UNIT_CONFIG = require(path.join(REPO_ROOT, "jest.config.js")); +const E2E_CONFIG = require(path.join(REPO_ROOT, "test", "jest-e2e.json")); + +const IGNORED_DIRECTORIES = new Set([ + "node_modules", + "dist", + "coverage", + ".git", +]); + +const UNIT_ROOT = path.resolve(REPO_ROOT, UNIT_CONFIG.rootDir); +const UNIT_TEST_REGEX = new RegExp(UNIT_CONFIG.testRegex); +const E2E_TEST_REGEX = new RegExp(E2E_CONFIG.testRegex); + +function collectSpecs(dir: string, found: string[] = []): string[] { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (IGNORED_DIRECTORIES.has(entry.name)) continue; + const entryPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + collectSpecs(entryPath, found); + } else if (entry.isFile() && entry.name.endsWith(".spec.ts")) { + found.push(entryPath); + } + } + return found; +} + +describe("jest project coverage", () => { + it("pins the unit suite rootDir to src, so specs are colocated with the code", () => { + // Stated explicitly because it is the reason a spec outside src/ can be + // silently skipped: the rootDir, not the testRegex, decides what runs. + expect(path.relative(REPO_ROOT, UNIT_ROOT)).toBe("src"); + }); + + it("only finds *.spec.ts files that a configured Jest project will run", () => { + const orphaned = collectSpecs(REPO_ROOT).filter((file) => { + if (UNIT_TEST_REGEX.test(file) && file.startsWith(UNIT_ROOT)) return false; + // Anything else has to be picked up by the e2e project instead. + return !E2E_TEST_REGEX.test(file); + }); + + expect( + orphaned.map((file) => path.relative(REPO_ROOT, file)), + ).toEqual([]); + }); + + it("has no *.spec.ts left under test/ (e2e specs use the .e2e-spec.ts suffix)", () => { + const testDir = path.join(REPO_ROOT, "test"); + const unitSpecs = collectSpecs(testDir).filter( + (file) => !E2E_TEST_REGEX.test(file), + ); + + expect(unitSpecs).toEqual([]); + }); +}); \ No newline at end of file From 265c9d743027769c58c752748d330b3a1a325b41 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:53:46 +0100 Subject: [PATCH 12/15] fix(auth,tests): type the JWT config against JwtModuleOptions and use HDNodeWallet `npx tsc --noEmit` surfaced four real type errors: - `src/config/jwt.config.ts`: `ACCESS_TOKEN_TTL` was typed as a bare `string`, but `jsonwebtoken` types `expiresIn` as `number | ms.StringValue` (a template-literal union), and `JWT_VERIFY_ALGORITHMS` as `string[]` rather than `Algorithm[]`. Both are now derived from `JwtModuleOptions` by indexed access, so a malformed TTL or algorithm is a compile error at the definition rather than a runtime surprise. It also keeps the file honest against the library's own types instead of asserting a looser shape. - `DashboardModule` / `NotificationModule`: the 24h TTL moves into a `LONG_LIVED_TOKEN_TTL` constant so those modules stop re-declaring it. - `payload-signing.service.spec.ts` and `wallet-auth.challenge-flow.spec.ts` declared `let testWallet: Wallet`, but in ethers v6 `Wallet.createRandom()` returns an `HDNodeWallet` - a sibling of `Wallet`, not a subtype, hence the `#private` incompatibility. Both now use the concrete `HDNodeWallet` type. Closes #160 --- .../services/payload-signing.service.spec.ts | 8 ++--- src/config/jwt.config.ts | 32 ++++++++++++++++--- .../auth/wallet-auth.challenge-flow.spec.ts | 16 ++++++---- src/dashboard/dashboard.module.ts | 8 ++--- src/notifications/notification.module.ts | 8 ++--- 5 files changed, 49 insertions(+), 23 deletions(-) diff --git a/src/blockchain/oracle/services/payload-signing.service.spec.ts b/src/blockchain/oracle/services/payload-signing.service.spec.ts index adbd826..16b41fc 100644 --- a/src/blockchain/oracle/services/payload-signing.service.spec.ts +++ b/src/blockchain/oracle/services/payload-signing.service.spec.ts @@ -1,16 +1,16 @@ import { Test, TestingModule } from "@nestjs/testing"; import { ConfigService } from "@nestjs/config"; -import { Wallet } from "ethers"; +import { HDNodeWallet } from "ethers"; import { PayloadSigningService } from "./payload-signing.service"; describe("PayloadSigningService", () => { let service: PayloadSigningService; - let testWallet: Wallet; + let testWallet: HDNodeWallet; let configService: ConfigService; beforeEach(async () => { // Create a test wallet - testWallet = Wallet.createRandom(); + testWallet = HDNodeWallet.createRandom(); // Mock ConfigService const mockConfigService = { @@ -207,7 +207,7 @@ describe("PayloadSigningService", () => { payload, ); - const differentWallet = Wallet.createRandom(); + const differentWallet = HDNodeWallet.createRandom(); const isValid = service.verifySignature( signature, diff --git a/src/config/jwt.config.ts b/src/config/jwt.config.ts index 426b37c..619ce95 100644 --- a/src/config/jwt.config.ts +++ b/src/config/jwt.config.ts @@ -1,5 +1,19 @@ import { JwtModuleOptions } from "@nestjs/jwt"; +/** + * Deriving the duration/algorithm types from `JwtModuleOptions` rather than + * re-declaring them as `string`/`string[]` keeps this file honest: `jsonwebtoken` + * types `expiresIn` as `number | ms.StringValue` (a template-literal union) and + * `algorithms` as `Algorithm[]`, so a bare `string` is not assignable and the + * compiler catches a malformed TTL at the definition rather than at runtime. + */ +type TokenLifetime = NonNullable< + NonNullable["expiresIn"] +>; +type VerifyAlgorithms = NonNullable< + NonNullable["algorithms"] +>; + /** * Single source of truth for JWT lifetimes and algorithm policy. * @@ -17,7 +31,7 @@ import { JwtModuleOptions } from "@nestjs/jwt"; * Access-token lifetime, as a `ms`-style string understood by `jsonwebtoken`. * Shared by `AuthService` and `EnhancedAuthService`. */ -export const ACCESS_TOKEN_TTL = "15m"; +export const ACCESS_TOKEN_TTL: TokenLifetime = "15m"; /** * `ACCESS_TOKEN_TTL` expressed in seconds, for assertions and for places that @@ -33,7 +47,15 @@ export const ACCESS_TOKEN_TTL_SECONDS = 15 * 60; export const REFRESH_TOKEN_TTL_MS = 7 * 24 * 60 * 60 * 1000; /** `REFRESH_TOKEN_TTL_MS` as a `jsonwebtoken` duration string. */ -export const REFRESH_TOKEN_TTL = "7d"; +export const REFRESH_TOKEN_TTL: TokenLifetime = "7d"; + +/** + * Lifetime for the long-lived tokens issued by `DashboardModule` (websocket + * gateway tickets) and `NotificationModule` (consumer tokens). These are + * separate issuers from the 15-minute access-token issuer in `AuthModule`, and + * their TTLs are unchanged. + */ +export const LONG_LIVED_TOKEN_TTL: TokenLifetime = "24h"; /** * Algorithm used to sign new tokens. Pinned explicitly rather than relying on @@ -47,13 +69,15 @@ export const JWT_SIGNING_ALGORITHM = "HS256"; * presented with some other algorithm is rejected outright instead of the * verifier deciding for itself. */ -export const JWT_VERIFY_ALGORITHMS: string[] = [JWT_SIGNING_ALGORITHM]; +export const JWT_VERIFY_ALGORITHMS: VerifyAlgorithms = [ + JWT_SIGNING_ALGORITHM, +]; export interface JwtIssuerConfig { /** Signing/verification secret. */ secret: string; /** Default lifetime applied when a caller does not pass its own `expiresIn`. */ - expiresIn: string; + expiresIn: TokenLifetime; } /** diff --git a/src/core/auth/wallet-auth.challenge-flow.spec.ts b/src/core/auth/wallet-auth.challenge-flow.spec.ts index 032bd94..a9bf535 100644 --- a/src/core/auth/wallet-auth.challenge-flow.spec.ts +++ b/src/core/auth/wallet-auth.challenge-flow.spec.ts @@ -3,14 +3,16 @@ import { getRepositoryToken } from "@nestjs/typeorm"; import { JwtService } from "@nestjs/jwt"; import { ConfigService } from "@nestjs/config"; import { UnauthorizedException } from "@nestjs/common"; -import { Wallet } from "ethers"; +import { HDNodeWallet } from "ethers"; import { WalletAuthService } from "./wallet-auth.service"; import { ChallengeService } from "./challenge.service"; import { EnhancedAuthService } from "./enhanced-auth.service"; import { EmailService } from "./email.service"; import { User, UserRole } from "../user/entities/user.entity"; -// Aliased: `Wallet` here is the ethers signing wallet, the entity is what -// `WalletAuthService` injects its repository for. +// Aliased: `HDNodeWallet` here is the ethers signing wallet, the entity is +// what `WalletAuthService` injects its repository for. In ethers v6 +// `Wallet.createRandom()` returns an `HDNodeWallet`, which is a sibling of +// `Wallet` rather than a subtype, so the variable needs the concrete type. import { Wallet as WalletEntity, WalletType, @@ -33,7 +35,7 @@ describe("WalletAuthService challenge and signature flow", () => { let jwtService: JwtService; let walletRepository: { findOne: jest.Mock }; let provenanceService: { createProvenanceRecord: jest.Mock }; - let testWallet: Wallet; + let testWallet: HDNodeWallet; const mockUser = { id: "user-1", @@ -110,7 +112,7 @@ describe("WalletAuthService challenge and signature flow", () => { challengeService = module.get(ChallengeService); jwtService = module.get(JwtService); - testWallet = Wallet.createRandom(); + testWallet = HDNodeWallet.createRandom(); }); describe("challenge issuance", () => { @@ -232,7 +234,7 @@ describe("WalletAuthService challenge and signature flow", () => { const message = challengeService.issueChallengeForAddress( testWallet.address, ); - const otherWallet = Wallet.createRandom(); + const otherWallet = HDNodeWallet.createRandom(); const signature = await otherWallet.signMessage(message); await expect( @@ -283,7 +285,7 @@ describe("WalletAuthService challenge and signature flow", () => { const message = challengeService.issueChallengeForAddress( testWallet.address, ); - const otherWallet = Wallet.createRandom(); + const otherWallet = HDNodeWallet.createRandom(); const signature = await otherWallet.signMessage(message); await expect( diff --git a/src/dashboard/dashboard.module.ts b/src/dashboard/dashboard.module.ts index b959331..d6f81e9 100644 --- a/src/dashboard/dashboard.module.ts +++ b/src/dashboard/dashboard.module.ts @@ -17,7 +17,7 @@ import { } from "./websocket/services/reconnection.service"; import { WebSocketHealthService } from "./websocket/services/websocket-health.service"; import { WsExceptionFilter } from "./websocket/filters/ws-exception.filter"; -import { buildJwtModuleOptions } from "src/config/jwt.config"; +import { buildJwtModuleOptions, LONG_LIVED_TOKEN_TTL } from "src/config/jwt.config"; @Module({ imports: [ @@ -30,9 +30,9 @@ import { buildJwtModuleOptions } from "src/config/jwt.config"; secret: configService.get("JWT_SECRET") || "default-secret-change-in-production", - // Unchanged: the websocket gateway issues its own long-lived ticket, - // separate from the 15m access-token issuer in AuthModule. - expiresIn: "24h", + // The websocket gateway issues its own long-lived ticket, separate + // from the 15m access-token issuer in AuthModule. Unchanged. + expiresIn: LONG_LIVED_TOKEN_TTL, }), }), ], diff --git a/src/notifications/notification.module.ts b/src/notifications/notification.module.ts index e5248b8..e2b09b1 100644 --- a/src/notifications/notification.module.ts +++ b/src/notifications/notification.module.ts @@ -3,7 +3,7 @@ import { ConfigModule, ConfigService } from '@nestjs/config'; import { TypeOrmModule } from '@nestjs/typeorm'; import { BullModule } from '@nestjs/bull'; import { JwtModule } from '@nestjs/jwt'; -import { buildJwtModuleOptions } from 'src/config/jwt.config'; +import { buildJwtModuleOptions, LONG_LIVED_TOKEN_TTL } from 'src/config/jwt.config'; // Entities import { Notification } from './entities/notification.entity'; @@ -65,9 +65,9 @@ import { NotificationController } from './notification.controller'; useFactory: (configService: ConfigService) => buildJwtModuleOptions({ secret: configService.get('JWT_SECRET', 'change-me-in-production'), - // Unchanged: notification consumers use their own longer-lived - // token, separate from the 15m access-token issuer in AuthModule. - expiresIn: '24h', + // Consumer tokens, separate from the 15m access-token issuer in + // AuthModule. Unchanged. + expiresIn: LONG_LIVED_TOKEN_TTL, }), }), ], From 7053ef143cc3d25b7e6fb644e30126e97b81dca6 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:53:46 +0100 Subject: [PATCH 13/15] test(email-linking): type the repository mocks so the create() assertion compiles emailVerificationRepository was read back off module.get(...) as a Repository in order to reach .create.mock.calls. Repository.create is an overloaded method, so .mock does not exist on that type and TS2339 fired. The mocks are now held in local variables typed as { create: jest.Mock; ... } and passed to the testing module directly, so the assertion reads off the mock rather than off a re-typed Repository. This also drops the jest.spyOn-on-a-shared-instance pattern the file used before. Closes #159 --- src/core/auth/email-linking.service.spec.ts | 184 +++++++++----------- 1 file changed, 80 insertions(+), 104 deletions(-) diff --git a/src/core/auth/email-linking.service.spec.ts b/src/core/auth/email-linking.service.spec.ts index e873bb1..99ea837 100644 --- a/src/core/auth/email-linking.service.spec.ts +++ b/src/core/auth/email-linking.service.spec.ts @@ -1,6 +1,5 @@ import { Test, TestingModule } from "@nestjs/testing"; import { getRepositoryToken } from "@nestjs/typeorm"; -import { Repository } from "typeorm"; import { BadRequestException, ConflictException, @@ -15,6 +14,19 @@ import { Wallet } from "./entities/wallet.entity"; const WALLET_ADDRESS = "0x1234567890123456789012345678901234567890"; +/** + * The repository mocks are held in local variables rather than being read back + * off `module.get(...)`: `Repository.create` is an overloaded method, so + * `repository.create.mock` is not typed on a `Repository` and the assertion + * would not compile. + */ +type RepositoryMock = { + findOne: jest.Mock; + create: jest.Mock; + save: jest.Mock; + delete: jest.Mock; +}; + /** * `EmailLinkingService` resolves everything through the Wallet repository * (`walletRepository.findOne({ where: { address }, relations: ["user"] })`), @@ -23,10 +35,10 @@ const WALLET_ADDRESS = "0x1234567890123456789012345678901234567890"; */ describe("EmailLinkingService", () => { let service: EmailLinkingService; - let userRepository: Repository; - let emailVerificationRepository: Repository; - let walletRepository: Repository; - let emailService: EmailService; + let userRepo: RepositoryMock; + let verificationRepo: RepositoryMock; + let walletRepo: RepositoryMock; + let emailService: { sendVerificationEmail: jest.Mock }; // The User entity has many required columns; these are the ones // EmailLinkingService actually reads. @@ -46,69 +58,51 @@ describe("EmailLinkingService", () => { user, }) as unknown as Wallet; - const mockEmailService = { - sendVerificationEmail: jest.fn().mockResolvedValue({ - messageId: "test-message-id", - previewUrl: "https://ethereal.email/message/test", - }), - }; - beforeEach(async () => { - jest.clearAllMocks(); + userRepo = { + findOne: jest.fn(), + create: jest.fn(), + save: jest.fn(async (user) => user), + delete: jest.fn(), + }; + verificationRepo = { + findOne: jest.fn(), + create: jest.fn((v) => v), + save: jest.fn(async (v) => v), + delete: jest.fn(), + }; + walletRepo = { + findOne: jest.fn(), + create: jest.fn((v) => v), + save: jest.fn(async (v) => v), + delete: jest.fn(), + }; + emailService = { + sendVerificationEmail: jest.fn().mockResolvedValue({ + messageId: "test-message-id", + previewUrl: "https://ethereal.email/message/test", + }), + }; const module: TestingModule = await Test.createTestingModule({ providers: [ EmailLinkingService, - { - provide: getRepositoryToken(User), - useValue: { - findOne: jest.fn(), - create: jest.fn(), - save: jest.fn(async (user) => user), - delete: jest.fn(), - }, - }, + { provide: getRepositoryToken(User), useValue: userRepo }, { provide: getRepositoryToken(EmailVerification), - useValue: { - findOne: jest.fn(), - create: jest.fn((v) => v), - save: jest.fn(async (v) => v), - delete: jest.fn(), - }, - }, - { - provide: getRepositoryToken(Wallet), - useValue: { - findOne: jest.fn(), - create: jest.fn((v) => v), - save: jest.fn(async (v) => v), - delete: jest.fn(), - }, - }, - { - provide: EmailService, - useValue: mockEmailService, + useValue: verificationRepo, }, + { provide: getRepositoryToken(Wallet), useValue: walletRepo }, + { provide: EmailService, useValue: emailService }, ], }).compile(); - service = module.get(EmailLinkingService); - userRepository = module.get>(getRepositoryToken(User)); - emailVerificationRepository = module.get>( - getRepositoryToken(EmailVerification), - ); - walletRepository = module.get>(getRepositoryToken(Wallet)); - emailService = module.get(EmailService); + service = module.get(EmailLinkingService); // Sensible defaults; individual tests override what they care about. - jest - .spyOn(walletRepository, "findOne") - .mockResolvedValue(buildWallet(buildUser())); - jest.spyOn(userRepository, "findOne").mockResolvedValue(null); - jest - .spyOn(emailVerificationRepository, "delete") - .mockResolvedValue(undefined as never); + walletRepo.findOne.mockResolvedValue(buildWallet(buildUser())); + userRepo.findOne.mockResolvedValue(null); + verificationRepo.delete.mockResolvedValue(undefined); }); describe("initiateEmailLinking", () => { @@ -130,8 +124,8 @@ describe("EmailLinkingService", () => { const before = Date.now(); await service.initiateEmailLinking(WALLET_ADDRESS, "Test@Example.com"); - const createArgs = - emailVerificationRepository.create.mock.calls[0][0] as EmailVerification; + const createArgs = verificationRepo.create.mock + .calls[0][0] as EmailVerification; expect(createArgs.email).toBe("test@example.com"); expect(createArgs.walletAddress).toBe(WALLET_ADDRESS.toLowerCase()); @@ -151,7 +145,7 @@ describe("EmailLinkingService", () => { }); it("rejects an unknown wallet", async () => { - jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + walletRepo.findOne.mockResolvedValue(null); await expect( service.initiateEmailLinking(WALLET_ADDRESS, "test@example.com"), @@ -159,9 +153,7 @@ describe("EmailLinkingService", () => { }); it("rejects an email already linked to a different account", async () => { - jest - .spyOn(userRepository, "findOne") - .mockResolvedValue(buildUser({ id: "someone-else" })); + userRepo.findOne.mockResolvedValue(buildUser({ id: "someone-else" })); await expect( service.initiateEmailLinking(WALLET_ADDRESS, "test@example.com"), @@ -169,13 +161,11 @@ describe("EmailLinkingService", () => { }); it("rejects an email that is already verified for this wallet", async () => { - jest - .spyOn(walletRepository, "findOne") - .mockResolvedValue( - buildWallet( - buildUser({ email: "test@example.com", emailVerified: true }), - ), - ); + walletRepo.findOne.mockResolvedValue( + buildWallet( + buildUser({ email: "test@example.com", emailVerified: true }), + ), + ); await expect( service.initiateEmailLinking(WALLET_ADDRESS, "test@example.com"), @@ -196,14 +186,10 @@ describe("EmailLinkingService", () => { it("marks the user email as verified and links it to the wallet", async () => { const user = buildUser(); - jest - .spyOn(emailVerificationRepository, "findOne") - .mockResolvedValue( - buildVerification(new Date(Date.now() + 10 * 60 * 1000)), - ); - jest.spyOn(walletRepository, "findOne").mockResolvedValue( - buildWallet(user), + verificationRepo.findOne.mockResolvedValue( + buildVerification(new Date(Date.now() + 10 * 60 * 1000)), ); + walletRepo.findOne.mockResolvedValue(buildWallet(user)); const result = await service.verifyEmailAndLink("a".repeat(64)); @@ -212,13 +198,11 @@ describe("EmailLinkingService", () => { expect(result.walletAddress).toBe(WALLET_ADDRESS); expect(user.email).toBe("test@example.com"); expect(user.emailVerified).toBe(true); - expect(userRepository.save).toHaveBeenCalled(); + expect(userRepo.save).toHaveBeenCalled(); }); it("rejects an unknown token", async () => { - jest - .spyOn(emailVerificationRepository, "findOne") - .mockResolvedValue(null); + verificationRepo.findOne.mockResolvedValue(null); await expect( service.verifyEmailAndLink("invalid-token"), @@ -226,25 +210,23 @@ describe("EmailLinkingService", () => { }); it("rejects an expired token and deletes the record", async () => { - jest - .spyOn(emailVerificationRepository, "findOne") - .mockResolvedValue(buildVerification(new Date(Date.now() - 1000))); + verificationRepo.findOne.mockResolvedValue( + buildVerification(new Date(Date.now() - 1000)), + ); await expect( service.verifyEmailAndLink("a".repeat(64)), ).rejects.toThrow(UnauthorizedException); - expect(emailVerificationRepository.delete).toHaveBeenCalledWith({ + expect(verificationRepo.delete).toHaveBeenCalledWith({ token: "a".repeat(64), }); }); it("rejects a verification whose wallet no longer exists", async () => { - jest - .spyOn(emailVerificationRepository, "findOne") - .mockResolvedValue( - buildVerification(new Date(Date.now() + 10 * 60 * 1000)), - ); - jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + verificationRepo.findOne.mockResolvedValue( + buildVerification(new Date(Date.now() + 10 * 60 * 1000)), + ); + walletRepo.findOne.mockResolvedValue(null); await expect( service.verifyEmailAndLink("a".repeat(64)), @@ -254,7 +236,7 @@ describe("EmailLinkingService", () => { describe("getAccountInfo", () => { it("returns the linked email for a wallet that has one", async () => { - jest.spyOn(walletRepository, "findOne").mockResolvedValue( + walletRepo.findOne.mockResolvedValue( buildWallet( buildUser({ email: "test@example.com", emailVerified: true }), ), @@ -268,9 +250,7 @@ describe("EmailLinkingService", () => { }); it("returns a null email when the wallet has none linked", async () => { - jest.spyOn(walletRepository, "findOne").mockResolvedValue( - buildWallet(buildUser()), - ); + walletRepo.findOne.mockResolvedValue(buildWallet(buildUser())); const result = await service.getAccountInfo(WALLET_ADDRESS); @@ -279,7 +259,7 @@ describe("EmailLinkingService", () => { }); it("returns a null email when the wallet is unknown", async () => { - jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + walletRepo.findOne.mockResolvedValue(null); const result = await service.getAccountInfo(WALLET_ADDRESS); @@ -294,24 +274,20 @@ describe("EmailLinkingService", () => { email: "test@example.com", emailVerified: true, }); - jest.spyOn(walletRepository, "findOne").mockResolvedValue( - buildWallet(user), - ); + walletRepo.findOne.mockResolvedValue(buildWallet(user)); const result = await service.unlinkEmail(WALLET_ADDRESS); expect(result.message).toContain("successfully unlinked"); expect(user.email).toBeNull(); expect(user.emailVerified).toBe(false); - expect(emailVerificationRepository.delete).toHaveBeenCalledWith({ + expect(verificationRepo.delete).toHaveBeenCalledWith({ walletAddress: WALLET_ADDRESS.toLowerCase(), }); }); it("throws when no email is linked", async () => { - jest - .spyOn(walletRepository, "findOne") - .mockResolvedValue(buildWallet(buildUser())); + walletRepo.findOne.mockResolvedValue(buildWallet(buildUser())); await expect( service.unlinkEmail(WALLET_ADDRESS), @@ -319,7 +295,7 @@ describe("EmailLinkingService", () => { }); it("throws when the wallet is unknown", async () => { - jest.spyOn(walletRepository, "findOne").mockResolvedValue(null); + walletRepo.findOne.mockResolvedValue(null); await expect( service.unlinkEmail(WALLET_ADDRESS), @@ -333,12 +309,12 @@ describe("EmailLinkingService", () => { email: "test@example.com", emailVerified: true, }); - jest.spyOn(userRepository, "findOne").mockResolvedValue(user); + userRepo.findOne.mockResolvedValue(user); await expect(service.getUserByEmail("TEST@example.com")).resolves.toBe( user, ); - expect(userRepository.findOne).toHaveBeenCalledWith({ + expect(userRepo.findOne).toHaveBeenCalledWith({ where: { email: "test@example.com", emailVerified: true }, }); }); From 179f2778e6235d3b23d33cd705066cbf9ab20d21 Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:53:55 +0100 Subject: [PATCH 14/15] test(jest): drop test/portfolio/portfolio.service.spec.ts rather than ship it half-repaired `tsc` reported 17 errors in this file once it became reachable, and reading the service showed the drift is behavioural, not just typing: - PortfolioService takes seven constructor dependencies. The spec provided four; PerformanceAnalyticsService, PortfolioConstraintService and AuditLogService were all missing, so the testing module could not have compiled. Those three are precisely the collaborators that gate the behaviour this spec asserts on. - deletePortfolio() now calls `remove()` (a hard delete). The spec asserted a soft delete - status archived, deletedAt set, save called. - restorePortfolio() no longer exists; the restore path is updatePortfolio(id, { status: ACTIVE }). Two tests target a removed API. - addHolding() looks a holding up by `{ portfolioId, ticker }`, not `{ portfolioId, ticker, chain }`, and the spec asserts the old shape. - Every add/update/remove holding test now also traverses validatePortfolioConstraints -> auditLogService.recordVerification and updatePortfolioMetrics -> performanceService.recordSnapshot, none of which the spec mocks. - `type` was dropped from CreatePortfolioDto, `status` moved to the PortfolioStatus enum, and CreateOptimizationDto gained a required portfolioId. Repairing this properly means rebuilding roughly thirty tests against a ~700 line service whose transitive collaborators are unmocked - and with no way to run the suite, there is no way to know whether the rebuilt assertions are actually right. A spec that merely compiles would look finished while almost certainly turning CI red, which is worse than the current honest state. Deleted, with the intent recorded here so it can be picked up as its own task once the suite is runnable. The issue allows exactly this outcome ("runs in the unit suite or is deliberately deleted, with the rationale in the PR"). This is not a duplicate of any existing spec, so no working coverage is lost - only a suite that never ran and could not have. The other two mechanically-moved specs on this branch (payload-signing.service.spec.ts, modern-portfolio-theory.spec.ts) type check cleanly. Closes #159 --- .../services/portfolio.service.spec.ts | 549 ------------------ 1 file changed, 549 deletions(-) delete mode 100644 src/investment/portfolio/services/portfolio.service.spec.ts diff --git a/src/investment/portfolio/services/portfolio.service.spec.ts b/src/investment/portfolio/services/portfolio.service.spec.ts deleted file mode 100644 index 10e4b61..0000000 --- a/src/investment/portfolio/services/portfolio.service.spec.ts +++ /dev/null @@ -1,549 +0,0 @@ -import { Test, TestingModule } from "@nestjs/testing"; -import { getRepositoryToken } from "@nestjs/typeorm"; -import { PortfolioService } from "src/investment/portfolio/services/portfolio.service"; -import { Portfolio } from "src/investment/portfolio/entities/portfolio.entity"; -import { - PortfolioAsset, - Chain, - AssetType, -} from "src/investment/portfolio/entities/portfolio-asset.entity"; -import { OptimizationHistory } from "src/investment/portfolio/entities/optimization-history.entity"; -import { RiskProfile } from "src/investment/portfolio/entities/risk-profile.entity"; -import { CreatePortfolioDto } from "src/investment/portfolio/dto/portfolio.dto"; -import { OptimizationMethod } from "src/investment/portfolio/entities/optimization-history.entity"; -import { - AddHoldingDto, - UpdateHoldingDto, -} from "src/investment/portfolio/dto/portfolio-asset.dto"; - -describe("PortfolioService", () => { - let service: PortfolioService; - let portfolioRepository: any; - let assetRepository: any; - let optimizationRepository: any; - let riskProfileRepository: any; - - const mockPortfolio = { - id: "test-portfolio-1", - userId: "test-user-1", - name: "Test Portfolio", - type: "custom", - status: "active", - totalValue: 100000, - currentAllocation: { AAPL: 30, MSFT: 70 }, - targetAllocation: null, - initialAllocation: {}, - allocationStrategy: "manual", - autoRebalanceEnabled: false, - rebalanceThreshold: 5, - assets: [], - deletedAt: null, - save: jest.fn(), - }; - - const mockAsset = { - id: "asset-1", - ticker: "AAPL", - name: "Apple", - chain: Chain.OTHER, - quantity: 100, - currentPrice: 150, - value: 15000, - allocationPercentage: 15, - portfolioId: "test-portfolio-1", - save: jest.fn(), - }; - - const mockOptimization = { - id: "opt-1", - portfolioId: "test-portfolio-1", - method: OptimizationMethod.MEAN_VARIANCE, - status: "completed", - suggestedAllocation: { AAPL: 40, MSFT: 60 }, - expectedReturn: 0.08, - expectedVolatility: 0.15, - expectedSharpeRatio: 0.5, - improvementScore: 10, - currentAllocation: mockPortfolio.currentAllocation, - save: jest.fn(), - }; - - beforeEach(async () => { - portfolioRepository = { - create: jest.fn().mockReturnValue(mockPortfolio), - save: jest.fn().mockResolvedValue(mockPortfolio), - findOne: jest.fn(), - find: jest.fn().mockResolvedValue([mockPortfolio]), - delete: jest.fn(), - }; - - assetRepository = { - create: jest.fn().mockReturnValue(mockAsset), - save: jest.fn().mockResolvedValue(mockAsset), - find: jest.fn().mockResolvedValue([mockAsset]), - findOne: jest.fn(), - }; - - optimizationRepository = { - create: jest.fn().mockReturnValue(mockOptimization), - save: jest.fn(), - find: jest.fn(), - findOne: jest.fn(), - }; - - riskProfileRepository = { - findOne: jest.fn(), - }; - - const module: TestingModule = await Test.createTestingModule({ - providers: [ - PortfolioService, - { - provide: getRepositoryToken(Portfolio), - useValue: portfolioRepository, - }, - { - provide: getRepositoryToken(PortfolioAsset), - useValue: assetRepository, - }, - { - provide: getRepositoryToken(OptimizationHistory), - useValue: optimizationRepository, - }, - { - provide: getRepositoryToken(RiskProfile), - useValue: riskProfileRepository, - }, - ], - }).compile(); - - service = module.get(PortfolioService); - }); - - it("should be defined", () => { - expect(service).toBeDefined(); - }); - - describe("createPortfolio", () => { - it("should create a new portfolio", async () => { - portfolioRepository.findOne.mockResolvedValue(null); - - const dto: CreatePortfolioDto = { - name: "Test Portfolio", - type: "custom", - }; - - const result = await service.createPortfolio("test-user-1", dto); - - expect(portfolioRepository.create).toHaveBeenCalledWith( - expect.objectContaining({ - name: dto.name, - userId: "test-user-1", - type: "custom", - status: "active", - }), - ); - expect(portfolioRepository.save).toHaveBeenCalled(); - expect(result).toEqual(mockPortfolio); - }); - - it("should throw if portfolio name already exists", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - const dto: CreatePortfolioDto = { - name: "Test Portfolio", - }; - - await expect(service.createPortfolio("test-user-1", dto)).rejects.toThrow( - "Portfolio with this name already exists", - ); - }); - - it("should throw for empty name", async () => { - const dto: CreatePortfolioDto = { - name: " ", - }; - - await expect(service.createPortfolio("test-user-1", dto)).rejects.toThrow( - BadRequestException, - ); - }); - }); - - describe("getPortfolio", () => { - it("should return a portfolio by ID", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - const result = await service.getPortfolio("test-portfolio-1"); - - expect(portfolioRepository.findOne).toHaveBeenCalledWith({ - where: { id: "test-portfolio-1" }, - relations: expect.any(Array), - }); - expect(result).toEqual(mockPortfolio); - }); - - it("should throw NotFoundException if portfolio not found", async () => { - portfolioRepository.findOne.mockResolvedValue(null); - - await expect(service.getPortfolio("non-existent")).rejects.toThrow( - NotFoundException, - ); - }); - - it("should throw NotFoundException if portfolio is deleted", async () => { - const deletedPortfolio = { ...mockPortfolio, deletedAt: new Date() }; - portfolioRepository.findOne.mockResolvedValue(deletedPortfolio); - - await expect(service.getPortfolio("test-portfolio-1")).rejects.toThrow( - NotFoundException, - ); - }); - }); - - describe("getUserPortfolios", () => { - it("should return all portfolios for a user", async () => { - const result = await service.getUserPortfolios("test-user-1"); - - expect(portfolioRepository.find).toHaveBeenCalledWith({ - where: { userId: "test-user-1" }, - relations: expect.any(Array), - order: expect.any(Object), - }); - expect(result).toEqual([mockPortfolio]); - }); - }); - - describe("updatePortfolio", () => { - it("should update portfolio properties", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - const updateDto = { - name: "Updated Portfolio", - description: "Updated description", - }; - - const result = await service.updatePortfolio( - "test-portfolio-1", - updateDto, - ); - - expect(portfolioRepository.save).toHaveBeenCalled(); - expect(result.name).toBe("Updated Portfolio"); - }); - - it("should archive portfolio when status is ARCHIVED", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - const updateDto = { - status: "archived", - }; - - const result = await service.updatePortfolio( - "test-portfolio-1", - updateDto, - ); - - expect(result.status).toBe("archived"); - expect(result.deletedAt).toBeDefined(); - }); - - it("should validate allocation percentages", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - const updateDto = { - currentAllocation: { AAPL: 60, MSFT: 50 }, - }; - - await expect( - service.updatePortfolio("test-portfolio-1", updateDto), - ).rejects.toThrow(BadRequestException); - }); - }); - - describe("deletePortfolio", () => { - it("should soft delete portfolio", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - await service.deletePortfolio("test-portfolio-1"); - - expect(mockPortfolio.status).toBe("archived"); - expect(mockPortfolio.deletedAt).toBeDefined(); - expect(portfolioRepository.save).toHaveBeenCalled(); - }); - - it("should throw NotFoundException for deleted portfolio", async () => { - const deletedPortfolio = { ...mockPortfolio, deletedAt: new Date() }; - portfolioRepository.findOne.mockResolvedValue(deletedPortfolio); - - await expect(service.deletePortfolio("test-portfolio-1")).rejects.toThrow( - NotFoundException, - ); - }); - }); - - describe("restorePortfolio", () => { - it("should restore archived portfolio", async () => { - const archivedPortfolio = { - ...mockPortfolio, - status: "archived", - deletedAt: new Date(), - }; - portfolioRepository.findOne.mockResolvedValue(archivedPortfolio); - - const result = await service.restorePortfolio("test-portfolio-1"); - - expect(result.status).toBe("active"); - expect(result.deletedAt).toBeNull(); - expect(portfolioRepository.save).toHaveBeenCalled(); - }); - - it("should throw NotFoundException when restoring non-archived portfolio", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - await expect( - service.restorePortfolio("test-portfolio-1"), - ).rejects.toThrow(NotFoundException); - }); - }); - - describe("addAsset", () => { - it("should add an asset to portfolio", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - assetRepository.findOne.mockResolvedValue(null); - - const result = await service.addAsset( - "test-portfolio-1", - "AAPL", - "Apple", - 100, - 150, - 0, - ); - - expect(assetRepository.create).toHaveBeenCalled(); - expect(assetRepository.save).toHaveBeenCalled(); - expect(result).toEqual(mockAsset); - }); - - it("should throw for invalid quantity", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - await expect( - service.addAsset("test-portfolio-1", "AAPL", "Apple", -10, 150, 0), - ).rejects.toThrow(BadRequestException); - }); - - it("should throw for negative price", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - await expect( - service.addAsset("test-portfolio-1", "AAPL", "Apple", 100, -150, 0), - ).rejects.toThrow(BadRequestException); - }); - }); - - describe("runOptimization", () => { - it("should run portfolio optimization", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - assetRepository.find.mockResolvedValue([mockAsset]); - optimizationRepository.create.mockReturnValue({ - ...mockOptimization, - status: "in_progress", - save: jest.fn(), - }); - optimizationRepository.save - .mockResolvedValueOnce({ - ...mockOptimization, - status: "in_progress", - }) - .mockResolvedValueOnce(mockOptimization); - - const result = await service.runOptimization("test-portfolio-1", { - method: OptimizationMethod.MEAN_VARIANCE, - }); - - expect(result.status).toBe("completed"); - expect(optimizationRepository.save).toHaveBeenCalled(); - }); - - it("should throw if portfolio has no assets", async () => { - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - assetRepository.find.mockResolvedValue([]); - - await expect( - service.runOptimization("test-portfolio-1", { - method: OptimizationMethod.MEAN_VARIANCE, - }), - ).rejects.toThrow("Portfolio has no assets to optimize"); - }); - }); - - describe("approveOptimization", () => { - it("should approve completed optimization", async () => { - optimizationRepository.findOne.mockResolvedValue(mockOptimization); - - const result = await service.approveOptimization("opt-1", "Looks good"); - - expect(result.status).toBe("approved"); - expect(optimizationRepository.save).toHaveBeenCalled(); - }); - - it("should throw if optimization not found", async () => { - optimizationRepository.findOne.mockResolvedValue(null); - - await expect(service.approveOptimization("non-existent")).rejects.toThrow( - NotFoundException, - ); - }); - - it("should throw if optimization is not completed", async () => { - const pendingOptimization = { - ...mockOptimization, - status: "in_progress", - }; - optimizationRepository.findOne.mockResolvedValue(pendingOptimization); - - await expect(service.approveOptimization("opt-1")).rejects.toThrow( - BadRequestException, - ); - }); - }); - - describe("implementOptimization", () => { - it("should implement approved optimization", async () => { - const approvedOptimization = { - ...mockOptimization, - status: "approved", - }; - optimizationRepository.findOne.mockResolvedValue(approvedOptimization); - portfolioRepository.findOne.mockResolvedValue(mockPortfolio); - - const result = await service.implementOptimization("opt-1"); - - expect(result.targetAllocation).toEqual( - approvedOptimization.suggestedAllocation, - ); - expect(optimizationRepository.save).toHaveBeenCalled(); - }); - - it("should throw if optimization not found", async () => { - optimizationRepository.findOne.mockResolvedValue(null); - - await expect( - service.implementOptimization("non-existent"), - ).rejects.toThrow(NotFoundException); - }); - - it("should throw if optimization is not approved", async () => { - optimizationRepository.findOne.mockResolvedValue(mockOptimization); - - await expect(service.implementOptimization("opt-1")).rejects.toThrow( - BadRequestException, - ); - }); - }); - - describe("addHolding", () => { - it("should add a new holding to portfolio", async () => { - const dto: AddHoldingDto = { - ticker: "ETH", - name: "Ethereum", - chain: Chain.ETHEREUM, - quantity: 10, - currentPrice: 2000, - costBasis: 15000, - }; - - assetRepository.findOne.mockResolvedValue(null); - assetRepository.create.mockReturnValue({ ...mockAsset, ...dto }); - assetRepository.save.mockResolvedValue({ ...mockAsset, ...dto }); - - const result = await service.addHolding("test-portfolio-1", dto); - - expect(assetRepository.findOne).toHaveBeenCalledWith({ - where: { - portfolioId: "test-portfolio-1", - ticker: dto.ticker, - chain: dto.chain, - }, - }); - expect(assetRepository.create).toHaveBeenCalled(); - expect(assetRepository.save).toHaveBeenCalled(); - expect(result.ticker).toBe(dto.ticker); - expect(result.chain).toBe(dto.chain); - }); - - it("should throw error if holding already exists", async () => { - const dto: AddHoldingDto = { - ticker: "ETH", - name: "Ethereum", - chain: Chain.ETHEREUM, - quantity: 10, - currentPrice: 2000, - costBasis: 15000, - }; - - assetRepository.findOne.mockResolvedValue(mockAsset); - - await expect(service.addHolding("test-portfolio-1", dto)).rejects.toThrow( - "Holding with same ticker and chain already exists", - ); - }); - }); - - describe("updateHolding", () => { - it("should update holding in portfolio", async () => { - const dto: UpdateHoldingDto = { - quantity: 20, - currentPrice: 2500, - }; - - const updatedAsset = { ...mockAsset, ...dto, value: 50000 }; - assetRepository.findOne.mockResolvedValue(mockAsset); - assetRepository.save.mockResolvedValue(updatedAsset); - - const result = await service.updateHolding( - "test-portfolio-1", - "asset-1", - dto, - ); - - expect(assetRepository.findOne).toHaveBeenCalledWith({ - where: { id: "asset-1", portfolioId: "test-portfolio-1" }, - }); - expect(assetRepository.save).toHaveBeenCalled(); - }); - - it("should throw error if holding not found", async () => { - const dto: UpdateHoldingDto = { quantity: 20 }; - assetRepository.findOne.mockResolvedValue(null); - - await expect( - service.updateHolding("test-portfolio-1", "non-existent", dto), - ).rejects.toThrow("Holding not found"); - }); - }); - - describe("removeHolding", () => { - it("should remove holding from portfolio", async () => { - assetRepository.findOne.mockResolvedValue(mockAsset); - assetRepository.remove.mockResolvedValue(null); - - await service.removeHolding("test-portfolio-1", "asset-1"); - - expect(assetRepository.findOne).toHaveBeenCalledWith({ - where: { id: "asset-1", portfolioId: "test-portfolio-1" }, - }); - expect(assetRepository.remove).toHaveBeenCalledWith(mockAsset); - }); - - it("should throw error if holding not found", async () => { - assetRepository.findOne.mockResolvedValue(null); - - await expect( - service.removeHolding("test-portfolio-1", "non-existent"), - ).rejects.toThrow("Holding not found"); - }); - }); -}); From 55baec60b4d6099041be22bb750faab59f20887e Mon Sep 17 00:00:00 2001 From: Aliyu Ibrahim <271224223+ykargeee-bit@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:00:22 +0100 Subject: [PATCH 15/15] fix(file-upload): enforce the MIME allowlist in the handler, not a decorator `npx tsc --noEmit` failed with src/infrastructure/file-upload/file-upload.controller.ts(142,25): error TS2532: Object is possibly 'undefined'. This is unrelated to the auth and jest work on this branch - the file and tsconfig.json are byte-identical to main - but it blocks the type-check gate, so it is fixed here and flagged as an unrelated change. The compiler was right to flag it, and it is not only a typing problem. A decorator expression is evaluated once, when the class is defined - not per request - so the arrow function inside it captures whatever `this` is at class-definition time (the class/prototype) and reuses that for every subsequent request. `getAllowedMimeTypes` is a prototype method, so it is not reachable through it. The MIME allowlist on the upload endpoint was therefore not being enforced the way it appeared to be. The check moves into `uploadFile()`, which runs per request against the real instance, so `this` is unambiguous: - no file -> BadRequestException("No file provided") - disallowed mimetype -> BadRequestException("Unsupported file type: ...") - otherwise -> unchanged, proceeds to storageService.upload() The `limits.fileSize` setting deliberately stays on the interceptor: multer enforces it while streaming, before the body is buffered, so moving it into the handler would remove the protection it exists for. The trade-off of moving the MIME check is that a disallowed file is now fully buffered before being rejected; it is still bounded by the 50MB size limit. Adds file-upload.controller.spec.ts, the first coverage this path has had - the absence of a test is the other half of why the defect went unnoticed. It covers the default allowlist, a configured allowlist including the empty-after-parsing fallback, the missing-file case, and that rejections happen before storage is touched. --- .../file-upload.controller.spec.ts | 104 ++++++++++++++++++ .../file-upload/file-upload.controller.ts | 41 ++++--- 2 files changed, 127 insertions(+), 18 deletions(-) create mode 100644 src/infrastructure/file-upload/file-upload.controller.spec.ts diff --git a/src/infrastructure/file-upload/file-upload.controller.spec.ts b/src/infrastructure/file-upload/file-upload.controller.spec.ts new file mode 100644 index 0000000..4158570 --- /dev/null +++ b/src/infrastructure/file-upload/file-upload.controller.spec.ts @@ -0,0 +1,104 @@ +import { Test, TestingModule } from "@nestjs/testing"; +import { BadRequestException } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; +import { FileUploadController } from "./file-upload.controller"; +import { FileStorageService } from "./services/file-storage.service"; +import { FileCleanupService } from "./services/file-cleanup.service"; +import { FileScanService } from "./services/file-scan.service"; +import { FileMetadataService } from "./services/file-metadata.service"; + +/** + * The upload MIME allowlist had no coverage at all, which is part of why the + * `fileFilter` problem went unnoticed: the check lived in a decorator + * expression, so it was neither reached by a test nor reliably invoked at + * runtime. It is now enforced in `uploadFile()`, which is what these exercise. + */ +describe("FileUploadController upload allowlist", () => { + let controller: FileUploadController; + let storageService: { upload: jest.Mock }; + let configService: { get: jest.Mock }; + + const uploadedFile = (mimetype: string) => + ({ + originalname: "upload", + mimetype, + size: 10, + buffer: Buffer.from("data"), + }) as unknown as Express.Multer.File; + + const user = { id: "user-1" } as never; + const dto = {} as never; + + beforeEach(async () => { + jest.clearAllMocks(); + + storageService = { upload: jest.fn().mockResolvedValue({ file: {}, thumbnails: [] }) }; + configService = { get: jest.fn().mockReturnValue(undefined) }; + + const module: TestingModule = await Test.createTestingModule({ + controllers: [FileUploadController], + providers: [ + { provide: FileStorageService, useValue: storageService }, + { provide: FileCleanupService, useValue: {} }, + { provide: FileScanService, useValue: {} }, + { provide: FileMetadataService, useValue: {} }, + { provide: ConfigService, useValue: configService }, + ], + }).compile(); + + controller = module.get(FileUploadController); + }); + + describe("default allowlist (FILE_ALLOWED_MIME_TYPES unset)", () => { + it("accepts an allowed mime type", async () => { + await expect( + controller.uploadFile(uploadedFile("image/png"), dto, user), + ).resolves.toBeDefined(); + expect(storageService.upload).toHaveBeenCalled(); + }); + + it("rejects a mime type that is not on the allowlist", async () => { + await expect( + controller.uploadFile(uploadedFile("application/x-msdownload"), dto, user), + ).rejects.toThrow(BadRequestException); + expect(storageService.upload).not.toHaveBeenCalled(); + }); + + it("rejects a request with no file", async () => { + await expect( + controller.uploadFile(undefined as never, dto, user), + ).rejects.toThrow("No file provided"); + expect(storageService.upload).not.toHaveBeenCalled(); + }); + }); + + describe("configured allowlist (FILE_ALLOWED_MIME_TYPES set)", () => { + it("honours the configured list instead of the default", async () => { + configService.get.mockReturnValue("text/plain, application/json"); + + await expect( + controller.uploadFile(uploadedFile("text/plain"), dto, user), + ).resolves.toBeDefined(); + + // On the default list, but not on the configured one. + await expect( + controller.uploadFile(uploadedFile("image/png"), dto, user), + ).rejects.toThrow(BadRequestException); + }); + + it("falls back to the default list when the configured value is only separators", async () => { + configService.get.mockReturnValue(" , ,"); + + await expect( + controller.uploadFile(uploadedFile("image/png"), dto, user), + ).resolves.toBeDefined(); + }); + }); + + it("rejects before touching storage when the user is unknown", async () => { + await expect( + controller.uploadFile(uploadedFile("image/png"), dto, undefined as never), + ).rejects.toThrow("User identity not found"); + expect(storageService.upload).not.toHaveBeenCalled(); + }); +}); \ No newline at end of file diff --git a/src/infrastructure/file-upload/file-upload.controller.ts b/src/infrastructure/file-upload/file-upload.controller.ts index 5fc797d..9e30d52 100644 --- a/src/infrastructure/file-upload/file-upload.controller.ts +++ b/src/infrastructure/file-upload/file-upload.controller.ts @@ -133,24 +133,19 @@ export class FileUploadController { limits: { fileSize: 50 * 1024 * 1024, // 50MB }, - fileFilter: (req, file, cb) => { - const f = file; - if (!f) { - cb(new BadRequestException("No file provided"), false); - return; - } - const allowed = this.getAllowedMimeTypes(); - if (allowed.has(f.mimetype)) { - cb(null, true); - } else { - cb( - new BadRequestException( - `Unsupported file type: ${f.mimetype}`, - ), - false, - ); - } - }, + // NOTE: the MIME allowlist check deliberately lives in `uploadFile()` + // rather than in a multer `fileFilter` here. + // + // Decorator expressions are evaluated once, when the class is defined - + // not per request - so an arrow function inside one captures whatever + // `this` is at class-definition time (the class/prototype), and then + // reuses it for every subsequent request. Calling a prototype method + // through it is not the instance method, which is both why the type + // checker flags this and why the check was never reliable. `uploadFile` + // runs per request against the real instance, so `this` is correct. + // + // The size limit stays on the interceptor: multer enforces it while + // streaming, before the body is buffered, so it must not be moved. }), ) @ApiOperation({ summary: "Upload a file" }) @@ -179,6 +174,16 @@ export class FileUploadController { if (!user?.id) { throw new BadRequestException("User identity not found in request"); } + + // MIME allowlist. See the note on the interceptor above for why this is + // here rather than in a multer `fileFilter`. + if (!file) { + throw new BadRequestException("No file provided"); + } + if (!this.getAllowedMimeTypes().has(file.mimetype)) { + throw new BadRequestException(`Unsupported file type: ${file.mimetype}`); + } + const userId = user.id; const result = await this.storageService.upload(file, userId, dto);