fix(mobile): use a valid OAuth callback URL (#10832)

* add root resource path '/' to mobile oauth scheme

* chore: add oauth-callback path

* add root resource path '/' to mobile oauth scheme

* chore: add oauth-callback path

* fix: make sure there are three forward slash in callback URL

---------

Co-authored-by: Jason Rasmussen <jason@rasm.me>
Co-authored-by: Alex <alex.tran1502@gmail.com>

docs/docs/administration/oauth.md

@@ -3,7 +3,7 @@
 This page contains details about using OAuth in Immich.
 
 :::tip
-Unable to set `app.immich:/` as a valid redirect URI? See [Mobile Redirect URI](#mobile-redirect-uri) for an alternative solution.
+Unable to set `app.immich:///oauth-callback` as a valid redirect URI? See [Mobile Redirect URI](#mobile-redirect-uri) for an alternative solution.
 :::
 
 ## Overview
@@ -30,7 +30,7 @@ Before enabling OAuth in Immich, a new client application needs to be configured
 
    The **Sign-in redirect URIs** should include:
 
-   - `app.immich:/` - for logging in with OAuth from the [Mobile App](/docs/features/mobile-app.mdx)
+   - `app.immich:///oauth-callback` - for logging in with OAuth from the [Mobile App](/docs/features/mobile-app.mdx)
    - `http://DOMAIN:PORT/auth/login` - for logging in with OAuth from the Web Client
    - `http://DOMAIN:PORT/user-settings` - for manually linking OAuth in the Web Client
 
@@ -38,7 +38,7 @@ Before enabling OAuth in Immich, a new client application needs to be configured
 
    Mobile
 
-   - `app.immich:/` (You **MUST** include this for iOS and Android mobile apps to work properly)
+   - `app.immich:///oauth-callback` (You **MUST** include this for iOS and Android mobile apps to work properly)
 
    Localhost
 
@@ -96,16 +96,16 @@ When Auto Launch is enabled, the login page will automatically redirect the user
 
 ## Mobile Redirect URI
 
-The redirect URI for the mobile app is `app.immich:/`, which is a [Custom Scheme](https://developer.apple.com/documentation/xcode/defining-a-custom-url-scheme-for-your-app). If this custom scheme is an invalid redirect URI for your OAuth Provider, you can work around this by doing the following:
+The redirect URI for the mobile app is `app.immich:///oauth-callback`, which is a [Custom Scheme](https://developer.apple.com/documentation/xcode/defining-a-custom-url-scheme-for-your-app). If this custom scheme is an invalid redirect URI for your OAuth Provider, you can work around this by doing the following:
 
-1. Configure an http(s) endpoint to forwards requests to `app.immich:/`
+1. Configure an http(s) endpoint to forwards requests to `app.immich:///oauth-callback`
 2. Whitelist the new endpoint as a valid redirect URI with your provider.
 3. Specify the new endpoint as the `Mobile Redirect URI Override`, in the OAuth settings.
 
 With these steps in place, you should be able to use OAuth from the [Mobile App](/docs/features/mobile-app.mdx) without a custom scheme redirect URI.
 
 :::info
-Immich has a route (`/api/oauth/mobile-redirect`) that is already configured to forward requests to `app.immich:/`, and can be used for step 1.
+Immich has a route (`/api/oauth/mobile-redirect`) that is already configured to forward requests to `app.immich:///oauth-callback`, and can be used for step 1.
 :::
 
 ## Example Configuration

mobile/android/app/src/main/AndroidManifest.xml

@@ -69,7 +69,7 @@
         <action android:name="android.intent.action.VIEW" />
         <category android:name="android.intent.category.DEFAULT" />
         <category android:name="android.intent.category.BROWSABLE" />
-        <data android:scheme="app.immich" />
+        <data android:scheme="app.immich" android:pathPrefix="/oauth-callback"/>
       </intent-filter>
     </activity>
     <!-- Don't delete the meta-data below.

mobile/lib/pages/login/login.page.dart

@@ -29,7 +29,7 @@ class LoginPage extends HookConsumerWidget {
     );
 
     return Scaffold(
-      body: const LoginForm(),
+      body: LoginForm(),
       bottomNavigationBar: SafeArea(
         child: Padding(
           padding: const EdgeInsets.only(bottom: 16.0),

mobile/lib/services/oauth.service.dart

@@ -3,7 +3,7 @@ import 'package:logging/logging.dart';
 import 'package:openapi/api.dart';
 import 'package:flutter_web_auth/flutter_web_auth.dart';
 
-// Redirect URL = app.immich://
+// Redirect URL = app.immich:///oauth-callback
 
 class OAuthService {
   final ApiService _apiService;
@@ -16,28 +16,40 @@ class OAuthService {
   ) async {
     // Resolve API server endpoint from user provided serverUrl
     await _apiService.resolveAndSetEndpoint(serverUrl);
+    final redirectUri = '$callbackUrlScheme:///oauth-callback';
+    log.info(
+      "Starting OAuth flow with redirect URI: $redirectUri",
+    );
 
     final dto = await _apiService.oAuthApi.startOAuth(
-      OAuthConfigDto(redirectUri: '$callbackUrlScheme:/'),
+      OAuthConfigDto(redirectUri: redirectUri),
     );
-    return dto?.url;
+
+    final authUrl = dto?.url;
+    log.info('Received Authorization URL: $authUrl');
+
+    return authUrl;
   }
 
   Future<LoginResponseDto?> oAuthLogin(String oauthUrl) async {
-    try {
-      var result = await FlutterWebAuth.authenticate(
+    String result = await FlutterWebAuth.authenticate(
       url: oauthUrl,
       callbackUrlScheme: callbackUrlScheme,
     );
 
+    log.info('Received OAuth callback: $result');
+
+    if (result.startsWith('app.immich:/oauth-callback')) {
+      result = result.replaceAll(
+        'app.immich:/oauth-callback',
+        'app.immich:///oauth-callback',
+      );
+    }
+
     return await _apiService.oAuthApi.finishOAuth(
       OAuthCallbackDto(
         url: result,
       ),
     );
-    } catch (e, stack) {
-      log.severe("OAuth login failed", e, stack);
-      return null;
-    }
   }
 }

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

@@ -27,12 +27,15 @@ import 'package:immich_mobile/widgets/forms/login/login_button.dart';
 import 'package:immich_mobile/widgets/forms/login/o_auth_login_button.dart';
 import 'package:immich_mobile/widgets/forms/login/password_input.dart';
 import 'package:immich_mobile/widgets/forms/login/server_endpoint_input.dart';
+import 'package:logging/logging.dart';
 import 'package:openapi/api.dart';
 import 'package:package_info_plus/package_info_plus.dart';
 import 'package:permission_handler/permission_handler.dart';
 
 class LoginForm extends HookConsumerWidget {
-  const LoginForm({super.key});
+  LoginForm({super.key});
+
+  final log = Logger('LoginForm');
 
   @override
   Widget build(BuildContext context, WidgetRef ref) {
@@ -229,7 +232,9 @@ class LoginForm extends HookConsumerWidget {
             .getOAuthServerUrl(sanitizeUrl(serverEndpointController.text));
 
         isLoading.value = true;
-      } catch (e) {
+      } catch (error, stack) {
+        log.severe('Error getting OAuth server Url: $error', stack);
+
         ImmichToast.show(
           context: context,
           msg: "login_form_failed_get_oauth_server_config".tr(),
@@ -241,10 +246,19 @@ class LoginForm extends HookConsumerWidget {
       }
 
       if (oAuthServerUrl != null) {
-        var loginResponseDto = await oAuthService.oAuthLogin(oAuthServerUrl);
+        try {
+          final loginResponseDto =
+              await oAuthService.oAuthLogin(oAuthServerUrl);
 
-        if (loginResponseDto != null) {
-          var isSuccess = await ref
+          if (loginResponseDto == null) {
+            return;
+          }
+
+          log.info(
+            "Finished OAuth login with response: ${loginResponseDto.userEmail}",
+          );
+
+          final isSuccess = await ref
               .watch(authenticationProvider.notifier)
               .setSuccessLoginInfo(
                 accessToken: loginResponseDto.accessToken,
@@ -258,17 +272,19 @@ class LoginForm extends HookConsumerWidget {
               ref.watch(backupProvider.notifier).resumeBackup();
             }
             context.replaceRoute(const TabControllerRoute());
-          } else {
+          }
+        } catch (error, stack) {
+          log.severe('Error logging in with OAuth: $error', stack);
+
           ImmichToast.show(
             context: context,
-              msg: "login_form_failed_login".tr(),
+            msg: error.toString(),
             toastType: ToastType.error,
             gravity: ToastGravity.TOP,
           );
-          }
-        }
-
+        } finally {
           isLoading.value = false;
+        }
       } else {
         ImmichToast.show(
           context: context,

server/src/constants.ts

@@ -51,7 +51,7 @@ export const resourcePaths = {
   },
 };
 
-export const MOBILE_REDIRECT = 'app.immich:/';
+export const MOBILE_REDIRECT = 'app.immich:///oauth-callback';
 export const LOGIN_URL = '/auth/login?autoLaunch=0';
 
 export enum AuthType {

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

@@ -423,11 +423,13 @@ describe('AuthService', () => {
 
   describe('getMobileRedirect', () => {
     it('should pass along the query params', () => {
-      expect(sut.getMobileRedirect('http://immich.app?code=123&state=456')).toEqual('app.immich:/?code=123&state=456');
+      expect(sut.getMobileRedirect('http://immich.app?code=123&state=456')).toEqual(
+        'app.immich:///oauth-callback?code=123&state=456',
+      );
     });
 
     it('should work if called without query params', () => {
-      expect(sut.getMobileRedirect('http://immich.app')).toEqual('app.immich:/?');
+      expect(sut.getMobileRedirect('http://immich.app')).toEqual('app.immich:///oauth-callback?');
     });
   });
 
@@ -488,25 +490,23 @@ describe('AuthService', () => {
       expect(userMock.create).toHaveBeenCalledTimes(1);
     });
 
-    it('should use the mobile redirect override', async () => {
+    for (const url of [
+      'app.immich:/',
+      'app.immich://',
+      'app.immich:///',
+      'app.immich:/oauth-callback?code=abc123',
+      'app.immich://oauth-callback?code=abc123',
+      'app.immich:///oauth-callback?code=abc123',
+    ]) {
+      it(`should use the mobile redirect override for a url of ${url}`, async () => {
         systemMock.get.mockResolvedValue(systemConfigStub.oauthWithMobileOverride);
         userMock.getByOAuthId.mockResolvedValue(userStub.user1);
         sessionMock.create.mockResolvedValue(sessionStub.valid);
 
-      await sut.callback({ url: `app.immich:/?code=abc123` }, loginDetails);
-
-      expect(callbackMock).toHaveBeenCalledWith('http://mobile-redirect', { state: 'state' }, { state: 'state' });
-    });
-
-    it('should use the mobile redirect override for ios urls with multiple slashes', async () => {
-      systemMock.get.mockResolvedValue(systemConfigStub.oauthWithMobileOverride);
-      userMock.getByOAuthId.mockResolvedValue(userStub.user1);
-      sessionMock.create.mockResolvedValue(sessionStub.valid);
-
-      await sut.callback({ url: `app.immich:///?code=abc123` }, loginDetails);
-
+        await sut.callback({ url }, loginDetails);
         expect(callbackMock).toHaveBeenCalledWith('http://mobile-redirect', { state: 'state' }, { state: 'state' });
       });
+    }
 
     it('should use the default quota', async () => {
       systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);

server/src/services/auth.service.ts

@@ -356,7 +356,7 @@ export class AuthService {
   }
 
   private normalize(config: SystemConfig, redirectUri: string) {
-    const isMobile = redirectUri.startsWith(MOBILE_REDIRECT);
+    const isMobile = redirectUri.startsWith('app.immich:/');
     const { mobileRedirectUri, mobileOverrideEnabled } = config.oauth;
     if (isMobile && mobileOverrideEnabled && mobileRedirectUri) {
       return mobileRedirectUri;

web/src/lib/components/admin-page/settings/auth/auth-settings.svelte

@@ -209,7 +209,9 @@
 
               <SettingSwitch
                 title={$t('admin.oauth_mobile_redirect_uri_override').toUpperCase()}
-                subtitle={$t('admin.oauth_mobile_redirect_uri_override_description')}
+                subtitle={$t('admin.oauth_mobile_redirect_uri_override_description', {
+                  values: { callback: 'app.immich:///oauth-callback' },
+                })}
                 disabled={disabled || !config.oauth.enabled}
                 on:click={() => handleToggleOverride()}
                 bind:checked={config.oauth.mobileOverrideEnabled}

web/src/lib/i18n/en.json

@@ -172,7 +172,7 @@
     "oauth_issuer_url": "Issuer URL",
     "oauth_mobile_redirect_uri": "Mobile redirect URI",
     "oauth_mobile_redirect_uri_override": "Mobile redirect URI override",
-    "oauth_mobile_redirect_uri_override_description": "Enable when 'app.immich:/' is an invalid redirect URI.",
+    "oauth_mobile_redirect_uri_override_description": "Enable when OAuth provider does not allow a mobile URI, like '{callback}'",
     "oauth_profile_signing_algorithm": "Profile signing algorithm",
     "oauth_profile_signing_algorithm_description": "Algorithm used to sign the user profile.",
     "oauth_scope": "Scope",