feat(auth): first-run local administrator onboarding - #68
feat(auth): first-run local administrator onboarding#681012839419a-alt wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughOpenCLI now supports local administrator onboarding. A bootstrap token creates the administrator password, local sessions authenticate console requests, and the frontend provides setup, login, recovery, status, and validation flows while retaining OIDC and machine-token access. ChangesLocal administrator onboarding
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant LoginPage
participant AuthProvider
participant LocalAuthAPI
participant LocalAdminStore
Operator->>LoginPage: Enter bootstrap token and password
LoginPage->>AuthProvider: setupLocalAdmin(bootstrapToken, password)
AuthProvider->>LocalAuthAPI: POST /auth/local/setup
LocalAuthAPI->>LocalAdminStore: Create password hash
LocalAuthAPI-->>AuthProvider: Return access_token
AuthProvider-->>LoginPage: Persist identity token
LoginPage-->>Operator: Open authenticated console
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
|
✅ Health of changed files: 5.3 → 5.9 (+0.6) 📋 At a glance Files & modules (3)
✅ Health gate: passed 📌 Before you merge
🔎 More signals (4)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (3 modules)"]
m_backend["backend (6 files)"]:::changed
m_frontend["frontend (5 files)"]:::changed
m_tests["tests (1 file)"]:::changed
end
d_backend["backend"]
m_frontend -->|13 files| d_backend
d_backend["backend"]
m_tests -->|3 files| d_backend
d_frontend["frontend"]
m_tests -->|1 file| d_frontend
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (100 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (5)
2 more
🔗 Hidden coupling (2 files)
💀 Dead code (10 findings)
7 more
👀 Suggested reviewers @2233admin 📊 See the full report for this PR |
There was a problem hiding this comment.
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 winRecord 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 winAdd component coverage for the local-auth flows.
frontend/scripts/check-login-theme-regressions.mjsonly 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
📒 Files selected for processing (30)
DESIGN.mdMOTION.mdREADME.mdbackend/api/v1/__init__.pybackend/api/v1/local_auth.pybackend/config.pybackend/migrations/versions/z7a8b9c0d1e2_add_local_admin.pybackend/models/__init__.pybackend/models/identity.pybackend/security/fleet_auth.pybackend/security/identity.pybackend/security/local_auth.pydocs/adr/0043-use-local-administrator-password-after-bootstrap.mdfrontend/app/login/page.tsxfrontend/components/auth/auth-provider.tsxfrontend/components/auth/local-admin-access.tsxfrontend/lib/api/endpoints.tsfrontend/lib/auth/session.tsfrontend/lib/auth/types.tsfrontend/scripts/check-login-theme-regressions.mjsopenspec/changes/local-admin-onboarding/brief.mdopenspec/changes/local-admin-onboarding/design.mdopenspec/changes/local-admin-onboarding/directions.mdopenspec/changes/local-admin-onboarding/motion.mdopenspec/changes/local-admin-onboarding/qa.mdopenspec/changes/local-admin-onboarding/tasks.mdscripts/install.ps1scripts/install.shtests/unit/security/test_local_auth.pytests/unit/test_identity_models.py
| ## 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. |
There was a problem hiding this comment.
📐 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 whetherreveal.trim-lineis 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.
CI: Next Frontend failure — 上游 CI 配置缺陷(next start vs output: standalone)现象: 根因:job 的 WebServer 步骤用
为什么历史 PR 是绿的(#65 等):同一警告也出现在 #65 的日志里,但那次
建议:上游把 CI WebServer 步骤改为 standalone 模式正确启动方式( |
…dpoints in capability matrix
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:6c65f69Add local administrator onboarding9873519accept local sessions with stale fleet headersWhat this PR does
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 limitingbackend/security/fleet_auth.py: accept either a valid local session or a valid fleet tokenbackend/api/v1/local_auth.py(new): status/setup/login endpoints behind unauthenticated Fleet boundarybackend/models/identity.py+ migrationz7a8b9c0d1e2_add_local_admin: fixed local-admin recordbackend/config.py: local-admin configurationfrontend/components/auth/local-admin-access.tsx(new): first-run password setup flowfrontend/app/login/page.tsx,frontend/components/auth/auth-provider.tsx,frontend/lib/auth/*,frontend/lib/api/endpoints.ts: local login wiringscripts/install.ps1,scripts/install.sh,README.md,DESIGN.md,MOTION.md: installer flow + docsopenspec/changes/local-admin-onboarding/*: brief/design/directions/motion/qa/taskstests/unit/security/test_local_auth.py(new) +tests/unit/test_identity_models.pySecurity notes
Verification (fresh run on this branch, 2026-08-08)
pytest tests/unit/security/test_local_auth.py tests/unit/security/test_fleet_auth.py tests/unit/security/test_identity.py tests/unit/test_identity_models.py -m "not live": 43 passed (matches the original local-auth + Fleet-auth + identity + identity-model suite count from Add first-run local administrator onboarding #61)ruff checkon all touched Python files: All checks passedagent/local-admin-onboarding) ontomain(Scope workflow execution to the active trigger graph #60) with zero conflicts; diff identical to the original T1 rangeEvidence 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