This is an automated email from the ASF dual-hosted git repository. moonming pushed a commit to branch fix/401-reject-not-fake-success in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git
commit 0c69992d5cea69373777c84189cec1dfbc7d2591 Author: Ming Wen <[email protected]> AuthorDate: Fri Jul 10 14:55:19 2026 +0800 fix: reject 401 responses instead of resolving fabricated empty data The axios response interceptor converted every 401 into a resolved response carrying '{ data: {} }'. Callers treated that as success: - a 401'd DELETE showed the green delete-success toast and invalidated queries even though nothing was deleted - add pages' onSuccess handlers read 'data.data.value.id' from the fabricated body and crashed with a TypeError - detail pages' suspense queries resolved with '{}' and crashed on 'data.value.*' access - list pages rendered an empty-cluster illusion Now the interceptor still opens the settings modal on 401 but rejects, so every caller takes its normal error path. Supporting changes: - root route gets an errorComponent that keeps the settings modal mounted (on a fresh install every request 401s until the admin key is entered, and the modal is the only way in) plus a retry button - queries no longer retry on 401 (retries cannot succeed until the user fixes the key; failing fast surfaces the modal immediately) - DeleteResourceBtn swallows the rejection after the interceptor's toast so a failed delete no longer leaves an unhandled rejection Fixes #3415 --- .../regression/auth.401-no-false-success.spec.ts | 88 ++++++++++++++++++++++ src/components/page/DeleteResourceBtn.tsx | 5 ++ src/config/global.ts | 19 ++++- src/config/req.ts | 7 +- src/locales/de/common.json | 4 + src/locales/en/common.json | 5 +- src/locales/es/common.json | 4 + src/locales/tr/common.json | 4 + src/locales/zh/common.json | 4 + src/routes/__root.tsx | 40 +++++++++- 10 files changed, 173 insertions(+), 7 deletions(-) diff --git a/e2e/tests/regression/auth.401-no-false-success.spec.ts b/e2e/tests/regression/auth.401-no-false-success.spec.ts new file mode 100644 index 000000000..f2e61557d --- /dev/null +++ b/e2e/tests/regression/auth.401-no-false-success.spec.ts @@ -0,0 +1,88 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Regression for the 401 interceptor behavior (apache/apisix-dashboard#3415): +// the axios interceptor used to convert a 401 into a *resolved* response with +// fabricated `{ data: {} }`. That made a 401'd DELETE show the green +// "Delete ... Successfully" toast (and fed `{}` into onSuccess handlers, +// crashing add/detail pages). The interceptor must open the Settings modal +// AND reject, so callers take their normal error path. + +import { routesPom } from '@e2e/pom/routes'; +import { randomId } from '@e2e/utils/common'; +import { e2eReq } from '@e2e/utils/req'; +import { test } from '@e2e/utils/test'; +import { expect } from '@playwright/test'; + +import { deleteAllRoutes, putRouteReq } from '@/apis/routes'; +import type { APISIXType } from '@/types/schema/apisix'; + +test.beforeAll(async () => { + await deleteAllRoutes(e2eReq); +}); + +test.afterAll(async () => { + await deleteAllRoutes(e2eReq); +}); + +test('a 401 response must not produce a success toast', async ({ page }) => { + const name = randomId('reg-401'); + await putRouteReq(e2eReq, { + name, + uri: `/regression/401/${name}`, + upstream: { + type: 'roundrobin', + nodes: { 'reg-401.local:80': 1 }, + }, + } as APISIXType['Route']); + + await routesPom.toIndex(page); + await routesPom.isIndexPage(page); + + const row = page.getByRole('row', { name }); + await expect(row).toBeVisible(); + + // Force the DELETE to come back as a 401, the way an expired/rotated + // admin key would. + await page.route('**/apisix/admin/routes/**', async (route) => { + if (route.request().method() === 'DELETE') { + await route.fulfill({ + status: 401, + contentType: 'application/json', + body: JSON.stringify({ message: 'failed to check token' }), + }); + } else { + await route.fallback(); + } + }); + + await row.getByRole('button', { name: 'Delete' }).click(); + await page + .getByRole('dialog') + .getByRole('button', { name: 'Delete' }) + .click(); + + // The settings modal must open so the user can fix the key ... + await expect(page.getByRole('dialog', { name: 'Settings' })).toBeVisible(); + + // ... and no success toast may appear; the row must survive. + const successToast = page + .getByRole('alert') + .filter({ hasText: /successfully/i }); + await expect(successToast).toHaveCount(0); + await expect(row).toBeVisible(); +}); diff --git a/src/components/page/DeleteResourceBtn.tsx b/src/components/page/DeleteResourceBtn.tsx index d6e6ef6ee..6d8aea6c6 100644 --- a/src/components/page/DeleteResourceBtn.tsx +++ b/src/components/page/DeleteResourceBtn.tsx @@ -85,6 +85,11 @@ export const DeleteResourceBtn = (props: DeleteResourceProps) => { // So this is a workaround for now // TODO: remove this queryClient.invalidateQueries(); + }) + .catch(() => { + // the axios interceptor owns the error toast; swallow the + // rejection so a failed delete (401/5xx/network) does not + // surface as an unhandled promise rejection }), }) ); diff --git a/src/config/global.ts b/src/config/global.ts index e39744e50..c1dfad63b 100644 --- a/src/config/global.ts +++ b/src/config/global.ts @@ -16,6 +16,7 @@ */ import { QueryClient } from '@tanstack/react-query'; import { createRouter } from '@tanstack/react-router'; +import { HttpStatusCode, isAxiosError } from 'axios'; import { routeTree } from '@/routeTree.gen'; @@ -25,4 +26,20 @@ export const router = createRouter({ routeTree, basepath: BASE_PATH }); export type Router = typeof router; -export const queryClient = new QueryClient({}); +export const queryClient = new QueryClient({ + defaultOptions: { + queries: { + retry: (failureCount, error) => { + // retrying a 401 cannot succeed until the user fixes the admin + // key; fail fast so the settings modal appears immediately + if ( + isAxiosError(error) && + error.response?.status === HttpStatusCode.Unauthorized + ) { + return false; + } + return failureCount < 3; + }, + }, + }, +}); diff --git a/src/config/req.ts b/src/config/req.ts index 078bd513f..7c94258f2 100644 --- a/src/config/req.ts +++ b/src/config/req.ts @@ -85,10 +85,13 @@ req.interceptors.response.use( message, color: 'red', }); - // Requires to enter admin key at 401 + // Requires to enter admin key at 401. + // Note: do NOT resolve with fabricated data here — callers must take + // their normal error path. Resolving `{ data: {} }` made a 401'd + // DELETE/PUT show success toasts and fed `{}` into `onSuccess` + // handlers and suspense queries, crashing detail/add pages. if (res.status === HttpStatusCode.Unauthorized) { getDefaultStore().set(isSettingsOpenAtom, true); - return Promise.resolve({ data: {} }); } } return Promise.reject(err); diff --git a/src/locales/de/common.json b/src/locales/de/common.json index 95cfb9ad5..0fbf9dfe1 100644 --- a/src/locales/de/common.json +++ b/src/locales/de/common.json @@ -370,5 +370,9 @@ }, "upstreams": { "singular": "Upstream" + }, + "error": { + "title": "Etwas ist schiefgelaufen", + "retry": "Erneut versuchen" } } diff --git a/src/locales/en/common.json b/src/locales/en/common.json index 9625981d2..5209795a4 100644 --- a/src/locales/en/common.json +++ b/src/locales/en/common.json @@ -336,7 +336,6 @@ "services": { "singular": "Service", "empty": "No services found. Click Add Service to create your first one." - }, "settings": { "adminKey": "Admin Key", @@ -371,5 +370,9 @@ }, "upstreams": { "singular": "Upstream" + }, + "error": { + "title": "Something went wrong", + "retry": "Retry" } } diff --git a/src/locales/es/common.json b/src/locales/es/common.json index b57953e9c..d6188dfcf 100644 --- a/src/locales/es/common.json +++ b/src/locales/es/common.json @@ -370,5 +370,9 @@ }, "upstreams": { "singular": "Upstream" + }, + "error": { + "title": "Algo salió mal", + "retry": "Reintentar" } } diff --git a/src/locales/tr/common.json b/src/locales/tr/common.json index 46da33e48..c20dccc76 100644 --- a/src/locales/tr/common.json +++ b/src/locales/tr/common.json @@ -370,5 +370,9 @@ }, "upstreams": { "singular": "Upstream" + }, + "error": { + "title": "Bir şeyler ters gitti", + "retry": "Yeniden dene" } } diff --git a/src/locales/zh/common.json b/src/locales/zh/common.json index a8097fa1d..6a82d155a 100644 --- a/src/locales/zh/common.json +++ b/src/locales/zh/common.json @@ -370,5 +370,9 @@ }, "upstreams": { "singular": "上游" + }, + "error": { + "title": "出错了", + "retry": "重试" } } diff --git a/src/routes/__root.tsx b/src/routes/__root.tsx index 0ce486324..070ed270e 100644 --- a/src/routes/__root.tsx +++ b/src/routes/__root.tsx @@ -14,12 +14,18 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { AppShell } from '@mantine/core'; +import { AppShell, Button, Code, Stack, Text } from '@mantine/core'; import { useDisclosure } from '@mantine/hooks'; import { ReactQueryDevtools } from '@tanstack/react-query-devtools'; -import { createRootRoute, HeadContent, Outlet } from '@tanstack/react-router'; +import { + createRootRoute, + type ErrorComponentProps, + HeadContent, + Outlet, + useRouter, +} from '@tanstack/react-router'; import { TanStackRouterDevtools } from '@tanstack/react-router-devtools'; -import { I18nextProvider } from 'react-i18next'; +import { I18nextProvider, useTranslation } from 'react-i18next'; import { Header } from '@/components/Header'; import { Navbar } from '@/components/Navbar'; @@ -63,6 +69,34 @@ const Root = () => { ); }; +const RootErrorContent = (props: ErrorComponentProps) => { + const { error } = props; + const { t } = useTranslation(); + const router = useRouter(); + return ( + <Stack align="center" justify="center" mih="60vh" gap="md" p="xl"> + <Text fw={700} size="lg"> + {t('error.title')} + </Text> + <Code block>{error.message}</Code> + <Button onClick={() => router.invalidate()}>{t('error.retry')}</Button> + </Stack> + ); +}; + +/** + * Loader/render failures land here when no child route handles them. + * The settings modal must stay mounted: on a fresh install every request + * 401s until the admin key is entered, and the modal is the only way in. + */ +const RootError = (props: ErrorComponentProps) => ( + <I18nextProvider i18n={i18n}> + <RootErrorContent {...props} /> + <SettingsModal /> + </I18nextProvider> +); + export const Route = createRootRoute({ component: Root, + errorComponent: RootError, });
