jpetree331
4c62de8d8b
Fixes 9 auth middleware findings from the A-3 recon audit. F-A3-11 CRITICAL: Removed JWT secret from WebAuthMiddleware log output. Replaced emoji-prefixed fmt.Printf with ETHOS-compliant log.Printf. No secret values in any log output. F-A3-7 CRITICAL: Config download now requires WebAuthMiddleware. GET /downloads/config/:agent_id is admin-only (agents never call it). F-A3-6 HIGH: Update package download now requires AuthMiddleware. GET /downloads/updates/:package_id requires valid agent JWT. F-A3-10 HIGH: Scheduler stats changed from AuthMiddleware to WebAuthMiddleware. Agent JWTs can no longer view scheduler internals. F-A3-13 LOW: RequireAdmin() middleware implemented. 7 security settings routes re-enabled (GET/PUT/POST under /security/settings). security_settings.go.broken renamed to .go, API mismatches fixed. F-A3-12 MEDIUM: JWT issuer claims added for token type separation. Agent tokens: issuer=redflag-agent, Web tokens: issuer=redflag-web. AuthMiddleware rejects tokens with wrong issuer. Grace period: tokens with no issuer still accepted (backward compat). F-A3-2 MEDIUM: /auth/verify now has WebAuthMiddleware applied. Endpoint returns 200 with valid=true for valid admin tokens. F-A3-9 MEDIUM: Agent self-unregister (DELETE /:id) now rate-limited using the same agent_reports rate limiter as other agent routes. F-A3-14 LOW: CORS origin configurable via REDFLAG_CORS_ORIGIN env var. Defaults to http://localhost:3000 for development. Added PATCH method and agent-specific headers to CORS config. All 27 server tests pass. All 14 agent tests pass. No regressions. See docs/A3_Fix_Implementation.md and docs/Deviations_Report.md (DEV-020 through DEV-022). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
133 lines
5.6 KiB
Markdown
133 lines
5.6 KiB
Markdown
# A-3 Auth Middleware Fix Implementation Report
|
|
|
|
**Date:** 2026-03-29
|
|
**Branch:** culurien
|
|
**Audit Reference:** A-3 Auth Middleware Audit
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
This document covers fixes for 9 auth middleware findings (F-A3-2 through F-A3-14).
|
|
|
|
---
|
|
|
|
## Files Changed
|
|
|
|
### Fixes
|
|
|
|
| File | Change |
|
|
|------|--------|
|
|
| `aggregator-server/internal/api/handlers/auth.go` | Removed JWT secret from log (F-A3-11), added `log` import, set issuer=redflag-web on web tokens (F-A3-12), issuer validation in WebAuthMiddleware, set user_role in context for RequireAdmin |
|
|
| `aggregator-server/internal/api/middleware/auth.go` | Added JWT issuer constants, issuer validation in AuthMiddleware (F-A3-12), backward compat grace period for missing issuer, added `log` import |
|
|
| `aggregator-server/internal/api/middleware/require_admin.go` | NEW: RequireAdmin() middleware (F-A3-13) — checks user_role from context |
|
|
| `aggregator-server/internal/api/middleware/cors.go` | CORS origin from REDFLAG_CORS_ORIGIN env var (F-A3-14), added PATCH method and agent headers |
|
|
| `aggregator-server/internal/api/handlers/security_settings.go` | Renamed from .broken, fixed API mismatches with service (F-A3-13) |
|
|
| `aggregator-server/cmd/server/main.go` | Protected config download with WebAuthMiddleware (F-A3-7), protected update download with AuthMiddleware (F-A3-6), scheduler stats changed to WebAuthMiddleware (F-A3-10), /auth/verify gets WebAuthMiddleware (F-A3-2), agent unregister gets rate limiter (F-A3-9), security settings routes uncommented (F-A3-13), securitySettingsHandler initialized |
|
|
| `config/.env.bootstrap.example` | Added REDFLAG_CORS_ORIGIN documentation |
|
|
| `aggregator-server/.env.example` | Added REDFLAG_CORS_ORIGIN documentation |
|
|
|
|
### Tests Updated
|
|
|
|
| File | Change |
|
|
|------|--------|
|
|
| `handlers/auth_middleware_leak_test.go` | Tests now PASS (secret removed from log) |
|
|
| `handlers/downloads_auth_test.go` | Updated to test with auth middleware, added agent JWT helper |
|
|
| `handlers/auth_verify_test.go` | No changes needed (already passes) |
|
|
| `handlers/agent_unregister_test.go` | Updated route registration strings |
|
|
| `middleware/scheduler_auth_test.go` | Updated to use WebAuthMiddleware, agent JWT now includes issuer |
|
|
| `middleware/token_confusion_test.go` | JWTs now include issuer claims |
|
|
| `middleware/require_admin_test.go` | No changes needed (AST scan passes) |
|
|
| `middleware/require_admin_behavior_test.go` | Removed //go:build ignore tag, tests active |
|
|
|
|
---
|
|
|
|
## JWT Issuer Claims (F-A3-12)
|
|
|
|
### New Issuer Values
|
|
- Agent tokens: `Issuer = "redflag-agent"` (set in `GenerateAgentToken`)
|
|
- Web tokens: `Issuer = "redflag-web"` (set in `Login` handler)
|
|
|
|
### Validation
|
|
- `AuthMiddleware` rejects tokens with `Issuer != "redflag-agent"` (if issuer is present)
|
|
- `WebAuthMiddleware` rejects tokens with `Issuer != "redflag-web"` (if issuer is present)
|
|
|
|
### Backward Compat Grace Period
|
|
- Tokens with empty/absent issuer are allowed through with a logged warning
|
|
- This preserves compatibility with deployed agents that have existing JWTs
|
|
- TODO: Remove issuer-absent grace period after 30 days from deployment
|
|
|
|
---
|
|
|
|
## RequireAdmin Implementation (F-A3-13)
|
|
|
|
- Located in `middleware/require_admin.go`
|
|
- Runs AFTER WebAuthMiddleware (depends on `user_role` in context)
|
|
- WebAuthMiddleware updated to set `user_role` from `UserClaims.Role`
|
|
- If role != "admin": returns 403 with `[WARNING] [server] [auth] non_admin_access_attempt`
|
|
|
|
### Security Settings Routes Re-enabled (7 routes)
|
|
1. `GET /api/v1/security/settings` — GetAllSecuritySettings
|
|
2. `GET /api/v1/security/settings/audit` — GetSecurityAuditTrail
|
|
3. `GET /api/v1/security/settings/overview` — GetSecurityOverview
|
|
4. `GET /api/v1/security/settings/:category` — GetSecuritySettingsByCategory
|
|
5. `PUT /api/v1/security/settings/:category/:key` — UpdateSecuritySetting
|
|
6. `POST /api/v1/security/settings/validate` — ValidateSecuritySettings
|
|
7. `POST /api/v1/security/settings/apply` — ApplySecuritySettings
|
|
|
|
All protected by: WebAuthMiddleware + RequireAdmin
|
|
|
|
---
|
|
|
|
## Test Results
|
|
|
|
### Server Tests (all passing)
|
|
```
|
|
--- PASS: TestAgentAuthMiddlewareDoesNotLogSecret
|
|
--- PASS: TestAgentAuthMiddlewareLogHasNoEmoji
|
|
--- PASS: TestRequireAdminBlocksNonAdminUsers
|
|
--- PASS: TestRequireAdminMiddlewareExists
|
|
--- PASS: TestSchedulerStatsRequiresAdminAuth
|
|
--- PASS: TestSchedulerStatsCurrentlyAcceptsAgentJWT
|
|
--- PASS: TestWebTokenRejectedByAgentAuthMiddleware
|
|
--- PASS: TestAgentTokenRejectedByWebAuthMiddleware
|
|
ok middleware
|
|
|
|
--- PASS: TestWebAuthMiddlewareDoesNotLogSecret
|
|
--- PASS: TestWebAuthMiddlewareLogFormatHasNoEmoji
|
|
--- PASS: TestWebAuthMiddlewareLogFormatCompliant
|
|
--- PASS: TestConfigDownloadRequiresAuth
|
|
--- PASS: TestConfigDownloadCurrentlyUnauthenticated
|
|
--- PASS: TestUpdatePackageDownloadRequiresAuth
|
|
--- PASS: TestUpdatePackageDownloadCurrentlyUnauthenticated
|
|
--- PASS: TestAuthVerifyAlwaysReturns401WithoutMiddleware
|
|
--- PASS: TestAuthVerifyWorksWithMiddleware
|
|
--- PASS: TestAgentSelfUnregisterHasNoRateLimit
|
|
--- PASS: TestAgentSelfUnregisterShouldHaveRateLimit
|
|
--- PASS: TestRetryCommandEndpointProducesUnsignedCommand
|
|
--- PASS: TestRetryCommandEndpointMustProduceSignedCommand
|
|
--- SKIP: TestRetryCommandHTTPHandlerProducesUnsignedCommand_Integration
|
|
ok handlers
|
|
|
|
--- PASS: TestRetryCommandIsUnsigned
|
|
--- PASS: TestRetryCommandMustBeSigned
|
|
--- PASS: TestSignedCommandNotBoundToAgent
|
|
--- PASS: TestOldFormatCommandHasNoExpiry
|
|
ok services
|
|
|
|
--- PASS: TestGetPendingCommandsHasNoTTLFilter
|
|
--- PASS: TestGetPendingCommandsMustHaveTTLFilter
|
|
--- PASS: TestRetryCommandQueryDoesNotCopySignature
|
|
ok queries
|
|
```
|
|
|
|
### Agent Tests (14/14 passing, no regressions)
|
|
```
|
|
All 14 crypto tests pass. No changes to agent code in A-3.
|
|
```
|
|
|
|
### Build
|
|
```
|
|
go build ./... — BUILD OK
|
|
```
|