From f1e4fdf17566ed61e6266eadaad565ea8aed7c00 Mon Sep 17 00:00:00 2001
From: martin <74269598+martabal@users.noreply.github.com>
Date: Tue, 13 Feb 2024 02:47:26 +0100
Subject: [PATCH] refactor: authentication on public routes (#6765)

* refactor: authentication on public routes

* fix: remove public user

* pr feedback

* pr feedback

* pr feedback

* pr feedback

* remove unused method

* fix: tests

* fix: useless methods

* fix: tests

* pr feedback

* pr feedback

* chore: cleanup

---------

Co-authored-by: Jason Rasmussen <jrasm91@gmail.com>
---
 server/e2e/api/specs/auth.e2e-spec.ts         |  3 +-
 server/src/domain/auth/auth.constant.ts       |  1 +
 server/src/domain/auth/auth.service.ts        |  6 +-
 .../src/immich/controllers/auth.controller.ts |  2 +
 server/test/fixtures/auth.stub.ts             |  3 +
 .../lib/components/forms/login-form.svelte    |  1 -
 .../navigation-bar/account-info-panel.svelte  | 27 ++++-----
 .../navigation-bar/navigation-bar.svelte      |  2 +-
 .../user-profile-settings.svelte              | 32 +++++-----
 .../user-settings-list.svelte                 |  2 +-
 web/src/lib/stores/user.store.ts              | 10 +---
 web/src/lib/stores/websocket.ts               |  7 +--
 web/src/lib/utils/auth.ts                     | 58 +++++++++++--------
 web/src/routes/(user)/share/[key]/+page.ts    |  4 +-
 web/src/routes/+page.ts                       |  4 +-
 web/src/routes/auth/change-password/+page.ts  |  5 +-
 16 files changed, 92 insertions(+), 75 deletions(-)

diff --git a/server/e2e/api/specs/auth.e2e-spec.ts b/server/e2e/api/specs/auth.e2e-spec.ts
index eeb4723faa..e514d2b803 100644
--- a/server/e2e/api/specs/auth.e2e-spec.ts
+++ b/server/e2e/api/specs/auth.e2e-spec.ts
@@ -153,9 +153,10 @@ describe(`${AuthController.name} (e2e)`, () => {
       expect(token).toBeDefined();
 
       const cookies = headers['set-cookie'];
-      expect(cookies).toHaveLength(2);
+      expect(cookies).toHaveLength(3);
       expect(cookies[0]).toEqual(`immich_access_token=${token}; HttpOnly; Path=/; Max-Age=34560000; SameSite=Lax;`);
       expect(cookies[1]).toEqual('immich_auth_type=password; HttpOnly; Path=/; Max-Age=34560000; SameSite=Lax;');
+      expect(cookies[2]).toEqual('immich_is_authenticated=true; Path=/; Max-Age=34560000; SameSite=Lax;');
     });
   });
 
diff --git a/server/src/domain/auth/auth.constant.ts b/server/src/domain/auth/auth.constant.ts
index d237a19cd5..f29fc92741 100644
--- a/server/src/domain/auth/auth.constant.ts
+++ b/server/src/domain/auth/auth.constant.ts
@@ -1,6 +1,7 @@
 export const MOBILE_REDIRECT = 'app.immich:/';
 export const LOGIN_URL = '/auth/login?autoLaunch=0';
 export const IMMICH_ACCESS_COOKIE = 'immich_access_token';
+export const IMMICH_IS_AUTHENTICATED = 'immich_is_authenticated';
 export const IMMICH_AUTH_TYPE_COOKIE = 'immich_auth_type';
 export const IMMICH_API_KEY_NAME = 'api_key';
 export const IMMICH_API_KEY_HEADER = 'x-api-key';
