refactor(routing): simplify and improve robustness (#2479)

Signed-off-by: Fernando Fernández <ferferga@hotmail.com>

frontend/locales/en.json

@@ -317,7 +317,7 @@
   "revokeFailure": "Error revoking API key",
   "revokeSuccess": "Successfully revoked API key",
   "role": "Role",
-  "routeValidationError": "The specified routeId in route params is not correct",
+  "routeValidationError": "The specified parameters for accessing this page are not correct",
   "runningTasks": "Running tasks",
   "runtime": "Duration",
   "save": "Save",

frontend/src/components/Forms/AddServerForm.vue

@@ -48,9 +48,8 @@ import { shallowRef, unref } from 'vue';
 import { useI18n } from 'vue-i18n';
 import { useRouter } from 'vue-router';
 import { remote } from '@/plugins/remote';
-import { getJSONConfig } from '@/utils/external-config';
+import { jsonConfig } from '@/utils/external-config';
 
-const jsonConfig = await getJSONConfig();
 const router = useRouter();
 const i18n = useI18n();
 const valid = shallowRef(false);
@@ -63,17 +62,16 @@ const rules = [
 ];
 
 /**
- * Attempts a connection to the given server
+ * Attempts a connection to the given server.
+ * If the connection is successful, the user will be redirected to the login page
+ * at the middleware level
  */
 async function connectToServer(): Promise<void> {
   loading.value = true;
 
   try {
     await remote.auth.connectServer(serverUrl.value);
-
-    await (previousServerLength === 0
-      ? router.push('/server/login')
-      : router.push('/server/select'));
+    await router.push('/server/login');
   } finally {
     loading.value = false;
   }

frontend/src/components/Forms/LoginForm.vue

@@ -2,7 +2,7 @@
   <div>
     <VForm
       v-model="valid"
-      :disabled="loading"
+      :disabled="loading || disabled"
       @submit.prevent="userLogin">
       <VTextField
         v-if="!user"
@@ -51,7 +51,7 @@
         </VCol>
         <VCol class="mr-2">
           <VBtn
-            :disabled="!valid"
+            :disabled="!valid || disabled"
             :loading="loading"
             block
             size="large"
@@ -75,15 +75,14 @@ import { useI18n } from 'vue-i18n';
 import { useRouter } from 'vue-router';
 import { fetchIndexPage } from '@/utils/items';
 import { remote } from '@/plugins/remote';
-import { getJSONConfig } from '@/utils/external-config';
+import { jsonConfig } from '@/utils/external-config';
 
-const { user } = defineProps<{ user?: UserDto }>();
+const { user, disabled } = defineProps<{ user?: UserDto; disabled?: boolean }>();
 
 defineEmits<{
   change: [];
 }>();
 
-const jsonConfig = await getJSONConfig();
 const { t } = useI18n();
 
 const router = useRouter();
@@ -121,9 +120,7 @@ async function userLogin(): Promise<void> {
      * loading spinner active until we redirect the user.
      */
     await fetchIndexPage();
-
-    await router.replace('/');
-  } finally {
+  } catch {
     loading.value = false;
   }
 }

frontend/src/components/Layout/AppBar/SearchField.vue

@@ -24,7 +24,7 @@ const searchQuery = computed({
   },
   set(value) {
     void router.replace({
-      ...router.currentRoute,
+      ...router.currentRoute.value,
       query: {
         q: value.trim() || undefined
       }

frontend/src/pages/server/login.vue

@@ -79,6 +79,7 @@
         </h5>
         <LoginForm
           :user="currentUser"
+          :disabled="!isConnectedToServer"
           @change="resetCurrentUser" />
         <p
           v-if="disclaimer"
@@ -98,27 +99,20 @@ meta:
 
 <script setup lang="ts">
 import type { UserDto } from '@jellyfin/sdk/lib/generated-client';
-import { ref, shallowRef, computed } from 'vue';
+import { ref, shallowRef, computed, watch } from 'vue';
 import { useI18n } from 'vue-i18n';
 import { useRouter } from 'vue-router';
-import { watchImmediate } from '@vueuse/core';
 import { remote } from '@/plugins/remote';
-import { getJSONConfig } from '@/utils/external-config';
-import { isConnectedToServer } from '@/store';
+import { jsonConfig } from '@/utils/external-config';
 import { usePageTitle } from '@/composables/page-title';
+import { useSnackbar } from '@/composables/use-snackbar';
+import { isConnectedToServer } from '@/store';
 
-const jsonConfig = await getJSONConfig();
 const { t } = useI18n();
 const router = useRouter();
 
 usePageTitle(() => t('login'));
 
-watchImmediate(isConnectedToServer, async () => {
-  if (!isConnectedToServer.value) {
-    await router.replace('/server/select');
-  }
-});
-
 const disclaimer = computed(() => remote.auth.currentServer?.BrandingOptions.LoginDisclaimer);
 const publicUsers = computed(() => remote.auth.currentServer?.PublicUsers ?? []);
 
@@ -132,7 +126,6 @@ async function setCurrentUser(user: UserDto): Promise<void> {
   if (!user.HasPassword && user.Name) {
     // If the user doesn't have a password, avoid showing the password form
     await remote.auth.loginUser(user.Name, '');
-    await router.replace('/');
   } else {
     currentUser.value = user;
   }
@@ -145,4 +138,10 @@ function resetCurrentUser(): void {
   currentUser.value = undefined;
   loginAsOther.value = false;
 }
+
+watch(isConnectedToServer, () => {
+  if (!isConnectedToServer.value) {
+    useSnackbar(t('noServerConnection'), 'error');
+  }
+});
 </script>

frontend/src/plugins/remote/auth.ts

@@ -320,10 +320,16 @@ class RemotePluginAuth extends CommonStore<AuthState> {
       }
     }
 
+    const serverIndex = this._state.servers.indexOf(server);
+
     this._state.servers.splice(
-      this._state.servers.indexOf(server),
+      serverIndex,
       1
     );
+
+    if (this._state.currentServerIndex === serverIndex) {
+      this._state.currentServerIndex = -1;
+    }
   };
 
   public constructor() {

frontend/src/plugins/remote/index.ts

@@ -11,7 +11,7 @@ import RemotePluginAuthInstance from './auth';
 import RemotePluginSDKInstance from './sdk';
 import RemotePluginSocketInstance from './socket';
 import { isNil, sealed } from '@/utils/validation';
-import { getJSONConfig } from '@/utils/external-config';
+import { jsonConfig } from '@/utils/external-config';
 
 @sealed
 class RemotePlugin {
@@ -38,8 +38,7 @@ export function createPlugin(): {
         = remote;
 
       const auth = remote.auth;
-      const config = await getJSONConfig();
-      const defaultServers = config.defaultServerURLs;
+      const defaultServers = jsonConfig.defaultServerURLs;
       /**
        * We reverse the list so the first server is the last to be connected,
        * and thus is the chosen one by default

frontend/src/plugins/router/index.ts

@@ -10,11 +10,11 @@ import { loginGuard } from './middlewares/login';
 import { metaGuard } from './middlewares/meta';
 import { validateGuard } from './middlewares/validate';
 import { isStr } from '@/utils/validation';
-import { getJSONConfig } from '@/utils/external-config';
+import { jsonConfig } from '@/utils/external-config';
 
 export const router = createRouter({
   history:
-    (await getJSONConfig()).routerMode === 'history'
+    jsonConfig.routerMode === 'history'
       ? createWebHistory()
       : createWebHashHistory(),
   routes: [],
@@ -62,35 +62,15 @@ router.back = () => {
 };
 
 /**
- * Re-run the middleware pipeline when the user logs out or state is cleared
+ * Re-run the middleware pipeline when the user logs out or state is cleared,
+ * no additional logic is here so we can keep the the login middleware
+ * is the only source of truth.
  */
 watch([
   () => remote.auth.currentUser,
-  () => remote.auth.servers,
   () => remote.auth.currentServer
 ], () => {
-  if (!remote.auth.currentUser && remote.auth.servers.length <= 0) {
-    /**
-     * We run the redirect to /server/add as it's the first page in the login flow
-     *
-     * In case the whole localStorage is gone at runtime, if we're at the login
-     * page, redirecting to /server/login wouldn't work, as we're in that same page.
-     * /server/add doesn't depend on the state of localStorage, so it's always safe to
-     * redirect there and leave the middleware take care of the final destination
-     * (when servers are already available, for example)
-     */
-    void router.replace('/server/add');
-  } else if (
-    !remote.auth.currentUser
-    && remote.auth.servers.length > 0
-    && remote.auth.currentServer
-  ) {
-    void (remote.auth.currentServer.StartupWizardCompleted ? router.replace('/server/login') : router.replace('/wizard'));
-  } else if (
-    !remote.auth.currentUser
-    && remote.auth.servers.length > 0
-    && !remote.auth.currentServer
-  ) {
-    void router.replace('/server/select');
-  }
+  void router.replace({
+    force: true
+  });
 }, { flush: 'sync' });

frontend/src/plugins/router/middlewares/admin-pages.ts

@@ -1,4 +1,4 @@
-import type { RouteLocationNormalized, RouteLocationRaw } from 'vue-router';
+import type { NavigationGuardReturn, RouteLocationNormalized } from 'vue-router';
 import { useSnackbar } from '@/composables/use-snackbar';
 import { i18n } from '@/plugins/i18n';
 import { remote } from '@/plugins/remote';
@@ -9,12 +9,10 @@ import { remote } from '@/plugins/remote';
  */
 export function adminGuard(
   to: RouteLocationNormalized
-): boolean | RouteLocationRaw {
+): NavigationGuardReturn {
   if (to.meta.admin && !remote.auth.currentUser?.Policy?.IsAdministrator) {
     useSnackbar(i18n.t('unauthorized'), 'error');
 
-    return { path: '/', replace: true };
+    return false;
   }
-
-  return true;
 }

frontend/src/plugins/router/middlewares/login.ts

@@ -1,58 +1,75 @@
 import type {
-  RouteLocationNormalized,
-  RouteLocationPathRaw,
-  RouteLocationRaw
+  NavigationGuardReturn,
+  RouteLocationNormalized
 } from 'vue-router';
+import type { RouteNamedMap } from 'vue-router/auto-routes';
 import { until } from '@vueuse/core';
 import { remote } from '@/plugins/remote';
 import { isNil } from '@/utils/validation';
-import { getJSONConfig } from '@/utils/external-config';
+import { jsonConfig } from '@/utils/external-config';
+import { useSnackbar } from '@/composables/use-snackbar';
+import { i18n } from '@/plugins/i18n';
 
 const serverAddUrl = '/server/add';
 const serverSelectUrl = '/server/select';
 const serverLoginUrl = '/server/login';
-const serverRoutes = new Set([serverAddUrl, serverSelectUrl]);
-const routes = new Set([...serverRoutes, serverLoginUrl]);
+const serverWizard = '/wizard';
+const serverPages = new Set<keyof RouteNamedMap>([serverAddUrl, serverSelectUrl, serverLoginUrl, serverWizard]);
 
 /**
- * Performs the login guard redirection ensuring no redirection loops happen
+ * Gets the best server page based on the current state.
+ * Note that the final page rendered might differ from the best one here
+ * in the loginGuard
  */
-function doRedir(dest: RouteLocationPathRaw, to: RouteLocationNormalized) {
-  return to.path === dest.path
-    ? true
-    : dest;
-}
-
-/**
- * Redirects to login page if there's no user logged in.
- */
-export async function loginGuard(
-  to: RouteLocationNormalized
-): Promise<boolean | RouteLocationRaw> {
-  const jsonConfig = await getJSONConfig();
-
+async function _getBestServerPage(): Promise<Nullish<keyof RouteNamedMap>> {
   if (jsonConfig.defaultServerURLs.length && isNil(remote.auth.currentServer)) {
     await until(() => remote.auth.currentServer).toBeTruthy({ flush: 'pre' });
   }
 
-  if (
-    !isNil(remote.auth.currentServer)
-    && !isNil(remote.auth.currentUser)
-    && !isNil(remote.auth.currentUserToken)
-    && routes.has(to.path)
-  ) {
-    return doRedir({ path: '/', replace: true }, to);
+  if (!remote.auth.servers.length) {
+    return serverAddUrl;
+  } else if (isNil(remote.auth.currentServer)) {
+    return serverSelectUrl;
+  } else if (!remote.auth.currentServer.StartupWizardCompleted) {
+    return serverWizard;
   }
-
-  if (jsonConfig.allowServerSelection) {
-    if (!remote.auth.servers.length) {
-      return doRedir({ path: serverAddUrl, replace: true }, to);
-    } else if (isNil(remote.auth.currentServer)) {
-      return doRedir({ path: serverSelectUrl, replace: true }, to);
-    }
-  } else {
-    return doRedir({ path: serverLoginUrl, replace: true }, to);
-  }
-
-  return true;
 }
+
+export const loginGuard = async (
+  to: RouteLocationNormalized,
+  from: RouteLocationNormalized
+): Promise<Exclude<NavigationGuardReturn, Error>> => {
+  const toServerPages = serverPages.has(to.name);
+
+  if (!jsonConfig.allowServerSelection && toServerPages) {
+    return false;
+  }
+
+  const fromServerPages = serverPages.has(from.name);
+  const res = await _getBestServerPage();
+
+  const loggedIn = !isNil(remote.auth.currentUser);
+  const shouldBlockToServer = loggedIn && toServerPages;
+  const shouldBlockToApp = !loggedIn && !toServerPages;
+  const shouldBlock = shouldBlockToServer || shouldBlockToApp;
+  const shouldRedirectToHome = loggedIn && fromServerPages;
+  /**
+   * Redirections between server and app pages are freely allowed
+   */
+  const shouldRedirect = !isNil(res) || shouldBlockToApp || shouldRedirectToHome;
+
+  if (shouldRedirect) {
+    const name = loggedIn ? '/' : res ?? serverLoginUrl;
+
+    if (to.name !== name) {
+      return {
+        name,
+        replace: true
+      };
+    }
+  } else if (shouldBlock) {
+    useSnackbar(i18n.t('unauthorized'), 'error');
+
+    return false;
+  }
+};

frontend/src/plugins/router/middlewares/meta.ts

@@ -1,8 +1,8 @@
 import { defu } from 'defu';
 import { ref, toRaw } from 'vue';
 import type {
+  NavigationGuardReturn,
   RouteLocationNormalized,
-  RouteLocationRaw,
   RouteMeta
 } from 'vue-router';
 
@@ -40,7 +40,7 @@ const reactiveMeta = ref(structuredClone(defaultMeta));
 export function metaGuard(
   to: RouteLocationNormalized,
   from: RouteLocationNormalized
-): boolean | RouteLocationRaw {
+): NavigationGuardReturn {
   reactiveMeta.value = defu(to.meta, structuredClone(defaultMeta));
   /**
    * This is needed to ensure all the meta matches the expected data
@@ -51,6 +51,4 @@ export function metaGuard(
   if (from.meta.layout.transition.leave) {
     to.meta.layout.transition.enter = from.meta.layout.transition.leave;
   }
-
-  return true;
 }

frontend/src/plugins/router/middlewares/playback.ts

@@ -1,14 +1,16 @@
-import type { RouteLocationRaw } from 'vue-router';
+import type { NavigationGuardReturn } from 'vue-router';
 import { playbackManager } from '@/store/playback-manager';
 import { isNil } from '@/utils/validation';
+import { useSnackbar } from '@/composables/use-snackbar';
+import { i18n } from '@/plugins/i18n';
 
 /**
  * Validates that no playback is happening when accesing a route
  */
-export function playbackGuard(): RouteLocationRaw | boolean {
+export function playbackGuard(): NavigationGuardReturn {
   if (isNil(playbackManager.currentItem)) {
-    return { path: '/', replace: true };
-  }
+    useSnackbar(i18n.t('routeValidationError'), 'error');
 
-  return true;
+    return false;
+  }
 }

frontend/src/plugins/router/middlewares/validate.ts

@@ -1,4 +1,4 @@
-import type { RouteLocationNormalized, RouteLocationRaw } from 'vue-router';
+import type { NavigationGuardReturn, RouteLocationNormalized } from 'vue-router';
 import { useSnackbar } from '@/composables/use-snackbar';
 import { i18n } from '@/plugins/i18n';
 import { isStr } from '@/utils/validation';
@@ -9,16 +9,14 @@ import { isStr } from '@/utils/validation';
  */
 export function validateGuard(
   to: RouteLocationNormalized
-): boolean | RouteLocationRaw {
+): NavigationGuardReturn {
   if (('itemId' in to.params) && isStr(to.params.itemId)) {
     const check = /[\da-f]{32}/i.test(to.params.itemId);
 
     if (!check) {
       useSnackbar(i18n.t('routeValidationError'), 'error');
 
-      return { path: '/', replace: true };
+      return false;
     }
   }
-
-  return true;
 }

frontend/src/utils/external-config.ts

@@ -50,7 +50,7 @@ function validateJsonConfig(
  * Fetch configuration at runtime from the config.json file
  * We use destr for serialization as it has better support for JS primitives.
  */
-export async function getJSONConfig(): Promise<ExternalJSONConfig> {
+async function getJSONConfig(): Promise<ExternalJSONConfig> {
   if (isNil(externalConfig)) {
     const loadedConfig: unknown = await (
       await fetch('config.json', { cache: 'no-store' })
@@ -63,3 +63,5 @@ export async function getJSONConfig(): Promise<ExternalJSONConfig> {
 
   return externalConfig;
 }
+
+export const jsonConfig = await getJSONConfig();