Skip to content

feat(auth): first-run local administrator onboarding - #68

Merged
2233admin merged 1 commit into
2233admin:mainfrom
1012839419a-alt:feat/local-admin-onboarding
Sep 2, 2026
Merged

feat(auth): first-run local administrator onboarding#68
2233admin merged 1 commit into
2233admin:mainfrom
1012839419a-alt:feat/local-admin-onboarding

Conversation

@1012839419a-alt

Copy link
Copy Markdown
Contributor

Summary

Focused split of T1 (local administrator onboarding) out of draft PR #61 (Add first-run local administrator onboarding). Contains only the first 2 of #61's 7 commits:

  • 6c65f69 Add local administrator onboarding
  • 9873519 accept local sessions with stale fleet headers

What this PR does

  • make local administrator password setup the primary first-run path for self-hosted installs
  • keep OIDC optional and move Bootstrap access behind an explicit emergency-recovery disclosure
  • persist a single salted scrypt credential and issue server-signed 12-hour local sessions
  • allow only the exact status/setup/login endpoints through the unauthenticated Fleet boundary, with per-client failure limiting
  • accept a valid local administrator session even when an upgraded browser also sends a stale Fleet transport token
  • document the decision, installer flow, design states, and verification evidence

Why a separate PR

#61 was a draft bundling 4 independent topics (T1 local-admin onboarding / T2 agent observability + persistence / T3 dev toolchain / T4 fixed API image — see the breakdown comments on #61). T1 is the core security-relevant piece and is cleanly separable: it touches only auth/identity/installer/frontend-auth files and shares no files with T2/T3/T4. Extracting it gives reviewers a small, reviewable, mergeable unit while #61 stays open for the rest.

Changes

30 files changed, 876 insertions(+), 96 deletions(-):

  • backend/security/local_auth.py (new): scrypt credential store, 12-hour HS256 local sessions, per-client failure limiting
  • backend/security/fleet_auth.py: accept either a valid local session or a valid fleet token
  • backend/api/v1/local_auth.py (new): status/setup/login endpoints behind unauthenticated Fleet boundary
  • backend/models/identity.py + migration z7a8b9c0d1e2_add_local_admin: fixed local-admin record
  • backend/config.py: local-admin configuration
  • frontend/components/auth/local-admin-access.tsx (new): first-run password setup flow
  • frontend/app/login/page.tsx, frontend/components/auth/auth-provider.tsx, frontend/lib/auth/*, frontend/lib/api/endpoints.ts: local login wiring
  • scripts/install.ps1, scripts/install.sh, README.md, DESIGN.md, MOTION.md: installer flow + docs
  • openspec/changes/local-admin-onboarding/*: brief/design/directions/motion/qa/tasks
  • tests/unit/security/test_local_auth.py (new) + tests/unit/test_identity_models.py

Security notes

  • setup still requires BOOTSTRAP_ADMIN_TOKEN and can create only the fixed local-admin record
  • passwords require 12-256 characters and are stored only as salted scrypt hashes with fixed work parameters
  • sessions are HS256 tokens signed by SECRET_KEY and expire after 12 hours
  • failed setup/login attempts are bounded per client; tracked-client memory is bounded
  • concurrent first-run setup is guarded by the database primary key and returns 409
  • a stale invalid Fleet header cannot override an independently valid local session
  • no registration, invitation, multi-user local accounts, or password-reset subsystem is introduced

Verification (fresh run on this branch, 2026-08-08)

Evidence from the original #61 verification (unchanged code, same commits) additionally included: 36 focused local-auth/Fleet-auth tests after the stale-header fix, TypeScript type-check, frontend ESLint, login regression suite 4 passed, Next.js production build, Alembic single head with fresh-SQLite upgrade, live Docker deployment, and a real browser login reaching the Studio project page with refresh-preserved session.

Follow-ups

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 7f48b9b8-aebc-487a-b9d4-1b24a2b7893d

📥 Commits

Reviewing files that changed from the base of the PR and between 03e03f5 and c0dacac.

📒 Files selected for processing (6)
  • backend/api/v1/identity.py
  • backend/api/v1/system.py
  • backend/security/identity.py
  • backend/security/local_auth.py
  • tests/unit/security/test_identity.py
  • tests/unit/security/test_local_auth.py

📝 Summary

Summary by CodeRabbit

  • New Features

    • Local login now limits repeated failed attempts per client and temporarily blocks further attempts when the limit is reached.
    • System configuration responses now include the OpenCLI runtime revision.
    • Local-session identities now provide more complete profile and claim information.
  • Bug Fixes

    • Password verification now rejects hashes that use unsupported scrypt parameters.
    • Successful local logins reset the client’s failed-attempt limit.

Walkthrough

The changes add per-client local-login throttling, enforce canonical scrypt parameters, enrich local-session identities, expose the runtime revision, and expand authentication tests.

Changes

Authentication hardening

Layer / File(s) Summary
Per-client login throttling
backend/security/local_auth.py, backend/api/v1/identity.py, tests/unit/security/test_local_auth.py
Adds bounded failure tracking with HTTP 429 responses and Retry-After. Local login checks, records failures, and resets successful clients.
Canonical scrypt password validation
backend/security/local_auth.py, tests/unit/security/test_local_auth.py
Centralizes scrypt parameters for hashing. Password verification rejects hashes with different encoded parameters.
Local identity claim population
backend/security/identity.py, tests/unit/security/test_identity.py
Local-session identities now include filtered email, name, username, picture, and complete claims data. Tests verify the expanded response.

Runtime metadata

Layer / File(s) Summary
Runtime revision configuration payload
backend/api/v1/system.py
Adds runtime_revision from settings.opencli_runtime_revision to the system configuration response.

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

Sequence Diagram(s)

sequenceDiagram
  participant local_login
  participant LoginAttemptLimiter
  participant verify_password
  local_login->>LoginAttemptLimiter: check(client_id)
  local_login->>verify_password: validate credentials
  local_login->>LoginAttemptLimiter: record_failure(client_id) on invalid credentials
  local_login->>LoginAttemptLimiter: reset(client_id) on successful login
Loading

Poem

I’m a rabbit guarding the login gate,
Counting failed hops before they accumulate.
Scrypt stones now fit the same-sized row,
Local claims carry what identities show.
Runtime revision joins the config stream,
And tests keep watch beside the beam.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.86% 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
Title check ✅ Passed The title clearly and concisely identifies the main change: first-run local administrator onboarding.
Description check ✅ Passed The description directly explains the local administrator onboarding changes, security design, affected components, and verification results.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@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: 1

🧹 Nitpick comments (2)
docs/adr/0043-use-local-administrator-password-after-bootstrap.md (1)

16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Record the maximum password length.

The PR contract is 12–256 characters, but Line 16 records only a minimum. Add the upper bound so the ADR matches the API and frontend validation contract.

Proposed wording
-- The operator chooses a password of at least 12 characters.
+- The operator chooses a password of 12–256 characters.

As per coding guidelines, docs/adr/** must record durable architecture decisions.

🤖 Prompt for 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.

In `@docs/adr/0043-use-local-administrator-password-after-bootstrap.md` at line
16, Update the password-length requirement in the ADR statement to document the
complete 12–256 character contract, preserving the existing minimum-length and
salted scrypt storage details.

Source: Coding guidelines

frontend/scripts/check-login-theme-regressions.mjs (1)

17-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add component coverage for the local-auth flows.

frontend/scripts/check-login-theme-regressions.mjs only checks source-name presence. The source wiring is correct, but the remaining behavioral checks for setup, local login, Bootstrap recovery, status refresh, and recovery disclosure should be covered by component tests.

🤖 Prompt for 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.

In `@frontend/scripts/check-login-theme-regressions.mjs` around lines 17 - 30,
Extend the coverage beyond source-name assertions in the test named “login keeps
local setup, recovery, OIDC, development, and reduced-motion paths” by adding
component tests for local-admin setup, local sign-in, Bootstrap recovery, status
refresh, and recovery-disclosure behavior. Keep the existing source checks
intact and exercise these flows through rendered component interactions.
🤖 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 `@MOTION.md`:
- Around line 23-35: Make the reduced-motion contract consistent across
MOTION.md (lines 23-35) and openspec/changes/local-admin-onboarding/motion.md
(lines 5-8): explicitly state whether reveal.trim-line is allowed on
authentication surfaces, require no animation when reduced motion is enabled,
and require the immediate static text and focus result in the onboarding motion
specification.

---

Nitpick comments:
In `@docs/adr/0043-use-local-administrator-password-after-bootstrap.md`:
- Line 16: Update the password-length requirement in the ADR statement to
document the complete 12–256 character contract, preserving the existing
minimum-length and salted scrypt storage details.

In `@frontend/scripts/check-login-theme-regressions.mjs`:
- Around line 17-30: Extend the coverage beyond source-name assertions in the
test named “login keeps local setup, recovery, OIDC, development, and
reduced-motion paths” by adding component tests for local-admin setup, local
sign-in, Bootstrap recovery, status refresh, and recovery-disclosure behavior.
Keep the existing source checks intact and exercise these flows through rendered
component interactions.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e670ccda-e2c1-4953-8e56-965d4515f930

📥 Commits

Reviewing files that changed from the base of the PR and between 94ab53d and 30fc4cf.

📒 Files selected for processing (30)
  • DESIGN.md
  • MOTION.md
  • README.md
  • backend/api/v1/__init__.py
  • backend/api/v1/local_auth.py
  • backend/config.py
  • backend/migrations/versions/z7a8b9c0d1e2_add_local_admin.py
  • backend/models/__init__.py
  • backend/models/identity.py
  • backend/security/fleet_auth.py
  • backend/security/identity.py
  • backend/security/local_auth.py
  • docs/adr/0043-use-local-administrator-password-after-bootstrap.md
  • frontend/app/login/page.tsx
  • frontend/components/auth/auth-provider.tsx
  • frontend/components/auth/local-admin-access.tsx
  • frontend/lib/api/endpoints.ts
  • frontend/lib/auth/session.ts
  • frontend/lib/auth/types.ts
  • frontend/scripts/check-login-theme-regressions.mjs
  • openspec/changes/local-admin-onboarding/brief.md
  • openspec/changes/local-admin-onboarding/design.md
  • openspec/changes/local-admin-onboarding/directions.md
  • openspec/changes/local-admin-onboarding/motion.md
  • openspec/changes/local-admin-onboarding/qa.md
  • openspec/changes/local-admin-onboarding/tasks.md
  • scripts/install.ps1
  • scripts/install.sh
  • tests/unit/security/test_local_auth.py
  • tests/unit/test_identity_models.py

Comment thread MOTION.md Outdated
Comment on lines +23 to +35
## Procedural Motion

No procedural motion is used for authentication or recovery surfaces.

## Runtime Policy

CSS transitions are the default adapter for small state changes. The existing Motion React adapter may preserve the selected primitive where it is already loaded; no new animation runtime is introduced.

## Reduced Motion

When `prefers-reduced-motion` is enabled, state changes use immediate opacity changes and do not animate position, scale, or background effects.

Fallback: every animated confirmation has an immediate static state change with the same text and focus result.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Make the reduced-motion contract consistent.

MOTION.md requires an immediate state change for reduced motion and says that authentication uses no procedural motion. The feature specification permits a Motion React adapter but does not require an immediate static reduced-motion state. This can produce an animated reduced-motion path.

  • MOTION.md#L23-L35: Define whether reveal.trim-line is permitted on authentication surfaces and state that reduced motion has no animation.
  • openspec/changes/local-admin-onboarding/motion.md#L5-L8: Require the immediate static text and focus result when reduced motion is enabled.
📍 Affects 2 files
  • MOTION.md#L23-L35 (this comment)
  • openspec/changes/local-admin-onboarding/motion.md#L5-L8
🤖 Prompt for 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.

In `@MOTION.md` around lines 23 - 35, Make the reduced-motion contract consistent
across MOTION.md (lines 23-35) and
openspec/changes/local-admin-onboarding/motion.md (lines 5-8): explicitly state
whether reveal.trim-line is allowed on authentication surfaces, require no
animation when reduced motion is enabled, and require the immediate static text
and focus result in the onboarding motion specification.

@1012839419a-alt

Copy link
Copy Markdown
Contributor Author

CI: Next Frontend failure — 上游 CI 配置缺陷(next start vs output: standalone)

现象Next Frontend job 失败,page.goto: net::ERR_CONNECTION_REFUSED at http://127.0.0.1:3000/login(e2e/login.spec.mjs:4)

根因:job 的 WebServer 步骤用 next start --hostname 127.0.0.1 --port 3000,但项目 next.configoutput: standalone。日志明确:

⚠ "next start" does not work with "output: standalone" configuration. Use "node .next/standalone/server.js" instead.

next start 在 standalone 模式下不监听端口(或直接退出)→ e2e 连 3000 被拒。

为什么历史 PR 是绿的#65 等):同一警告也出现在 #65 的日志里,但那次 next start 恰好起来了(1 passed)。该失败是 standalone 生效与否的 runner 环境差异——间歇性 flake,不是 #68/#70 的前端代码问题:

  • next build 完全成功:✓ Compiled successfully + ✓ Generating static pages (35/35)
  • 本 PR 前端改动(global-agent-dock.tsx 等)已通过编译 + 全页 prerender,无 SSR 错误

建议:上游把 CI WebServer 步骤改为 standalone 模式正确启动方式(node .next/standalone/server.js,参考 Next.js 官方 standalone 部署文档),或 rerun 该 job 等 runner 恢复正常。本 PR 代码无需改动。

@2233admin
2233admin force-pushed the feat/local-admin-onboarding branch from ca9aec8 to c0dacac Compare September 2, 2026 07:21
@2233admin
2233admin merged commit f04a95d into 2233admin:main Sep 2, 2026
3 of 8 checks passed
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

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.

2 participants