Skip to content

Commit

Permalink
refactor(routing): simplify and improve robustness (#2479)
Browse files Browse the repository at this point in the history
Signed-off-by: Fernando Fernández <[email protected]>
  • Loading branch information
ferferga authored Oct 24, 2024
1 parent f15bcde commit 3b796b8
Show file tree
Hide file tree
Showing 14 changed files with 113 additions and 119 deletions.
2 changes: 1 addition & 1 deletion frontend/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 5 additions & 7 deletions frontend/src/components/Forms/AddServerForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down
13 changes: 5 additions & 8 deletions frontend/src/components/Forms/LoginForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div>
<VForm
v-model="valid"
:disabled="loading"
:disabled="loading || disabled"
@submit.prevent="userLogin">
<VTextField
v-if="!user"
Expand Down Expand Up @@ -51,7 +51,7 @@
</VCol>
<VCol class="mr-2">
<VBtn
:disabled="!valid"
:disabled="!valid || disabled"
:loading="loading"
block
size="large"
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/Layout/AppBar/SearchField.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const searchQuery = computed({
},
set(value) {
void router.replace({
...router.currentRoute,
...router.currentRoute.value,
query: {
q: value.trim() || undefined
}
Expand Down
23 changes: 11 additions & 12 deletions frontend/src/pages/server/login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
</h5>
<LoginForm
:user="currentUser"
:disabled="!isConnectedToServer"
@change="resetCurrentUser" />
<p
v-if="disclaimer"
Expand All @@ -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 ?? []);
Expand All @@ -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;
}
Expand All @@ -145,4 +138,10 @@ function resetCurrentUser(): void {
currentUser.value = undefined;
loginAsOther.value = false;
}
watch(isConnectedToServer, () => {
if (!isConnectedToServer.value) {
useSnackbar(t('noServerConnection'), 'error');
}
});
</script>
8 changes: 7 additions & 1 deletion frontend/src/plugins/remote/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
5 changes: 2 additions & 3 deletions frontend/src/plugins/remote/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
36 changes: 8 additions & 28 deletions frontend/src/plugins/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down Expand Up @@ -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' });
8 changes: 3 additions & 5 deletions frontend/src/plugins/router/middlewares/admin-pages.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
}
93 changes: 55 additions & 38 deletions frontend/src/plugins/router/middlewares/login.ts
Original file line number Diff line number Diff line change
@@ -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);
export const loginGuard = async (
to: RouteLocationNormalized,
from: RouteLocationNormalized
): Promise<Exclude<NavigationGuardReturn, Error>> => {
const toServerPages = serverPages.has(to.name);

if (!jsonConfig.allowServerSelection && toServerPages) {
return false;
}

return true;
}
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;
}
};
Loading

0 comments on commit 3b796b8

Please sign in to comment.