fix(auth): gate onboarding replacement on known token state
This commit is contained in:
Palash Debnath
@@ -428,8 +428,8 @@ jobs:
|
||||
PY
|
||||
|
||||
- name: Run smoke tests
|
||||
if: matrix.backend_supported
|
||||
# Exercise credential paths on native Windows as well as POSIX hosts.
|
||||
if: matrix.backend_supported
|
||||
run: uv run --no-sync pytest tests/smoke/ tests/test_hf_token_cache_paths.py -q --tb=short
|
||||
env:
|
||||
HF_HUB_OFFLINE: "1" # same no-silent-downloads guard as the main pytest job
|
||||
|
||||
+1
-1
@@ -20,7 +20,7 @@ the frozen-backend fallback mirror it for their toolchains.
|
||||
|
||||
### Fixed
|
||||
|
||||
- Onboarding recognizes local Hugging Face tokens without network checks, preserves Windows CLI logins, and clears saved token files explicitly (#1852) — thanks @psiberfunk!
|
||||
- Onboarding reads Hugging Face tokens locally, preserves Windows CLI logins, and requires successful discovery before replacing saved credentials (#1852) — thanks @psiberfunk!
|
||||
|
||||
|
||||
## [0.5.2] — 2026-09-02
|
||||
|
||||
@@ -122,3 +122,6 @@ Opening onboarding or Settings only reads local token presence and masked previe
|
||||
Windows automatically shortens the model cache path while keeping the normal CLI token location. If only a previous VoiceStudio short-cache token exists, the app continues using that file. An existing normal CLI token takes priority; explicit token or cache overrides remain authoritative. Credentials are never copied between these locations.
|
||||
|
||||
The CLI row reads only the selected local file, without OAuth refresh or environment-token fallback. **Also clear saved HuggingFace CLI token files** removes both active and stored-token files at recognized automatic locations, so an older app token cannot reappear on restart. Explicit overrides limit clearing to their selected location. Clearing only the app token preserves CLI files; neither action revokes tokens on Hugging Face or changes Git credentials. A file permission failure is reported instead of claiming the files were cleared.
|
||||
|
||||
|
||||
If onboarding cannot read token state, it shows an error and **Retry**, keeping token entry hidden until discovery succeeds. **Replace token** writes the encrypted app token through the same endpoint as Settings, so it replaces the highest-priority app credential even when an older one exists. The success message confirms saving only; validation remains a separate explicit action.
|
||||
|
||||
@@ -3,7 +3,7 @@ import { useTranslation } from 'react-i18next';
|
||||
import { Check, Zap } from 'lucide-react';
|
||||
import { cn } from '@/lib/utils';
|
||||
import { openExternal } from '../api/external';
|
||||
import { apiJson } from '../api/client';
|
||||
import { apiJson, apiPost } from '../api/client';
|
||||
import { Button, Input } from '../ui';
|
||||
|
||||
/**
|
||||
@@ -11,7 +11,7 @@ import { Button, Input } from '../ui';
|
||||
* the wizard's pinned action area, right by the "Waiting for required models…"
|
||||
* / Continue button. A free token gives authenticated downloads (faster,
|
||||
* higher rate limits, fewer stalls) and unlocks gated models (pyannote
|
||||
* diarization). Persisted via the same `set-env` endpoint Settings uses, so
|
||||
* diarization). Persisted via the same encrypted-app-token endpoint Settings uses, so
|
||||
* it survives restarts.
|
||||
*
|
||||
* Before pitching a token, it checks the resolver state (same endpoint the
|
||||
@@ -20,7 +20,7 @@ import { Button, Input } from '../ui';
|
||||
* login` — sees that instead of a blind "add a token" prompt (#FR-006).
|
||||
* Replacing an already-active token is gated behind an explicit "Replace…"
|
||||
* click rather than being one blind paste-and-Save away, since Save persists
|
||||
* via `huggingface_hub.login()`, which overwrites `$HF_HOME/token` outright.
|
||||
* in the app store and the selected local Hub token file.
|
||||
*
|
||||
* @param {string=} className extra class on the root (e.g. layout pinning).
|
||||
*/
|
||||
@@ -34,6 +34,7 @@ export default function HfTokenCard({ className = '' }) {
|
||||
// want to flash a false "you have no token" pitch before we actually know.
|
||||
const [tokenState, setTokenState] = useState(null);
|
||||
const [checkFailed, setCheckFailed] = useState(false);
|
||||
const [checkAttempt, setCheckAttempt] = useState(0);
|
||||
// Explicit gate: revealing the paste-a-token form when a token is already
|
||||
// active requires this deliberate click, so Save can never blind-clobber a
|
||||
// working token.
|
||||
@@ -44,6 +45,14 @@ export default function HfTokenCard({ className = '' }) {
|
||||
(async () => {
|
||||
try {
|
||||
const data = await apiJson('/system/hf-token/state');
|
||||
if (
|
||||
!Array.isArray(data?.sources) ||
|
||||
data.sources.length !== 3 ||
|
||||
!['app', 'env', 'hf-cli'].every((source) =>
|
||||
data.sources.some((row) => row?.source === source && typeof row.set === 'boolean'),
|
||||
)
|
||||
)
|
||||
throw new Error('Invalid token state');
|
||||
if (!cancelled) setTokenState(data);
|
||||
} catch {
|
||||
if (!cancelled) setCheckFailed(true);
|
||||
@@ -52,19 +61,14 @@ export default function HfTokenCard({ className = '' }) {
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, []);
|
||||
}, [checkAttempt]);
|
||||
|
||||
const saveHfToken = async () => {
|
||||
const value = hfToken.trim();
|
||||
if (!value || hfState === 'saving') return;
|
||||
if (!value || hfState === 'saving' || tokenState == null || checkFailed) return;
|
||||
setHfState('saving');
|
||||
try {
|
||||
const { apiFetch } = await import('../api/client');
|
||||
await apiFetch('/system/set-env', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ key: 'HF_TOKEN', value }),
|
||||
});
|
||||
await apiPost('/api/settings/hf-token', { token: value });
|
||||
setHfState('saved');
|
||||
setHfToken('');
|
||||
} catch {
|
||||
@@ -82,15 +86,41 @@ export default function HfTokenCard({ className = '' }) {
|
||||
>
|
||||
<span className="inline-flex items-center gap-1.5 font-semibold text-success">
|
||||
<Check size={14} aria-hidden="true" />
|
||||
{t('firstrun.hf_token_saved_fast', 'Hugging Face token saved — downloads are now faster')}
|
||||
{t('firstrun.hf_token_saved', 'Hugging Face token saved')}
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
if (checkFailed) {
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
'flex flex-wrap items-center gap-2 rounded-md bg-danger/10 px-3 py-2 text-sm',
|
||||
className,
|
||||
)}
|
||||
>
|
||||
<span role="alert" className="text-danger">
|
||||
{t('common.error', 'Something went wrong')}
|
||||
</span>
|
||||
<Button
|
||||
size="sm"
|
||||
variant="secondary"
|
||||
onClick={() => {
|
||||
setCheckFailed(false);
|
||||
setTokenState(null);
|
||||
setCheckAttempt((attempt) => attempt + 1);
|
||||
}}
|
||||
>
|
||||
{t('bootstrap.retry', 'Retry')}
|
||||
</Button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// Still checking — never flash the "add a token" pitch before we know
|
||||
// whether one is already active.
|
||||
if (tokenState == null && !checkFailed) {
|
||||
if (tokenState == null) {
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
@@ -108,8 +138,12 @@ export default function HfTokenCard({ className = '' }) {
|
||||
app: t('settings.hf_source_app_label', {
|
||||
defaultValue: 'VoiceStudio (encrypted, recommended)',
|
||||
}),
|
||||
env: t('settings.hf_source_env_label', { defaultValue: 'Environment variable' }),
|
||||
'hf-cli': t('settings.hf_source_cli_label', { defaultValue: 'HuggingFace CLI' }),
|
||||
env: t('settings.hf_source_env_label', {
|
||||
defaultValue: 'Environment variable',
|
||||
}),
|
||||
'hf-cli': t('settings.hf_source_cli_label', {
|
||||
defaultValue: 'HuggingFace CLI',
|
||||
}),
|
||||
};
|
||||
const activeRow = tokenState?.sources?.find((row) => row.set);
|
||||
|
||||
@@ -142,9 +176,7 @@ export default function HfTokenCard({ className = '' }) {
|
||||
);
|
||||
}
|
||||
|
||||
// No active token (or the state check failed — fail toward the pre-fix
|
||||
// behavior rather than hiding the card), or the user explicitly chose to
|
||||
// replace an active one.
|
||||
// A successful state read found no token, or replacement was explicit.
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
@@ -171,7 +203,9 @@ export default function HfTokenCard({ className = '' }) {
|
||||
placeholder={t('firstrun.hf_token_inline_ph', 'Paste hf_… token (optional)')}
|
||||
value={hfToken}
|
||||
autoComplete="off"
|
||||
disabled={hfState === 'saving'}
|
||||
onChange={(e) => {
|
||||
if (hfState === 'saving') return;
|
||||
setHfToken(e.target.value);
|
||||
if (hfState !== 'idle') setHfState('idle');
|
||||
}}
|
||||
@@ -197,6 +231,7 @@ export default function HfTokenCard({ className = '' }) {
|
||||
<button
|
||||
type="button"
|
||||
className="cursor-pointer appearance-none whitespace-nowrap border-0 bg-transparent p-0 text-[0.76rem] text-fg-muted underline hover:no-underline"
|
||||
disabled={hfState === 'saving'}
|
||||
onClick={() => {
|
||||
setReplacing(false);
|
||||
setHfToken('');
|
||||
|
||||
@@ -7,9 +7,27 @@ import HfTokenCard from './HfTokenCard';
|
||||
const STATE_NONE_ACTIVE = {
|
||||
active: null,
|
||||
sources: [
|
||||
{ source: 'app', set: false, masked: null, whoami_user: null, whoami_ok: false },
|
||||
{ source: 'env', set: false, masked: null, whoami_user: null, whoami_ok: false },
|
||||
{ source: 'hf-cli', set: false, masked: null, whoami_user: null, whoami_ok: false },
|
||||
{
|
||||
source: 'app',
|
||||
set: false,
|
||||
masked: null,
|
||||
whoami_user: null,
|
||||
whoami_ok: false,
|
||||
},
|
||||
{
|
||||
source: 'env',
|
||||
set: false,
|
||||
masked: null,
|
||||
whoami_user: null,
|
||||
whoami_ok: false,
|
||||
},
|
||||
{
|
||||
source: 'hf-cli',
|
||||
set: false,
|
||||
masked: null,
|
||||
whoami_user: null,
|
||||
whoami_ok: false,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
@@ -17,8 +35,20 @@ const STATE_NONE_ACTIVE = {
|
||||
const STATE_HF_CLI_ACTIVE = {
|
||||
active: null,
|
||||
sources: [
|
||||
{ source: 'app', set: false, masked: null, whoami_user: null, whoami_ok: false },
|
||||
{ source: 'env', set: false, masked: null, whoami_user: null, whoami_ok: false },
|
||||
{
|
||||
source: 'app',
|
||||
set: false,
|
||||
masked: null,
|
||||
whoami_user: null,
|
||||
whoami_ok: false,
|
||||
},
|
||||
{
|
||||
source: 'env',
|
||||
set: false,
|
||||
masked: null,
|
||||
whoami_user: null,
|
||||
whoami_ok: false,
|
||||
},
|
||||
{
|
||||
source: 'hf-cli',
|
||||
set: true,
|
||||
@@ -129,16 +159,30 @@ describe('HfTokenCard', () => {
|
||||
await waitFor(() => expect(screen.getByPlaceholderText(/hf_/)).toBeInTheDocument());
|
||||
});
|
||||
|
||||
it('falls back to the pitch (pre-fix behavior) when the state check fails, rather than hiding the card', async () => {
|
||||
global.fetch = vi.fn().mockRejectedValueOnce(new Error('network error'));
|
||||
it('keeps Save hidden after a failed state read until Retry discovers the existing token', async () => {
|
||||
const failed = mockFetchOnce({ detail: 'private server failure' }, 503);
|
||||
failed.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => STATE_HF_CLI_ACTIVE,
|
||||
});
|
||||
global.fetch = failed;
|
||||
render(<HfTokenCard />);
|
||||
await waitFor(() => expect(screen.getByPlaceholderText(/hf_/)).toBeInTheDocument());
|
||||
const retry = await screen.findByRole('button', { name: 'Retry' });
|
||||
expect(screen.queryByPlaceholderText(/hf_/)).toBeNull();
|
||||
expect(screen.queryByRole('button', { name: /^save$/i })).toBeNull();
|
||||
expect(screen.queryByText(/private server failure/)).toBeNull();
|
||||
fireEvent.click(retry);
|
||||
await screen.findByText(/hf_…Sfb/);
|
||||
expect(screen.queryByPlaceholderText(/hf_/)).toBeNull();
|
||||
fireEvent.click(screen.getByRole('button', { name: /replace/i }));
|
||||
expect(screen.getByText(/replaces the token saved for this app/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('Save still POSTs HF_TOKEN via /system/set-env when no token was active', async () => {
|
||||
it('saves an app token through the canonical Settings endpoint without claiming validation', async () => {
|
||||
const fetchMock = mockFetchSequence(
|
||||
{ status: 200, body: STATE_NONE_ACTIVE }, // GET state
|
||||
{ status: 200, body: { key: 'HF_TOKEN', set: true, shadowed: false } }, // POST set-env
|
||||
{ status: 200, body: STATE_HF_CLI_ACTIVE }, // POST app token
|
||||
);
|
||||
global.fetch = fetchMock;
|
||||
|
||||
@@ -150,9 +194,122 @@ describe('HfTokenCard', () => {
|
||||
await waitFor(() => {
|
||||
const postCall = fetchMock.mock.calls.find(([, opts]) => opts?.method === 'POST');
|
||||
expect(postCall).toBeTruthy();
|
||||
expect(postCall[0]).toMatch(/\/system\/set-env$/);
|
||||
expect(JSON.parse(postCall[1].body)).toEqual({ key: 'HF_TOKEN', value: 'hf_newtoken123' });
|
||||
expect(postCall[0]).toMatch(/\/api\/settings\/hf-token$/);
|
||||
expect(JSON.parse(postCall[1].body)).toEqual({ token: 'hf_newtoken123' });
|
||||
});
|
||||
await waitFor(() => expect(screen.getByText(/Hugging Face token saved/)).toBeInTheDocument());
|
||||
await screen.findByText('Hugging Face token saved');
|
||||
expect(screen.queryByText(/downloads are now faster/)).toBeNull();
|
||||
});
|
||||
it.each(['app', 'env', 'hf-cli'])(
|
||||
'replaces the used token from %s through the app source',
|
||||
async (source) => {
|
||||
const state = {
|
||||
active: null,
|
||||
sources: STATE_NONE_ACTIVE.sources.map((row) => ({
|
||||
...row,
|
||||
set: row.source === source,
|
||||
masked: row.source === source ? 'hf_…old' : null,
|
||||
})),
|
||||
};
|
||||
global.fetch = mockFetchSequence(
|
||||
{ status: 200, body: state },
|
||||
{ status: 200, body: STATE_NONE_ACTIVE },
|
||||
);
|
||||
render(<HfTokenCard />);
|
||||
await screen.findByText(/hf_…old/);
|
||||
expect(screen.queryByPlaceholderText(/hf_/)).toBeNull();
|
||||
fireEvent.click(screen.getByRole('button', { name: /replace/i }));
|
||||
fireEvent.change(screen.getByPlaceholderText(/hf_/), {
|
||||
target: { value: ' hf_replacement ' },
|
||||
});
|
||||
fireEvent.click(screen.getByRole('button', { name: /^save$/i }));
|
||||
await screen.findByText('Hugging Face token saved');
|
||||
const [url, options] = global.fetch.mock.calls.find(([, opts]) => opts?.method === 'POST');
|
||||
expect(url).toMatch(/\/api\/settings\/hf-token$/);
|
||||
expect(JSON.parse(options.body)).toEqual({ token: 'hf_replacement' });
|
||||
},
|
||||
);
|
||||
|
||||
it('retains the replacement warning and token after a failed save so it can be retried', async () => {
|
||||
global.fetch = mockFetchSequence(
|
||||
{ status: 200, body: STATE_HF_CLI_ACTIVE },
|
||||
{ status: 500, body: { detail: 'private server failure' } },
|
||||
{ status: 200, body: STATE_NONE_ACTIVE },
|
||||
);
|
||||
render(<HfTokenCard />);
|
||||
await screen.findByText(/hf_…Sfb/);
|
||||
fireEvent.click(screen.getByRole('button', { name: /replace/i }));
|
||||
fireEvent.change(screen.getByPlaceholderText(/hf_/), {
|
||||
target: { value: 'hf_replacement' },
|
||||
});
|
||||
fireEvent.click(screen.getByRole('button', { name: /^save$/i }));
|
||||
await screen.findByText(/Could not save the token/);
|
||||
expect(screen.getByPlaceholderText(/hf_/)).toHaveValue('hf_replacement');
|
||||
expect(screen.getByText(/replaces the token saved for this app/i)).toBeInTheDocument();
|
||||
expect(screen.queryByText(/private server failure/)).toBeNull();
|
||||
fireEvent.click(screen.getByRole('button', { name: /^save$/i }));
|
||||
await screen.findByText('Hugging Face token saved');
|
||||
});
|
||||
it.each([
|
||||
null,
|
||||
{},
|
||||
{ sources: [] },
|
||||
{ sources: [null] },
|
||||
{ sources: [...STATE_NONE_ACTIVE.sources, null] },
|
||||
])('keeps malformed token state gated: %j', async (state) => {
|
||||
global.fetch = mockFetchOnce(state);
|
||||
render(<HfTokenCard />);
|
||||
await screen.findByRole('button', { name: 'Retry' });
|
||||
expect(screen.queryByPlaceholderText(/hf_/)).toBeNull();
|
||||
expect(screen.queryByRole('button', { name: /^save$/i })).toBeNull();
|
||||
});
|
||||
|
||||
it('waits for Retry to finish before showing the empty-state form', async () => {
|
||||
let resolveRetry;
|
||||
global.fetch = mockFetchOnce({ detail: 'failed' }, 503);
|
||||
global.fetch.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveRetry = resolve;
|
||||
}),
|
||||
);
|
||||
render(<HfTokenCard />);
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Retry' }));
|
||||
expect(screen.getByText(/checking/i)).toBeInTheDocument();
|
||||
expect(screen.queryByPlaceholderText(/hf_/)).toBeNull();
|
||||
await waitFor(() => expect(resolveRetry).toBeTypeOf('function'));
|
||||
resolveRetry({
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => STATE_NONE_ACTIVE,
|
||||
});
|
||||
await screen.findByPlaceholderText(/hf_/);
|
||||
});
|
||||
|
||||
it('does not issue duplicate saves or cancel an in-flight replacement', async () => {
|
||||
let resolveSave;
|
||||
global.fetch = mockFetchOnce(STATE_HF_CLI_ACTIVE);
|
||||
global.fetch.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveSave = resolve;
|
||||
}),
|
||||
);
|
||||
render(<HfTokenCard />);
|
||||
fireEvent.click(await screen.findByRole('button', { name: /replace/i }));
|
||||
fireEvent.change(screen.getByPlaceholderText(/hf_/), {
|
||||
target: { value: 'hf_replacement' },
|
||||
});
|
||||
fireEvent.click(screen.getByRole('button', { name: /^save$/i }));
|
||||
expect(screen.getByRole('button', { name: /saving/i })).toBeDisabled();
|
||||
expect(screen.getByPlaceholderText(/hf_/)).toBeDisabled();
|
||||
fireEvent.change(screen.getByPlaceholderText(/hf_/), { target: { value: 'hf_racing_edit' } });
|
||||
expect(screen.getByPlaceholderText(/hf_/)).toHaveValue('hf_replacement');
|
||||
expect(screen.getByRole('button', { name: /cancel/i })).toBeDisabled();
|
||||
fireEvent.keyDown(screen.getByPlaceholderText(/hf_/), { key: 'Enter' });
|
||||
await waitFor(() => expect(resolveSave).toBeTypeOf('function'));
|
||||
expect(global.fetch.mock.calls.filter(([, opts]) => opts?.method === 'POST')).toHaveLength(1);
|
||||
resolveSave({ ok: true, status: 200, json: async () => STATE_NONE_ACTIVE });
|
||||
await screen.findByText('Hugging Face token saved');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -160,14 +160,14 @@ def test_get_hf_token_state_fresh_busts_whoami_cache(fresh_app, monkeypatch):
|
||||
# An explicit test fails; ordinary reads must never revalidate.
|
||||
r = c.get("/api/settings/hf-token/state?fresh=1")
|
||||
env_row = next(s for s in r.json()["sources"] if s["source"] == "env")
|
||||
assert env_row["set"] and not env_row["whoami_ok"]
|
||||
assert env_row["set"] and env_row["whoami_ok"] is False
|
||||
first_calls = calls["n"]
|
||||
|
||||
# Network recovers, but a plain GET still serves the cached failure.
|
||||
# Network recovers, but a plain GET only reports unvalidated local presence.
|
||||
verdict["ok"] = True
|
||||
r = c.get("/api/settings/hf-token/state")
|
||||
env_row = next(s for s in r.json()["sources"] if s["source"] == "env")
|
||||
assert not env_row["whoami_ok"], "plain GET must keep the cache (no re-run)"
|
||||
assert env_row["whoami_ok"] is None, "plain GET must not validate"
|
||||
assert calls["n"] == first_calls
|
||||
|
||||
# "Test now" (fresh=1) drops the cache and re-runs whoami → verified.
|
||||
@@ -305,3 +305,21 @@ def test_token_state_reads_never_contact_hugging_face(fresh_app, monkeypatch, en
|
||||
row = next(row for row in response.json()["sources"] if row["source"] == "env")
|
||||
assert row["set"] and row["whoami_ok"] is None
|
||||
whoami.assert_not_called()
|
||||
|
||||
|
||||
def test_canonical_save_replaces_the_existing_app_source(fresh_app, monkeypatch):
|
||||
import huggingface_hub
|
||||
from services import settings_store, token_resolver
|
||||
settings_store.set_hf_token("hf_old_app")
|
||||
monkeypatch.setenv("HF_TOKEN", "hf_old_env")
|
||||
monkeypatch.setattr(huggingface_hub, "login", lambda **kwargs: None)
|
||||
monkeypatch.setattr(huggingface_hub, "whoami", lambda token: {"name": token})
|
||||
response = _client(fresh_app).post("/api/settings/hf-token", json={"token": SAMPLE_TOKEN})
|
||||
assert response.status_code == 200
|
||||
assert settings_store.get_hf_token() == SAMPLE_TOKEN
|
||||
resolved = token_resolver.resolve()
|
||||
assert resolved.source == "app"
|
||||
assert resolved.token == SAMPLE_TOKEN
|
||||
app_row = next(row for row in response.json()["sources"] if row["source"] == "app")
|
||||
assert app_row["set"] is True
|
||||
assert app_row["whoami_ok"] is None
|
||||
|
||||
Reference in New Issue
Block a user