Skip to content

Enable password manager autofill for admin tokens - #873

Open
mincia1110 wants to merge 2 commits into
lidge-jun:devfrom
mincia1110:agent/admin-token-password-manager
Open

Enable password manager autofill for admin tokens#873
mincia1110 wants to merge 2 commits into
lidge-jun:devfrom
mincia1110:agent/admin-token-password-manager

Conversation

@mincia1110

@mincia1110 mincia1110 commented Aug 2, 2026

Copy link
Copy Markdown

Summary

  • replace the remote dashboard's window.prompt() admin-token flow with a standard sign-in form that password managers can recognize
  • keep the submitted admin token in memory only; OpenCodex does not write it to localStorage or sessionStorage
  • preserve the shared 401 resolution gate, cancellation behavior, and focus restoration
  • add localized dialog copy, focused regression coverage, and dashboard documentation

Why

Remote dashboards must authenticate with the raw admin token, but window.prompt() does not expose the standard form and autocomplete semantics used by browser password managers. This forces operators to copy and paste the credential again after reloads instead of letting their chosen password manager offer save and autofill behavior.

The root cause is the prompt primitive itself: it cannot provide stable username/password fields, field names, or autocomplete="current-password" metadata.

Security and privacy

  • token persistence remains entirely browser/password-manager owned and opt-in
  • OpenCodex continues to retain the token only in module memory
  • no token, account identifier, hostname, or operator-specific value is added to source or tests
  • bun run privacy:scan passes

This PR touches management-credential handling and therefore requests explicit security review.

Validation

  • bun run test — 6,867 passed, 8 skipped
  • bun run typecheck
  • bun run privacy:scan
  • cd gui && bun test tests — 492 passed
  • cd gui && bun run lint
  • cd gui && bun run lint:i18n
  • cd gui && bun run build
  • cd docs-site && bun run build

Summary by CodeRabbit

  • New Features

    • Added a localized administrator sign-in dialog with browser autofill and password-manager support.
    • Added server-side token verification, clearer validation errors, cancellation handling, and secure in-memory token use.
    • Added administrator authentication translations across supported languages.
    • Streamlined locale handling for authentication prompts.
  • Documentation

    • Documented local and remote dashboard authentication, including browser-managed autofill and non-persistent token handling.
  • Tests

    • Added coverage for validation, retries, cancellation, cleanup, focus restoration, and token security.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The dashboard now uses a localized password form for admin authentication. The API verifies submitted tokens through /api/settings and uses an injectable prompt callback after unauthorized responses. Tokens remain out of browser storage. Tests cover submission, cancellation, cleanup, focus restoration, locale handling, and API retries.

Changes

Admin token authentication

Layer / File(s) Summary
Token dialog and localized form
gui/src/admin-token-dialog.ts, gui/src/i18n/*, gui/tests/admin-token-dialog.test.ts
Adds a localized modal with a fixed read-only username and autofill-compatible token field. The dialog trims tokens, rejects empty input, handles rejected and unavailable states, restores focus, removes itself, and does not write to browser storage.
API verification and prompt injection
gui/src/api.ts, gui/tests/api-auth-memory.test.ts
Validates tokens through /api/settings, replaces window.prompt with an injectable callback, limits prompting per request wave, and restores custom prompt callbacks during test reset. Tests verify invalid-token retry behavior and empty or canceled input handling.
Dashboard authentication documentation
docs-site/src/content/docs/guides/web-dashboard.md, docs-site/src/content/docs/ja/guides/web-dashboard.md, docs-site/src/content/docs/ko/guides/web-dashboard.md, docs-site/src/content/docs/ru/guides/web-dashboard.md, docs-site/src/content/docs/zh-cn/guides/web-dashboard.md
Documents loopback sessions, remote admin-token requirements, password-manager support, and in-memory token handling.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ApiAuthFetch
  participant requestAdminToken
  participant verifyAdminToken
  participant promptForAdminToken
  participant BrowserForm
  ApiAuthFetch->>requestAdminToken: receive unauthorized response
  requestAdminToken->>promptForAdminToken: request admin token
  promptForAdminToken->>BrowserForm: show localized password form
  BrowserForm-->>promptForAdminToken: submit token or cancel
  promptForAdminToken->>verifyAdminToken: verify submitted token
  verifyAdminToken-->>promptForAdminToken: accepted, rejected, or unavailable
  promptForAdminToken-->>requestAdminToken: return token or null
  requestAdminToken-->>ApiAuthFetch: retry authenticated request
Loading

Possibly related PRs

Suggested labels: enhancement, documentation

Suggested reviewers: ingwannu, wibias, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: enabling password-manager autofill for the admin-token sign-in flow.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@mincia1110
mincia1110 marked this pull request as ready for review August 2, 2026 02:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1ee3644b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread gui/src/admin-token-dialog.ts Outdated
Comment thread gui/src/admin-token-dialog.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs-site/src/content/docs/guides/web-dashboard.md`:
- Around line 32-36: Update the translated dashboard guide pages for ja, ko, ru,
and zh-cn by adding the missing sign-in section immediately after their opening
instructions. Mirror the source guide’s behavior: explain the loopback
exception, require an admin token for remote dashboards, describe the standard
password form and browser/password-manager autofill, and state that the
dashboard retains the token only in memory without localStorage or
sessionStorage persistence.

In `@gui/src/admin-token-dialog.ts`:
- Around line 39-69: Replace the duplicated field-label text in the admin token
dialog with purpose-specific i18n entries: add account and admin-token
field-label keys to all six locale dictionaries, then use the localized values
for accountLabel.textContent and tokenLabel.textContent. Keep
ADMIN_TOKEN_USERNAME only as the readonly username value, and do not reuse
titleText for the token label or hardcoded text for user-visible labels.

In `@gui/tests/admin-token-dialog.test.ts`:
- Around line 56-67: Add a test alongside the existing admin-token dialog tests
that submits an empty or whitespace-only password, verifies the dialog remains
open and the prompt promise is still pending, then submits a valid token and
asserts the promise resolves successfully. Reuse the existing
promptForAdminToken dialog setup and submission interaction symbols.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 708924f4-06db-4671-9dd0-801701333b8e

📥 Commits

Reviewing files that changed from the base of the PR and between 6ac6bea and d1ee364.

📒 Files selected for processing (11)
  • docs-site/src/content/docs/guides/web-dashboard.md
  • gui/src/admin-token-dialog.ts
  • gui/src/api.ts
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/tests/admin-token-dialog.test.ts
  • gui/tests/api-auth-memory.test.ts

Comment thread docs-site/src/content/docs/guides/web-dashboard.md
Comment thread gui/src/admin-token-dialog.ts
Comment thread gui/tests/admin-token-dialog.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant