docs: local authentication mode design spec#1354
Merged
Conversation
Design spec for AUTH_MODE=local covering credential bootstrap, Argon2id hashing, login API, rate limiting, login form architecture, nginx integration, and security considerations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace SHA-256 hash-based timing-safe comparison with direct timingSafeEqual on padded buffers — simpler and equally secure since usernames are not secret values. Addresses CodeRabbit review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughAdds a detailed design for a new local authentication mode (AUTH_MODE=local), describing credential bootstrap, Argon2id hashing, login endpoints and UI, in-memory rate limiting, session handling, Nginx serving changes, environment variables, and testing/migration guidance. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Nginx
participant Frontend as Login Page
participant API as Rackula API
participant Auth as Local Auth Module
participant Store as Session Store
User->>Nginx: GET /auth/login
Nginx->>Frontend: serve login page (static)
User->>Frontend: submit credentials (POST /api/auth/login)
Frontend->>API: POST /api/auth/login (credentials)
API->>Auth: validate credentials, check rate-limit
Auth->>Auth: verify Argon2id hash (timing-safe)
alt success
Auth->>Store: create session, set cookie
API-->>Frontend: 200 + Set-Cookie
Frontend-->>User: redirect to app
else failure
Auth-->>API: error (401 or 429)
API-->>Frontend: error response
Frontend-->>User: show error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
codeant-ai
bot
added
the
size:L
Contributor
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/2026-02-27-local-auth-design.md`:
- Around line 79-86: The fenced code block showing Argon2id parameters is
missing a language identifier, which affects rendering; update the block that
contains "type: argon2id" / "memoryCost: 65536" etc. to include a language tag
such as `text` or `properties` after the opening triple backticks (e.g.,
```text) so the configuration-style content renders correctly.
- Around line 282-286: Clarify and enforce session invalidation when switching
RACKULA_AUTH_MODE from "oidc" to "local": update the migration docs to recommend
invalidating existing sessions by rotating the session secret or expiring
session cookies and/or provide a clear command/step to purge server-side session
store after setting RACKULA_LOCAL_USERNAME and RACKULA_LOCAL_PASSWORD and
changing RACKULA_AUTH_MODE, and add a note that automated tooling could
optionally trigger this rotation to avoid subject mismatches (e.g.,
"user@example.com" vs "admin") that break authorization or audit trails.
- Line 39: Implement Docker secrets support by checking for environment
variables with a _FILE suffix (e.g., RACKULA_LOCAL_PASSWORD_FILE and analogous
session secret file var) and reading the secret from that file when present,
falling back to the plain env var if not; update the local auth initialization
logic that currently reads RACKULA_LOCAL_PASSWORD to prefer file-based values
when RACKULA_LOCAL_PASSWORD_FILE is set and apply the same pattern to the
session secret handling so both secrets use the secure Docker secrets flow.
- Around line 374-380: Add unit tests that assert the application fails fast
during credential bootstrap by exercising the validation logic (e.g., the
function/method responsible for bootstrap validation such as
validateBootstrapCredentials or bootstrapStartup in the auth/init code): include
tests that (1) supplying invalid credentials causes startup to throw with a
clear message, (2) omitting required env vars RACKULA_LOCAL_USERNAME or
RACKULA_LOCAL_PASSWORD triggers startup failure, (3) providing a password
shorter than 12 characters fails validation, and (4) if you implement Docker
secrets file reading, include a test that missing/invalid secret file content
causes startup failure; make assertions on thrown error types/messages so the
tests guarantee the fail-fast behavior described in section 1.
- Around line 143-151: The fenced code block showing the credential verification
flow is missing a language identifier; update the opening fence for that
numbered list (the credential verification flow block: "1. Rate limit check ...
7. On success -> reset rate limit counter, create signed session token") to
include a language identifier such as text or pseudocode (e.g., change ``` to
```text) so the block is treated as plain/pseudo code by renderers.
- Around line 354-369: Update the "## 13. Files to Modify" table to include the
documentation files referenced elsewhere in the plan: add rows for
docs/deployment/AUTHENTICATION.md (new/modified — migration steps, local mode
setup), and consider adding docs/ARCHITECTURE.md and docs/reference/SPEC.md
where appropriate; ensure the table entries mirror the style used for other rows
(file name in first column, brief change description in second) so the plan's
mention of AUTHENTICATION.md (line 289) is reflected in this modification list.
| 4. Discard plaintext password from process memory | ||
| 5. If either missing or invalid -> **fail startup** with clear error | ||
|
|
||
| **Docker secrets support** (stretch goal): `RACKULA_LOCAL_PASSWORD_FILE=/run/secrets/local_password` reads from file instead of env. Same pattern documented (but not yet implemented) for session secret. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider prioritizing Docker secrets support.
While the environment variable approach is simpler for MVP, marking Docker secrets as a "stretch goal" may be too conservative given the security implications. The plaintext password visibility in docker inspect is a real risk (acknowledged in the Devil's Advocate section at line 325). Since Docker secrets support is relatively straightforward to implement (read from file if _FILE suffix exists, pattern already documented for session secret), consider including it in the MVP scope rather than deferring it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-27-local-auth-design.md` at line 39, Implement Docker
secrets support by checking for environment variables with a _FILE suffix (e.g.,
RACKULA_LOCAL_PASSWORD_FILE and analogous session secret file var) and reading
the secret from that file when present, falling back to the plain env var if
not; update the local auth initialization logic that currently reads
RACKULA_LOCAL_PASSWORD to prefer file-based values when
RACKULA_LOCAL_PASSWORD_FILE is set and apply the same pattern to the session
secret handling so both secrets use the secure Docker secrets flow.
Comment on lines
374
to
380
| ### Unit Tests | ||
|
|
||
| - Argon2id hash/verify round-trip | ||
| - Rate limiter: 5 failures trigger lockout, lockout expires, success resets counter, separate IPs tracked independently | ||
| - Login endpoint: valid creds -> 200 + cookie, invalid -> 401, missing fields -> 400, rate limited -> 429 | ||
| - Timing-safe username comparison | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add startup validation tests.
The testing strategy is comprehensive, but consider adding unit tests for credential bootstrap validation:
- Invalid credentials cause startup failure with clear error messages
- Missing required env vars (
RACKULA_LOCAL_USERNAME,RACKULA_LOCAL_PASSWORD) cause startup failure - Password below 12 characters causes startup failure
- Docker secrets file reading (if implemented in MVP)
These tests ensure the fail-fast behavior described in section 1 works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-27-local-auth-design.md` around lines 374 - 380, Add unit
tests that assert the application fails fast during credential bootstrap by
exercising the validation logic (e.g., the function/method responsible for
bootstrap validation such as validateBootstrapCredentials or bootstrapStartup in
the auth/init code): include tests that (1) supplying invalid credentials causes
startup to throw with a clear message, (2) omitting required env vars
RACKULA_LOCAL_USERNAME or RACKULA_LOCAL_PASSWORD triggers startup failure, (3)
providing a password shorter than 12 characters fails validation, and (4) if you
implement Docker secrets file reading, include a test that missing/invalid
secret file content causes startup failure; make assertions on thrown error
types/messages so the tests guarantee the fail-fast behavior described in
section 1.
- Add language tags to fenced code blocks (text) - Recommend session secret rotation on OIDC->local migration - Add credential bootstrap fail-fast tests to testing strategy - Add AUTHENTICATION.md to files-to-modify table Skipped: Docker secrets implementation (explicitly deferred as stretch goal in the design spec). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/plans/2026-02-27-local-auth-design.md (1)
355-371: 🧹 Nitpick | 🔵 TrivialConsider updates to ARCHITECTURE.md and SPEC.md.
The files list includes
docs/deployment/AUTHENTICATION.mdbut doesn't mention whetherdocs/reference/ARCHITECTURE.mdordocs/reference/SPEC.mdneed updates. As per coding guidelines, ARCHITECTURE.md provides the high-level overview and SPEC.md is the authoritative technical specification. Adding a new authentication mode likely requires updates to both.📋 Suggested additions to the table
| `docs/deployment/AUTHENTICATION.md` | Add local auth setup guide, migration steps between modes | +| `docs/reference/ARCHITECTURE.md` | Document AUTH_MODE=local in authentication architecture overview | +| `docs/reference/SPEC.md` | Add local auth specification (credential flow, endpoints, session handling) |Based on learnings: Start with
docs/ARCHITECTURE.mdfor codebase overview and consultdocs/reference/SPEC.mdas the authoritative technical specification. Organize documentation by purpose: ARCHITECTURE.md for high-level overview, guides/ for testing and accessibility patterns, reference/ for specifications and technical details, planning/ for roadmap.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/plans/2026-02-27-local-auth-design.md` around lines 355 - 371, Update the documentation to include the new local-auth mode: add a high-level overview of local authentication and where it fits into the system in docs/reference/ARCHITECTURE.md (referencing the new files like src/login.ts, src/lib/components/LoginForm.svelte, and api/src/auth-logger.ts), and add concrete technical requirements, env var definitions (RACKULA_LOCAL_*), API behavior (api/src/app.ts / api/src/security.ts conditional routes and config parsing), and deployment/config steps to docs/reference/SPEC.md so it aligns with the new docs/deployment/AUTHENTICATION.md guide; ensure SPEC.md includes precise env names, expected request/response shapes for /auth/login, and migration notes for deploy/docker-compose.persist.yml and deploy/nginx.conf.template.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/2026-02-27-local-auth-design.md`:
- Line 284: The document uses mixed British/American spelling for
"authorization" (e.g., "authorisation" near the RACKULA_AUTH_SESSION_SECRET
recommendation), so search the file for all occurrences of "authorisation" and
replace them with the American spelling "authorization" to standardize
terminology; verify headings, inline text, code comments, and any related
phrases (e.g., "authorize"/"authorise") are consistent and update them as needed
to "authorize"/"authorization".
- Line 145: Update the spec to describe the simplified timing-safe username
comparison: replace the padded-buffer technique with computing a fixed-length
cryptographic hash (e.g., SHA-256) of each username and comparing those hashes
with Node.js crypto.timingSafeEqual; mention the need to normalize usernames
before hashing (case/whitespace rules), that hashes are equal-length so no
manual padding is required, and reference the use of crypto.timingSafeEqual (or
equivalent) as the comparison primitive so readers can implement
verifyUsernameHash() or similar consistently.
---
Duplicate comments:
In `@docs/plans/2026-02-27-local-auth-design.md`:
- Around line 355-371: Update the documentation to include the new local-auth
mode: add a high-level overview of local authentication and where it fits into
the system in docs/reference/ARCHITECTURE.md (referencing the new files like
src/login.ts, src/lib/components/LoginForm.svelte, and api/src/auth-logger.ts),
and add concrete technical requirements, env var definitions (RACKULA_LOCAL_*),
API behavior (api/src/app.ts / api/src/security.ts conditional routes and config
parsing), and deployment/config steps to docs/reference/SPEC.md so it aligns
with the new docs/deployment/AUTHENTICATION.md guide; ensure SPEC.md includes
precise env names, expected request/response shapes for /auth/login, and
migration notes for deploy/docker-compose.persist.yml and
deploy/nginx.conf.template.
|
|
||
| ```text | ||
| 1. Rate limit check (per-IP sliding window) | ||
| 2. Timing-safe username comparison (padded buffers + timingSafeEqual) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify timing-safe username comparison implementation.
The specification mentions "padded buffers + timingSafeEqual" but this approach adds complexity. According to the PR summary, there was a CodeRabbit reviewer change to "simplify timing-safe username comparison."
Consider documenting the final simplified approach here. If using Node.js crypto.timingSafeEqual, note that it requires equal-length buffers, so padding is necessary. However, a simpler approach might be to compare fixed-length hashes of the usernames rather than padding the raw strings.
💡 Simpler alternative approach
-2. Timing-safe username comparison (padded buffers + timingSafeEqual)
+2. Timing-safe username comparison (compare SHA-256 hashes of usernames)This avoids manual buffer padding and leverages cryptographic hashing for constant-time comparison.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-02-27-local-auth-design.md` at line 145, Update the spec to
describe the simplified timing-safe username comparison: replace the
padded-buffer technique with computing a fixed-length cryptographic hash (e.g.,
SHA-256) of each username and comparing those hashes with Node.js
crypto.timingSafeEqual; mention the need to normalize usernames before hashing
(case/whitespace rules), that hashes are equal-length so no manual padding is
required, and reference the use of crypto.timingSafeEqual (or equivalent) as the
comparison primitive so readers can implement verifyUsernameHash() or similar
consistently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
AUTH_MODE=local(feat: Local authentication mode (username/password) #1117) — username/password authentication for homelabbers without an IdPCloses #1117
Test plan
🤖 Generated with Claude Code
CodeAnt-AI Description
Design spec for local username/password authentication (AUTH_MODE=local)
What Changed
Impact
✅ Shorter homelab setup for single-admin deployments✅ Clearer login flow when no external IdP is available✅ Fewer successful brute-force attempts due to built-in rate limiting💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.