Skip to content

Stop persisting admin key to localStorage #3398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
potiuk wants to merge 2 commits into
base: master
Choose a base branch
from
+57 −61
Open

Stop persisting admin key to localStorage #3398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cfa8478
Stop persisting admin key to localStorage
potiuk May 30, 2026
1707b21
e2e: authenticate via Admin API header injection (in-memory admin key)
potiuk Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions e2e/tests/auth.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ import { getAPISIXConf } from '@e2e/utils/common';
import { test } from '@e2e/utils/test';
import { expect } from '@playwright/test';

// use empty storage state to avoid auth
test.use({ storageState: { cookies: [], origins: [] } });
// This suite exercises the manual auth flow itself, so opt out of the
// automatic Admin API key injection and drive the Settings modal directly.
test.use({ injectAdminKey: false });

test('can auth with admin key', { tag: '@auth' }, async ({ page }) => {
const settingsModal = page.getByRole('dialog', { name: 'Settings' });
Expand Down Expand Up @@ -60,9 +61,19 @@ test('can auth with admin key', { tag: '@auth' }, async ({ page }) => {
.getByRole('button')
.click();

await page.reload();
// The key authenticates the current session immediately (it is held in
// memory), so the token check now succeeds without a reload.
await expect(failedMsg).toBeHidden();
});

await test.step('admin key is not persisted across a full reload', async () => {
// The admin key is kept in memory only and never written to browser
// storage, so a hard reload drops it and re-authentication is required.
await page.reload();
await expect(failedMsg).toBeVisible();
await expect(settingsModal).toBeVisible();
await expect(adminKeyInput).toBeEmpty();

@Baoyuantop Baoyuantop Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean I have to reconfigure the admin key every time I refresh the page? I'm not sure if this will affect the user experience.

@potiuk potiuk Jun 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - not really, it's the UX change that is explained in the PR description -> it's per browsing session.

As I understand it and worth double checking what happens with this change in - it behaves in this way:

  • A page session lasts as long as the browser is open, and survives over page reloads and restores.
  • Opening a page in a new tab or window creates a new session with the value of the top-level browsing context, which differs from how session cookies work.
  • Opening multiple tabs/windows with the same URL creates sessionStorage for each tab/window.
  • Closing a tab/window ends the session and clears objects in sessionStorage.

});
});

test('password input can toggle visibility', { tag: '@auth' }, async ({ page }) => {
Expand Down
86 changes: 38 additions & 48 deletions e2e/utils/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,59 +14,49 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { readFile } from 'node:fs/promises';
import path from 'node:path';
import { test as baseTest } from '@playwright/test';

import { expect, test as baseTest } from '@playwright/test';
import { API_HEADER_KEY, API_PREFIX } from '@/config/constant';

import { fileExists, getAPISIXConf } from './common';
import { env } from './env';
import { getAPISIXConf } from './common';

export type Test = typeof test;
export const test = baseTest.extend<object, { workerStorageState: string }>({
storageState: ({ workerStorageState }, use) => use(workerStorageState),
workerStorageState: [
async ({ browser }, use) => {
// Use parallelIndex as a unique identifier for each worker.
const id = test.info().parallelIndex;
const fileName = path.resolve(
test.info().project.outputDir,
`.auth/${id}.json`
);
const { adminKey } = await getAPISIXConf();

// file exists and contains admin key, use it
if (
(await fileExists(fileName)) &&
(await readFile(fileName)).toString().includes(adminKey)
) {
return use(fileName);
}

const page = await browser.newPage({ storageState: undefined });

// have to use env here, because the baseURL is not available in worker
await page.goto(env.E2E_TARGET_URL);

// we need to authenticate
const settingsModal = page.getByRole('dialog', { name: 'Settings' });
await expect(settingsModal).toBeVisible();
// PasswordInput renders with a label, use getByLabel instead
const adminKeyInput = page.getByLabel('Admin Key');
await adminKeyInput.clear();
await adminKeyInput.fill(adminKey);
await page
.getByRole('dialog', { name: 'Settings' })
.getByRole('button')
.click();

await page.context().storageState({ path: fileName });
await page.close();
await use(fileName);
},
{ scope: 'worker' },
],
page: async ({ baseURL, page }, use) => {
type TestOptions = {
/**
* Whether to authenticate Admin API requests by injecting the
* `X-API-KEY` header at the network layer. On by default.
*
* The dashboard holds the admin key in memory only (it is no longer
* persisted to `localStorage`), so auth cannot be replayed through
* Playwright's `storageState` the way it used to be. Instead we add the
* header to every Admin API request for the whole browser context, which
* keeps the app authenticated across full-page navigations and reloads
* (the e2e suite navigates with `page.goto`, i.e. fresh document loads)
* without re-driving the Settings modal on every page.
*
* Tests that exercise the manual auth flow itself (`auth.spec.ts`) opt
* out via `test.use({ injectAdminKey: false })`.
*/
injectAdminKey: boolean;
};

export const test = baseTest.extend<TestOptions>({
injectAdminKey: [true, { option: true }],
page: async ({ baseURL, page, injectAdminKey }, use) => {
if (injectAdminKey) {
const { adminKey } = await getAPISIXConf();
await page.route(
(url) => url.pathname.startsWith(API_PREFIX),
(route) =>
route.continue({
headers: {
...route.request().headers(),
[API_HEADER_KEY.toLowerCase()]: adminKey,
},
})
);
}
await page.goto(baseURL);
await use(page);
},
Expand Down
15 changes: 5 additions & 10 deletions src/stores/global.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,12 @@
* limitations under the License.
*/
import { atom } from 'jotai';
import { atomWithStorage } from 'jotai/utils';

// Admin key with persistent storage
export const adminKeyAtom = atomWithStorage<string>(
'settings:adminKey',
'',
undefined,
{
getOnInit: true,
}
);
// Admin key — in-memory only. Operators re-enter the key each browser
// session by design: persisting it to localStorage broadens the
// credential's audience to anything that can read the dashboard origin's
// storage (XSS, browser extensions, shared workstations).
export const adminKeyAtom = atom<string>('');

// Settings modal visibility state
export const isSettingsOpenAtom = atom<boolean>(false);
Loading