forked from baron/baron-sso
레포 업데이트
This commit is contained in:
Lectom C Han
@@ -0,0 +1,150 @@
|
||||
# Headless Login Debug Improvement Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Make headless password login failures return safe reason-specific response codes and produce debug-only diagnostic logs.
|
||||
|
||||
**Architecture:** Keep the change local to `AuthHandler` by introducing a headless-password-specific error classification helper and reusing it across response mapping and structured logging. Preserve current success-path behavior while adding explicit tests for reason codes and debug-field gating.
|
||||
|
||||
**Tech Stack:** Go, Fiber, `log/slog`, `stretchr/testify`, `go-jose`
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Add failing response-classification tests
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/internal/handler/auth_handler_login_test.go`
|
||||
- Reference: `backend/internal/handler/auth_handler.go`
|
||||
|
||||
- [ ] **Step 1: Write failing tests for classified 401 responses**
|
||||
|
||||
Add tests for:
|
||||
|
||||
- audience mismatch -> `code=invalid_client_assertion_audience`
|
||||
- signature mismatch -> `code=invalid_client_assertion_signature`
|
||||
- iss/sub mismatch -> `code=invalid_client_assertion_iss_sub`
|
||||
- expired assertion -> `code=invalid_client_assertion_expired`
|
||||
- invalid credentials -> `code=password_or_email_mismatch`
|
||||
|
||||
- [ ] **Step 2: Run only the new response tests and verify they fail for the expected reason**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd backend && go test ./internal/handler -run 'TestHeadlessPasswordLogin_(AudienceMismatchReturnsDetailedCode|SignatureMismatchReturnsDetailedCode|IssSubMismatchReturnsDetailedCode|ExpiredAssertionReturnsDetailedCode)|TestPasswordLogin_InvalidCredentials_ReturnsCode' -v
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- new headless response tests fail because the handler still returns generic codes/messages
|
||||
- existing invalid-credentials test remains green unless intentionally updated
|
||||
|
||||
### Task 2: Add failing log-gating tests
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/internal/handler/auth_handler_login_test.go`
|
||||
- Reference: `backend/internal/logger/logger.go`
|
||||
|
||||
- [ ] **Step 1: Write tests for debug-only diagnostic logging**
|
||||
|
||||
Add tests that install a temporary `slog` handler backed by a buffer and assert:
|
||||
|
||||
- debug logger emits `expected_audiences` and `received_audiences`
|
||||
- info logger does not emit those debug-only fields
|
||||
|
||||
- [ ] **Step 2: Run only the new log tests and verify they fail**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd backend && go test ./internal/handler -run 'TestHeadlessPasswordLogin_(DebugLogIncludesDiagnostics|InfoLogOmitsDebugDiagnostics)' -v
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- tests fail because the current handler does not emit reason-specific structured logs yet
|
||||
|
||||
### Task 3: Implement classified headless assertion failures
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/internal/handler/auth_handler.go`
|
||||
|
||||
- [ ] **Step 1: Add a local classified error type and helpers**
|
||||
|
||||
Implement a small helper shape for:
|
||||
|
||||
- `status`
|
||||
- `code`
|
||||
- `safeMessage`
|
||||
- `logMessage`
|
||||
- `debugAttrs`
|
||||
|
||||
- [ ] **Step 2: Make claim validation return classified reasons**
|
||||
|
||||
Replace generic string errors from `validateHeadlessClientAssertionClaims` with typed/classified failures for:
|
||||
|
||||
- iss/sub mismatch
|
||||
- expired
|
||||
- not active yet
|
||||
- iat in future
|
||||
- audience mismatch
|
||||
|
||||
- [ ] **Step 3: Make assertion verification return signature/jwks-specific failures**
|
||||
|
||||
Ensure parse, jwks load, claims, and signature-verification paths map to distinct safe codes/messages.
|
||||
|
||||
### Task 4: Implement structured logging and handler integration
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/internal/handler/auth_handler.go`
|
||||
|
||||
- [ ] **Step 1: Add a helper to detect whether debug logging is enabled**
|
||||
|
||||
Use the default `slog` logger handler to gate diagnostic fields without changing global logger initialization.
|
||||
|
||||
- [ ] **Step 2: Log classified assertion failures in `HeadlessPasswordLogin`**
|
||||
|
||||
Log:
|
||||
|
||||
- reason code
|
||||
- client id
|
||||
- path
|
||||
- debug-only attrs when debug logging is enabled
|
||||
|
||||
- [ ] **Step 3: Log classified credential failures**
|
||||
|
||||
Keep response behavior safe while adding a structured log line for credential mismatch.
|
||||
|
||||
### Task 5: Verify and document
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/test-plan/backend-test-inventory.md` if headless password login inventory needs expansion
|
||||
- Modify: issue `#502` comment with failing-test evidence and implementation summary
|
||||
|
||||
- [ ] **Step 1: Run focused handler tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd backend && go test ./internal/handler -run 'TestHeadlessPasswordLogin_|TestPasswordLogin_InvalidCredentials_ReturnsCode' -v
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- all targeted tests pass
|
||||
|
||||
- [ ] **Step 2: Run broader backend handler regression tests if practical**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cd backend && go test ./internal/handler -v
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- no regressions in auth handler tests
|
||||
|
||||
- [ ] **Step 3: Update issue `#502` with what failed first, what changed, and what passed**
|
||||
|
||||
- [ ] **Step 4: Review whether `docs/test-plan/backend-test-inventory.md` or troubleshooting docs should be updated**
|
||||
146
docs/superpowers/specs/2026-04-01-headless-login-debug-design.md
Normal file
146
docs/superpowers/specs/2026-04-01-headless-login-debug-design.md
Normal file
@@ -0,0 +1,146 @@
|
||||
# Headless Login Debug and Error Classification Design
|
||||
|
||||
## Scope
|
||||
|
||||
This design covers issue `#502` only.
|
||||
|
||||
- Target endpoint: `POST /api/v1/auth/headless/password/login`
|
||||
- Goal: classify 401 failure reasons for headless password login, return a safe response to RP clients, and emit richer diagnostics only when debug logging is enabled
|
||||
- Non-goal: introduce a shared auth-wide error/observability layer across all handlers. That follow-up work is tracked separately in issue `#503`.
|
||||
|
||||
## Problem
|
||||
|
||||
Current behavior makes production debugging slow.
|
||||
|
||||
- Multiple assertion failures collapse into `invalid_client_assertion`
|
||||
- The handler often returns early without emitting a reason-specific log line
|
||||
- RP clients cannot distinguish safe failure categories such as audience mismatch vs signature mismatch
|
||||
- Server operators cannot quickly tell whether the failure came from client assertion validation or user credential validation
|
||||
|
||||
## Constraints
|
||||
|
||||
- Response policy must follow the agreed level `2`: return `code + short safe message`
|
||||
- Debug logs may include level `3`-style diagnostics, but only when the logger is configured for debug level
|
||||
- Sensitive values must never be logged: raw `client_assertion`, password, session token, cookies
|
||||
- Existing successful headless password login flow must remain unchanged
|
||||
- Existing policy requires failing tests first
|
||||
|
||||
## Recommended Approach
|
||||
|
||||
Introduce a small headless-password-specific failure classification layer inside `auth_handler.go`.
|
||||
|
||||
The classification layer should map each failure into:
|
||||
|
||||
- `status`
|
||||
- `code`
|
||||
- `safeMessage`
|
||||
- `logMessage`
|
||||
- `debugFields`
|
||||
|
||||
This stays local to the current handler so the change remains small, but the structure should be reusable enough to extract later in issue `#503`.
|
||||
|
||||
## Failure Categories
|
||||
|
||||
### Client assertion failures
|
||||
|
||||
- `invalid_client_assertion_parse`
|
||||
- `invalid_client_assertion_signature`
|
||||
- `invalid_client_assertion_iss_sub`
|
||||
- `invalid_client_assertion_expired`
|
||||
- `invalid_client_assertion_not_before`
|
||||
- `invalid_client_assertion_iat_future`
|
||||
- `invalid_client_assertion_audience`
|
||||
- `invalid_client_assertion_jwks_load`
|
||||
|
||||
### Credential failure
|
||||
|
||||
- `password_or_email_mismatch`
|
||||
|
||||
## Response Contract
|
||||
|
||||
Responses should expose only safe details.
|
||||
|
||||
Examples:
|
||||
|
||||
- `{"code":"invalid_client_assertion_audience","error":"Client assertion audience mismatch"}`
|
||||
- `{"code":"invalid_client_assertion_signature","error":"Client assertion signature verification failed"}`
|
||||
- `{"code":"password_or_email_mismatch","error":"Invalid credentials"}`
|
||||
|
||||
The response must not echo:
|
||||
|
||||
- expected vs received audience values
|
||||
- full claim payload
|
||||
- kid mismatch internals beyond the safe message
|
||||
- raw token values
|
||||
|
||||
## Logging Strategy
|
||||
|
||||
### Normal log level
|
||||
|
||||
Emit one structured warning/error log per classified failure with:
|
||||
|
||||
- `reason_code`
|
||||
- `client_id`
|
||||
- `path`
|
||||
- `req_id` if present in context/log pipeline
|
||||
|
||||
Message examples:
|
||||
|
||||
- `headless password login client assertion failed`
|
||||
- `headless password login credential authentication failed`
|
||||
|
||||
### Debug log level
|
||||
|
||||
Add diagnostic fields that are useful for root cause analysis:
|
||||
|
||||
- `expected_audiences`
|
||||
- `received_audiences`
|
||||
- `received_kid`
|
||||
- `jwks_refreshed`
|
||||
- `claim_issuer`
|
||||
- `claim_subject`
|
||||
- `claim_expires_at`
|
||||
- `claim_not_before`
|
||||
- `claim_issued_at`
|
||||
- `login_challenge_prefix`
|
||||
|
||||
The `login_challenge` should be truncated before logging.
|
||||
|
||||
## Implementation Shape
|
||||
|
||||
Inside `backend/internal/handler/auth_handler.go`:
|
||||
|
||||
1. Add a small internal type for classified headless assertion errors.
|
||||
2. Make `validateHeadlessClientAssertionClaims` return structured reasons instead of generic strings.
|
||||
3. Make `verifyHeadlessClientAssertion` return a classified failure that the handler can both:
|
||||
- convert to a safe HTTP response
|
||||
- log with optional debug fields
|
||||
4. Add a small helper that checks whether debug logging is enabled for the current default `slog` logger.
|
||||
5. Log credential authentication failures with their reason code as well.
|
||||
|
||||
## Testing
|
||||
|
||||
Add failing tests first in `backend/internal/handler/auth_handler_login_test.go`.
|
||||
|
||||
Required tests:
|
||||
|
||||
- audience mismatch returns `401 + invalid_client_assertion_audience`
|
||||
- signature mismatch returns `401 + invalid_client_assertion_signature`
|
||||
- iss/sub mismatch returns `401 + invalid_client_assertion_iss_sub`
|
||||
- expired assertion returns `401 + invalid_client_assertion_expired`
|
||||
- invalid credentials still returns `401 + password_or_email_mismatch`
|
||||
- debug logger captures diagnostic fields for assertion audience mismatch
|
||||
- non-debug logger does not emit debug-only fields
|
||||
|
||||
## Risks
|
||||
|
||||
- Over-classifying too much detail into RP responses could create unnecessary information disclosure
|
||||
- Logging helper implementation must not assume a specific handler type beyond standard `slog`
|
||||
- Existing tests may assume the older generic `invalid_client_assertion` code and will need intentional updates
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- Operators can distinguish signature, claims, and credential failures from logs
|
||||
- RP clients get safe but actionable reason codes
|
||||
- Debug mode includes additional diagnostics without leaking secrets
|
||||
- Existing success path and previously covered headless login behavior remain green
|
||||
Reference in New Issue
Block a user