Merge 5ba3446fdf into ffc83eae36

mobile/lib/services/oauth.service.dart

@@ -11,7 +11,7 @@ class OAuthService {
   final log = Logger('OAuthService');
   OAuthService(this._apiService);
 
-  Future<String?> getOAuthServerUrl(String serverUrl, String state, String codeChallenge) async {
+  Future<String?> getOAuthServerUrl(String serverUrl, String state, String codeChallenge, String nonce) async {
     // Resolve API server endpoint from user provided serverUrl
     await _apiService.resolveAndSetEndpoint(serverUrl);
     final redirectUri = '$callbackUrlScheme:///oauth-callback';
@@ -22,6 +22,7 @@ class OAuthService {
         redirectUri: redirectUri,
         state: Optional.present(state),
         codeChallenge: Optional.present(codeChallenge),
+        nonce: Optional.present(nonce),
       ),
     );
 
@@ -31,7 +32,7 @@ class OAuthService {
     return authUrl;
   }
 
-  Future<LoginResponseDto?> oAuthLogin(String oauthUrl, String state, String codeVerifier) async {
+  Future<LoginResponseDto?> oAuthLogin(String oauthUrl, String state, String codeVerifier, String nonce) async {
     String result = await FlutterWebAuth2.authenticate(url: oauthUrl, callbackUrlScheme: callbackUrlScheme);
 
     log.info('Received OAuth callback: $result');
@@ -41,7 +42,12 @@ class OAuthService {
     }
 
     return await _apiService.oAuthApi.finishOAuth(
-      OAuthCallbackDto(url: result, state: Optional.present(state), codeVerifier: Optional.present(codeVerifier)),
+      OAuthCallbackDto(
+        url: result,
+        state: Optional.present(state),
+        codeVerifier: Optional.present(codeVerifier),
+        nonce: Optional.present(nonce),
+      ),
     );
   }
 }

mobile/lib/widgets/forms/login/login_form.dart

@@ -326,6 +326,7 @@ class LoginForm extends HookConsumerWidget {
       String? oAuthServerUrl;
 
       final state = generateRandomString(32);
+      final nonce = generateRandomString(32);
 
       final codeVerifier = randomCodeVerifier();
       final codeChallenge = await generatePKCECodeChallenge(codeVerifier);
@@ -335,6 +336,7 @@ class LoginForm extends HookConsumerWidget {
           normalizeServerUrl(serverEndpointController.text),
           state,
           codeChallenge,
+          nonce,
         );
 
         // Invalidate all api repository provider instance to take into account new access token
@@ -357,7 +359,7 @@ class LoginForm extends HookConsumerWidget {
 
       if (oAuthServerUrl != null) {
         try {
-          final loginResponseDto = await oAuthService.oAuthLogin(oAuthServerUrl, state, codeVerifier);
+          final loginResponseDto = await oAuthService.oAuthLogin(oAuthServerUrl, state, codeVerifier, nonce);
 
           if (loginResponseDto == null || !context.mounted) {
             return;

open-api/immich-openapi-specs.json

@@ -20657,6 +20657,10 @@
             "description": "OAuth code verifier (PKCE)",
             "type": "string"
           },
+          "nonce": {
+            "description": "OAuth nonce parameter",
+            "type": "string"
+          },
           "state": {
             "description": "OAuth state parameter",
             "type": "string"
@@ -20678,6 +20682,10 @@
             "description": "OAuth code challenge (PKCE)",
             "type": "string"
           },
+          "nonce": {
+            "description": "OAuth nonce parameter",
+            "type": "string"
+          },
           "redirectUri": {
             "description": "OAuth redirect URI",
             "type": "string"

packages/sdk/src/fetch-client.ts

@@ -1405,6 +1405,8 @@ export type NotificationUpdateDto = {
 export type OAuthConfigDto = {
     /** OAuth code challenge (PKCE) */
     codeChallenge?: string;
+    /** OAuth nonce parameter */
+    nonce?: string;
     /** OAuth redirect URI */
     redirectUri: string;
     /** OAuth state parameter */
@@ -1421,6 +1423,8 @@ export type OAuthBackchannelLogoutDto = {
 export type OAuthCallbackDto = {
     /** OAuth code verifier (PKCE) */
     codeVerifier?: string;
+    /** OAuth nonce parameter */
+    nonce?: string;
     /** OAuth state parameter */
     state?: string;
     /** OAuth callback URL */

server/src/controllers/oauth.controller.ts

@@ -49,7 +49,7 @@ export class OAuthController {
     @Res({ passthrough: true }) res: Response,
     @GetLoginDetails() loginDetails: LoginDetails,
   ): Promise<OAuthAuthorizeResponseDto> {
-    const { url, state, codeVerifier } = await this.service.authorize(dto);
+    const { url, state, nonce, codeVerifier } = await this.service.authorize(dto);
     return respondWithCookie(
       res,
       { url },
@@ -57,6 +57,7 @@ export class OAuthController {
         isSecure: loginDetails.isSecure,
         values: [
           { key: ImmichCookie.OAuthState, value: state },
+          { key: ImmichCookie.OAuthNonce, value: nonce },
           { key: ImmichCookie.OAuthCodeVerifier, value: codeVerifier },
         ],
       },
@@ -78,6 +79,7 @@ export class OAuthController {
   ): Promise<LoginResponseDto> {
     const body = await this.service.callback(dto, request.headers, loginDetails);
     res.clearCookie(ImmichCookie.OAuthState);
+    res.clearCookie(ImmichCookie.OAuthNonce);
     res.clearCookie(ImmichCookie.OAuthCodeVerifier);
     return respondWithCookie(res, body, {
       isSecure: loginDetails.isSecure,

server/src/dtos/auth.dto.ts

@@ -107,6 +107,7 @@ const OAuthCallbackSchema = z
     url: z.string().min(1).describe('OAuth callback URL'),
     state: z.string().optional().describe('OAuth state parameter'),
     codeVerifier: z.string().optional().describe('OAuth code verifier (PKCE)'),
+    nonce: z.string().optional().describe('OAuth nonce parameter'),
   })
   .meta({ id: 'OAuthCallbackDto' });
 
@@ -115,6 +116,7 @@ const OAuthConfigSchema = z
     redirectUri: z.string().describe('OAuth redirect URI'),
     state: z.string().optional().describe('OAuth state parameter'),
     codeChallenge: z.string().optional().describe('OAuth code challenge (PKCE)'),
+    nonce: z.string().optional().describe('OAuth nonce parameter'),
   })
   .meta({ id: 'OAuthConfigDto' });
 

server/src/enum.ts

@@ -14,6 +14,7 @@ export enum ImmichCookie {
   SharedLinkToken = 'immich_shared_link_token',
   OAuthState = 'immich_oauth_state',
   OAuthCodeVerifier = 'immich_oauth_code_verifier',
+  OAuthNonce = 'immich_oauth_nonce',
 }
 
 export enum ImmichHeader {

server/src/repositories/oauth.repository.ts

@@ -10,6 +10,7 @@ import {
   discovery,
   fetchUserInfo,
   None,
+  randomNonce,
   randomPKCECodeVerifier,
   randomState,
   skipSubjectCheck,
@@ -41,9 +42,10 @@ export class OAuthRepository {
     this.logger.setContext(OAuthRepository.name);
   }
 
-  async authorize(config: OAuthConfig, redirectUrl: string, state?: string, codeChallenge?: string) {
+  async authorize(config: OAuthConfig, redirectUrl: string, state?: string, codeChallenge?: string, nonce?: string) {
     const client = await this.getClient(config);
     state ??= randomState();
+    nonce ??= randomNonce();
 
     let codeVerifier: string | null;
     if (codeChallenge) {
@@ -57,6 +59,7 @@ export class OAuthRepository {
       redirect_uri: redirectUrl,
       scope: config.scope,
       state,
+      nonce,
     };
 
     if (config.prompt) {
@@ -70,7 +73,7 @@ export class OAuthRepository {
 
     const url = buildAuthorizationUrl(client, params).href;
 
-    return { url, state, codeVerifier };
+    return { url, state, nonce, codeVerifier };
   }
 
   async getLogoutEndpoint(config: OAuthConfig) {
@@ -83,12 +86,17 @@ export class OAuthRepository {
     url: string,
     expectedState: string,
     codeVerifier: string,
+    expectedNonce?: string,
   ): Promise<{ profile: OAuthProfile; sid?: string; idToken?: string }> {
     const client = await this.getClient(config);
     const pkceCodeVerifier = client.serverMetadata().supportsPKCE() ? codeVerifier : undefined;
 
     try {
-      const tokens = await authorizationCodeGrant(client, new URL(url), { expectedState, pkceCodeVerifier });
+      const tokens = await authorizationCodeGrant(client, new URL(url), {
+        expectedState,
+        pkceCodeVerifier,
+        expectedNonce,
+      });
 
       let profile: OAuthProfile;
       const tokenClaims = tokens.claims();

server/src/services/auth.service.spec.ts

@@ -38,7 +38,12 @@ describe(AuthService.name, () => {
   beforeEach(() => {
     ({ sut, mocks } = newTestService(AuthService));
 
-    mocks.oauth.authorize.mockResolvedValue({ url: 'http://test', state: 'state', codeVerifier: 'codeVerifier' });
+    mocks.oauth.authorize.mockResolvedValue({
+      url: 'http://test',
+      state: 'state',
+      nonce: 'nonce',
+      codeVerifier: 'codeVerifier',
+    });
     mocks.oauth.getLogoutEndpoint.mockResolvedValue('http://end-session-endpoint');
   });
 
@@ -853,6 +858,7 @@ describe(AuthService.name, () => {
           'http://mobile-redirect?code=abc123',
           'xyz789',
           'foo',
+          undefined,
         );
       });
     }

server/src/services/auth.service.ts

@@ -279,6 +279,7 @@ export class AuthService extends BaseService {
       this.resolveRedirectUri(oauth, dto.redirectUri),
       dto.state,
       dto.codeChallenge,
+      dto.nonce,
     );
   }
 
@@ -298,12 +299,14 @@ export class AuthService extends BaseService {
       throw new BadRequestException('OAuth code verifier is missing');
     }
 
+    const expectedNonce = dto.nonce ?? this.getCookieOauthNonce(headers) ?? undefined;
+
     const url = this.resolveRedirectUri(oauth, dto.url);
     const {
       profile,
       sid: oauthSid,
       idToken: oauthBearerToken,
-    } = await this.oauthRepository.getProfileAndOAuthSid(oauth, url, expectedState, codeVerifier);
+    } = await this.oauthRepository.getProfileAndOAuthSid(oauth, url, expectedState, codeVerifier, expectedNonce);
     const normalizedEmail = profile.email ? profile.email.trim().toLowerCase() : undefined;
     const { autoRegister, defaultStorageQuota, storageLabelClaim, storageQuotaClaim, roleClaim } = oauth;
     this.logger.debug(`Logging in with OAuth: ${JSON.stringify(profile)}`);
@@ -409,12 +412,14 @@ export class AuthService extends BaseService {
       throw new BadRequestException('OAuth code verifier is missing');
     }
 
+    const expectedNonce = dto.nonce ?? this.getCookieOauthNonce(headers) ?? undefined;
+
     const { oauth } = await this.getConfig({ withCache: false });
     const {
       profile: { sub: oauthId },
       sid,
       idToken,
-    } = await this.oauthRepository.getProfileAndOAuthSid(oauth, dto.url, expectedState, codeVerifier);
+    } = await this.oauthRepository.getProfileAndOAuthSid(oauth, dto.url, expectedState, codeVerifier, expectedNonce);
     const duplicate = await this.userRepository.getByOAuthId(oauthId);
     if (duplicate && duplicate.id !== auth.user.id) {
       this.logger.warn(`OAuth link account failed: sub is already linked to another user (${duplicate.email}).`);
@@ -491,6 +496,11 @@ export class AuthService extends BaseService {
     return cookies[ImmichCookie.OAuthCodeVerifier] || null;
   }
 
+  private getCookieOauthNonce(headers: IncomingHttpHeaders): string | null {
+    const cookies = parse(headers.cookie || '');
+    return cookies[ImmichCookie.OAuthNonce] || null;
+  }
+
   async validateSharedLinkKey(key: string | string[]): Promise<AuthDto> {
     key = Array.isArray(key) ? key[0] : key;
 

server/src/utils/response.ts

@@ -17,6 +17,7 @@ export const respondWithCookie = <T>(res: Response, body: T, { isSecure, values
     [ImmichCookie.AccessToken]: defaults,
     [ImmichCookie.MaintenanceToken]: { ...defaults, maxAge: Duration.fromObject({ days: 1 }).toMillis() },
     [ImmichCookie.OAuthState]: defaults,
+    [ImmichCookie.OAuthNonce]: defaults,
     [ImmichCookie.OAuthCodeVerifier]: defaults,
     // no httpOnly so that the client can know the auth state
     [ImmichCookie.IsAuthenticated]: { ...defaults, httpOnly: false },