From 08f426fcd5e51200ee277790c1c55ef548e45e29 Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 18 Aug 2026 10:05:57 +0200 Subject: [PATCH] fix(auth): don't auto-redirect to the OIDC provider when the login URL must stay copyable The single-provider auto-redirect added in 18ee92f2 fires on every visit to /login, including the two cases where the login page has to stay put: - A native client's /oauth/authorize URL is parked in the login hash so it can be copied into the browser the user is actually signed in to (#2654). The auto-redirect replaced that URL with the provider's before it could be copied. Copying the provider URL instead does not work either, because the OIDC state lives in the originating browser's localStorage, so finishing the flow elsewhere fails the state check. - Inside the Electron window, login is handed off to the system browser by DesktopLogin. Redirecting to the provider in-window stranded the user there. Move the decision into getAutoRedirectProvider() and cover it with unit tests. Fixes #3473 --- .../src/helpers/redirectToProvider.test.ts | 51 +++++++++++++++++++ frontend/src/helpers/redirectToProvider.ts | 45 ++++++++++++++++ frontend/src/views/user/Login.vue | 33 ++++++------ 3 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 frontend/src/helpers/redirectToProvider.test.ts diff --git a/frontend/src/helpers/redirectToProvider.test.ts b/frontend/src/helpers/redirectToProvider.test.ts new file mode 100644 index 000000000..d867e5ef9 --- /dev/null +++ b/frontend/src/helpers/redirectToProvider.test.ts @@ -0,0 +1,51 @@ +import {describe, it, expect} from 'vitest' + +import {getAutoRedirectProvider} from './redirectToProvider' +import type {IProvider} from '@/types/IProvider' + +const provider = {key: 'authentik', name: 'Authentik'} as IProvider + +const soleProviderContext = { + localAuthEnabled: false, + ldapAuthEnabled: false, + openIdEnabled: true, + providers: [provider], + isDesktopApp: false, + justLoggedOut: false, + hasCopyableRedirect: false, +} + +describe('getAutoRedirectProvider', () => { + it('returns the provider when it is the only way to log in', () => { + expect(getAutoRedirectProvider(soleProviderContext)).toBe(provider) + }) + + it('does not redirect when the login url carries a copyable oauth destination', () => { + expect(getAutoRedirectProvider({...soleProviderContext, hasCopyableRedirect: true})).toBeUndefined() + }) + + it('does not redirect inside the desktop app', () => { + expect(getAutoRedirectProvider({...soleProviderContext, isDesktopApp: true})).toBeUndefined() + }) + + it('does not redirect right after an explicit logout', () => { + expect(getAutoRedirectProvider({...soleProviderContext, justLoggedOut: true})).toBeUndefined() + }) + + it('does not redirect when local or ldap auth is available', () => { + expect(getAutoRedirectProvider({...soleProviderContext, localAuthEnabled: true})).toBeUndefined() + expect(getAutoRedirectProvider({...soleProviderContext, ldapAuthEnabled: true})).toBeUndefined() + }) + + it('does not redirect when there is a choice of providers', () => { + expect(getAutoRedirectProvider({ + ...soleProviderContext, + providers: [provider, {key: 'other', name: 'Other'} as IProvider], + })).toBeUndefined() + }) + + it('does not redirect when openid is disabled or has no providers', () => { + expect(getAutoRedirectProvider({...soleProviderContext, openIdEnabled: false})).toBeUndefined() + expect(getAutoRedirectProvider({...soleProviderContext, providers: []})).toBeUndefined() + }) +}) diff --git a/frontend/src/helpers/redirectToProvider.ts b/frontend/src/helpers/redirectToProvider.ts index df2031ec3..211d8adc9 100644 --- a/frontend/src/helpers/redirectToProvider.ts +++ b/frontend/src/helpers/redirectToProvider.ts @@ -31,3 +31,48 @@ export const redirectToProviderOnLogout = (provider: IProvider): boolean => { } return false } + +interface AutoRedirectContext { + localAuthEnabled: boolean + ldapAuthEnabled: boolean + openIdEnabled: boolean + providers: IProvider[] + isDesktopApp: boolean + justLoggedOut: boolean + hasCopyableRedirect: boolean +} + +/** + * The provider the login page should redirect to without the user clicking anything, + * or undefined when it must render the login form instead. + */ +export function getAutoRedirectProvider(ctx: AutoRedirectContext): IProvider | undefined { + // The Electron window hands login off to the system browser via DesktopLogin – redirecting + // to the provider in-window would strand the user there with no way back to the app. + if (ctx.isDesktopApp) { + return undefined + } + + // Otherwise we'd immediately re-authenticate the user we just logged out. + if (ctx.justLoggedOut) { + return undefined + } + + // A native client's authorize URL is parked in the login hash so it stays copyable into the + // browser the user is actually signed in to (#2654). Redirecting to the provider replaces it + // before it can be copied, and the provider URL itself is not transferable: the OIDC state + // lives in this browser's localStorage, so finishing the flow elsewhere fails the state check. + if (ctx.hasCopyableRedirect) { + return undefined + } + + if (ctx.localAuthEnabled || ctx.ldapAuthEnabled) { + return undefined + } + + if (!ctx.openIdEnabled || ctx.providers.length !== 1) { + return undefined + } + + return ctx.providers[0] +} diff --git a/frontend/src/views/user/Login.vue b/frontend/src/views/user/Login.vue index e4af1160c..f4c1ed854 100644 --- a/frontend/src/views/user/Login.vue +++ b/frontend/src/views/user/Login.vue @@ -117,7 +117,7 @@