From 743d18932c0e7353ce5efe4eeabc818ac6d49a3e Mon Sep 17 00:00:00 2001
From: Jason Rasmussen <jason@rasm.me>
Date: Wed, 28 Aug 2024 09:58:43 -0400
Subject: [PATCH] chore: add oauth-callback path

---
 docs/docs/administration/oauth.md             | 12 +++---
 .../android/app/src/main/AndroidManifest.xml  |  4 +-
 mobile/lib/services/oauth.service.dart        |  4 +-
 server/src/constants.ts                       |  2 +-
 server/src/services/auth.service.spec.ts      | 40 +++++++++----------
 server/src/services/auth.service.ts           |  2 +-
 .../settings/auth/auth-settings.svelte        |  4 +-
 web/src/lib/i18n/en.json                      |  2 +-
 8 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/docs/docs/administration/oauth.md b/docs/docs/administration/oauth.md
index 432477064fd1dc..96dca68e4fa9db 100644
--- a/docs/docs/administration/oauth.md
+++ b/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
diff --git a/mobile/android/app/src/main/AndroidManifest.xml b/mobile/android/app/src/main/AndroidManifest.xml
index edb41510f01568..e5e3e2a396b5ec 100644
--- a/mobile/android/app/src/main/AndroidManifest.xml
+++ b/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.
@@ -94,4 +94,4 @@
       <data android:scheme="geo" />
     </intent>
   </queries>
-</manifest>
\ No newline at end of file
+</manifest>
diff --git a/mobile/lib/services/oauth.service.dart b/mobile/lib/services/oauth.service.dart
index d46705f05d41ae..c78fc479b117f9 100644
--- a/mobile/lib/services/oauth.service.dart
+++ b/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:/// i.e., {scheme}://{resource}
+// Redirect URL = app.immich:///oauth-callback
 
 class OAuthService {
   final ApiService _apiService;
@@ -18,7 +18,7 @@ class OAuthService {
     await _apiService.resolveAndSetEndpoint(serverUrl);
 
     final dto = await _apiService.oAuthApi.startOAuth(
-      OAuthConfigDto(redirectUri: '$callbackUrlScheme:///'),
+      OAuthConfigDto(redirectUri: '$callbackUrlScheme://oauth-callback'),
     );
     return dto?.url;
   }
diff --git a/server/src/constants.ts b/server/src/constants.ts
index c2b10b0969a0b9..29ed5f6d37c850 100644
--- a/server/src/constants.ts
+++ b/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 {
diff --git a/server/src/services/auth.service.spec.ts b/server/src/services/auth.service.spec.ts
index 26295c0b23d33b..d73896edb1be02 100644
--- a/server/src/services/auth.service.spec.ts
+++ b/server/src/services/auth.service.spec.ts
@@ -424,12 +424,12 @@ 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',
+        '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?');
     });
   });
 
@@ -490,25 +490,23 @@ describe('AuthService', () => {
       expect(userMock.create).toHaveBeenCalledTimes(1);
     });
 
-    it('should use the mobile redirect override', 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);
-
-      expect(callbackMock).toHaveBeenCalledWith('http://mobile-redirect', { state: 'state' }, { state: 'state' });
-    });
+    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 }, loginDetails);
+        expect(callbackMock).toHaveBeenCalledWith('http://mobile-redirect', { state: 'state' }, { state: 'state' });
+      });
+    }
 
     it('should use the default quota', async () => {
       systemMock.get.mockResolvedValue(systemConfigStub.oauthWithStorageQuota);
diff --git a/server/src/services/auth.service.ts b/server/src/services/auth.service.ts
index 18b4268292dba2..10cf93b6a4e6d5 100644
--- a/server/src/services/auth.service.ts
+++ b/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;
diff --git a/web/src/lib/components/admin-page/settings/auth/auth-settings.svelte b/web/src/lib/components/admin-page/settings/auth/auth-settings.svelte
index 34514dc1252edf..37f875c604f167 100644
--- a/web/src/lib/components/admin-page/settings/auth/auth-settings.svelte
+++ b/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}
diff --git a/web/src/lib/i18n/en.json b/web/src/lib/i18n/en.json
index e274ae01a5a86d..c790b91f6f9949 100644
--- a/web/src/lib/i18n/en.json
+++ b/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 OAuth provider does not allow callback 'app.immich:///'",
+    "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",