From b3f203c4ff35e33626f0b1327c7c41b57f7ffc7c Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 25 Oct 2026 18:01:50 +0200 Subject: [PATCH 1/7] test(frontend): expect one side margin for each filter control in the sheet --- .../filter-demo/filter-demo.locators.ts | 2 ++ .../features/filter-demo/filter-demo.spec.ts | 20 ++++++++++++++++++ .../features/filter-demo/filter-demo.tasks.ts | 21 +++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/frontend/tests/features/filter-demo/filter-demo.locators.ts b/frontend/tests/features/filter-demo/filter-demo.locators.ts index 3414f5d7..a59266bd 100644 --- a/frontend/tests/features/filter-demo/filter-demo.locators.ts +++ b/frontend/tests/features/filter-demo/filter-demo.locators.ts @@ -13,6 +13,8 @@ export const filterDemo = (page: Page) => { valueGroup: (propertyLabel: string) => control(propertyLabel).getByRole('group', { name: propertyLabel, exact: true }), controlLabel: (propertyLabel: string) => control(propertyLabel).getByText(propertyLabel, { exact: true }), button: (propertyLabel: string, name: string) => control(propertyLabel).getByRole('button', { name, exact: true }), + fields: (propertyLabel: string) => + control(propertyLabel).getByRole('button').or(control(propertyLabel).getByRole('textbox')), pressedButtons: (propertyLabel: string) => control(propertyLabel).getByRole('button', { pressed: true }), dateField: (propertyLabel: string, name: string) => control(propertyLabel).getByRole('group', { name, exact: true }), calendar: (propertyLabel: string) => control(propertyLabel).getByRole('grid'), diff --git a/frontend/tests/features/filter-demo/filter-demo.spec.ts b/frontend/tests/features/filter-demo/filter-demo.spec.ts index 72b715a9..5095ce91 100644 --- a/frontend/tests/features/filter-demo/filter-demo.spec.ts +++ b/frontend/tests/features/filter-demo/filter-demo.spec.ts @@ -21,6 +21,26 @@ test.describe('the Add Filter menu on a phone', () => { }) } + test('gives each control the same 16 px side margins in the sheet', async ({ page }) => { + const demo = filterDemoPage(page) + await demo.open() + const margins: Record = {} + + for (const property of ['First Name', 'Status', 'Created Date', 'Comment Count', 'Is Blocked']) { + await demo.chooseProperty(property) + margins[property] = await demo.sideMarginsOf(property) + await demo.closeControl(property) + } + + expect(margins).toEqual({ + 'First Name': { left: 16, right: 16 }, + Status: { left: 16, right: 16 }, + 'Created Date': { left: 16, right: 16 }, + 'Comment Count': { left: 16, right: 16 }, + 'Is Blocked': { left: 16, right: 16 }, + }) + }) + test('names the Is Blocked control only in the title of the sheet', async ({ page }) => { const demo = filterDemoPage(page) await demo.open() diff --git a/frontend/tests/features/filter-demo/filter-demo.tasks.ts b/frontend/tests/features/filter-demo/filter-demo.tasks.ts index dc457bcd..e5fcc658 100644 --- a/frontend/tests/features/filter-demo/filter-demo.tasks.ts +++ b/frontend/tests/features/filter-demo/filter-demo.tasks.ts @@ -25,6 +25,11 @@ export function filterDemoPage(page: Page) { await expect(at.loadingControl).toHaveCount(0) }, + async closeControl(propertyLabel: string) { + await at.button(propertyLabel, 'Close').click() + await expect(at.control(propertyLabel)).toBeHidden() + }, + async chooseBetween(propertyLabel: string) { await at.button(propertyLabel, 'Between').click() await expect(at.dateField(propertyLabel, 'Start date')).toBeVisible() @@ -91,6 +96,22 @@ export function filterDemoPage(page: Page) { return { left: box.x, right: box.x + box.width } }, + /** The space between each side of the sheet and the nearest button or text box of the control, without the Close button. */ + async sideMarginsOf(propertyLabel: string) { + const sheet = await at.control(propertyLabel).boundingBox() + const close = await at.button(propertyLabel, 'Close').boundingBox() + if (!sheet || !close) throw new Error('The sheet is not on the screen.') + let left = Infinity + let right = Infinity + for (const field of await at.fields(propertyLabel).all()) { + const box = await field.boundingBox() + if (!box || box.width === 0 || (box.x === close.x && box.y === close.y)) continue + left = Math.min(left, box.x - sheet.x) + right = Math.min(right, sheet.x + sheet.width - (box.x + box.width)) + } + return { left, right } + }, + async pageScrollWidth() { return page.evaluate(() => document.documentElement.scrollWidth) }, From 1387391ed7e62f14601c05dd67b60abc9ddb50f5 Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 25 Oct 2026 18:02:50 +0200 Subject: [PATCH 2/7] fix(frontend): let the filter dialog own the side padding of its control --- .../src/components/filter-builder/controls/filter-control.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/filter-builder/controls/filter-control.tsx b/frontend/src/components/filter-builder/controls/filter-control.tsx index 5f443596..fbaa3b34 100644 --- a/frontend/src/components/filter-builder/controls/filter-control.tsx +++ b/frontend/src/components/filter-builder/controls/filter-control.tsx @@ -54,7 +54,8 @@ export function FilterControlDialog({ }} className={FILTER_CONTROLS[config.controlType].dialogClassName ?? DEFAULT_DIALOG_CLASS_NAME} > -
+ {/* A control pads its sides for the submenu popover. The dialog pads its own body, so here the control does not. */} +
{initialFilter ? 'Edit' : 'Add'} {config.propertyLabel} Filter From f1cca602cccaa4ce3594de721dadc4e68ea41f83 Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 25 Oct 2026 18:03:20 +0200 Subject: [PATCH 3/7] test(frontend): expect a tall bottom sheet to stay clear of the toast at the top --- frontend/src/components/toaster.test.tsx | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/frontend/src/components/toaster.test.tsx b/frontend/src/components/toaster.test.tsx index dba63f15..56d2e28d 100644 --- a/frontend/src/components/toaster.test.tsx +++ b/frontend/src/components/toaster.test.tsx @@ -146,6 +146,33 @@ test.each(Object.entries(bottomPanels))( }, ) +test('on a phone on its side, the toast at the top does not cover a tall bottom sheet', async () => { + await page.viewport(667, 375) + await render( + <> + ( + + + Edit filter + +
+ + )} + /> + + , + ) + const dialog = page.getByRole('dialog', { name: 'Edit filter' }) + await expect.element(dialog).toBeVisible() + + Notification.error('Could not save the changes') + await expect.element(page.getByText('Could not save the changes')).toBeVisible() + await animationsFinished() + + expect(boxOf('Could not save the changes').bottom).toBeLessThanOrEqual(dialog.element().getBoundingClientRect().top) +}) + test('on a phone on its side, a toast that came while the pointer was on the toasts keeps waiting when a newer toast hides it', async () => { await page.viewport(667, 375) await render( From f6b6c7e1d243cd901f56ba35c21af32ccc895ad0 Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 25 Oct 2026 18:03:51 +0200 Subject: [PATCH 4/7] fix(frontend): keep a short bottom sheet below the toast at the top --- frontend/src/components/ui/responsive-dialog.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/ui/responsive-dialog.tsx b/frontend/src/components/ui/responsive-dialog.tsx index 580a2a45..f0068f3c 100644 --- a/frontend/src/components/ui/responsive-dialog.tsx +++ b/frontend/src/components/ui/responsive-dialog.tsx @@ -79,8 +79,10 @@ function ResponsiveDialog({ data-side={isMobile ? "bottom" : undefined} className={cn( "fixed z-50 w-full bg-background text-sm ring-1 ring-foreground/10 outline-none", - // Mobile: bottom sheet - "right-0 bottom-0 left-0 max-h-[85vh] rounded-t-xl", + // Mobile: bottom sheet. While it is open, index.css shows the toasts at the top: 1rem from the edge, and 3rem + // for one toast. The sheet stops 0.5rem below that, so the toast does not cover its drag handle. dvh is the visible + // height of a phone with its toolbar shown. vh is the height without the toolbar. + "right-0 bottom-0 left-0 max-h-[85vh] supports-[height:100dvh]:max-h-[min(85vh,calc(100dvh-4.5rem))] rounded-t-xl", "md:top-1/2 md:bottom-auto md:left-1/2 md:max-h-[calc(100vh-4rem)] md:max-w-md md:-translate-x-1/2 md:-translate-y-1/2 md:rounded-xl", className )} From f1a42ebab374dc930c11a9eb9c41c4445d7e653e Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 25 Oct 2026 18:10:42 +0200 Subject: [PATCH 5/7] test: expect /bff/user to answer an anonymous visitor with 204 --- .../weather/weather-forecast.locators.ts | 1 + .../features/weather/weather-forecast.spec.ts | 21 +++++++++++++++++++ .../weather/weather-forecast.tasks.ts | 5 +++++ frontend/tests/support/api/bff-api.ts | 11 ++++++++-- .../Integration/BackChannelLogoutTests.cs | 20 +++++++++--------- .../Integration/BffSessionTests.cs | 6 +++--- .../Integration/LoginEndpointTests.cs | 2 +- .../Integration/LoginHardeningTests.cs | 8 +++---- .../Integration/LogoutEndpointTests.cs | 4 ++-- .../Integration/ProviderFeatureTests.cs | 2 +- .../Integration/RefreshFailureTests.cs | 2 +- .../Integration/RefreshIdTokenTests.cs | 2 +- .../Integration/SpaRedirectTests.cs | 2 +- .../Integration/UserEndpointTests.cs | 4 ++-- 14 files changed, 62 insertions(+), 28 deletions(-) diff --git a/frontend/tests/features/weather/weather-forecast.locators.ts b/frontend/tests/features/weather/weather-forecast.locators.ts index 0266971e..36e4426a 100644 --- a/frontend/tests/features/weather/weather-forecast.locators.ts +++ b/frontend/tests/features/weather/weather-forecast.locators.ts @@ -17,6 +17,7 @@ export const weatherForecast = (page: Page) => { emptyMessage: page.getByText('No weather data available'), error: page.getByRole('alert').filter({ hasText: 'Error loading weather data' }), toast: (text: string) => page.getByRole('alertdialog').filter({ hasText: text }), + toasts: page.getByRole('alertdialog'), login: page.getByRole('button', { name: 'Login', exact: true }), logout: page.getByRole('button', { name: 'Logout', exact: true }), welcome: (name: string) => page.getByText(`Welcome, ${name}`, { exact: true }), diff --git a/frontend/tests/features/weather/weather-forecast.spec.ts b/frontend/tests/features/weather/weather-forecast.spec.ts index 3a60c0aa..dbfdc2e8 100644 --- a/frontend/tests/features/weather/weather-forecast.spec.ts +++ b/frontend/tests/features/weather/weather-forecast.spec.ts @@ -91,6 +91,27 @@ test.describe('an anonymous visitor', () => { expect(api.all('get', weatherPaths.secureList)).toHaveLength(0) }) + test('is a normal state: no error in the console and no toast', async ({ page, api }) => { + api.mock(weatherApi.listReturns(aForecast())) + + const weather = weatherForecastPage(page) + await weather.open() + + await expect(weather.at.login).toBeVisible() + expect(weather.consoleErrors).toEqual([]) + expect(await weather.at.toasts.count()).toBe(0) + }) + + test('still sees an error when the BFF fails to read the session', async ({ page, api }) => { + api.mock(weatherApi.listReturns(aForecast()), bffApi.userFails(aProblem().saying('The session store is down.'))) + + const weather = weatherForecastPage(page) + await weather.open() + + await expect(weather.at.login).toBeVisible() + expect(await weather.at.toast('The session store is down.').count()).toBe(1) + }) + test('goes to the BFF login with the page to come back to', async ({ page, api }) => { api.mock(weatherApi.listReturns(aForecast()), bffApi.loginOpens()) const weather = weatherForecastPage(page) diff --git a/frontend/tests/features/weather/weather-forecast.tasks.ts b/frontend/tests/features/weather/weather-forecast.tasks.ts index 26278395..1da509e8 100644 --- a/frontend/tests/features/weather/weather-forecast.tasks.ts +++ b/frontend/tests/features/weather/weather-forecast.tasks.ts @@ -5,12 +5,17 @@ import { weatherForecast } from './weather-forecast.locators' export function weatherForecastPage(page: Page) { const at = weatherForecast(page) + const consoleErrors: string[] = [] + page.on('console', (message) => { + if (message.type() === 'error') consoleErrors.push(message.text()) + }) // The exact path: `/api/v1/weather/secure` also starts with `/api/v1/weather`. const answered = (path: string) => page.waitForResponse((r) => r.request().method() === 'GET' && new URL(r.url()).pathname === path) return { at, + consoleErrors, async open() { const settled = Promise.all([answered(bffPaths.user), answered(weatherPaths.list)]) diff --git a/frontend/tests/support/api/bff-api.ts b/frontend/tests/support/api/bff-api.ts index eaed4728..d1c8223b 100644 --- a/frontend/tests/support/api/bff-api.ts +++ b/frontend/tests/support/api/bff-api.ts @@ -1,3 +1,4 @@ +import type { ProblemDetails } from '@/lib/problem-details' import type { Identity } from '../auth/actors' import type { MockRoute } from './mock-route' @@ -8,11 +9,17 @@ export const bffPaths = { } export const bffApi = { - /** The claims of the signed-in user, or a 401 for an anonymous visitor, as the BFF answers. */ + /** The claims of the signed-in user, or a 204 with no body for an anonymous visitor, as the BFF answers. */ userIs(identity: Identity): MockRoute { return identity.signedIn ? { method: 'get', path: bffPaths.user, status: 200, body: identity.claims } - : { method: 'get', path: bffPaths.user, status: 401 } + : { method: 'get', path: bffPaths.user, status: 204 } + }, + + /** The BFF fails to read the session. */ + userFails(problem: { build(): ProblemDetails }): MockRoute { + const body = problem.build() + return { method: 'get', path: bffPaths.user, status: body.status ?? 500, body } }, /** The SPA navigates to the login of the BFF. The mock answers with an empty page, not with the redirect to the IdP. */ diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/BackChannelLogoutTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/BackChannelLogoutTests.cs index 2929a2ab..80248c72 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/BackChannelLogoutTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/BackChannelLogoutTests.cs @@ -158,7 +158,7 @@ public async Task a_logout_token_with_a_sid_ends_only_the_sessions_of_that_sid() response.StatusCode.ShouldBe(HttpStatusCode.OK); (await app.WithStoreAsync(store => store.GetAsync(endedKey, CancellationToken.None))).ShouldBeNull(); - (await ended.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await ended.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); (await app.WithStoreAsync(store => store.GetAsync(otherKey, CancellationToken.None))).ShouldNotBeNull(); (await otherIdpSession.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.OK); } @@ -175,7 +175,7 @@ public async Task a_logout_token_with_only_a_sid_ends_the_session_of_that_sid() response.StatusCode.ShouldBe(HttpStatusCode.OK); (await app.WithStoreAsync(store => store.GetAsync(key, CancellationToken.None))).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] @@ -195,8 +195,8 @@ public async Task a_logout_token_with_only_a_sub_ends_all_the_sessions_of_that_s response.StatusCode.ShouldBe(HttpStatusCode.OK); (await app.WithStoreAsync(store => store.FindBySubjectAsync(subject, CancellationToken.None))).ShouldBeEmpty(); - (await first.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); - (await second.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await first.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); + (await second.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); (await otherUser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.OK); } @@ -215,8 +215,8 @@ public async Task a_back_channel_logout_sends_no_revocation_request_to_the_ident (await PostAsync(Token(c => { c["sub"] = subject; c.Remove("sid"); }))).StatusCode.ShouldBe(HttpStatusCode.OK); (await app.WithStoreAsync(store => store.FindBySubjectAsync(subject, CancellationToken.None))).ShouldBeEmpty(); - (await first.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); - (await second.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await first.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); + (await second.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); app.IdentityProvider.IssuedRefreshTokens(subject).Count.ShouldBe(2); app.IdentityProvider.WasRefreshTokenRevoked(subject).ShouldBeFalse(); } @@ -298,7 +298,7 @@ public async Task a_failed_write_of_the_jti_gets_400_and_ends_no_session() // The identity provider sends the token again to a BFF that can write the jti (await PostAsync(logoutToken)).StatusCode.ShouldBe(HttpStatusCode.OK); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] @@ -320,7 +320,7 @@ public async Task a_failed_delete_of_a_session_gets_500_and_lets_the_same_token_ // The identity provider sends the token again to a BFF that can delete the row (await PostAsync(logoutToken)).StatusCode.ShouldBe(HttpStatusCode.OK); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] @@ -344,8 +344,8 @@ public async Task a_request_that_aborts_during_the_logout_still_ends_all_the_ses await WaitUntilAsync(async () => (await app.WithStoreAsync(store => store.FindBySubjectAsync(subject, CancellationToken.None))).Count == 0); - (await first.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); - (await second.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await first.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); + (await second.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/BffSessionTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/BffSessionTests.cs index bbbaba89..2024c2ce 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/BffSessionTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/BffSessionTests.cs @@ -51,7 +51,7 @@ public async Task logout_deletes_the_row() (await browser.GetAsync("/bff/logout")).StatusCode.ShouldBe(HttpStatusCode.Redirect); (await app.WithStoreAsync(store => store.GetAsync(key, CancellationToken.None))).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] @@ -136,7 +136,7 @@ public async Task a_refresh_that_ends_after_logout_does_not_bring_the_session_ba issued.Count.ShouldBe(2); provider.RevokedTokens.ShouldContain(issued[0]); provider.RevokedTokens.ShouldContain(issued[1]); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] @@ -167,7 +167,7 @@ public async Task a_refresh_that_writes_between_the_read_and_the_delete_of_a_log var issued = provider.IssuedRefreshTokens(subject); issued.Count.ShouldBe(2); provider.RevokedTokens.ShouldContain(issued[1]); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/LoginEndpointTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/LoginEndpointTests.cs index 0d04d57b..af14a2de 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/LoginEndpointTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/LoginEndpointTests.cs @@ -187,7 +187,7 @@ public async Task an_error_for_a_prompt_login_redirects_to_the_return_url_with_n callback.StatusCode.ShouldBe(HttpStatusCode.Redirect); callback.Headers.Location!.OriginalString.ShouldBe($"{BffApp.SpaDevServer}/back"); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } public static async Task ShouldBeProblemAsync(HttpResponseMessage response, string title, string field) diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/LoginHardeningTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/LoginHardeningTests.cs index c588fec9..e577e922 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/LoginHardeningTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/LoginHardeningTests.cs @@ -35,7 +35,7 @@ public async Task a_callback_with_a_wrong_or_missing_iss_gets_400_problem_and_no await LoginEndpointTests.ShouldBeProblemAsync(callback, CallbackRejectedException.Title, "iss"); BffApp.SessionCookieValue(callback).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); // The BFF does not redeem the code of a response that it does not trust BffApp.Instance.IdentityProvider.TokenRequests.ShouldNotContain(r => r.Subject == subject); } @@ -94,7 +94,7 @@ public async Task an_unsigned_id_token_gets_400_problem_and_no_session() await LoginEndpointTests.ShouldBeProblemAsync(callback, CallbackRejectedException.Title, IdTokenChecks.AlgorithmHeader); BffApp.SessionCookieValue(callback).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] @@ -106,7 +106,7 @@ public async Task an_id_token_with_another_audience_gets_400_problem_and_no_sess await LoginEndpointTests.ShouldBeProblemAsync(callback, CallbackRejectedException.Title, IdTokenChecks.AudienceClaim); BffApp.SessionCookieValue(callback).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } /// @@ -125,7 +125,7 @@ public async Task an_id_token_that_fails_a_token_response_check_gets_400_problem await LoginEndpointTests.ShouldBeProblemAsync(callback, CallbackRejectedException.Title, CallbackRejection.CallbackField); BffApp.SessionCookieValue(callback).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } /// diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/LogoutEndpointTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/LogoutEndpointTests.cs index 5b8af28c..4b756817 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/LogoutEndpointTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/LogoutEndpointTests.cs @@ -38,7 +38,7 @@ public async Task logout_with_the_sid_revokes_the_refresh_token_ends_the_session provider.WasRefreshTokenRevoked("user-logout").ShouldBeTrue(); response.Headers.GetValues("Set-Cookie").ShouldContain(c => c.StartsWith("__Host-bff=;", StringComparison.Ordinal)); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] @@ -59,7 +59,7 @@ public async Task a_revocation_error_does_not_stop_the_logout(string revokeMode) provider.WasRefreshTokenRevoked(subject).ShouldBeTrue(); (await app.WithStoreAsync(store => store.GetAsync(key, CancellationToken.None))).ShouldBeNull(); response.Headers.GetValues("Set-Cookie").ShouldContain(c => c.StartsWith("__Host-bff=;", StringComparison.Ordinal)); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/ProviderFeatureTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/ProviderFeatureTests.cs index 61f81efe..e7d72757 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/ProviderFeatureTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/ProviderFeatureTests.cs @@ -165,7 +165,7 @@ public async Task without_end_session_and_revocation_logout_ends_the_session_and logout.Headers.Location!.OriginalString.ShouldBe($"{BffApp.SpaDevServer}/bye"); logout.Headers.GetValues("Set-Cookie").ShouldContain(c => c.StartsWith("__Host-bff=;", StringComparison.Ordinal)); (await _minimalBff.WithStoreAsync(store => store.GetAsync(key, CancellationToken.None))).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); _minimalProvider.RevokedTokens.ShouldBeEmpty(); } diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/RefreshFailureTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/RefreshFailureTests.cs index a3c2e172..385fc5e3 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/RefreshFailureTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/RefreshFailureTests.cs @@ -25,7 +25,7 @@ public async Task an_invalid_grant_ends_the_session_and_the_request_goes_with_no (await response.ReadEchoAsync()).Headers.ShouldNotContainKey("Authorization"); BffApp.SessionCookieValue(response).ShouldBe(""); (await app.WithStoreAsync(store => store.GetAsync(key, CancellationToken.None))).ShouldBeNull(); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/RefreshIdTokenTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/RefreshIdTokenTests.cs index 54543c23..de76fa1d 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/RefreshIdTokenTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/RefreshIdTokenTests.cs @@ -75,7 +75,7 @@ public async Task an_invalid_id_token_in_the_refresh_response_ends_the_session_a BffApp.SessionCookieValue(response).ShouldBe(""); (await app.WithStoreAsync(store => store.GetAsync(key, CancellationToken.None))).ShouldBeNull(); provider.RevokedTokens.ShouldContain(provider.IssuedRefreshTokens(subject)[1]); - (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + (await browser.GetApiAsync("/bff/user")).StatusCode.ShouldBe(HttpStatusCode.NoContent); } [Test] diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/SpaRedirectTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/SpaRedirectTests.cs index 33c9a74f..4d05d338 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/SpaRedirectTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/SpaRedirectTests.cs @@ -152,7 +152,7 @@ public async Task a_dev_or_published_bff_allows_no_cross_origin_request_from_the var response = await browser.SendAsync(request); preflightResponse.Headers.Contains("Access-Control-Allow-Origin").ShouldBeFalse(); - response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + response.StatusCode.ShouldBe(HttpStatusCode.NoContent); response.Headers.Contains("Access-Control-Allow-Origin").ShouldBeFalse(); response.Headers.Contains("Access-Control-Allow-Credentials").ShouldBeFalse(); } diff --git a/tests/FullstackTemplate.Bff.Tests/Integration/UserEndpointTests.cs b/tests/FullstackTemplate.Bff.Tests/Integration/UserEndpointTests.cs index 89144e51..820e7db8 100644 --- a/tests/FullstackTemplate.Bff.Tests/Integration/UserEndpointTests.cs +++ b/tests/FullstackTemplate.Bff.Tests/Integration/UserEndpointTests.cs @@ -7,13 +7,13 @@ namespace FullstackTemplate.Bff.Tests.Integration; public class UserEndpointTests { [Test] - public async Task an_anonymous_request_gets_401_with_an_empty_body() + public async Task an_anonymous_request_gets_204_with_an_empty_body() { using var browser = BffApp.Instance.CreateBrowser(); var response = await browser.GetApiAsync("/bff/user"); - response.StatusCode.ShouldBe(HttpStatusCode.Unauthorized); + response.StatusCode.ShouldBe(HttpStatusCode.NoContent); (await response.Content.ReadAsStringAsync()).ShouldBeEmpty(); } From 7e5ac40d710b6af1728484ff1a4da54f31b2585c Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 25 Oct 2026 18:14:10 +0200 Subject: [PATCH 6/7] fix(bff): answer an anonymous /bff/user with 204, not 401 --- .agents/skills/verify-fullstack-template/SKILL.md | 4 ++-- .../verify-fullstack-template/features/login-logout.md | 8 ++++---- .../verify-fullstack-template/recipes/login-logout.mjs | 8 ++++---- .../skills/verify-fullstack-template/scripts/doctor.sh | 2 +- FullstackTemplate.Bff/User/UserEndpoint.cs | 5 +++-- docs/authentication.md | 4 ++-- docs/frontend/testing.md | 2 +- docs/specs/home-rolled-bff.md | 9 +++++---- frontend/conformance/run-conformance.ts | 2 +- frontend/e2e/auth-flows.spec.ts | 2 +- frontend/src/domain/auth/apis/get-user.ts | 10 ++++------ 11 files changed, 28 insertions(+), 28 deletions(-) diff --git a/.agents/skills/verify-fullstack-template/SKILL.md b/.agents/skills/verify-fullstack-template/SKILL.md index 6b1409cd..9d2b3f5d 100644 --- a/.agents/skills/verify-fullstack-template/SKILL.md +++ b/.agents/skills/verify-fullstack-template/SKILL.md @@ -60,7 +60,7 @@ Each line starts with `ok` or `FAIL`. Exit code 0 means the stack is worth drivi - The current run and its `stack.json` exist. - The AppHost pid of this checkout is the pid that `launch.sh` recorded. Thus the stack is ours. - Each resource is `Running` and `Healthy`. -- The web URL is unchanged, `/api/v1/weather` is 200, `/bff/user` is 401 or 200, and the server `/health` is 200. +- The web URL is unchanged, `/api/v1/weather` is 200, `/bff/user` is 204 or 200, and the server `/health` is 200. - If the stack uses `DuendeDemo`, the Duende demo IdP is reachable. If it is not, no login recipe can pass. A local provider, such as ZeroFactor, is a resource of the stack, so the health check above examines it. - `HEAD` is the commit of the launch. If it moved, the doctor prints `warn`. Frontend code hot-reloads. For server or BFF code, run `dotnet aspire resource server rebuild --apphost FullstackTemplate.AppHost` (or `bff`). @@ -144,7 +144,7 @@ Proof standards: - Capture the action and the result. A recipe needs a capture before and after the change of state, not only the last screen. - Prove side effects next to the visible result. Examples: the HTTP status and the JSON of the API response, the `__Host-bff` cookie, the `ui-theme` value in `localStorage`. - Compare the screen with the data. For example, each weather card must show the `summary` and the temperature of the API row, not only "5 cards". -- Read `console.log` and `network.log` after each run. These lines are normal: one `401 /bff/user` for each anonymous page load, the 401 lines from the anonymous probes that a recipe makes on purpose, and, with DuendeDemo, the iframe `sandbox` warning from the Duende pages. Report each other error, even if the recipe passed. +- Read `console.log` and `network.log` after each run. These lines are normal: the 401 lines from the anonymous probes that a recipe makes on purpose, and, with DuendeDemo, the iframe `sandbox` warning from the Duende pages. Report each other error, even if the recipe passed. - Look at the screenshots. If something looks wrong (clipped, misaligned, unreadable in dark mode), report it, even if it is not part of your change. - Do not mock the app. ZeroFactor is a mock IdP, but it gives real OIDC tokens. The BFF and the server validate them as they validate the tokens of any other provider. The Duende demo IdP is the only external system. diff --git a/.agents/skills/verify-fullstack-template/features/login-logout.md b/.agents/skills/verify-fullstack-template/features/login-logout.md index ddc01f6b..6d78df79 100644 --- a/.agents/skills/verify-fullstack-template/features/login-logout.md +++ b/.agents/skills/verify-fullstack-template/features/login-logout.md @@ -4,7 +4,7 @@ A visitor logs in with the `Login` button. The BFF sends the browser to the IdP ## Sub-features -- `auth-anonymous` shows `Login`, and `GET /bff/user` is 401. +- `auth-anonymous` shows `Login`, and `GET /bff/user` is 204. - `auth-login` goes through `/bff/login`, the login page of the IdP, and `/signin-oidc`, back to `/`. - `auth-session` shows `Welcome, ` and `Logout`. The session survives a reload. - `auth-logout` goes through the BFF logout and the IdP logout, back to `/` with `Login`. @@ -22,12 +22,12 @@ Preconditions: - `doctor.sh` exits with 0. - The browser context is new, so nobody is logged in. -- **Anonymous state.** Open `/`. Run `page.goto('/')` after `waitForResponse` on `/bff/user`. The response is 401 and the button `Login` is visible. +- **Anonymous state.** Open `/`. Run `page.goto('/')` after `waitForResponse` on `/bff/user`. The response is 204 and the button `Login` is visible. - **Log in.** Choose `Login`, then sign in at the IdP. Run `const { name } = await login()`. The helper clicks `getByRole('button', { name: 'Login' })`. At ZeroFactor, it clicks the user `[data-testid="user-admin@example.com"]`. At DuendeDemo, it fills `#Input_Username` and `#Input_Password` with `bob` and clicks `button[value="login"]`. The browser returns to the `web` origin, and the button `Logout` is visible. `name` is the `name` claim from `GET /bff/user`. - **Session.** Read the greeting. Run ``page.getByText(`Welcome, ${name}`)``. The greeting is visible. `context.cookies()` contains `__Host-bff`. - **Reload.** Run `page.reload()`. The greeting is visible again. - **Log out.** Choose `Logout`. Run `logout()`. The helper clicks `Logout`. ZeroFactor sends the browser back at once. At DuendeDemo, the helper clicks the link `a[href*="signout-callback-oidc"]`. The button `Login` is visible on `/`. -- **Logged-out state.** Run `fetch('/bff/user', { headers: { 'X-CSRF': '1' } })` in the page. The status is 401. +- **Logged-out state.** Run `fetch('/bff/user', { headers: { 'X-CSRF': '1' } })` in the page. The status is 204. - **Proof.** Run `node $S/scripts/browse.mjs $S/recipes/login-logout.mjs`. The captures `anonymous`, `logged-in`, and `logged-out` show the three states. ## Gotchas @@ -35,7 +35,7 @@ Preconditions: - With DuendeDemo, the IdP is a public server. If it is down, login fails. The doctor examines it. - The Duende demo server sometimes sends its CSS and scripts very slowly (17 to 29 s were seen). Thus the page `load` event can take more than 30 s. In a new login script, wait for `domcontentloaded` and for `#Input_Username`, never for `load`. `login()` does this. - Each call to `/bff/*` and `/api/*` needs the header `X-CSRF: 1`. Without it, the BFF answers 401 even with a valid session. -- Each anonymous page load logs a `401 /bff/user` line in `network.log` and `console.log`. This is normal. +- An anonymous visitor is a normal state. `/bff/user` answers 204, so `network.log` and `console.log` get no line for it. - The greeting shows the `name` claim (`Admin User`, or `Bob Smith` at DuendeDemo), not the user name. - Duende shows a "return to the application" page after logout. If the recipe does not follow its link, the page stays on the IdP. - `launch.sh` sets the provider, whatever the default in `appsettings.json` is. ZeroFactor is the default. `login()` supports only ZeroFactor and DuendeDemo. Other providers use other forms and users. See `frontend/e2e/idps/` for their selectors. diff --git a/.agents/skills/verify-fullstack-template/recipes/login-logout.mjs b/.agents/skills/verify-fullstack-template/recipes/login-logout.mjs index 2fd827aa..774a2093 100644 --- a/.agents/skills/verify-fullstack-template/recipes/login-logout.mjs +++ b/.agents/skills/verify-fullstack-template/recipes/login-logout.mjs @@ -1,9 +1,9 @@ // Feature: login-logout. BFF login through the IdP of the stack. See features/login-logout.md. export default async ({ page, context, step, capture, expect, login, logout }) => { - await step('anonymous: /bff/user is 401 and the page shows Login', async () => { + await step('anonymous: /bff/user is 204 and the page shows Login', async () => { const res = page.waitForResponse((r) => r.url().endsWith('/bff/user')) await page.goto('/') - expect((await res).status()).toBe(401) + expect((await res).status()).toBe(204) await expect(page.getByRole('button', { name: 'Login' })).toBeVisible() }) await capture('anonymous') @@ -24,9 +24,9 @@ export default async ({ page, context, step, capture, expect, login, logout }) = await step('log out', () => logout()) - await step('logged out: /bff/user is 401 again', async () => { + await step('logged out: /bff/user is 204 again', async () => { const status = await page.evaluate(async () => (await fetch('/bff/user', { headers: { 'X-CSRF': '1' } })).status) - expect(status).toBe(401) + expect(status).toBe(204) await expect(page.getByText(`Welcome, ${name}`)).toHaveCount(0) }) await capture('logged-out') diff --git a/.agents/skills/verify-fullstack-template/scripts/doctor.sh b/.agents/skills/verify-fullstack-template/scripts/doctor.sh index 1c698d54..f9ae3cc1 100755 --- a/.agents/skills/verify-fullstack-template/scripts/doctor.sh +++ b/.agents/skills/verify-fullstack-template/scripts/doctor.sh @@ -41,7 +41,7 @@ server="$(jq -r .urls.server "$stack")" code="$(curl -s -o /dev/null -w '%{http_code}' -H 'X-CSRF: 1' "$web/api/v1/weather")" [[ "$code" == "200" ]] && ok "GET $web/api/v1/weather -> 200" || fail "GET $web/api/v1/weather -> $code" code="$(curl -s -o /dev/null -w '%{http_code}' -H 'X-CSRF: 1' "$web/bff/user")" -[[ "$code" == "401" || "$code" == "200" ]] && ok "GET /bff/user -> $code" || fail "GET /bff/user -> $code" +[[ "$code" == "204" || "$code" == "200" ]] && ok "GET /bff/user -> $code" || fail "GET /bff/user -> $code" code="$(curl -sk -o /dev/null -w '%{http_code}' "$server/health")" [[ "$code" == "200" ]] && ok "GET $server/health -> 200" || fail "GET $server/health -> $code" diff --git a/FullstackTemplate.Bff/User/UserEndpoint.cs b/FullstackTemplate.Bff/User/UserEndpoint.cs index 2d206dc0..a83bc5d1 100644 --- a/FullstackTemplate.Bff/User/UserEndpoint.cs +++ b/FullstackTemplate.Bff/User/UserEndpoint.cs @@ -9,7 +9,8 @@ namespace FullstackTemplate.Bff.User; /// /// /bff/user returns the claims of the BFF session to the SPA. It is a BFF API endpoint, so it needs -/// X-CSRF: 1. An anonymous request gets a 401 with an empty body. +/// X-CSRF: 1. An anonymous request gets a 204 with an empty body. A signed-out visitor is a normal state, and a +/// browser logs each 4xx response as an error in the console. /// public static class UserEndpoint { @@ -27,7 +28,7 @@ public static async Task HandleAsync(HttpContext context, TimeProvider timeProvi var result = await context.AuthenticateAsync(CookieAuthenticationDefaults.AuthenticationScheme); if (!result.Succeeded) { - context.Response.StatusCode = StatusCodes.Status401Unauthorized; + context.Response.StatusCode = StatusCodes.Status204NoContent; return; } diff --git a/docs/authentication.md b/docs/authentication.md index 777d7c80..8e6eacf4 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -10,7 +10,7 @@ The design is in [docs/specs/home-rolled-bff.md](specs/home-rolled-bff.md). The |---|---|---|---| | `/bff/login` | any | no | Starts a login | | `/bff/logout` | GET | no | Ends the BFF session and starts the logout at the provider | -| `/bff/user` | GET | yes | The claims of the BFF session, or 401 | +| `/bff/user` | GET | yes | The claims of the BFF session, or 204 when no one is signed in | | `/bff/backchannel` | POST | no | The back-channel logout receiver. Only the provider calls it. | | `/signin-oidc` | GET | no | The login callback | | `/signout-callback-oidc` | GET | no | The return from the logout at the provider | @@ -330,7 +330,7 @@ The script also gives the tests the client id and the client secret of the BFF, - The refresh rotation test uses the client of the BFF for its own code flow. It proves that the provider rejects the old refresh token after one refresh. It runs if `sessionFeatures.refreshRotation` is true in the IdP file: Keycloak, FusionAuth, and Authentik. - The BFF refresh test makes two API calls through the BFF, and each call refreshes. The second call succeeds only if the BFF stored the rotated refresh token of the first call. Then the test logs out, and the `id_token_hint` proves that the BFF stored the ID token of the second refresh. It runs if `sessionFeatures.refreshRotation` is true in the IdP file: Keycloak, FusionAuth, and Authentik. On a provider that does not reject a reused refresh token, the test cannot prove the rotation. - The refresh lock test makes two API calls through the BFF at the same time, and both need the same refresh. Both calls must succeed, and the BFF session must still work after them. With no refresh lock, both calls send the same refresh token, and the provider rejects the second one. It runs on the same providers as the BFF refresh test. -- The back-channel logout test ends the session at the provider through its admin API, and expects a 401 from `/bff/user`. It runs if `sessionFeatures.backChannelLogoutReachesBff` is true in the IdP file: on Keycloak, with the bootstrap admin, and on Authentik, with the `authentik-bootstrap-token` parameter of the AppHost. Authentik creates this token only on the first start of its database. +- The back-channel logout test ends the session at the provider through its admin API, and expects a 204 from `/bff/user`. It runs if `sessionFeatures.backChannelLogoutReachesBff` is true in the IdP file: on Keycloak, with the bootstrap admin, and on Authentik, with the `authentik-bootstrap-token` parameter of the AppHost. Authentik creates this token only on the first start of its database. ## Conformance diff --git a/docs/frontend/testing.md b/docs/frontend/testing.md index f6b73fc1..8fa650f0 100644 --- a/docs/frontend/testing.md +++ b/docs/frontend/testing.md @@ -54,7 +54,7 @@ An error body must have the shape that ASP.NET sends. Make it with `aProblem()`. ### Identity -The SPA knows the user only from `GET /bff/user`. The `actor` fixture gives the claims of that answer, so a test never drives a login form. The default actor is anonymous, and `/bff/user` answers 401. To act as a signed-in user, add this line to a `describe` block: +The SPA knows the user only from `GET /bff/user`. The `actor` fixture gives the claims of that answer, so a test never drives a login form. The default actor is anonymous, and `/bff/user` answers 204. To act as a signed-in user, add this line to a `describe` block: ```ts test.use({ actor: as.user('Bob Smith') }) diff --git a/docs/specs/home-rolled-bff.md b/docs/specs/home-rolled-bff.md index 89fe8c9c..579bdca6 100644 --- a/docs/specs/home-rolled-bff.md +++ b/docs/specs/home-rolled-bff.md @@ -113,7 +113,7 @@ These paths go away on purpose ([Duende BFF features to drop](https://github.com ### `/bff/user` - Without `X-CSRF: 1`, the response is 401 with an empty body. -- Anonymous: 401 with an empty body. +- Anonymous: 204 with an empty body. A browser logs each 4xx response as a console error, and a signed-out visitor is a normal state. - Signed in: 200 `application/json`. The body is an array of `{ "type": string, "value": string, "valueType": null }`. The array has these items, in this order: 1. Each claim of the BFF session principal, in the stored order. 2. `bff:logout_url`, if the principal does not have it: `PathBase + /bff/logout`, plus `?sid=` if a `sid` claim exists. @@ -401,7 +401,7 @@ If a check fails, the refresh fails with the error `bff_invalid_refresh_id_token The checks and the revocation do not use the cancellation of `Bff:Refresh:TokenRequestTimeout`, because that cancellation gives a temporary failure and keeps the BFF session. Each gets its own limit of `Bff:Refresh:TokenRequestTimeout` instead. If the signing keys do not load within the limit, the check fails. A revocation that fails or reaches its limit is logged, and the refresh still fails with `bff_invalid_refresh_id_token`. -Each failed refresh adds to `bff.refresh.failures` with `kind=permanent` or `kind=temporary`. After a permanent failure, the next `/bff/user` gives a 401, so the SPA shows the user as signed out. The SPA does not change. The refresh has no retry inside the call. The next request that needs a token tries again. A retry inside the call can send a rotated refresh token two times, after the identity provider used it but the response got lost, and then the session ends. A retry also holds the refresh lock and its `bffdb` connection longer. The refresh goes through the backchannel `HttpClient` of the OIDC handler, which does not come from `IHttpClientFactory`, so the standard resilience handler of the service defaults does not apply to it. +Each failed refresh adds to `bff.refresh.failures` with `kind=permanent` or `kind=temporary`. After a permanent failure, the next `/bff/user` gives a 204, so the SPA shows the user as signed out. The SPA does not change. The refresh has no retry inside the call. The next request that needs a token tries again. A retry inside the call can send a rotated refresh token two times, after the identity provider used it but the response got lost, and then the session ends. A retry also holds the refresh lock and its `bffdb` connection longer. The refresh goes through the backchannel `HttpClient` of the OIDC handler, which does not come from `IHttpClientFactory`, so the standard resilience handler of the service defaults does not apply to it. **The refresh lock.** Our own `IUserTokenRequestConcurrencyControl` replaces the default. It has two layers: @@ -491,7 +491,8 @@ Each behavior of the current BFF, from [the inventory](https://github.com/pdevit | 6 | `/bff/logout`: local `returnUrl` | Kept | Open-redirect defense | 3.4 | | 7 | `/bff/logout`: refresh token revocation | Changed | `Auth:Features:Revocation` replaces `RevokeRefreshTokenOnLogout`. A missing endpoint is safe. [#18](https://github.com/pdevito3/FullstackTemplate/issues/18) | 3.3, 3.4 | | 8 | `/bff/logout`: RP-initiated logout with `id_token_hint` | Kept | A missing `end_session_endpoint` now falls back to the local `returnUrl` | 3.4 | -| 9 | `/bff/user`: `X-CSRF`, 401 empty body when anonymous | Kept | Wire contract | 2 | +| 9 | `/bff/user`: `X-CSRF`, 401 empty body without it | Kept | Wire contract | 2 | +| 9a | `/bff/user`: 204 empty body when anonymous, not 401 | Changed | A browser logs each 4xx response as a console error, and a signed-out visitor is a normal state | 2 | | 10 | `/bff/user`: claim array, `bff:logout_url`, `bff:session_state` | Kept | Wire contract | 2 | | 11 | `/bff/user`: `bff:session_expires_in` as a number | Changed | It now also reflects the absolute limit | 2, 3.1 | | 12 | `/bff/user?slide=false` | Kept | [#22](https://github.com/pdevito3/FullstackTemplate/issues/22) | 2 | @@ -621,7 +622,7 @@ These tests come from the tickets. Each one is in addition to the tests of secti | Unit | Each `Auth:Features:PushedAuthorization` value maps to the correct `PushedAuthorizationBehavior`. | [#13](https://github.com/pdevito3/FullstackTemplate/issues/13) | | Integration | A forged `X-Forwarded-Host` on `/bff/login` still gives the configured `redirect_uri`. | [#16](https://github.com/pdevito3/FullstackTemplate/issues/16) | | Integration | A `Host` that is not on the list gets a 400. | [#16](https://github.com/pdevito3/FullstackTemplate/issues/16) | -| Integration | `invalid_grant`: the BFF session row is gone, the request goes with no token, and `/bff/user` returns 401. | [#20](https://github.com/pdevito3/FullstackTemplate/issues/20) | +| Integration | `invalid_grant`: the BFF session row is gone, the request goes with no token, and `/bff/user` returns 204. | [#20](https://github.com/pdevito3/FullstackTemplate/issues/20) | | Integration | A 503 while the token is still valid: the request goes with the current token, and the BFF session stays. | [#20](https://github.com/pdevito3/FullstackTemplate/issues/20) | | Integration | A 503 after the token expired: the request goes with no token, the BFF session stays, and the next call tries the refresh again. | [#20](https://github.com/pdevito3/FullstackTemplate/issues/20) | | Integration | A browser `Authorization` header never reaches the API, for an anonymous request too. | [#21](https://github.com/pdevito3/FullstackTemplate/issues/21) | diff --git a/frontend/conformance/run-conformance.ts b/frontend/conformance/run-conformance.ts index 6cc04b9b..f0e73515 100644 --- a/frontend/conformance/run-conformance.ts +++ b/frontend/conformance/run-conformance.ts @@ -210,7 +210,7 @@ async function drive(browser: Browser, bff: string, flow: Flow, screenshot: stri } const user = async () => { const response = await context.request.get(`${bff}/bff/user`, { headers: { 'X-CSRF': '1' } }) - return { status: response.status(), claims: response.ok() ? ((await response.json()) as { type: string; value: string }[]) : [] } + return { status: response.status(), claims: response.status() === 200 ? ((await response.json()) as { type: string; value: string }[]) : [] } } await step('login', login) diff --git a/frontend/e2e/auth-flows.spec.ts b/frontend/e2e/auth-flows.spec.ts index 41090dbf..c2f8e594 100644 --- a/frontend/e2e/auth-flows.spec.ts +++ b/frontend/e2e/auth-flows.spec.ts @@ -591,7 +591,7 @@ test.describe(`Back-channel logout - ${currentIdp.name}`, () => { } // The BFF deletes the session row when the logout token arrives. Authentik sends it from a worker task. - await expect.poll(() => bffUserStatus(page), { timeout: 30000 }).toBe(401) + await expect.poll(() => bffUserStatus(page), { timeout: 30000 }).toBe(204) }) }) diff --git a/frontend/src/domain/auth/apis/get-user.ts b/frontend/src/domain/auth/apis/get-user.ts index 0faa82a3..536c85eb 100644 --- a/frontend/src/domain/auth/apis/get-user.ts +++ b/frontend/src/domain/auth/apis/get-user.ts @@ -4,14 +4,12 @@ import type { AuthState, BffClaim } from '../types' import { AuthKeys } from './auth.keys' /** - * Fetch user claims from BFF. Returns null when nobody is signed in: the BFF answers 401 for an - * anonymous visitor, and that is a normal state, not an error to show in a toast. + * Fetch user claims from BFF. Returns null when nobody is signed in: the BFF answers 204 for an + * anonymous visitor. A 4xx or 5xx status is an error, and apiClient shows it in a toast. */ export async function getUser(): Promise { - const response = await apiClient.get('/bff/user', { - validateStatus: (status) => status === 200 || status === 401, - }) - return response.status === 401 ? null : response.data + const response = await apiClient.get('/bff/user') + return response.status === 204 ? null : response.data } /** From 8dbc29779921df69c78f220b46d8dc26f8042482 Mon Sep 17 00:00:00 2001 From: Paul DeVito Date: Sun, 25 Oct 2026 19:37:05 +0200 Subject: [PATCH 7/7] fix(frontend): retry a failed user query once, as other queries do The query skipped retries for a 401 or 403. /bff/user now answers a signed-out visitor with 204, so each error left is a real failure, and one retry keeps a short 5xx from showing a signed-in user as signed out. The spec now marks the 204 as a change from Duende BFF. --- docs/specs/home-rolled-bff.md | 2 +- frontend/src/domain/auth/apis/get-user.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/specs/home-rolled-bff.md b/docs/specs/home-rolled-bff.md index 579bdca6..468f5144 100644 --- a/docs/specs/home-rolled-bff.md +++ b/docs/specs/home-rolled-bff.md @@ -113,7 +113,7 @@ These paths go away on purpose ([Duende BFF features to drop](https://github.com ### `/bff/user` - Without `X-CSRF: 1`, the response is 401 with an empty body. -- Anonymous: 204 with an empty body. A browser logs each 4xx response as a console error, and a signed-out visitor is a normal state. +- Anonymous: 204 with an empty body. Change: Duende BFF answered 401. A browser logs each 4xx response as a console error, and a signed-out visitor is a normal state. - Signed in: 200 `application/json`. The body is an array of `{ "type": string, "value": string, "valueType": null }`. The array has these items, in this order: 1. Each claim of the BFF session principal, in the stored order. 2. `bff:logout_url`, if the principal does not have it: `PathBase + /bff/logout`, plus `?sid=` if a `sid` claim exists. diff --git a/frontend/src/domain/auth/apis/get-user.ts b/frontend/src/domain/auth/apis/get-user.ts index 536c85eb..54d71d27 100644 --- a/frontend/src/domain/auth/apis/get-user.ts +++ b/frontend/src/domain/auth/apis/get-user.ts @@ -75,7 +75,6 @@ export function useAuth(): AuthState { const { data, isLoading, isError } = useQuery({ queryKey: AuthKeys.user(), queryFn: getUser, - retry: false, // Don't retry on 401/403 staleTime: 5 * 60 * 1000, // Consider auth data fresh for 5 minutes })