From 4fdf59899e835958bc9da19558b07e7087712312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Mart=C3=ADn?= Date: Fri, 28 Nov 2025 23:28:51 +0100 Subject: [PATCH] Refactor OIDC authentication flow and update dependencies --- apps/api/src/app/auth/auth.controller.ts | 2 - apps/api/src/app/auth/auth.module.ts | 4 +- apps/api/src/app/auth/oidc-state.store.ts | 42 ++++++++++++------- apps/api/src/app/auth/oidc.strategy.ts | 10 ++--- .../configuration/configuration.service.ts | 14 +++---- .../login-with-access-token-dialog.html | 4 +- package-lock.json | 4 +- package.json | 4 +- 8 files changed, 46 insertions(+), 38 deletions(-) diff --git a/apps/api/src/app/auth/auth.controller.ts b/apps/api/src/app/auth/auth.controller.ts index cd51e2954..29af3a3c2 100644 --- a/apps/api/src/app/auth/auth.controller.ts +++ b/apps/api/src/app/auth/auth.controller.ts @@ -84,7 +84,6 @@ export class AuthController { @Req() request: Request, @Res() response: Response ) { - // Handles the Google OAuth2 callback const jwt: string = (request.user as any).jwt; if (jwt) { @@ -120,7 +119,6 @@ export class AuthController { @UseGuards(AuthGuard('oidc')) @Version(VERSION_NEUTRAL) public oidcLoginCallback(@Req() request: Request, @Res() response: Response) { - // Handles the OIDC callback const jwt: string = (request.user as any).jwt; if (jwt) { diff --git a/apps/api/src/app/auth/auth.module.ts b/apps/api/src/app/auth/auth.module.ts index 60a3a64d1..c31e66299 100644 --- a/apps/api/src/app/auth/auth.module.ts +++ b/apps/api/src/app/auth/auth.module.ts @@ -40,6 +40,7 @@ import { OidcStrategy } from './oidc.strategy'; GoogleStrategy, JwtStrategy, { + inject: [AuthService, ConfigurationService], provide: OidcStrategy, useFactory: async ( authService: AuthService, @@ -95,8 +96,7 @@ import { OidcStrategy } from './oidc.strategy'; } return new OidcStrategy(authService, options); - }, - inject: [AuthService, ConfigurationService] + } }, WebAuthService ] diff --git a/apps/api/src/app/auth/oidc-state.store.ts b/apps/api/src/app/auth/oidc-state.store.ts index d1b578e2b..437846cf1 100644 --- a/apps/api/src/app/auth/oidc-state.store.ts +++ b/apps/api/src/app/auth/oidc-state.store.ts @@ -1,3 +1,5 @@ +import ms from 'ms'; + /** * Custom state store for OIDC authentication that doesn't rely on express-session. * This store manages OAuth2 state parameters in memory with automatic cleanup. @@ -7,15 +9,17 @@ export class OidcStateStore { string, { appState?: unknown; - ctx: { maxAge?: number; nonce?: string; issued?: Date }; + ctx: { issued?: Date; maxAge?: number; nonce?: string }; meta?: unknown; timestamp: number; } >(); - private readonly STATE_EXPIRY_MS = 10 * 60 * 1000; // 10 minutes + private readonly STATE_EXPIRY_MS = ms('10 minutes'); - // Store request state. - // Signature matches passport-openidconnect SessionStore + /** + * Store request state. + * Signature matches passport-openidconnect SessionStore + */ public store( _req: unknown, _meta: unknown, @@ -43,8 +47,10 @@ export class OidcStateStore { } } - // Verify request state. - // Signature matches passport-openidconnect SessionStore + /** + * Verify request state. + * Signature matches passport-openidconnect SessionStore + */ public verify( _req: unknown, handle: string, @@ -76,16 +82,9 @@ export class OidcStateStore { } } - // Generate a cryptographically secure random handle - private generateHandle(): string { - return ( - Math.random().toString(36).substring(2, 15) + - Math.random().toString(36).substring(2, 15) + - Date.now().toString(36) - ); - } - - // Clean up expired states + /** + * Clean up expired states + */ private cleanup(): void { const now = Date.now(); const expiredKeys: string[] = []; @@ -100,4 +99,15 @@ export class OidcStateStore { this.stateMap.delete(key); } } + + /** + * Generate a cryptographically secure random handle + */ + private generateHandle(): string { + return ( + Math.random().toString(36).substring(2, 15) + + Math.random().toString(36).substring(2, 15) + + Date.now().toString(36) + ); + } } diff --git a/apps/api/src/app/auth/oidc.strategy.ts b/apps/api/src/app/auth/oidc.strategy.ts index 8366c58bc..58fd7bd87 100644 --- a/apps/api/src/app/auth/oidc.strategy.ts +++ b/apps/api/src/app/auth/oidc.strategy.ts @@ -59,6 +59,11 @@ export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') { params?.sub ?? context?.claims?.sub; + const jwt = await this.authService.validateOAuthLogin({ + provider: Provider.OIDC, + thirdPartyId + }); + if (!thirdPartyId) { Logger.error( `Missing subject identifier in OIDC response from ${issuer}`, @@ -67,11 +72,6 @@ export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') { throw new Error('Missing subject identifier in OIDC response'); } - const jwt = await this.authService.validateOAuthLogin({ - provider: Provider.OIDC, - thirdPartyId - }); - return { jwt }; } catch (error) { Logger.error(error, 'OidcStrategy'); diff --git a/apps/api/src/services/configuration/configuration.service.ts b/apps/api/src/services/configuration/configuration.service.ts index 59de60354..6f139b305 100644 --- a/apps/api/src/services/configuration/configuration.service.ts +++ b/apps/api/src/services/configuration/configuration.service.ts @@ -58,14 +58,14 @@ export class ConfigurationService { JWT_SECRET_KEY: str({}), MAX_ACTIVITIES_TO_IMPORT: num({ default: Number.MAX_SAFE_INTEGER }), MAX_CHART_ITEMS: num({ default: 365 }), - OIDC_AUTHORIZATION_URL: str({ default: '' }), - OIDC_CALLBACK_URL: str({ default: '' }), - OIDC_CLIENT_ID: str({ default: '' }), - OIDC_CLIENT_SECRET: str({ default: '' }), - OIDC_ISSUER: str({ default: '' }), + OIDC_AUTHORIZATION_URL: str({ default: undefined }), + OIDC_CALLBACK_URL: str({ default: undefined }), + OIDC_CLIENT_ID: str({ default: undefined }), + OIDC_CLIENT_SECRET: str({ default: undefined }), + OIDC_ISSUER: str({ default: undefined }), OIDC_SCOPE: json({ default: ['openid'] }), - OIDC_TOKEN_URL: str({ default: '' }), - OIDC_USER_INFO_URL: str({ default: '' }), + OIDC_TOKEN_URL: str({ default: undefined }), + OIDC_USER_INFO_URL: str({ default: undefined }), PORT: port({ default: DEFAULT_PORT }), PROCESSOR_GATHER_ASSET_PROFILE_CONCURRENCY: num({ default: DEFAULT_PROCESSOR_GATHER_ASSET_PROFILE_CONCURRENCY diff --git a/apps/client/src/app/components/login-with-access-token-dialog/login-with-access-token-dialog.html b/apps/client/src/app/components/login-with-access-token-dialog/login-with-access-token-dialog.html index 6570ab3d6..d345c4df5 100644 --- a/apps/client/src/app/components/login-with-access-token-dialog/login-with-access-token-dialog.html +++ b/apps/client/src/app/components/login-with-access-token-dialog/login-with-access-token-dialog.html @@ -41,7 +41,7 @@ class="mr-2" src="../assets/icons/google.svg" style="height: 1rem" - />Sign in with GoogleSign in with Google } @@ -52,7 +52,7 @@ class="px-4 rounded-pill" href="../api/auth/oidc" mat-stroked-button - >Sign in with OIDCSign in with OIDC } diff --git a/package-lock.json b/package-lock.json index ac64925a8..81e6c2677 100644 --- a/package-lock.json +++ b/package-lock.json @@ -83,7 +83,7 @@ "passport-google-oauth20": "2.0.0", "passport-headerapikey": "1.2.2", "passport-jwt": "4.0.1", - "passport-openidconnect": "^0.1.2", + "passport-openidconnect": "0.1.2", "reflect-metadata": "0.2.2", "rxjs": "7.8.1", "stripe": "18.5.0", @@ -131,7 +131,7 @@ "@types/node": "22.15.17", "@types/papaparse": "5.3.7", "@types/passport-google-oauth20": "2.0.16", - "@types/passport-openidconnect": "^0.1.3", + "@types/passport-openidconnect": "0.1.3", "@typescript-eslint/eslint-plugin": "8.43.0", "@typescript-eslint/parser": "8.43.0", "cypress": "6.2.1", diff --git a/package.json b/package.json index d10f81a73..4de533702 100644 --- a/package.json +++ b/package.json @@ -129,7 +129,7 @@ "passport-google-oauth20": "2.0.0", "passport-headerapikey": "1.2.2", "passport-jwt": "4.0.1", - "passport-openidconnect": "^0.1.2", + "passport-openidconnect": "0.1.2", "reflect-metadata": "0.2.2", "rxjs": "7.8.1", "stripe": "18.5.0", @@ -177,7 +177,7 @@ "@types/node": "22.15.17", "@types/papaparse": "5.3.7", "@types/passport-google-oauth20": "2.0.16", - "@types/passport-openidconnect": "^0.1.3", + "@types/passport-openidconnect": "0.1.3", "@typescript-eslint/eslint-plugin": "8.43.0", "@typescript-eslint/parser": "8.43.0", "cypress": "6.2.1",