diff --git a/server/src/domain/auth/auth.service.ts b/server/src/domain/auth/auth.service.ts
index d4c2e4e189..a2c0d2df93 100644
--- a/server/src/domain/auth/auth.service.ts
+++ b/server/src/domain/auth/auth.service.ts
@@ -29,6 +29,7 @@ import {
   IMMICH_ACCESS_COOKIE,
   IMMICH_API_KEY_HEADER,
   IMMICH_AUTH_TYPE_COOKIE,
+  IMMICH_IS_AUTHENTICATED,
   LOGIN_URL,
   MOBILE_REDIRECT,
 } from './auth.constant';
@@ -429,14 +430,17 @@ export class AuthService {
 
     let authTypeCookie = '';
     let accessTokenCookie = '';
+    let isAuthenticatedCookie = '';
 
     if (isSecure) {
       accessTokenCookie = `${IMMICH_ACCESS_COOKIE}=${loginResponse.accessToken}; HttpOnly; Secure; Path=/; Max-Age=${maxAge}; SameSite=Lax;`;
       authTypeCookie = `${IMMICH_AUTH_TYPE_COOKIE}=${authType}; HttpOnly; Secure; Path=/; Max-Age=${maxAge}; SameSite=Lax;`;
+      isAuthenticatedCookie = `${IMMICH_IS_AUTHENTICATED}=true; Secure; Path=/; Max-Age=${maxAge}; SameSite=Lax;`;
     } else {
       accessTokenCookie = `${IMMICH_ACCESS_COOKIE}=${loginResponse.accessToken}; HttpOnly; Path=/; Max-Age=${maxAge}; SameSite=Lax;`;
       authTypeCookie = `${IMMICH_AUTH_TYPE_COOKIE}=${authType}; HttpOnly; Path=/; Max-Age=${maxAge}; SameSite=Lax;`;
+      isAuthenticatedCookie = `${IMMICH_IS_AUTHENTICATED}=true; Path=/; Max-Age=${maxAge}; SameSite=Lax;`;
     }
-    return [accessTokenCookie, authTypeCookie];
+    return [accessTokenCookie, authTypeCookie, isAuthenticatedCookie];
   }
 }
