feat: improve error handling on authorize page

backend/internal/oidc/authorization_handler.go

@@ -1,6 +1,8 @@
 package oidc
 
 import (
+	"context"
+	"errors"
 	"log/slog"
 	"net/http"
 	"strings"
@@ -49,7 +51,7 @@ func (h *authorizationHandler) authorize(c *gin.Context) {
 		query, err := h.authorizationService.interactionRequestQuery(ctx, interactionID)
 		if err != nil {
 			slog.WarnContext(ctx, "Failed to restore authorize request from interaction session", "error", err.Error())
-			h.provider.WriteAuthorizeError(ctx, c.Writer, fosite.NewAuthorizeRequest(), err)
+			h.writeAuthorizeError(ctx, c, fosite.NewAuthorizeRequest(), err)
 			return
 		}
 		c.Request.URL.RawQuery = query.Encode()
@@ -63,7 +65,7 @@ func (h *authorizationHandler) authorize(c *gin.Context) {
 	ar, err := h.provider.NewAuthorizeRequest(ctx, c.Request)
 	if err != nil {
 		slog.ErrorContext(ctx, "Failed to create authorize request", "error", err.Error())
-		h.provider.WriteAuthorizeError(ctx, c.Writer, ar, err)
+		h.writeAuthorizeError(ctx, c, ar, err)
 		return
 	}
 
@@ -80,7 +82,7 @@ func (h *authorizationHandler) authorize(c *gin.Context) {
 	})
 	if err != nil {
 		slog.ErrorContext(ctx, "Failed to authorize request", "error", err.Error())
-		h.provider.WriteAuthorizeError(ctx, c.Writer, ar, err)
+		h.writeAuthorizeError(ctx, c, ar, err)
 		return
 	}
 
@@ -92,7 +94,7 @@ func (h *authorizationHandler) authorize(c *gin.Context) {
 	response, err := h.provider.NewAuthorizeResponse(ctx, ar, authorization.Session)
 	if err != nil {
 		slog.ErrorContext(ctx, "Failed to create authorize response", "error", err.Error())
-		h.provider.WriteAuthorizeError(ctx, c.Writer, ar, err)
+		h.writeAuthorizeError(ctx, c, ar, err)
 		return
 	}
 
@@ -103,25 +105,6 @@ func (h *authorizationHandler) authorize(c *gin.Context) {
 	h.provider.WriteAuthorizeResponse(ctx, c.Writer, ar, response)
 }
 
-func requestMetaFromGin(c *gin.Context) requestMeta {
-	return requestMeta{
-		IPAddress: c.ClientIP(),
-		UserAgent: c.Request.UserAgent(),
-	}
-}
-
-func authorizeRequestParams(requester fosite.AuthorizeRequester) map[string]string {
-	params := make(map[string]string)
-	for key, values := range requester.GetRequestForm() {
-		if len(values) == 0 || key == "request_uri" || key == "interaction" {
-			continue
-		}
-		params[key] = values[0]
-	}
-
-	return params
-}
-
 func (h *authorizationHandler) getInteractionSession(c *gin.Context) {
 	interactionID := c.Param("id")
 
@@ -154,3 +137,43 @@ func (h *authorizationHandler) completeInteraction(c *gin.Context) {
 
 	c.JSON(http.StatusOK, response)
 }
+
+func (h *authorizationHandler) writeAuthorizeError(ctx context.Context, c *gin.Context, ar fosite.AuthorizeRequester, err error) {
+	if ar.IsRedirectURIValid() {
+		// Send the error to the client
+		h.provider.WriteAuthorizeError(ctx, c.Writer, ar, err)
+		return
+	}
+
+	// If no redirect URI is available, we can't send the error to the client,
+	// so we redirect to a generic error page instead.
+	errorMessage := "An unknown error occurred during the authorization request."
+	if err, ok := errors.AsType[*fosite.RFC6749Error](err); ok {
+		if err.HintField != "" {
+			errorMessage = err.HintField
+		} else if err.DescriptionField != "" {
+			errorMessage = err.DescriptionField
+		}
+	}
+
+	c.Redirect(http.StatusFound, "/interaction/error?error="+errorMessage)
+}
+
+func requestMetaFromGin(c *gin.Context) requestMeta {
+	return requestMeta{
+		IPAddress: c.ClientIP(),
+		UserAgent: c.Request.UserAgent(),
+	}
+}
+
+func authorizeRequestParams(requester fosite.AuthorizeRequester) map[string]string {
+	params := make(map[string]string)
+	for key, values := range requester.GetRequestForm() {
+		if len(values) == 0 || key == "request_uri" || key == "interaction" {
+			continue
+		}
+		params[key] = values[0]
+	}
+
+	return params
+}

frontend/src/lib/components/header/header.svelte

@@ -9,6 +9,7 @@
 
 	const authUrls = [
 		/^\/interaction$/,
+		/^\/interaction\/error$/,
 		/^\/device$/,
 		/^\/login(?:\/.*)?$/,
 		/^\/logout$/,

frontend/src/lib/components/login-wrapper.svelte

@@ -68,9 +68,8 @@
 			: 'justify-center'}"
 	>
 		<div
-			class="relative z-10 flex h-full p-16 {cn(
-				showAlternativeSignInMethodButton && 'pb-0',
-				backgroundImageExists && 'w-[650px] 2xl:w-[800px]'
+			class="relative z-10 flex h-full w-full max-w-[650px] 2xl:max-w-[800px] p-16 {cn(
+				showAlternativeSignInMethodButton && 'pb-0'
 			)}"
 		>
 			<div class="flex h-full w-full flex-col overflow-hidden">

frontend/src/lib/utils/redirection-util.ts

@@ -18,7 +18,7 @@ export function getAuthRedirectPath(url: URL, user: User | null) {
 
 	const isPublicPath =
 		path.startsWith('/lc/') ||
-		['/interaction', '/login/alternative/code', '/device', '/health', '/healthz'].includes(path);
+		['/interaction', '/interaction/error', '/login/alternative/code', '/device', '/health', '/healthz'].includes(path);
 
 	const isAdminPath = path == '/settings/admin' || path.startsWith('/settings/admin/');
 

frontend/src/routes/authorize/components/client-provider-images.svelte

@@ -13,18 +13,16 @@
 		error,
 		client
 	}: {
-		success: boolean;
-		error: boolean;
-		client: OidcClientMetaData;
+		success?: boolean;
+		error?: boolean;
+		client?: OidcClientMetaData;
 	} = $props();
 
 	let animationDone = $state(false);
 
 	$effect(() => {
 		if (success || error) {
-			setTimeout(() => {
-				animationDone = true;
-			}, 500);
+			setTimeout(() => (animationDone = true), client ? 500 : 0);
 		} else {
 			animationDone = false;
 		}
@@ -61,14 +59,14 @@
 			<div class="flex size-10 items-center justify-center">
 				<CrossAnimated class="size-5" />
 			</div>
-		{:else if client.hasLogo}
+		{:else if client?.hasLogo}
 			<img
 				class="aspect-square size-10 object-contain"
 				src={cachedOidcClientLogo.getUrl(client.id, isLightMode)}
 				draggable={false}
 				alt={m.client_logo()}
 			/>
-		{:else}
+		{:else if client?.name}
 			<div class="flex size-10 items-center justify-center text-3xl font-bold">
 				{client.name.charAt(0).toUpperCase()}
 			</div>

frontend/src/routes/interaction/+page.svelte

@@ -170,7 +170,7 @@
 		</div>
 	{:else if currentStep === 'consent'}
 		<div class="w-full max-w-md" transition:slide={{ duration: 300 }}>
-			<Card.Root class="mt-6 mb-10">
+			<Card.Root class="mb-10">
 				<Card.Header>
 					<p class="text-muted-foreground text-start">
 						<FormattedMessage

frontend/src/routes/interaction/error/+page.svelte

@@ -0,0 +1,34 @@
+<script lang="ts">
+	import SignInWrapper from '$lib/components/login-wrapper.svelte';
+	import { Button } from '$lib/components/ui/button';
+	import { m } from '$lib/paraglide/messages';
+	import OidcService from '$lib/services/oidc-service';
+	import WebAuthnService from '$lib/services/webauthn-service';
+	import userStore from '$lib/stores/user-store';
+	import ClientProviderImages from '../../authorize/components/client-provider-images.svelte';
+	import type { PageProps } from './$types';
+
+	const webauthnService = new WebAuthnService();
+	const oidcService = new OidcService();
+
+	let { data }: PageProps = $props();
+	let { error } = data;
+</script>
+
+<svelte:head>
+	<title>{m.error()}</title>
+</svelte:head>
+
+<SignInWrapper>
+	<ClientProviderImages error success={false} />
+	<h1 class="font-gloock mt-5 text-3xl font-bold sm:text-4xl">
+		{m.error()}
+	</h1>
+	<p class="text-muted-foreground mt-2 mb-10">
+		{error}
+	</p>
+
+	<Button class="w-full sm:w-[50%]" variant="secondary" href={document.referrer || '/'}>
+		{m.go_back()}
+	</Button>
+</SignInWrapper>

frontend/src/routes/interaction/error/+page.ts

@@ -0,0 +1,9 @@
+import type { PageLoad } from './$types';
+
+export const load: PageLoad = async ({ url }) => {
+	const error = url.searchParams.get('error') ?? "An unknown error occured."
+
+	return {
+		error
+	};
+};

frontend/src/routes/login/+page.svelte

@@ -9,7 +9,6 @@
 	import { getWebauthnErrorMessage } from '$lib/utils/error-util';
 	import { startAuthentication } from '@simplewebauthn/browser';
 	import { fade } from 'svelte/transition';
-	import { cn } from 'tailwind-variants';
 	import LoginLogoErrorSuccessIndicator from './components/login-logo-error-success-indicator.svelte';
 
 	let { data } = $props();
@@ -63,7 +62,7 @@
 			</Button>
 		{/if}
 		<Button
-			class={cn($appConfigStore.allowUserSignups === 'open' && 'w-[50%]')}
+			class={$appConfigStore.allowUserSignups === 'open' ? 'w-[50%]' : 'w-[80%] sm:w-[40%]'}
 			{isLoading}
 			onclick={authenticate}
 			autofocus={true}

tests/specs/oidc.spec.ts

@@ -908,10 +908,11 @@ test.describe('OIDC prompt parameter', () => {
 			await route.fulfill({ status: 200, body: 'attacker' });
 		});
 
-		const response = await page.goto(`/authorize?${urlParams.toString()}`);
+		await page.goto(`/authorize?${urlParams.toString()}`);
 
-		expect(response?.status()).toBe(400);
-		await expect(page.locator('body')).toContainText('invalid_request');
+		await expect(page.locator('body')).toContainText(
+			"The 'redirect_uri' parameter does not match any of the OAuth 2.0 Client's pre-registered redirect urls."
+		);
 		expect(attackerRedirected).toBe(false);
 	});
 
@@ -962,10 +963,12 @@ test.describe('OIDC prompt parameter', () => {
 			await route.fulfill({ status: 200, body: 'attacker' });
 		});
 
-		const response = await page.goto(`/authorize?${urlParams.toString()}`);
+		await page.goto(`/authorize?${urlParams.toString()}`);
+
+		await expect(page.locator('body')).toContainText(
+			"The 'redirect_uri' parameter does not match any of the OAuth 2.0 Client's pre-registered redirect urls."
+		);
 
-		expect(response?.status()).toBe(400);
-		await expect(page.locator('body')).toContainText('invalid_request');
 		expect(attackerRedirected).toBe(false);
 	});
 
@@ -1303,7 +1306,7 @@ test.describe('Pushed Authorization Requests (PAR)', () => {
 		expect(firstCallbackUrl.searchParams.get('code')).toBeTruthy();
 
 		await page.goto(`/authorize?${urlParams.toString()}`);
-		await expect(page.getByText('invalid_request_uri')).toBeVisible();
+		await expect(page.locator('body')).toContainText('Invalid PAR session');
 	});
 
 	test('PAR endpoint rejects confidential client request without client credentials', async ({