Skip to content

refactor: harden security validations and simplify OAuth handlers#139

Merged
appleboy merged 2 commits intomainfrom
worktree-authgate
Mar 30, 2026
Merged

refactor: harden security validations and simplify OAuth handlers#139
appleboy merged 2 commits intomainfrom
worktree-authgate

Conversation

@appleboy
Copy link
Copy Markdown
Member

Summary

  • Security: Enforce minimum 32-byte JWT secret for HS256, reject PKCE plain method (S256 only), add session remember-me max-age upper bound (30 days), fix unsafe type assertions in OAuth callback (panic/DoS vector)
  • Simplification: Replace duplicate scopesAreCovered() with util.IsScopeSubset(), extract validateStateAndNonce() helper, define OAuth error code constants replacing ~20 inline magic strings, remove redundant "what" comments
  • Tests: Add validation tests for JWT secret length, session max-age, and PKCE plain rejection

Test plan

  • make generate — templates compile
  • make test — all tests pass (pre-existing Redis failures unrelated)
  • make lint — 0 issues
  • Manual verification of OAuth authorization code flow with S256 PKCE
  • Manual verification that short JWT secrets are rejected on startup

🤖 Generated with Claude Code

- Enforce minimum 32-byte JWT secret length for HS256 signing
- Reject PKCE plain method, only accept S256
- Add session remember-me max-age upper bound validation (30 days)
- Use safe type assertions in OAuth callback to prevent panics
- Replace duplicate scopesAreCovered with util.IsScopeSubset
- Extract validateStateAndNonce helper to deduplicate auth handlers
- Define OAuth error code constants to replace inline magic strings
- Remove redundant comments restating function names

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 41.42857% with 41 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/handlers/authorization.go 0.00% 23 Missing ⚠️
internal/handlers/token.go 33.33% 16 Missing ⚠️
internal/bootstrap/providers.go 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

- Add AlgHS256, AlgRS256, AlgES256 constants in config package
- Replace inline "HS256"/"RS256"/"ES256" strings across config,
  token, bootstrap, and handlers packages

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@appleboy appleboy merged commit a88d1d4 into main Mar 30, 2026
17 checks passed
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.

1 participant