From 39ba6b98e6726ec272c76ec196bb732a75fdd489 Mon Sep 17 00:00:00 2001 From: Kenrick Tandrian <60643640+KenTandrian@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:54:47 +0700 Subject: [PATCH] Task/improve type safety in auth and user services (#7630) Improve type safety --- apps/api/src/app/auth/api-key.strategy.ts | 28 ++++++++++---------- apps/api/src/app/auth/auth.module.ts | 21 +++++++++++---- apps/api/src/app/auth/oidc-state.store.ts | 31 ++++++++++++----------- apps/api/src/app/auth/oidc.strategy.ts | 14 +++++----- apps/api/src/app/auth/web-auth.service.ts | 9 +++++-- apps/api/src/app/user/user.service.ts | 13 ++++++---- 6 files changed, 67 insertions(+), 49 deletions(-) diff --git a/apps/api/src/app/auth/api-key.strategy.ts b/apps/api/src/app/auth/api-key.strategy.ts index 232a272bc..af83f4c60 100644 --- a/apps/api/src/app/auth/api-key.strategy.ts +++ b/apps/api/src/app/auth/api-key.strategy.ts @@ -56,22 +56,20 @@ export class ApiKeyStrategy extends PassportStrategy( } private async validateApiKey(apiKey: string) { - if (!apiKey) { - throw new HttpException( - getReasonPhrase(StatusCodes.UNAUTHORIZED), - StatusCodes.UNAUTHORIZED - ); - } - - try { - const { id } = await this.apiKeyService.getUserByApiKey(apiKey); + if (apiKey) { + try { + const { id } = await this.apiKeyService.getUserByApiKey(apiKey); + const user = await this.userService.user({ id }); - return this.userService.user({ id }); - } catch { - throw new HttpException( - getReasonPhrase(StatusCodes.UNAUTHORIZED), - StatusCodes.UNAUTHORIZED - ); + if (user) { + return user; + } + } catch {} } + + throw new HttpException( + getReasonPhrase(StatusCodes.UNAUTHORIZED), + StatusCodes.UNAUTHORIZED + ); } } diff --git a/apps/api/src/app/auth/auth.module.ts b/apps/api/src/app/auth/auth.module.ts index ddc41abad..e2a71ca84 100644 --- a/apps/api/src/app/auth/auth.module.ts +++ b/apps/api/src/app/auth/auth.module.ts @@ -69,7 +69,7 @@ import { OidcStrategy } from './oidc.strategy'; const issuer = configurationService.get('OIDC_ISSUER'); const scope = configurationService.get('OIDC_SCOPE'); - const callbackUrl = + const callbackURL = configurationService.get('OIDC_CALLBACK_URL') || `${configurationService.get('ROOT_URL')}/api/auth/oidc/callback`; @@ -114,15 +114,26 @@ import { OidcStrategy } from './oidc.strategy'; } } + const clientID = configurationService.get('OIDC_CLIENT_ID'); + const clientSecret = configurationService.get('OIDC_CLIENT_SECRET'); + + if (!clientID || !clientSecret || !issuer) { + logger.error( + 'OIDC configuration incomplete: issuer, clientID, or clientSecret missing' + ); + + throw new Error('OIDC configuration incomplete'); + } + const options: StrategyOptions = { authorizationURL, + callbackURL, + clientID, + clientSecret, issuer, scope, tokenURL, - userInfoURL, - callbackURL: callbackUrl, - clientID: configurationService.get('OIDC_CLIENT_ID'), - clientSecret: configurationService.get('OIDC_CLIENT_SECRET') + userInfoURL }; return new OidcStrategy(authService, options); diff --git a/apps/api/src/app/auth/oidc-state.store.ts b/apps/api/src/app/auth/oidc-state.store.ts index 653451166..aebd99892 100644 --- a/apps/api/src/app/auth/oidc-state.store.ts +++ b/apps/api/src/app/auth/oidc-state.store.ts @@ -1,17 +1,24 @@ +import type { Request } from 'express'; import ms from 'ms'; +import type { + SessionStore, + SessionStoreCallback, + SessionStoreContext, + SessionVerifyCallback +} from 'passport-openidconnect'; /** * Custom state store for OIDC authentication that doesn't rely on express-session. * This store manages OAuth2 state parameters in memory with automatic cleanup. */ -export class OidcStateStore { +export class OidcStateStore implements SessionStore { private readonly STATE_EXPIRY_MS = ms('10 minutes'); private stateMap = new Map< string, { appState?: unknown; - ctx: { issued?: Date; maxAge?: number; nonce?: string }; + ctx: SessionStoreContext; meta?: unknown; timestamp: number; } @@ -19,14 +26,13 @@ export class OidcStateStore { /** * Store request state. - * Signature matches passport-openidconnect SessionStore */ public store( - _req: unknown, - _meta: unknown, + _req: Request, + ctx: SessionStoreContext, appState: unknown, - ctx: { maxAge?: number; nonce?: string; issued?: Date }, - callback: (err: Error | null, handle?: string) => void + meta: unknown, + callback: SessionStoreCallback ) { try { // Generate a unique handle for this state @@ -35,7 +41,7 @@ export class OidcStateStore { this.stateMap.set(handle, { appState, ctx, - meta: _meta, + meta, timestamp: Date.now() }); @@ -50,16 +56,11 @@ export class OidcStateStore { /** * Verify request state. - * Signature matches passport-openidconnect SessionStore */ public verify( - _req: unknown, + _req: Request, handle: string, - callback: ( - err: Error | null, - appState?: unknown, - ctx?: { maxAge?: number; nonce?: string; issued?: Date } - ) => void + callback: SessionVerifyCallback ) { try { const data = this.stateMap.get(handle); diff --git a/apps/api/src/app/auth/oidc.strategy.ts b/apps/api/src/app/auth/oidc.strategy.ts index 661f2a821..30c7673c7 100644 --- a/apps/api/src/app/auth/oidc.strategy.ts +++ b/apps/api/src/app/auth/oidc.strategy.ts @@ -15,10 +15,10 @@ import { OidcStateStore } from './oidc-state.store'; @Injectable() export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') { - private readonly logger = new Logger(OidcStrategy.name); - private static readonly stateStore = new OidcStateStore(); + private readonly logger = new Logger(OidcStrategy.name); + public constructor( private readonly authService: AuthService, options: StrategyOptions @@ -48,11 +48,6 @@ export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') { params?.sub ?? context?.claims?.sub; - const jwt = await this.authService.validateOAuthLogin({ - thirdPartyId, - provider: Provider.OIDC - }); - if (!thirdPartyId) { this.logger.error( `Missing subject identifier in OIDC response from ${issuer}` @@ -61,6 +56,11 @@ export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') { throw new Error('Missing subject identifier in OIDC response'); } + const jwt = await this.authService.validateOAuthLogin({ + thirdPartyId, + provider: Provider.OIDC + }); + return { jwt }; } catch (error) { this.logger.error(error); diff --git a/apps/api/src/app/auth/web-auth.service.ts b/apps/api/src/app/auth/web-auth.service.ts index cb9dd8cb7..568822fb8 100644 --- a/apps/api/src/app/auth/web-auth.service.ts +++ b/apps/api/src/app/auth/web-auth.service.ts @@ -99,6 +99,11 @@ export class WebAuthService { ): Promise { const user = this.request.user; const expectedChallenge = user.authChallenge; + + if (!expectedChallenge) { + throw new Error('Missing authentication challenge'); + } + let verification: VerifiedRegistrationResponse; try { @@ -213,7 +218,7 @@ export class WebAuthService { id: isoBase64URL.fromBuffer(device.credentialId), publicKey: device.credentialPublicKey }, - expectedChallenge: `${user.authChallenge}`, + expectedChallenge: `${user?.authChallenge}`, expectedOrigin: this.expectedOrigin, expectedRPID: this.rpID, requireUserVerification: false, @@ -243,7 +248,7 @@ export class WebAuthService { }); return this.jwtService.sign({ - id: user.id + id: user?.id }); } diff --git a/apps/api/src/app/user/user.service.ts b/apps/api/src/app/user/user.service.ts index 1175662eb..0a4f21190 100644 --- a/apps/api/src/app/user/user.service.ts +++ b/apps/api/src/app/user/user.service.ts @@ -171,25 +171,28 @@ export class UserService { userSettings: settings.settings as UserSettings }); - let referralPartners: ReferralPartner[]; + let referralPartners: ReferralPartner[] = []; if ( this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && - subscription.type === SubscriptionType.Basic + subscription?.type === SubscriptionType.Basic ) { referralPartners = await this.propertyService.getByKey( PROPERTY_REFERRAL_PARTNERS ); } - let systemMessage: SystemMessage; + let systemMessage: SystemMessage | undefined; const systemMessageProperty = await this.propertyService.getByKey( PROPERTY_SYSTEM_MESSAGE ); - if (systemMessageProperty?.targetGroups?.includes(subscription?.type)) { + if ( + subscription?.type && + systemMessageProperty?.targetGroups?.includes(subscription.type) + ) { systemMessage = systemMessageProperty; } @@ -197,7 +200,7 @@ export class UserService { if ( this.configurationService.get('ENABLE_FEATURE_SUBSCRIPTION') && - subscription.type === SubscriptionType.Basic + subscription?.type === SubscriptionType.Basic ) { tags = tags.filter(({ id }) => { return [TAG_ID_DRAFT, TAG_ID_EXCLUDE_FROM_ANALYSIS].includes(id);