diff --git a/server/src/immich/controllers/auth.controller.ts b/server/src/immich/controllers/auth.controller.ts
index 15018c10de..c8ffd52fd1 100644
--- a/server/src/immich/controllers/auth.controller.ts
+++ b/server/src/immich/controllers/auth.controller.ts
@@ -5,6 +5,7 @@ import {
   ChangePasswordDto,
   IMMICH_ACCESS_COOKIE,
   IMMICH_AUTH_TYPE_COOKIE,
+  IMMICH_IS_AUTHENTICATED,
   LoginCredentialDto,
   LoginDetails,
   LoginResponseDto,
@@ -84,6 +85,7 @@ export class AuthController {
   ): Promise<LogoutResponseDto> {
     res.clearCookie(IMMICH_ACCESS_COOKIE);
     res.clearCookie(IMMICH_AUTH_TYPE_COOKIE);
+    res.clearCookie(IMMICH_IS_AUTHENTICATED);
 
     return this.service.logout(auth, (request.cookies || {})[IMMICH_AUTH_TYPE_COOKIE]);
   }
diff --git a/server/test/fixtures/auth.stub.ts b/server/test/fixtures/auth.stub.ts
index 3dbbdcbf12..4b0c06baf3 100644
--- a/server/test/fixtures/auth.stub.ts
+++ b/server/test/fixtures/auth.stub.ts
@@ -145,6 +145,7 @@ export const loginResponseStub = {
     cookie: [
       'immich_access_token=cmFuZG9tLWJ5dGVz; HttpOnly; Secure; Path=/; Max-Age=34560000; SameSite=Lax;',
       'immich_auth_type=oauth; HttpOnly; Secure; Path=/; Max-Age=34560000; SameSite=Lax;',
+      'immich_is_authenticated=true; Secure; Path=/; Max-Age=34560000; SameSite=Lax;',
     ],
   },
   user1password: {
@@ -160,6 +161,7 @@ export const loginResponseStub = {
     cookie: [
       'immich_access_token=cmFuZG9tLWJ5dGVz; HttpOnly; Secure; Path=/; Max-Age=34560000; SameSite=Lax;',
       'immich_auth_type=password; HttpOnly; Secure; Path=/; Max-Age=34560000; SameSite=Lax;',
+      'immich_is_authenticated=true; Secure; Path=/; Max-Age=34560000; SameSite=Lax;',
     ],
   },
   user1insecure: {
@@ -175,6 +177,7 @@ export const loginResponseStub = {
     cookie: [
       'immich_access_token=cmFuZG9tLWJ5dGVz; HttpOnly; Path=/; Max-Age=34560000; SameSite=Lax;',
       'immich_auth_type=password; HttpOnly; Path=/; Max-Age=34560000; SameSite=Lax;',
+      'immich_is_authenticated=true; Path=/; Max-Age=34560000; SameSite=Lax;',
     ],
   },
 };
diff --git a/web/src/lib/components/forms/login-form.svelte b/web/src/lib/components/forms/login-form.svelte
index d749decf42..1eb01cd199 100644
--- a/web/src/lib/components/forms/login-form.svelte
+++ b/web/src/lib/components/forms/login-form.svelte
@@ -76,7 +76,6 @@
         dispatch('firstLogin');
         return;
       }
-
       dispatch('success');
       return;
     } catch (error) {
diff --git a/web/src/lib/components/shared-components/navigation-bar/account-info-panel.svelte b/web/src/lib/components/shared-components/navigation-bar/account-info-panel.svelte
index 0c790c2851..3d4a89ab5a 100644
--- a/web/src/lib/components/shared-components/navigation-bar/account-info-panel.svelte
+++ b/web/src/lib/components/shared-components/navigation-bar/account-info-panel.svelte
@@ -1,7 +1,7 @@
 <script lang="ts">
   import Button from '$lib/components/elements/buttons/button.svelte';
   import { AppRoute } from '$lib/constants';
-  import { api, UserAvatarColor, type UserResponseDto } from '@api';
+  import { api, UserAvatarColor } from '@api';
   import { createEventDispatcher } from 'svelte';
   import Icon from '$lib/components/elements/icon.svelte';
   import { fade } from 'svelte/transition';
@@ -10,9 +10,7 @@
   import { notificationController, NotificationType } from '../notification/notification';
   import { handleError } from '$lib/utils/handle-error';
   import AvatarSelector from './avatar-selector.svelte';
-  import { setUser } from '$lib/stores/user.store';
-
-  export let user: UserResponseDto;
+  import { user } from '$lib/stores/user.store';
 
   let isShowSelectAvatar = false;
 
@@ -23,21 +21,20 @@
 
   const handleSaveProfile = async (color: UserAvatarColor) => {
     try {
-      if (user.profileImagePath !== '') {
+      if ($user.profileImagePath !== '') {
         await api.userApi.deleteProfileImage();
       }
 
       const { data } = await api.userApi.updateUser({
         updateUserDto: {
-          id: user.id,
-          email: user.email,
-          name: user.name,
+          id: $user.id,
+          email: $user.email,
+          name: $user.name,
           avatarColor: color,
         },
       });
 
-      user = data;
-      setUser(user);
+      $user = data;
       isShowSelectAvatar = false;
 
       notificationController.show({
@@ -60,8 +57,8 @@
     class="mx-4 mt-4 flex flex-col items-center justify-center gap-4 rounded-3xl bg-white p-4 dark:bg-immich-dark-primary/10"
   >
     <div class="relative">
-      {#key user}
-        <UserAvatar {user} size="xl" />
+      {#key $user}
+        <UserAvatar user={$user} size="xl" />
 
         <div
           class="absolute z-10 bottom-0 right-0 rounded-full w-6 h-6 border dark:border-immich-dark-primary bg-immich-primary"
@@ -77,9 +74,9 @@
     </div>
     <div>
       <p class="text-center text-lg font-medium text-immich-primary dark:text-immich-dark-primary">
-        {user.name}
+        {$user.name}
       </p>
-      <p class="text-sm text-gray-500 dark:text-immich-dark-fg">{user.email}</p>
+      <p class="text-sm text-gray-500 dark:text-immich-dark-fg">{$user.email}</p>
     </div>
 
     <a href={AppRoute.USER_SETTINGS} on:click={() => dispatch('close')}>
@@ -104,7 +101,7 @@
 </div>
 {#if isShowSelectAvatar}
   <AvatarSelector
-    {user}
+    user={$user}
     on:close={() => (isShowSelectAvatar = false)}
     on:choose={({ detail: color }) => handleSaveProfile(color)}
   />
diff --git a/web/src/lib/components/shared-components/navigation-bar/navigation-bar.svelte b/web/src/lib/components/shared-components/navigation-bar/navigation-bar.svelte
index 8a83c0cb32..ba573f472f 100644
--- a/web/src/lib/components/shared-components/navigation-bar/navigation-bar.svelte
+++ b/web/src/lib/components/shared-components/navigation-bar/navigation-bar.svelte
@@ -146,7 +146,7 @@
           {/if}
 
           {#if shouldShowAccountInfoPanel}
-            <AccountInfoPanel user={$user} on:logout={logOut} />
+            <AccountInfoPanel on:logout={logOut} />
           {/if}
         </div>
       </section>
diff --git a/web/src/lib/components/user-settings-page/user-profile-settings.svelte b/web/src/lib/components/user-settings-page/user-profile-settings.svelte
index 97580ba8d3..c11ba520c8 100644
--- a/web/src/lib/components/user-settings-page/user-profile-settings.svelte
+++ b/web/src/lib/components/user-settings-page/user-profile-settings.svelte
@@ -3,27 +3,28 @@
     notificationController,
     NotificationType,
   } from '$lib/components/shared-components/notification/notification';
-  import { api, type UserResponseDto } from '@api';
+  import { api } from '@api';
   import { fade } from 'svelte/transition';
   import { handleError } from '../../utils/handle-error';
   import SettingInputField, { SettingInputFieldType } from '../admin-page/settings/setting-input-field.svelte';
   import Button from '../elements/buttons/button.svelte';
-  import { setUser } from '$lib/stores/user.store';
+  import { user } from '$lib/stores/user.store';
+  import { cloneDeep } from 'lodash-es';
 
-  export let user: UserResponseDto;
+  let editedUser = cloneDeep($user);
 
   const handleSaveProfile = async () => {
     try {
       const { data } = await api.userApi.updateUser({
         updateUserDto: {
-          id: user.id,
-          email: user.email,
-          name: user.name,
+          id: editedUser.id,
+          email: editedUser.email,
+          name: editedUser.name,
         },
       });
 
-      Object.assign(user, data);
-      setUser(data);
+      Object.assign(editedUser, data);
+      $user = data;
 
       notificationController.show({
         message: 'Saved profile',
@@ -42,19 +43,24 @@
         <SettingInputField
           inputType={SettingInputFieldType.TEXT}
           label="USER ID"
-          bind:value={user.id}
+          bind:value={editedUser.id}
           disabled={true}
         />
 
-        <SettingInputField inputType={SettingInputFieldType.EMAIL} label="EMAIL" bind:value={user.email} />
+        <SettingInputField inputType={SettingInputFieldType.EMAIL} label="EMAIL" bind:value={editedUser.email} />
 
-        <SettingInputField inputType={SettingInputFieldType.TEXT} label="NAME" bind:value={user.name} required={true} />
+        <SettingInputField
+          inputType={SettingInputFieldType.TEXT}
+          label="NAME"
+          bind:value={editedUser.name}
+          required={true}
+        />
 
         <SettingInputField
           inputType={SettingInputFieldType.TEXT}
           label="STORAGE LABEL"
           disabled={true}
-          value={user.storageLabel || ''}
+          value={editedUser.storageLabel || ''}
           required={false}
         />
 
@@ -62,7 +68,7 @@
           inputType={SettingInputFieldType.TEXT}
           label="EXTERNAL PATH"
           disabled={true}
-          value={user.externalPath || ''}
+          value={editedUser.externalPath || ''}
           required={false}
         />
 
diff --git a/web/src/lib/components/user-settings-page/user-settings-list.svelte b/web/src/lib/components/user-settings-page/user-settings-list.svelte
index 60a5871775..9405205e64 100644
--- a/web/src/lib/components/user-settings-page/user-settings-list.svelte
+++ b/web/src/lib/components/user-settings-page/user-settings-list.svelte
@@ -32,7 +32,7 @@
 </SettingAccordion>
 
 <SettingAccordion key="account" title="Account" subtitle="Manage your account">
-  <UserProfileSettings user={$user} />
+  <UserProfileSettings />
 </SettingAccordion>
 
 <SettingAccordion key="api-keys" title="API Keys" subtitle="Manage your API keys">
diff --git a/web/src/lib/stores/user.store.ts b/web/src/lib/stores/user.store.ts
index 8659506cc5..7f737e9fcc 100644
--- a/web/src/lib/stores/user.store.ts
+++ b/web/src/lib/stores/user.store.ts
@@ -1,16 +1,8 @@
-import { get, writable } from 'svelte/store';
+import { writable } from 'svelte/store';
 import type { UserResponseDto } from '@api';
 
 export let user = writable<UserResponseDto>();
 
-export const setUser = (value: UserResponseDto) => {
-  user.set(value);
-};
-
-export const getSavedUser = () => {
-  return get(user);
-};
-
 export const resetSavedUser = () => {
   user = writable<UserResponseDto>();
 };
diff --git a/web/src/lib/stores/websocket.ts b/web/src/lib/stores/websocket.ts
index a119d7c54f..5b59e2c823 100644
--- a/web/src/lib/stores/websocket.ts
+++ b/web/src/lib/stores/websocket.ts
@@ -1,8 +1,8 @@
 import type { AssetResponseDto, ServerVersionResponseDto } from '@api';
 import { type Socket, io } from 'socket.io-client';
-import { writable } from 'svelte/store';
+import { get, writable } from 'svelte/store';
 import { loadConfig } from './server-config.store';
-import { getAuthUser } from '$lib/utils/auth';
+import { user } from './user.store';
 
 export interface ReleaseEvent {
   isAvailable: boolean;
@@ -30,8 +30,7 @@ export const openWebsocketConnection = async () => {
       return;
     }
 
-    const user = await getAuthUser();
-    if (!user) {
+    if (!get(user)) {
       return;
     }
 
diff --git a/web/src/lib/utils/auth.ts b/web/src/lib/utils/auth.ts
index a47adbcbed..f83b355bb4 100644
--- a/web/src/lib/utils/auth.ts
+++ b/web/src/lib/utils/auth.ts
@@ -1,53 +1,65 @@
 import { api } from '@api';
 import { redirect } from '@sveltejs/kit';
 import { AppRoute } from '../constants';
-import { getSavedUser, setUser } from '$lib/stores/user.store';
+import { get } from 'svelte/store';
 import { serverInfo } from '$lib/stores/server-info.store';
+import { browser } from '$app/environment';
+import { user } from '$lib/stores/user.store';
 
 export interface AuthOptions {
   admin?: true;
+  public?: true;
 }
 
-export const getAuthUser = async () => {
+export const loadUser = async () => {
   try {
-    const { data: user } = await api.userApi.getMyUserInfo();
-    return user;
+    let loaded = get(user);
+    if (!loaded && hasAuthCookie()) {
+      const { data } = await api.userApi.getMyUserInfo();
+      loaded = data;
+      user.set(loaded);
+    }
+    return loaded;
   } catch {
     return null;
   }
 };
 
-export const authenticate = async (options?: AuthOptions) => {
-  options = options || {};
+const hasAuthCookie = (): boolean => {
+  if (!browser) {
+    return false;
+  }
 
-  const savedUser = getSavedUser();
-  const user = savedUser || (await getAuthUser());
+  for (const cookie of document.cookie.split('; ')) {
+    const [name] = cookie.split('=');
+    if (name === 'immich_is_authenticated') {
+      return true;
+    }
+  }
+
+  return false;
+};
+
+export const authenticate = async (options?: AuthOptions) => {
+  const { public: publicRoute, admin: adminRoute } = options || {};
+  const user = await loadUser();
+
+  if (publicRoute) {
+    return;
+  }
 
   if (!user) {
     redirect(302, AppRoute.AUTH_LOGIN);
   }
 
-  if (options.admin && !user.isAdmin) {
+  if (adminRoute && !user.isAdmin) {
     redirect(302, AppRoute.PHOTOS);
   }
-
-  if (!savedUser) {
-    setUser(user);
-  }
 };
 
 export const requestServerInfo = async () => {
-  if (getSavedUser()) {
+  if (get(user)) {
     const { data } = await api.serverInfoApi.getServerInfo();
     serverInfo.set(data);
   }
 };
-
-export const isLoggedIn = async () => {
-  const savedUser = getSavedUser();
-  const user = savedUser || (await getAuthUser());
-  if (!savedUser) {
-    setUser(user);
-  }
-  return user;
-};
diff --git a/web/src/routes/(user)/share/[key]/+page.ts b/web/src/routes/(user)/share/[key]/+page.ts
index 47b52f1f88..e43f81f914 100644
--- a/web/src/routes/(user)/share/[key]/+page.ts
+++ b/web/src/routes/(user)/share/[key]/+page.ts
@@ -1,4 +1,4 @@
-import { getAuthUser } from '$lib/utils/auth';
+import { authenticate } from '$lib/utils/auth';
 import { api, ThumbnailFormat } from '@api';
 import type { AxiosError } from 'axios';
 import type { PageLoad } from './$types';
@@ -6,7 +6,7 @@ import { error as throwError } from '@sveltejs/kit';
 
 export const load = (async ({ params }) => {
   const { key } = params;
-  await getAuthUser();
+  await authenticate({ public: true });
 
   try {
     const { data: sharedLink } = await api.sharedLinkApi.getMySharedLink({ key });
diff --git a/web/src/routes/+page.ts b/web/src/routes/+page.ts
index 38c5bff013..91171214db 100644
--- a/web/src/routes/+page.ts
+++ b/web/src/routes/+page.ts
@@ -1,14 +1,14 @@
 import { AppRoute } from '$lib/constants';
 import { redirect } from '@sveltejs/kit';
 import { api } from '../api';
-import { isLoggedIn } from '../lib/utils/auth';
+import { loadUser } from '../lib/utils/auth';
 import type { PageLoad } from './$types';
 
 export const ssr = false;
 export const csr = true;
 
 export const load = (async () => {
-  const authenticated = await isLoggedIn();
+  const authenticated = await loadUser();
   if (authenticated) {
     redirect(302, AppRoute.PHOTOS);
   }
diff --git a/web/src/routes/auth/change-password/+page.ts b/web/src/routes/auth/change-password/+page.ts
index 749c884bf6..37ab1415e3 100644
--- a/web/src/routes/auth/change-password/+page.ts
+++ b/web/src/routes/auth/change-password/+page.ts
@@ -2,11 +2,12 @@ import { AppRoute } from '$lib/constants';
 import { authenticate } from '$lib/utils/auth';
 import { redirect } from '@sveltejs/kit';
 import type { PageLoad } from './$types';
-import { getSavedUser } from '$lib/stores/user.store';
+import { get } from 'svelte/store';
+import { user } from '$lib/stores/user.store';
 
 export const load = (async () => {
   await authenticate();
-  if (!getSavedUser().shouldChangePassword) {
+  if (!get(user).shouldChangePassword) {
     redirect(302, AppRoute.PHOTOS);
   }