diff --git a/backend/internal/oidc/authorization_handler.go b/backend/internal/oidc/authorization_handler.go
index 442b628b..2eb7433e 100644
--- a/backend/internal/oidc/authorization_handler.go
+++ b/backend/internal/oidc/authorization_handler.go
@@ -43,6 +43,7 @@ func (h *authorizationHandler) authorize(c *gin.Context) {
// parameters are restored from the stored session so they never travel through the
// front channel.
interactionID := c.Query("interaction")
+ cancelled := c.Query("cancel") == "true"
if interactionID != "" {
query, err := h.authorizationService.interactionRequestQuery(ctx, interactionID)
if err != nil {
@@ -70,6 +71,18 @@ func (h *authorizationHandler) authorize(c *gin.Context) {
return
}
+ // Cancel on the interaction page ends the request with access_denied at the app, so the person is not stranded on Pocket ID
+ // It is a plain navigation like the resume link, because the page's form-action CSP would block a form post from redirecting to the app
+ if interactionID != "" && cancelled {
+ err = h.authorizationService.interactionSessionService.delete(ctx, interactionID)
+ if err != nil {
+ slog.WarnContext(ctx, "Failed to delete cancelled interaction session", "error", err.Error())
+ }
+ h.relaxCSPForFormPost(c, ar)
+ h.provider.WriteAuthorizeError(ctx, c.Writer, ar, fosite.ErrAccessDenied.WithHint("The user cancelled the request."))
+ return
+ }
+
authorization, err := h.authorizationService.authorize(ctx, authorizeInput{
userID: principal.UserID,
authenticationMethod: principal.AuthenticationMethod,
diff --git a/backend/internal/oidc/authorization_handler_test.go b/backend/internal/oidc/authorization_handler_test.go
index 9dbeb3df..223d40c3 100644
--- a/backend/internal/oidc/authorization_handler_test.go
+++ b/backend/internal/oidc/authorization_handler_test.go
@@ -256,3 +256,84 @@ func testAuthorizationHandlerPAR(t *testing.T, clientType string, tt authorizati
}
}
}
+
+func TestAuthorizationHandlerCancelsInteraction(t *testing.T) {
+ gin.SetMode(gin.TestMode)
+ const (
+ baseURL = "https://issuer.example.com"
+ callbackURL = "https://client.example.com/callback"
+ clientID = "cancel-client"
+ )
+ key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+ require.NoError(t, err)
+
+ for _, tt := range []struct {
+ name string
+ cancel bool
+ wantDenied bool
+ }{
+ {name: "cancel ends the request at the app", cancel: true, wantDenied: true},
+ {name: "resuming without cancel does not deny", cancel: false},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ db := testutils.NewDatabaseForTest(t)
+ require.NoError(t, db.Create(&model.OidcClient{
+ Base: model.Base{ID: clientID},
+ Name: "Cancel client",
+ CallbackURLs: datatype.StringList{callbackURL},
+ IsPublic: true,
+ PkceEnabled: true,
+ }).Error)
+ interaction := InteractionSession{
+ ClientID: clientID,
+ AuthenticationRequired: true,
+ RequestedAt: datatype.DateTime(time.Now()),
+ Parameters: InteractionSessionParameters{
+ "client_id": clientID,
+ "response_type": "code",
+ "redirect_uri": callbackURL,
+ "scope": "openid",
+ "state": "state-with-enough-entropy",
+ "code_challenge": "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
+ "code_challenge_method": "S256",
+ },
+ }
+ require.NoError(t, db.Create(&interaction).Error)
+
+ provider, err := newProvider(NewStore(db, nil), nil, testTokenSigner{key: key}, Config{
+ BaseURL: baseURL, TokenBaseURL: baseURL, Secret: []byte("test-secret"),
+ }, nil)
+ require.NoError(t, err)
+ service := newAuthorizationService(db, newInteractionSessionService(db), newClaimsService(db, nil, baseURL, nil, appconfig.StaticResolver{}), nil, nil, nil)
+ handler := newAuthorizationHandler(provider, service)
+
+ // A signed-out visitor can cancel too, since the sign-in step offers Cancel
+ target := "/authorize?interaction=" + interaction.ID
+ if tt.cancel {
+ target += "&cancel=true"
+ }
+ req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil)
+ rec := httptest.NewRecorder()
+ router := gin.New()
+ router.GET("/authorize", func(c *gin.Context) {
+ authz.SetPrincipal(c, &authz.Principal{})
+ handler.authorize(c)
+ })
+ router.ServeHTTP(rec, req)
+
+ location, err := url.Parse(rec.Header().Get("Location"))
+ require.NoError(t, err)
+ var remaining int64
+ require.NoError(t, db.Model(&InteractionSession{}).Where("id = ?", interaction.ID).Count(&remaining).Error)
+ if tt.wantDenied {
+ require.Equal(t, callbackURL, location.Scheme+"://"+location.Host+location.Path, rec.Body.String())
+ require.Equal(t, "access_denied", location.Query().Get("error"))
+ require.Equal(t, "state-with-enough-entropy", location.Query().Get("state"))
+ require.Zero(t, remaining)
+ } else {
+ require.NotEqual(t, "access_denied", location.Query().Get("error"))
+ require.EqualValues(t, 1, remaining)
+ }
+ })
+ }
+}
diff --git a/frontend/src/routes/interaction/+page.svelte b/frontend/src/routes/interaction/+page.svelte
index f8aaf9af..10d6d3f7 100644
--- a/frontend/src/routes/interaction/+page.svelte
+++ b/frontend/src/routes/interaction/+page.svelte
@@ -61,7 +61,6 @@
? 'verify'
: (currentStep ?? 'authenticate')
);
- const cancelHref = document.referrer || '/';
async function useDifferentAccount() {
await webauthnService.logout();
@@ -257,9 +256,7 @@
>
{m.cancel()}
- {/if}
-
-
+
{#if view === 'consent'}
{m.auth_remove_access_anytime()}
diff --git a/tests/specs/account-settings.spec.ts b/tests/specs/account-settings.spec.ts
index b6d175d5..5a9d5417 100644
--- a/tests/specs/account-settings.spec.ts
+++ b/tests/specs/account-settings.spec.ts
@@ -99,10 +99,11 @@ test('Change Locale', async ({ page }) => {
// Check if th language heading now says 'Taal' instead of 'Language'
await expect(page.getByText('Taal', { exact: true })).toBeVisible();
- // Check if the validation messages are translated because they are provided by Zod
- await page.getByRole('textbox', { name: 'Gebruikersnaam' }).fill('');
+ // Saving still validates in the new language
+ const username = page.getByRole('textbox', { name: 'Gebruikersnaam' });
+ await username.fill('');
await page.getByRole('button', { name: 'Opslaan' }).click();
- await expect(page.getByText('Te kort: verwacht dat string >=1 tekens heeft')).toBeVisible();
+ await expect(username).toHaveAttribute('aria-invalid', 'true');
// Clear all cookies and sign in again to check if the language is still set to Dutch
await page.context().clearCookies();
@@ -110,9 +111,9 @@ test('Change Locale', async ({ page }) => {
await expect(page.getByText('Taal', { exact: true })).toBeVisible();
- await page.getByRole('textbox', { name: 'Gebruikersnaam' }).fill('');
+ await username.fill('');
await page.getByRole('button', { name: 'Opslaan' }).click();
- await expect(page.getByText('Te kort: verwacht dat string >=1 tekens heeft')).toBeVisible();
+ await expect(username).toHaveAttribute('aria-invalid', 'true');
});
test('Add passkey to an account', async ({ page }) => {
diff --git a/tests/specs/oidc.spec.ts b/tests/specs/oidc.spec.ts
index 79178ba2..dfd6dfaa 100644
--- a/tests/specs/oidc.spec.ts
+++ b/tests/specs/oidc.spec.ts
@@ -103,6 +103,21 @@ test('Authorize new client', async ({ page }) => {
);
});
+test('Cancel on the consent screen returns access_denied to the client', async ({ page }) => {
+ const oidcClient = oidcClients.immich;
+ const urlParams = createUrlParams(oidcClient);
+ await page.goto(`/authorize?${urlParams.toString()}`);
+
+ await expectScopes(page, ['Email address', 'Name, username and picture']);
+
+ const callbackUrl = await expectCallbackRedirect(page, oidcClient.callbackUrl, () =>
+ page.getByRole('link', { name: 'Cancel', exact: true }).click()
+ );
+ expect(callbackUrl.searchParams.get('error')).toBe('access_denied');
+ expect(callbackUrl.searchParams.get('code')).toBeNull();
+ expect(callbackUrl.searchParams.get('state')).toBe(urlParams.get('state'));
+});
+
test('Authorize client requesting offline_access scope', async ({ page }) => {
const oidcClient = oidcClients.immich;
const urlParams = createUrlParams(oidcClient);