Skip to content

feat: auth primitives - JWT/SIWE (pkg/auth/jwt)#352

Open
sadiq1971 wants to merge 2 commits into
mainfrom
feat/auth-jwt
Open

feat: auth primitives - JWT/SIWE (pkg/auth/jwt)#352
sadiq1971 wants to merge 2 commits into
mainfrom
feat/auth-jwt

Conversation

@sadiq1971

Copy link
Copy Markdown
Member

1 of 3 — read-API authentication, split for review. Merge order: this → auth-service → auth-integration.

Self-contained, reusable auth building blocks with no app wiring (nothing calls them yet), so this can be reviewed as a library in isolation.

  • Issuer — mint short-lived RS256 JWTs; publish the public key as JWKS
  • Validator — verify bearer JWTs (in-process key or remote JWKS URL)
  • SIWEVerifier — validate EIP-4361 login messages, recover the signer, return the nonce
  • RequireAuth — bearer-token HTTP middleware
  • ParseRSAPrivateKey — load the base64-PEM signing key

Also folds in the previously-dead auth.JWTValidator (relocated from pkg/auth/jwt.go, zero callers). Full unit tests per file.

Self-contained, reusable auth building blocks with no app wiring:
- Issuer: mint short-lived RS256 JWTs; publish the public key as JWKS
- Validator: verify bearer JWTs (in-process key or remote JWKS)
- SIWEVerifier: validate EIP-4361 login messages and recover the signer
- RequireAuth: bearer-token HTTP middleware
- ParseRSAPrivateKey: load the base64-PEM signing key

Folds in the previously-dead auth.JWTValidator (relocated from pkg/auth/jwt.go).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a comprehensive JWT authentication package (pkg/auth/jwt) that supports Sign-In with Ethereum (SIWE) login flows, JWT session issuance, and verification via JWKS, replacing the previous auth implementation. The feedback identifies several important improvements: adding a mutex to serialize JWKS key refreshes to prevent a thundering herd problem, implementing a double-checked locking pattern in getKey to avoid redundant fetches, completely replacing the cached keys map during refreshes to ensure revoked keys are evicted, and adding a defensive nil check on msg.GetURI() in the SIWE verifier to prevent potential panics.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread pkg/auth/jwt/validator.go
Comment thread pkg/auth/jwt/validator.go
Comment thread pkg/auth/jwt/validator.go Outdated
Comment thread pkg/auth/jwt/verifier.go
@codecov-commenter

codecov-commenter commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 38 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@83d4116). Learn more about missing BASE report.

Files with missing lines Patch % Lines
pkg/auth/jwt/validator.go 73.80% 11 Missing and 11 partials ⚠️
pkg/auth/jwt/middleware.go 77.77% 6 Missing and 2 partials ⚠️
pkg/auth/jwt/issuer.go 93.54% 1 Missing and 1 partial ⚠️
pkg/auth/jwt/jwks.go 90.47% 1 Missing and 1 partial ⚠️
pkg/auth/jwt/keys.go 87.50% 1 Missing and 1 partial ⚠️
pkg/auth/jwt/verifier.go 90.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #352   +/-   ##
=======================================
  Coverage        ?   33.08%           
=======================================
  Files           ?      159           
  Lines           ?    12376           
  Branches        ?        0           
=======================================
  Hits            ?     4094           
  Misses          ?     7949           
  Partials        ?      333           
Flag Coverage Δ
unittests 33.08% <81.81%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/auth/jwt/issuer.go 93.54% <93.54%> (ø)
pkg/auth/jwt/jwks.go 90.47% <90.47%> (ø)
pkg/auth/jwt/keys.go 87.50% <87.50%> (ø)
pkg/auth/jwt/verifier.go 90.47% <90.47%> (ø)
pkg/auth/jwt/middleware.go 77.77% <77.77%> (ø)
pkg/auth/jwt/validator.go 73.80% <73.80%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sadiq1971 sadiq1971 changed the title Auth primitives: JWT/SIWE (pkg/auth/jwt) feat: Auth primitives - JWT/SIWE (pkg/auth/jwt) Jul 10, 2026
@sadiq1971 sadiq1971 changed the title feat: Auth primitives - JWT/SIWE (pkg/auth/jwt) feat: auth primitives - JWT/SIWE (pkg/auth/jwt) Jul 10, 2026
Address review: guard against a thundering-herd of JWKS fetches on concurrent
cache misses (refreshMu + double-check), and replace the key map on refresh so
rotated-out keys stop being trusted.
@sadiq1971

Copy link
Copy Markdown
Member Author

Addressed Gemini's review in 9fa7b9c:

  • validator.go (thundering herd, high): added a refreshMu and a double-checked cache read in getKey, so a burst of concurrent misses triggers a single JWKS fetch.
  • validator.go (key rotation, security-high): refreshKeys now replaces the key cache wholesale instead of merging, so rotated-out keys stop being trusted.
  • verifier.go (GetURI nil-guard, medium): skipped — not applicable. siwe-go's Message.GetURI() returns a url.URL value, not a pointer, so it can't be nil and the suggested gotURI == nil check wouldn't compile.

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.

2 participants