fix(auth): preserve desktop authorize URL when not signed in (#2944)

frontend/src/constants/redirectHash.ts

@@ -0,0 +1,12 @@
+/**
+ * Hash-fragment prefix used to carry a post-login destination in the URL.
+ *
+ * Unlike the localStorage redirect, this lives in the address bar so the URL
+ * stays copyable between browsers (needed for native OAuth clients that open
+ * /oauth/authorize, see #2654). It uses the hash – not a query param – so the
+ * embedded OAuth parameters never reach server or proxy access logs.
+ *
+ * Must stay distinct from LINK_SHARE_HASH_PREFIX, which router.beforeEach
+ * special-cases.
+ */
+export const REDIRECT_HASH_PREFIX = '#redirect='

frontend/src/router/index.ts

@@ -6,6 +6,7 @@ import {getProjectViewId} from '@/helpers/projectView'
 import {parseDateOrString} from '@/helpers/time/parseDateOrString'
 import {getNextWeekDate} from '@/helpers/time/getNextWeekDate'
 import {LINK_SHARE_HASH_PREFIX} from '@/constants/linkShareHash'
+import {REDIRECT_HASH_PREFIX} from '@/constants/redirectHash'
 import {AUTH_ROUTE_NAMES} from '@/constants/authRouteNames'
 import {PRO_FEATURE} from '@/constants/proFeatures'
 
@@ -30,7 +31,7 @@ const router = createRouter({
 		}
 
 		// Scroll to anchor should still work
-		if (to.hash && !to.hash.startsWith(LINK_SHARE_HASH_PREFIX)) {
+		if (to.hash && !to.hash.startsWith(LINK_SHARE_HASH_PREFIX) && !to.hash.startsWith(REDIRECT_HASH_PREFIX)) {
 			return {el: to.hash}
 		}
 
@@ -472,10 +473,22 @@ const router = createRouter({
 })
 
 export async function getAuthForRoute(to: RouteLocation, authStore) {
+	// vue-router already decoded to.hash once, so slicing off the prefix yields the original
+	// fullPath (e.g. /oauth/authorize?...) losslessly — no extra decodeURIComponent needed.
+	const redirectDest = to.name === 'user.login' && to.hash.startsWith(REDIRECT_HASH_PREFIX)
+		? to.hash.slice(REDIRECT_HASH_PREFIX.length)
+		: ''
+
 	if (authStore.authUser || authStore.authLinkShare) {
+		// An already-signed-in browser that opens a copied /login#redirect=<oauth.authorize> URL
+		// must run the OAuth flow with its existing session instead of short-circuiting to home.
+		// The destination has no redirect hash, so the second guard pass just early-returns (#2654).
+		if (redirectDest) {
+			return redirectDest
+		}
 		return
 	}
-	
+
 	// Check if password reset token is in query params
 	const resetToken = to.query.userPasswordReset as string | undefined
 	
@@ -499,15 +512,35 @@ export async function getAuthForRoute(to: RouteLocation, authStore) {
 		}
 	}
 
+	// Keep the destination in the address bar (not just per-browser localStorage) so a native
+	// client's /oauth/authorize URL stays copyable into another browser. Hash, not query, so the
+	// embedded OAuth params never reach access logs (#2654). Pass fullPath raw: vue-router encodes
+	// the hash itself, so an extra encodeURIComponent here would be double-encoded in the URL.
+	if (to.name === 'oauth.authorize') {
+		return {
+			name: 'user.login',
+			hash: REDIRECT_HASH_PREFIX + to.fullPath,
+		}
+	}
+
+	// Fold the hash destination into localStorage: it's the only bridge that survives the
+	// external OIDC round-trip out of the SPA, so redirectIfSaved() works after any auth method.
+	// vue-router already decoded to.hash once, so it equals the fullPath we wrote above as-is.
+	if (to.hash.startsWith(REDIRECT_HASH_PREFIX)) {
+		const destination = to.hash.slice(REDIRECT_HASH_PREFIX.length)
+		const resolved = router.resolve(destination)
+		saveLastVisited(resolved.name as string, resolved.params, resolved.query)
+	}
+
 	// Check if the route the user wants to go to is a route which needs authentication. We use this to
 	// redirect the user after successful login.
 	const isValidUserAppRoute = !AUTH_ROUTE_NAMES.has(to.name as string) &&
 		localStorage.getItem('emailConfirmToken') === null
-	
+
 	if (isValidUserAppRoute) {
 		saveLastVisited(to.name as string, to.params, to.query)
 	}
-	
+
 	if (isValidUserAppRoute) {
 		return {name: 'user.login'}
 	}
@@ -565,12 +598,25 @@ router.beforeEach(async (to, from) => {
 
 	const newRoute = await getAuthForRoute(to, authStore)
 	if(newRoute) {
+		// A string target (the decoded redirect destination for an authed browser) already
+		// carries its own query/path and no redirect hash, so navigate to it verbatim — don't
+		// re-attach to.hash or it would re-enter the redirect loop.
+		if (typeof newRoute === 'string') {
+			return newRoute
+		}
 		return {
-			...newRoute,
 			hash: to.hash,
+			...newRoute,
 		}
 	}
-	
+
+	// to.fullPath keeps the redirect hash url-encoded while to.hash is decoded, so the endsWith
+	// check below never matches and would re-append the hash forever. The hash is already on the
+	// URL here, so skip the re-attach (#2654).
+	if (to.hash.startsWith(REDIRECT_HASH_PREFIX)) {
+		return
+	}
+
 	if(!to.fullPath.endsWith(to.hash)) {
 		return to.fullPath + to.hash
 	}

frontend/tests/e2e/user/oauth-authorize.spec.ts

@@ -32,10 +32,20 @@ test.describe('OAuth 2.0 Authorization Flow', () => {
 		})
 
 		// Navigate to the OAuth authorize frontend route.
-		// The user is not logged in, so the router guard saves the route
-		// and redirects to /login.
+		// The user is not logged in, so the router guard redirects to /login while
+		// carrying the authorize destination in a copyable #redirect= hash (not a
+		// query param, to keep the OAuth params out of access logs).
 		await page.goto(`/oauth/authorize?${authorizeParams}`)
-		await expect(page).toHaveURL(/\/login/)
+		await expect(page).toHaveURL(/\/login#redirect=/)
+
+		// The decoded #redirect= destination must carry the full authorize URL, including the
+		// OAuth params — checking only for the path would pass even if the query were dropped.
+		const redirectHash = decodeURIComponent(new URL(page.url()).hash)
+		expect(redirectHash).toContain('/oauth/authorize')
+		expect(redirectHash).toContain('response_type=code')
+		expect(redirectHash).toContain('client_id=vikunja')
+		expect(redirectHash).toContain(`code_challenge=${codeChallenge}`)
+		expect(redirectHash).toContain(`state=${state}`)
 
 		// Register the response listener BEFORE clicking Login, because after
 		// login redirectIfSaved() navigates back to /oauth/authorize and the
@@ -77,4 +87,70 @@ test.describe('OAuth 2.0 Authorization Flow', () => {
 		expect(tokenBody.token_type).toBe('bearer')
 		expect(tokenBody.expires_in).toBeGreaterThan(0)
 	})
+
+	// The primary #2654 scenario: the native client opened a different default browser that is
+	// already signed in to Vikunja. Opening the copied /login#redirect=<oauth.authorize> URL must
+	// run the OAuth flow with the existing session instead of short-circuiting to home.
+	test('Already-authenticated browser opening the copied login redirect runs the authorize flow', async ({authenticatedPage, apiContext, currentUser}) => {
+		const page = authenticatedPage
+
+		const codeVerifier = randomBytes(32).toString('base64url')
+		const codeChallenge = createHash('sha256').update(codeVerifier).digest('base64url')
+		const state = randomBytes(16).toString('base64url')
+
+		const authorizeParams = new URLSearchParams({
+			response_type: 'code',
+			client_id: 'vikunja',
+			redirect_uri: 'vikunja-flutter://callback',
+			code_challenge: codeChallenge,
+			code_challenge_method: 'S256',
+			state,
+		})
+
+		// The component POSTs as soon as it mounts with the existing session, so register the
+		// listener before navigating.
+		const authorizeResponsePromise = page.waitForResponse(
+			response => response.url().includes('/api/v1/oauth/authorize') && response.request().method() === 'POST',
+			{timeout: 15000},
+		)
+
+		// Open the copyable login URL exactly as it would be pasted from another browser
+		// (#redirect= is REDIRECT_HASH_PREFIX from @/constants/redirectHash, inlined here because
+		// the e2e runner has no @ alias).
+		const redirectDestination = `/oauth/authorize?${authorizeParams}`
+		await page.goto(`/login#redirect=${encodeURIComponent(redirectDestination)}`)
+
+		// The authed guard must send us straight to /oauth/authorize, not home.
+		await expect(page).toHaveURL(/\/oauth\/authorize/)
+		const landed = new URL(page.url())
+		expect(landed.pathname).toBe('/oauth/authorize')
+		expect(landed.searchParams.get('response_type')).toBe('code')
+		expect(landed.searchParams.get('client_id')).toBe('vikunja')
+		expect(landed.searchParams.get('code_challenge')).toBe(codeChallenge)
+		expect(landed.searchParams.get('state')).toBe(state)
+
+		// The PKCE flow completes with the existing session — no second login.
+		const authorizeResponse = await authorizeResponsePromise
+		const authorizeBody = await authorizeResponse.json()
+		expect(authorizeBody.code).toBeTruthy()
+		expect(authorizeBody.redirect_uri).toBe('vikunja-flutter://callback')
+		expect(authorizeBody.state).toBe(state)
+
+		const tokenResponse = await apiContext.post('oauth/token', {
+			data: {
+				grant_type: 'authorization_code',
+				code: authorizeBody.code,
+				client_id: 'vikunja',
+				redirect_uri: 'vikunja-flutter://callback',
+				code_verifier: codeVerifier,
+			},
+		})
+
+		expect(tokenResponse.ok()).toBe(true)
+		const tokenBody = await tokenResponse.json()
+		expect(tokenBody.access_token).toBeTruthy()
+		expect(tokenBody.refresh_token).toBeTruthy()
+		expect(tokenBody.token_type).toBe('bearer')
+		expect(tokenBody.expires_in).toBeGreaterThan(0)
+	})
 })