From b2ac4f4398b78ad5ea4dcc82c4d48672165ca407 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 14 May 2024 11:03:47 -0500 Subject: [PATCH 1/2] add logout button to error page --- app/components/ErrorBoundary.tsx | 22 +++++++++++++++++++++- app/components/TopBar.tsx | 7 +------ mock-api/msw/handlers.ts | 7 +++++-- mock-api/msw/util.ts | 3 +++ test/e2e/error-pages.e2e.ts | 11 +++++++++++ 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/app/components/ErrorBoundary.tsx b/app/components/ErrorBoundary.tsx index f99e9de78a..95b6cf9544 100644 --- a/app/components/ErrorBoundary.tsx +++ b/app/components/ErrorBoundary.tsx @@ -8,7 +8,10 @@ import { ErrorBoundary as BaseErrorBoundary } from 'react-error-boundary' import { useRouteError } from 'react-router-dom' -import type { ApiError } from '@oxide/api' +import { useApiMutation } from '~/api/client' +import { type ApiError } from '~/api/errors' +import { navToLogin } from '~/api/nav-to-login' +import { Button } from '~/ui/lib/Button' import { ErrorPage, NotFound } from './ErrorPage' @@ -27,6 +30,7 @@ function ErrorFallback({ error }: Props) {

Please try again. If the problem persists, contact your administrator.

+ ) } @@ -40,3 +44,19 @@ export function RouterDataErrorBoundary() { const error = useRouteError() as Props['error'] return } + +export function SignOutButton({ className }: { className?: string }) { + const logout = useApiMutation('logout', { + onSuccess: () => navToLogin({ includeCurrent: false }), + }) + return ( + + ) +} diff --git a/app/components/TopBar.tsx b/app/components/TopBar.tsx index e9fd48a283..c119712917 100644 --- a/app/components/TopBar.tsx +++ b/app/components/TopBar.tsx @@ -19,12 +19,7 @@ import { pb } from '~/util/path-builder' export function TopBar({ children }: { children: React.ReactNode }) { const navigate = useNavigate() const logout = useApiMutation('logout', { - onSuccess: () => { - // server will respond to /login with a login redirect - // TODO-usability: do we just want to dump them back to login or is there - // another page that would make sense, like a logged out homepage - navToLogin({ includeCurrent: false }) - }, + onSuccess: () => navToLogin({ includeCurrent: false }), }) // fetch happens in loader wrapping all authed pages const { me } = useCurrentUser() diff --git a/mock-api/msw/handlers.ts b/mock-api/msw/handlers.ts index 09e3168e3d..016a4e958d 100644 --- a/mock-api/msw/handlers.ts +++ b/mock-api/msw/handlers.ts @@ -33,6 +33,7 @@ import { currentUser, errIfExists, errIfInvalidDiskSize, + forbiddenErr, getStartAndEndTime, getTimestamps, handleMetrics, @@ -51,6 +52,7 @@ import { // is *JSON type. export const handlers = makeHandlers({ + logout: () => 204, ping: () => ({ status: 'ok' }), deviceAuthRequest: () => 200, deviceAuthConfirm: ({ body }) => (body.user_code === 'ERRO-RABC' ? 400 : 200), @@ -74,7 +76,9 @@ export const handlers = makeHandlers({ }, projectView: ({ path }) => { if (path.project.endsWith('error-503')) { - throw unavailableErr + throw unavailableErr() + } else if (path.project.endsWith('error-403')) { + throw forbiddenErr() } return lookup.project({ ...path }) @@ -1263,7 +1267,6 @@ export const handlers = makeHandlers({ localIdpUserDelete: NotImplemented, localIdpUserSetPassword: NotImplemented, loginSaml: NotImplemented, - logout: NotImplemented, networkingAddressLotBlockList: NotImplemented, networkingAddressLotCreate: NotImplemented, networkingAddressLotDelete: NotImplemented, diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts index 3727c1c3aa..7731af26c2 100644 --- a/mock-api/msw/util.ts +++ b/mock-api/msw/util.ts @@ -92,6 +92,9 @@ export function getTimestamps() { return { time_created: now, time_modified: now } } +export const forbiddenErr = () => + json({ error_code: 'Forbidden', request_id: 'fake-id' }, { status: 403 }) + export const unavailableErr = () => json({ error_code: 'ServiceUnavailable', request_id: 'fake-id' }, { status: 503 }) diff --git a/test/e2e/error-pages.e2e.ts b/test/e2e/error-pages.e2e.ts index 9dafa24717..f3f54c781e 100644 --- a/test/e2e/error-pages.e2e.ts +++ b/test/e2e/error-pages.e2e.ts @@ -13,6 +13,9 @@ test('Shows 404 page when a resource is not found', async ({ page }) => { await page.goto('/projects/nonexistent') await expect(page.locator('text=Page not found')).toBeVisible() + + // no logout button on 404 + await expect(page.getByRole('button', { name: 'Sign out' })).toBeHidden() }) test('Shows something went wrong page on other errors', async ({ page }) => { @@ -31,4 +34,12 @@ test('Shows something went wrong page on other errors', async ({ page }) => { const error = 'Invariant failed: Expected query to be prefetched. Key: ["projectView",{"path":{"project":"error-503"}}]' expect(errors.some((e) => e.message.includes(error))).toBeTruthy() + + // test clicking sign out + await page.getByRole('button', { name: 'Sign out' }).click() + // login route doesn't actually work in the mock setup (in production this + // is handled by nexus, and it's hard to get Vite to do the right thing here + // without getting elaborate with middleware), so this is a 404, but we do end + // up at the right URL + await expect(page).toHaveURL('/login') }) From 36257e0c8d3457d491ce595c68357cf927b53a83 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 15 May 2024 16:17:49 -0500 Subject: [PATCH 2/2] move sign out button to the corner, show on all error pages --- app/components/ErrorBoundary.tsx | 20 -------------------- app/components/ErrorPage.tsx | 23 ++++++++++++++++++++++- test/e2e/error-pages.e2e.ts | 3 +-- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/components/ErrorBoundary.tsx b/app/components/ErrorBoundary.tsx index 95b6cf9544..91085e08fe 100644 --- a/app/components/ErrorBoundary.tsx +++ b/app/components/ErrorBoundary.tsx @@ -8,10 +8,7 @@ import { ErrorBoundary as BaseErrorBoundary } from 'react-error-boundary' import { useRouteError } from 'react-router-dom' -import { useApiMutation } from '~/api/client' import { type ApiError } from '~/api/errors' -import { navToLogin } from '~/api/nav-to-login' -import { Button } from '~/ui/lib/Button' import { ErrorPage, NotFound } from './ErrorPage' @@ -30,7 +27,6 @@ function ErrorFallback({ error }: Props) {

Please try again. If the problem persists, contact your administrator.

- ) } @@ -44,19 +40,3 @@ export function RouterDataErrorBoundary() { const error = useRouteError() as Props['error'] return } - -export function SignOutButton({ className }: { className?: string }) { - const logout = useApiMutation('logout', { - onSuccess: () => navToLogin({ includeCurrent: false }), - }) - return ( - - ) -} diff --git a/app/components/ErrorPage.tsx b/app/components/ErrorPage.tsx index ba43a86a01..8292e38af6 100644 --- a/app/components/ErrorPage.tsx +++ b/app/components/ErrorPage.tsx @@ -10,6 +10,10 @@ import { Link } from 'react-router-dom' import { Error12Icon, PrevArrow12Icon } from '@oxide/design-system/icons/react' +import { useApiMutation } from '~/api/client' +import { navToLogin } from '~/api/nav-to-login' +import { Button } from '~/ui/lib/Button' + const GradientBackground = () => (
-
+
Back to console +
@@ -58,3 +63,19 @@ export function NotFound() { ) } + +export function SignOutButton({ className }: { className?: string }) { + const logout = useApiMutation('logout', { + onSuccess: () => navToLogin({ includeCurrent: false }), + }) + return ( + + ) +} diff --git a/test/e2e/error-pages.e2e.ts b/test/e2e/error-pages.e2e.ts index f3f54c781e..26f6a5982e 100644 --- a/test/e2e/error-pages.e2e.ts +++ b/test/e2e/error-pages.e2e.ts @@ -14,8 +14,7 @@ test('Shows 404 page when a resource is not found', async ({ page }) => { await page.goto('/projects/nonexistent') await expect(page.locator('text=Page not found')).toBeVisible() - // no logout button on 404 - await expect(page.getByRole('button', { name: 'Sign out' })).toBeHidden() + await expect(page.getByRole('button', { name: 'Sign out' })).toBeVisible() }) test('Shows something went wrong page on other errors', async ({ page }) => {