Authenticate read APIs (SIWE + JWT)#351
Conversation
Add optional Sign-In with Ethereum (EIP-4361) login that issues short-lived RS256 JWTs, and gate the transfer read endpoints and /profile behind them so a caller can only read their own data. - pkg/auth/jwt: Issuer, Validator, SIWEVerifier, JWKS, bearer middleware, key parsing - pkg/auth/service: login orchestration (consumer interfaces + mockery), handlers, log decorator - pkg/auth/service/nonce_provider: address-keyed in-memory nonce store (reuse per address, reject-when-full) - Read endpoints resolve the caller from the bearer token when auth is enabled, else fall back to ?address= (auth is optional; nil config disables it) - Write endpoints keep the existing EIP-191 signature path unchanged
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/outbound-transfer-authz #351 +/- ##
================================================================
+ Coverage 32.09% 33.74% +1.65%
================================================================
Files 154 163 +9
Lines 12270 12540 +270
================================================================
+ Hits 3938 4232 +294
+ Misses 8016 7970 -46
- Partials 316 338 +22
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust Sign-In with Ethereum (SIWE, EIP-4361) login flow and JWT-based session authentication for read endpoints, replacing the previous basic JWKS validation. It adds comprehensive authentication primitives, including an in-memory nonce provider with capacity limits, a JWT issuer/validator, and a SIWE verifier. Read endpoints such as /profile and transfer list endpoints are now guarded by the new authentication middleware. Feedback on the implementation highlights opportunities to prevent cache stampedes during concurrent JWKS fetches by serializing the fetch operation with a mutex, and to defensively guard against a potential nil pointer dereference panic in the login logging decorator.
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.
| type Validator struct { | ||
| jwksURL string | ||
| issuer string | ||
| keys map[string]any | ||
| keysMu sync.RWMutex | ||
| client *http.Client | ||
| } |
There was a problem hiding this comment.
To prevent concurrent JWKS fetches (cache stampede) when a missing key ID is requested under high load, consider adding a separate sync.Mutex to serialize the fetch operation. This avoids holding the main keysMu read-write lock during the HTTP request (which would block all token validations) while still ensuring only a single concurrent request is made to the JWKS endpoint.
| type Validator struct { | |
| jwksURL string | |
| issuer string | |
| keys map[string]any | |
| keysMu sync.RWMutex | |
| client *http.Client | |
| } | |
| type Validator struct { | |
| jwksURL string | |
| issuer string | |
| keys map[string]any | |
| keysMu sync.RWMutex | |
| client *http.Client | |
| fetchMu sync.Mutex | |
| } |
| func (v *Validator) getKey(kid string) (any, error) { | ||
| v.keysMu.RLock() | ||
| key, exists := v.keys[kid] | ||
| v.keysMu.RUnlock() | ||
| if exists { | ||
| return key, nil | ||
| } | ||
|
|
||
| if err := v.refreshKeys(); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| v.keysMu.RLock() | ||
| key, exists = v.keys[kid] | ||
| v.keysMu.RUnlock() | ||
| if !exists { | ||
| return nil, fmt.Errorf("key not found: %s", kid) | ||
| } | ||
| return key, nil | ||
| } |
There was a problem hiding this comment.
Use the new fetchMu to serialize the JWKS refresh operation. This prevents multiple concurrent goroutines from triggering duplicate HTTP requests to the JWKS endpoint on a cache miss, while ensuring that existing keys can still be read concurrently without blocking.
func (v *Validator) getKey(kid string) (any, error) {
v.keysMu.RLock()
key, exists := v.keys[kid]
v.keysMu.RUnlock()
if exists {
return key, nil
}
v.fetchMu.Lock()
defer v.fetchMu.Unlock()
// Double-check under RLock in case another goroutine fetched it while we waited
v.keysMu.RLock()
key, exists = v.keys[kid]
v.keysMu.RUnlock()
if exists {
return key, nil
}
if err := v.refreshKeys(); err != nil {
return nil, err
}
v.keysMu.RLock()
key, exists = v.keys[kid]
v.keysMu.RUnlock()
if !exists {
return nil, fmt.Errorf("key not found: %s", kid)
}
return key, nil
}| } else { | ||
| ls.logger.Info("Login completed", | ||
| zap.String("service", loginServiceName), | ||
| zap.String("method", "Login"), | ||
| zap.Duration("duration", duration), | ||
| zap.Int64("expires_at", resp.ExpiresAt), | ||
| ) | ||
| } |
There was a problem hiding this comment.
If ls.svc.Login returns a nil response and a nil error (which is syntactically possible for any function returning a pointer and an error), accessing resp.ExpiresAt will cause a nil pointer dereference panic. It is safer to defensively check if resp is non-nil before accessing its fields.
} else {
var expiresAt int64
if resp != nil {
expiresAt = resp.ExpiresAt
}
ls.logger.Info("Login completed",
zap.String("service", loginServiceName),
zap.String("method", "Login"),
zap.Duration("duration", duration),
zap.Int64("expires_at", expiresAt),
)
}
Enable read auth in the docker/e2e stack and authenticate the transfer list
calls with a real SIWE-issued JWT instead of ?address=.
- docker api-server config gets an auth block (localhost domain/uri, chain 31337)
- Makefile generates a base64 RSA PEM into JWT_PRIVATE_KEY; compose passes it to
the api-server and bootstrap (both load the api-server config)
- APIServerShim.Login runs GET /auth/nonce -> EIP-4361 sign -> POST /auth/login,
caches the JWT per address; the List{Incoming,Outgoing,Completed} calls attach
it as a bearer token (lazy login on first read, so tests need no new step)
|
Superseded by the 3-way split: auth-jwt → auth-service → auth-integration (all based on main, no dependency on the outbound-transfer-authz PR). |
2 participants
Closes #341.
Read data (transfer offers/history, profile) was served publicly by
?address=. This adds optional authentication for the read path; write endpoints keep the existing EIP-191 signature path unchanged.What it does
GET /auth/nonce?address=→ sign an EIP-4361 message →POST /auth/loginreturns a short-lived RS256 JWT.GET /.well-known/jwks.jsonpublishes the public key so other services (e.g. the indexer) verify tokens without a shared secret. Token claims:sub+ explicitcanton_party_id,evm_address./api/v2/transfer/{incoming,outgoing,completed}and/profileresolve the caller from the bearer token — a caller can only read their own data./auth/nonceflood can't invalidate an in-flight login).The e2e suite authenticates for real:
APIServerShim.Loginruns the nonce → EIP-4361 sign →/auth/loginflow and attaches the JWT as a bearer token on the transfer read calls.Optional by design
Auth is configured via an
auth:block (seepkg/auth/config.go). When the block is absent it is disabled: the middleware is a passthrough and the read endpoints fall back to?address=(the pre-existing behaviour). Devnet/mainnet and the docker/e2e stack ship with it enabled; a config without the block runs unauthenticated. The RSA signing key is supplied as a base64-encoded PEM via${JWT_PRIVATE_KEY}(the Makefile generates one for the e2e run).Why no refresh token
Deliberately omitted. Short-lived access tokens with no refresh token are the simplest secure-revocation story: there is no long-lived credential to steal or to build rotation/reuse-detection around, and a leaked JWT is useless after
token_ttl. When a token expires the client re-runs SIWE login (silent for custodial users; one wallet prompt for non-custodial). If seamless non-custodial sessions later require it,token_ttlis already a config knob to widen first, and a refresh token can be added without changing this design.Stacked on #348 (outbound transfer authorization); will retarget to
mainonce that merges.