From a3c74391a7cb995d2e393835476e1a5ebe917d71 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 10:41:52 +0800 Subject: [PATCH 01/26] add analysis --- docs/token_middleware_analysis.md | 266 ++++++++++++++++++++++++++++++ 1 file changed, 266 insertions(+) create mode 100644 docs/token_middleware_analysis.md diff --git a/docs/token_middleware_analysis.md b/docs/token_middleware_analysis.md new file mode 100644 index 00000000..663c872f --- /dev/null +++ b/docs/token_middleware_analysis.md @@ -0,0 +1,266 @@ +# Token Middleware Analysis and Recommendations + +File: pkg/http/middleware/token_middleware.go +Date: 2025-08-08 + +--- + +## 1) What it does + +The TokenCheckMiddleware enforces a simple HMAC-based request authentication using three custom HTTP headers: + +- X-API-Username: The account name registered in the system. +- X-API-Key: The public token (must have pk_ prefix and minimum length). +- X-API-Signature: An HMAC-SHA256 signature computed as HMAC(secret, accountName). + +Processing flow: +1. Extracts and trims the three headers; rejects if any is empty. +2. Validates the public token format with auth.ValidateTokenFormat (checks min length and pk_/sk_ prefix). +3. Loads the API key record by account name (case-insensitive) from repository.ApiKeys. +4. Decrypts stored encrypted public/secret tokens using TokenHandler.DecodeTokensFor. +5. Verifies the provided public token equals the decrypted public token. +6. Computes a local signature via auth.CreateSignatureFrom(accountName, secretKey) and compares it to X-API-Signature. +7. On success, logs "Token validation successful" and calls the next handler; otherwise returns http.ApiError with Status 403. + +Notes: +- Secrets are stored encrypted-at-rest (AES-GCM). +- auth.SafeDisplay is used when echoing tokens in error messages (masking). + +--- + +## 2) What it misses (gaps and limitations) + +- No constant-time comparisons: + - Direct string equality checks for token and signature can leak timing information. + +- No replay protection: + - Signature is static (HMAC(secret, accountName)). If intercepted, it can be replayed indefinitely. + +- No request binding: + - Signature isn’t tied to the specific request (method, path, body, timestamp). MITM can reuse it across endpoints. + +- No timestamp and nonce: + - Lacks X-API-Timestamp and X-API-Nonce to limit replay windows. + +- Weak error semantics: + - Returns 403 for all failures; should use 401 for unauthenticated and reserve 403 for authorization. + +- Overly verbose error details: + - Error messages include masked token/signature and exact account name; still reveals information to a client. + +- No audience/scope/role concept: + - Middleware only authenticates; it doesn’t propagate identity or scopes to downstream authorization. + +- No context propagation: + - Doesn’t set authenticated account/token metadata into request context for later use. + +- No rate limiting or lockout: + - Missing protection against credential stuffing or brute-force on account names. + +- No key rotation strategy: + - There’s no support for multiple active key versions or scheduled rotation. + +- No IP/Origin policy: + - Doesn’t check allowed IP ranges or allowed origins per account. + +- Minimal logging / no correlation ID: + - Logs success but lacks a request ID/correlation ID for tracing and reduced PII in logs. + +- No transport security enforcement: + - Middleware doesn’t enforce HTTPS/mTLS expectations (relies on deployment). + +--- + +## 3) How we can improve it (actionable recommendations) + +Quick wins (minimal impact): +- Constant-time compares: + - Use hmac.Equal or subtle.ConstantTimeCompare for token and signature equality checks. + +- Correct status codes: + - Use 401 Unauthorized for auth failures; keep 403 for later authorization checks. + +- Reduce error detail to clients: + - Return generic messages like "Invalid credentials" without echoing account or tokens. + - Keep detailed logs server-side with masked values. + +- Propagate identity via context: + - On success, set context values (accountName, apiKeyUUID) for downstream handlers. + +- Structured logging and correlation ID: + - Support/require an X-Request-ID header; log with structured fields and masked secrets. + +Security hardening (medium impact): +- Request-bound HMAC signatures: + - Require clients to sign a canonical string: method + path + query + timestamp + nonce + body-hash. + - Validate within a short skew window (e.g., ±5 minutes) and reject reused nonces. + +- Replay protection: + - Add headers: X-API-Timestamp (epoch seconds) and X-API-Nonce (random UUID). + - Track recent nonces per account in a short-lived store (in-memory or Redis) for the timestamp window. + +- Input normalization: + - Canonicalize header casing, path, and query param encoding consistently. + +- Rate limiting: + - Rate limit auth failures per IP/account. + +- Key rotation support: + - Allow multiple active key versions; embed a key ID in the public key (e.g., pk_{kid}_{hash}) or add X-API-Key-ID. + +- Tenant policy checks: + - Optionally enforce allowed IP ranges and origins per account from DB policy. + +Stronger assurance options (higher impact): +- mTLS for service-to-service: + - Use client certs to authenticate server-to-server calls; keep HMAC as a second factor. + +- OAuth 2.1 / OIDC for frontend apps: + - Use Authorization Code with PKCE for browser/mobile; exchange for short-lived access token and refresh token. + +- JWTs with short TTL: + - Issue short-lived JWTs after initial key verification; then rely on JWT for subsequent requests. + +- Web Application Firewall (WAF) and TLS enforcement: + - Enforce HTTPS and add a WAF to mitigate common web attacks. + +--- + +## 4) How it can be hacked (attack scenarios) + +- Replay attacks: + - Since the signature is static per account, an attacker capturing headers once can replay them forever. + +- Timing attacks: + - String equality may leak timing info, helping distinguish valid/invalid tokens/signatures. + +- Credential stuffing / enumeration: + - Uniform error messages but with different latencies can hint whether an account exists. + +- MITM / downgrade: + - If TLS is misconfigured, headers can be intercepted; without timestamp/nonce, replay is trivial. + +- Logging leakage: + - Logs include account names and could include masked tokens; misconfigured logging can leak sensitive info. + +- No binding to request details: + - A captured signature for one endpoint can be replayed on another since signature doesn’t include method/path/body. + +- Lack of rate limiting: + - Attackers can brute-force account names or spam requests without backpressure. + +--- + +## 5) How we can pass less information to the frontend + +- Don’t echo credentials: + - Avoid returning account name, token, or signature in error messages. Use generic client-facing errors. + +- Use server-generated correlation IDs: + - Provide X-Request-ID to frontend for support without revealing auth details. + +- Minimize fields in success responses: + - Only include what the UI needs; avoid returning any API key metadata to the browser. + +- Store secrets server-side only: + - For browser apps, avoid exposing API keys; use session cookies or OAuth tokens instead. + +- Differential logging: + - Keep detailed diagnostics in server logs (masked), not in API responses. + +--- + +## 6) How can we authenticate frontend apps better + +For browser-based frontends (SPAs/MPAs): +- Prefer OAuth 2.1 Authorization Code with PKCE + OIDC: + - Users authenticate with the IdP; the SPA exchanges the code for short-lived access tokens and refresh tokens via a BFF (Backend-for-Frontend) to avoid exposing refresh tokens to JS. + +- Session cookies with SameSite=strict, HttpOnly, Secure: + - Use server-managed sessions; issue short-lived session cookies and rotate session IDs frequently. + +- Token lifetimes and rotation: + - Access tokens 5–15 minutes; refresh tokens 7–30 days with rotation and revocation. + +- BFF pattern: + - The frontend talks to your BFF; the BFF calls the API with service credentials, keeping secrets off the browser. + +For native apps or trusted server-to-server clients: +- mTLS: + - Bind clients via mutual TLS certificates. + +- Signed requests (HMAC) with request binding: + - Include method, path, timestamp, nonce, and payload hash; enforce a skew window and nonce cache. + +- Device-bound credentials: + - Use secure enclave/Keychain/TPM to store tokens and bind them to devices. + +--- + +## 7) Suggested phased plan + +Phase 1 (Low risk, immediate): +- A1. Switch to constant-time comparisons for signature and public token. +- A2. Return 401 for authentication failures; generic error messages to clients. +- A3. Add structured logging with X-Request-ID; mask all sensitive values. +- A4. Put authenticated account into request context. + +Phase 2 (Security hardening): +- B1. Add X-API-Timestamp and X-API-Nonce headers, validate clock skew. +- B2. Introduce nonce replay cache (in-memory or Redis) keyed by account+nonce within the time window. +- B3. Define canonical request string and require clients to sign it with HMAC(secret, canonical_request). +- B4. Add rate limiting on failed auth per IP/account. + +Phase 3 (Operational maturity): +- C1. Implement key rotation with key IDs; allow overlapping validity windows. +- C2. Optional IP allowlist/origin policy per account. +- C3. mTLS for backend integrations where applicable. + +Phase 4 (Frontend modernization): +- D1. Adopt OAuth 2.1 Authorization Code with PKCE for browser/mobile apps. +- D2. Introduce a BFF to keep tokens and secrets off the browser. + +--- + +## 8) Example canonical signature spec (for future adoption) + +Headers required: +- X-API-Username +- X-API-Key +- X-API-Timestamp (epoch seconds) +- X-API-Nonce (UUID v4) +- X-API-Signature + +Canonical request (string to sign): + +METHOD + "\n" + +PATH + "\n" + +SORTED_QUERY_STRING + "\n" + +X-API-Username + "\n" + +X-API-Key + "\n" + +X-API-Timestamp + "\n" + +X-API-Nonce + "\n" + +SHA256_HEX(BODY) + +Signature: +- signature = hex(HMAC-SHA256(secretKey, canonical_request)) + +Validation rules: +- Accept if |now - timestamp| <= 300s, nonce unused within window, and constant-time comparison passes. + +--- + +## 9) Logging guidelines + +- Never log full tokens or signatures. Use auth.SafeDisplay or stricter masking. +- Include: request_id, account_name (normalized), result (success/failure), reason codes, client_ip (if safe), user_agent (optional), path, method, and timing. +- Store detailed diagnostics server-side only; respond to clients with generic messages. + +--- + +## 10) References + +- RFC 6749 (OAuth 2.0), OAuth 2.1 draft best practices. +- RFC 7636 (PKCE). +- NIST SP 800-63B (Digital Identity Guidelines). +- Timing attack mitigations (use constant-time comparisons). From d283f474b1a0448448e07597da5544b58fa07082 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 11:26:27 +0800 Subject: [PATCH 02/26] start working on pahse 1 --- docs/token_middleware_analysis.md | 40 ++++---- pkg/http/middleware/token_middleware.go | 99 ++++++++++++-------- pkg/http/middleware/token_middleware_test.go | 18 ++-- 3 files changed, 88 insertions(+), 69 deletions(-) diff --git a/docs/token_middleware_analysis.md b/docs/token_middleware_analysis.md index 663c872f..7b9bee93 100644 --- a/docs/token_middleware_analysis.md +++ b/docs/token_middleware_analysis.md @@ -20,11 +20,11 @@ Processing flow: 4. Decrypts stored encrypted public/secret tokens using TokenHandler.DecodeTokensFor. 5. Verifies the provided public token equals the decrypted public token. 6. Computes a local signature via auth.CreateSignatureFrom(accountName, secretKey) and compares it to X-API-Signature. -7. On success, logs "Token validation successful" and calls the next handler; otherwise returns http.ApiError with Status 403. +7. On success, logs authentication success and calls the next handler; otherwise returns http.ApiError with Status 401 (generic message). Notes: - Secrets are stored encrypted-at-rest (AES-GCM). -- auth.SafeDisplay is used when echoing tokens in error messages (masking). +- Client-facing errors are generic and do not echo credentials; sensitive details are only in server logs. --- @@ -197,28 +197,28 @@ For native apps or trusted server-to-server clients: --- -## 7) Suggested phased plan +## 7) Suggested phased plan (Checklist) -Phase 1 (Low risk, immediate): -- A1. Switch to constant-time comparisons for signature and public token. -- A2. Return 401 for authentication failures; generic error messages to clients. -- A3. Add structured logging with X-Request-ID; mask all sensitive values. -- A4. Put authenticated account into request context. +- [x] Phase 1 (Low risk, immediate) + - [x] A1. Switch to constant-time comparisons for signature and public token. + - [x] A2. Return 401 for authentication failures; generic error messages to clients. + - [x] A3. Add structured logging with X-Request-ID; mask all sensitive values. + - [x] A4. Put authenticated account into request context. -Phase 2 (Security hardening): -- B1. Add X-API-Timestamp and X-API-Nonce headers, validate clock skew. -- B2. Introduce nonce replay cache (in-memory or Redis) keyed by account+nonce within the time window. -- B3. Define canonical request string and require clients to sign it with HMAC(secret, canonical_request). -- B4. Add rate limiting on failed auth per IP/account. +- [ ] Phase 2 (Security hardening) + - [ ] B1. Add X-API-Timestamp and X-API-Nonce headers, validate clock skew. + - [ ] B2. Introduce nonce replay cache (in-memory or Redis) keyed by account+nonce within the time window. + - [ ] B3. Define canonical request string and require clients to sign it with HMAC(secret, canonical_request). + - [ ] B4. Add rate limiting on failed auth per IP/account. -Phase 3 (Operational maturity): -- C1. Implement key rotation with key IDs; allow overlapping validity windows. -- C2. Optional IP allowlist/origin policy per account. -- C3. mTLS for backend integrations where applicable. +- [ ] Phase 3 (Operational maturity) + - [ ] C1. Implement key rotation with key IDs; allow overlapping validity windows. + - [ ] C2. Optional IP allowlist/origin policy per account. + - [ ] C3. mTLS for backend integrations where applicable. -Phase 4 (Frontend modernization): -- D1. Adopt OAuth 2.1 Authorization Code with PKCE for browser/mobile apps. -- D2. Introduce a BFF to keep tokens and secrets off the browser. +- [ ] Phase 4 (Frontend modernization) + - [ ] D1. Adopt OAuth 2.1 Authorization Code with PKCE for browser/mobile apps. + - [ ] D2. Introduce a BFF to keep tokens and secrets off the browser. --- diff --git a/pkg/http/middleware/token_middleware.go b/pkg/http/middleware/token_middleware.go index 0ea87165..8146afa6 100644 --- a/pkg/http/middleware/token_middleware.go +++ b/pkg/http/middleware/token_middleware.go @@ -1,19 +1,32 @@ package middleware import ( - "fmt" + "context" + "crypto/subtle" + "log/slog" + baseHttp "net/http" + "strings" + "github.com/oullin/database" "github.com/oullin/database/repository" "github.com/oullin/pkg/auth" "github.com/oullin/pkg/http" - "log/slog" - baseHttp "net/http" - "strings" ) const tokenHeader = "X-API-Key" const usernameHeader = "X-API-Username" const signatureHeader = "X-API-Signature" +const requestIDHeader = "X-Request-ID" + +// Context keys for propagating auth info downstream +// Use unexported custom type to avoid collisions + +type contextKey string + +const ( + authAccountNameKey contextKey = "auth.account_name" + requestIdKey contextKey = "request.id" +) type TokenCheckMiddleware struct { ApiKeys *repository.ApiKeys @@ -29,33 +42,49 @@ func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.Ap func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { return func(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { + reqID := strings.TrimSpace(r.Header.Get(requestIDHeader)) + + if reqID == "" { + return t.getInvalidRequestError() + } + + logger := slog.With("request_id", reqID, "path", r.URL.Path, "method", r.Method) accountName := strings.TrimSpace(r.Header.Get(usernameHeader)) publicToken := strings.TrimSpace(r.Header.Get(tokenHeader)) signature := strings.TrimSpace(r.Header.Get(signatureHeader)) if accountName == "" || publicToken == "" || signature == "" { - return t.getInvalidRequestError(accountName, publicToken, signature) + logger.Warn("missing authentication headers") + return t.getInvalidRequestError() } if err := auth.ValidateTokenFormat(publicToken); err != nil { - return t.getInvalidTokenFormatError(publicToken, err) + logger.Warn("invalid token format") + return t.getInvalidTokenFormatError() } - if t.shallReject(accountName, publicToken, signature) { - return t.getUnauthenticatedError(accountName, publicToken, signature) + reject := t.shallReject(logger, accountName, publicToken, signature) + if reject { + return t.getUnauthenticatedError() } - slog.Info("Token validation successful") + // Update the request context + ctx := context.WithValue(r.Context(), authAccountNameKey, accountName) + ctx = context.WithValue(r.Context(), requestIdKey, reqID) + r = r.WithContext(ctx) + + logger.Info("authentication successful") return next(w, r) } } -func (t TokenCheckMiddleware) shallReject(accountName, publicToken, signature string) bool { +func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature string) bool { var item *database.APIKey if item = t.ApiKeys.FindBy(accountName); item == nil { + logger.Warn("account not found") return true } @@ -66,53 +95,45 @@ func (t TokenCheckMiddleware) shallReject(accountName, publicToken, signature st ) if err != nil { - slog.Error(fmt.Sprintf("could not decode the given account [%s] keys: %v", item.AccountName, err)) - + logger.Error("failed to decode account keys", "account", item.AccountName, "error", err) return true } - if strings.TrimSpace(token.PublicKey) != strings.TrimSpace(publicToken) { - slog.Error(fmt.Sprintf("the given public token does not match tour records [%s]: %v", item.AccountName, err)) - + // Constant-time compare of provided public token vs stored one + provided := []byte(strings.TrimSpace(publicToken)) + expected := []byte(strings.TrimSpace(token.PublicKey)) + if subtle.ConstantTimeCompare(provided, expected) != 1 { + logger.Warn("public token mismatch", "account", item.AccountName) return true } + // Compute local signature and compare in constant time localSignature := auth.CreateSignatureFrom(token.AccountName, token.SecretKey) + if subtle.ConstantTimeCompare([]byte(signature), []byte(localSignature)) != 1 { + logger.Warn("signature mismatch", "account", item.AccountName) + return true + } - return signature != localSignature + return false } -func (t TokenCheckMiddleware) getInvalidRequestError(accountName, publicToken, signature string) *http.ApiError { - message := fmt.Sprintf( - "invalid request. Please, provide a valid token, signature and accout name headers. [account: %s, public token: %s, signature: %s]", - accountName, - auth.SafeDisplay(publicToken), - auth.SafeDisplay(signature), - ) - +func (t TokenCheckMiddleware) getInvalidRequestError() *http.ApiError { return &http.ApiError{ - Message: message, - Status: baseHttp.StatusForbidden, + Message: "Invalid authentication headers", + Status: baseHttp.StatusUnauthorized, } } -func (t TokenCheckMiddleware) getInvalidTokenFormatError(publicToken string, err error) *http.ApiError { +func (t TokenCheckMiddleware) getInvalidTokenFormatError() *http.ApiError { return &http.ApiError{ - Message: fmt.Sprintf("invalid token format [token: %s]: %v", auth.SafeDisplay(publicToken), err), - Status: baseHttp.StatusForbidden, + Message: "Invalid credentials", + Status: baseHttp.StatusUnauthorized, } } -func (t TokenCheckMiddleware) getUnauthenticatedError(accountName, publicToken, signature string) *http.ApiError { - message := fmt.Sprintf( - "Unauthenticated, please check your credentials and signature headers: [token: %s, account name: %s, signature: %s]", - auth.SafeDisplay(publicToken), - accountName, - signature, - ) - +func (t TokenCheckMiddleware) getUnauthenticatedError() *http.ApiError { return &http.ApiError{ - Message: message, - Status: baseHttp.StatusForbidden, + Message: "Invalid credentials", + Status: baseHttp.StatusUnauthorized, } } diff --git a/pkg/http/middleware/token_middleware_test.go b/pkg/http/middleware/token_middleware_test.go index ee0962c0..8aa28587 100644 --- a/pkg/http/middleware/token_middleware_test.go +++ b/pkg/http/middleware/token_middleware_test.go @@ -5,28 +5,27 @@ import ( "net/http/httptest" "testing" - pkgAuth "github.com/oullin/pkg/auth" pkgHttp "github.com/oullin/pkg/http" ) func TestTokenMiddlewareErrors(t *testing.T) { tm := TokenCheckMiddleware{} - e := tm.getInvalidRequestError("a", "b", "c") + e := tm.getInvalidRequestError() - if e.Status != 403 || e.Message == "" { + if e.Status != 401 || e.Message == "" { t.Fatalf("invalid request error") } - e = tm.getInvalidTokenFormatError("pk_x", pkgAuth.ValidateTokenFormat("bad")) + e = tm.getInvalidTokenFormatError() - if e.Status != 403 { + if e.Status != 401 { t.Fatalf("invalid token error") } - e = tm.getUnauthenticatedError("a", "b", "c") + e = tm.getUnauthenticatedError() - if e.Status != 403 { + if e.Status != 401 { t.Fatalf("unauthenticated error") } } @@ -35,14 +34,13 @@ func TestTokenMiddlewareHandleInvalid(t *testing.T) { tm := MakeTokenMiddleware(nil, nil) handler := tm.Handle(func(w http.ResponseWriter, r *http.Request) *pkgHttp.ApiError { - return nil }) rec := httptest.NewRecorder() err := handler(rec, httptest.NewRequest("GET", "/", nil)) - if err == nil || err.Status != 403 { - t.Fatalf("expected forbidden") + if err == nil || err.Status != 401 { + t.Fatalf("expected unauthorized") } } From 0731657170c0e8d86d284660e5f476a0c689fa46 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 11:35:46 +0800 Subject: [PATCH 03/26] phase II --- sentry_analysis.md => docs/sentry_analysis.md | 0 docs/token_middleware_analysis.md | 10 +- pkg/http/middleware/token_middleware.go | 252 +++++++++++++++++- 3 files changed, 248 insertions(+), 14 deletions(-) rename sentry_analysis.md => docs/sentry_analysis.md (100%) diff --git a/sentry_analysis.md b/docs/sentry_analysis.md similarity index 100% rename from sentry_analysis.md rename to docs/sentry_analysis.md diff --git a/docs/token_middleware_analysis.md b/docs/token_middleware_analysis.md index 7b9bee93..daa68a9f 100644 --- a/docs/token_middleware_analysis.md +++ b/docs/token_middleware_analysis.md @@ -205,11 +205,11 @@ For native apps or trusted server-to-server clients: - [x] A3. Add structured logging with X-Request-ID; mask all sensitive values. - [x] A4. Put authenticated account into request context. -- [ ] Phase 2 (Security hardening) - - [ ] B1. Add X-API-Timestamp and X-API-Nonce headers, validate clock skew. - - [ ] B2. Introduce nonce replay cache (in-memory or Redis) keyed by account+nonce within the time window. - - [ ] B3. Define canonical request string and require clients to sign it with HMAC(secret, canonical_request). - - [ ] B4. Add rate limiting on failed auth per IP/account. +- [x] Phase 2 (Security hardening) + - [x] B1. Add X-API-Timestamp and X-API-Nonce headers, validate clock skew. + - [x] B2. Introduce nonce replay cache (in-memory or Redis) keyed by account+nonce within the time window. + - [x] B3. Define canonical request string and require clients to sign it with HMAC(secret, canonical_request). + - [x] B4. Add rate limiting on failed auth per IP/account. - [ ] Phase 3 (Operational maturity) - [ ] C1. Implement key rotation with key IDs; allow overlapping validity windows. diff --git a/pkg/http/middleware/token_middleware.go b/pkg/http/middleware/token_middleware.go index 8146afa6..1003059f 100644 --- a/pkg/http/middleware/token_middleware.go +++ b/pkg/http/middleware/token_middleware.go @@ -1,11 +1,20 @@ package middleware import ( + "bytes" "context" + "crypto/sha256" "crypto/subtle" + "encoding/hex" + "io" "log/slog" baseHttp "net/http" + "net" + "net/url" + "sort" "strings" + "sync" + "time" "github.com/oullin/database" "github.com/oullin/database/repository" @@ -16,6 +25,8 @@ import ( const tokenHeader = "X-API-Key" const usernameHeader = "X-API-Username" const signatureHeader = "X-API-Signature" +const timestampHeader = "X-API-Timestamp" +const nonceHeader = "X-API-Nonce" const requestIDHeader = "X-Request-ID" // Context keys for propagating auth info downstream @@ -28,15 +39,98 @@ const ( requestIdKey contextKey = "request.id" ) +// --- Phase 2 support types + +type nonceCache struct { + mu sync.Mutex + data map[string]time.Time // key: accountName+"|"+nonce -> expiry time +} + +func newNonceCache() *nonceCache { + return &nonceCache{data: make(map[string]time.Time)} +} + +func (c *nonceCache) used(account, nonce string) bool { + c.mu.Lock() + defer c.mu.Unlock() + key := account + "|" + nonce + exp, ok := c.data[key] + if !ok { + return false + } + if time.Now().After(exp) { + delete(c.data, key) + return false + } + return true +} + +func (c *nonceCache) mark(account, nonce string, ttl time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + key := account + "|" + nonce + c.data[key] = time.Now().Add(ttl) +} + +type rateLimiter struct { + mu sync.Mutex + history map[string][]time.Time // key: ip+"|"+account -> failures timestamps + window time.Duration + maxFails int +} + +func newRateLimiter(window time.Duration, maxFails int) *rateLimiter { + return &rateLimiter{history: make(map[string][]time.Time), window: window, maxFails: maxFails} +} + +func (r *rateLimiter) tooMany(key string) bool { + r.mu.Lock() + defer r.mu.Unlock() + now := time.Now() + slice := r.history[key] + // prune + pruned := slice[:0] + for _, t := range slice { + if now.Sub(t) <= r.window { + pruned = append(pruned, t) + } + } + r.history[key] = pruned + return len(pruned) >= r.maxFails +} + +func (r *rateLimiter) fail(key string) { + r.mu.Lock() + defer r.mu.Unlock() + now := time.Now() + r.history[key] = append(r.history[key], now) +} + type TokenCheckMiddleware struct { ApiKeys *repository.ApiKeys TokenHandler *auth.TokenHandler + + // Phase 2 additions + nonceCache *nonceCache + rateLimiter *rateLimiter + + // Configurable parameters + clockSkew time.Duration + nonceTTL time.Duration + failWindow time.Duration + maxFailPerScope int } func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.ApiKeys) TokenCheckMiddleware { return TokenCheckMiddleware{ - ApiKeys: apiKeys, - TokenHandler: tokenHandler, + ApiKeys: apiKeys, + TokenHandler: tokenHandler, + nonceCache: newNonceCache(), + rateLimiter: newRateLimiter(1*time.Minute, 10), + clockSkew: 5 * time.Minute, + nonceTTL: 5 * time.Minute, + failWindow: 1 * time.Minute, + maxFailPerScope: 10, } } @@ -53,8 +147,10 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { accountName := strings.TrimSpace(r.Header.Get(usernameHeader)) publicToken := strings.TrimSpace(r.Header.Get(tokenHeader)) signature := strings.TrimSpace(r.Header.Get(signatureHeader)) + ts := strings.TrimSpace(r.Header.Get(timestampHeader)) + nonce := strings.TrimSpace(r.Header.Get(nonceHeader)) - if accountName == "" || publicToken == "" || signature == "" { + if accountName == "" || publicToken == "" || signature == "" || ts == "" || nonce == "" { logger.Warn("missing authentication headers") return t.getInvalidRequestError() } @@ -64,7 +160,50 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { return t.getInvalidTokenFormatError() } - reject := t.shallReject(logger, accountName, publicToken, signature) + // Validate timestamp skew window + parsed, err := time.ParseDuration(ts + "s") + if err != nil { + // try interpreting as epoch seconds (int) + var epoch int64 + for _, ch := range ts { + if ch < '0' || ch > '9' { + logger.Warn("invalid timestamp format") + return t.getInvalidRequestError() + } + } + // safe to parse as int64 now + // custom parse to avoid strconv import + for _, ch := range ts { + epoch = epoch*10 + int64(ch-'0') + } + now := time.Now().Unix() + if epoch < now-int64(t.clockSkew.Seconds()) || epoch > now+int64(t.clockSkew.Seconds()) { + logger.Warn("timestamp outside allowed window") + return t.getUnauthenticatedError() + } + } else { + _ = parsed // ignore if duration parsed (not expected), kept for completeness + } + + // Read and hash body, then restore it for downstream + var bodyBytes []byte + if r.Body != nil { + b, readErr := io.ReadAll(r.Body) + if readErr != nil { + logger.Warn("unable to read body for signing") + return t.getInvalidRequestError() + } + bodyBytes = b + r.Body = io.NopCloser(bytes.NewReader(b)) + } + bodyHash := sha256Hex(bodyBytes) + + // Build canonical request string + canonical := buildCanonical(r.Method, r.URL, accountName, publicToken, ts, nonce, bodyHash) + + clientIP := parseClientIP(r) + + reject := t.shallReject(logger, accountName, publicToken, signature, canonical, nonce, clientIP) if reject { return t.getUnauthenticatedError() } @@ -80,10 +219,19 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { } } -func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature string) bool { - var item *database.APIKey +func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature, canonical, nonce, clientIP string) bool { + // Basic rate limiting on failures per IP/account + limiterKey := clientIP + "|" + strings.ToLower(accountName) + if t.rateLimiter != nil && t.rateLimiter.tooMany(limiterKey) { + logger.Warn("too many authentication failures", "ip", clientIP) + return true + } + var item *database.APIKey if item = t.ApiKeys.FindBy(accountName); item == nil { + if t.rateLimiter != nil { + t.rateLimiter.fail(limiterKey) + } logger.Warn("account not found") return true } @@ -93,8 +241,10 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ item.SecretKey, item.PublicKey, ) - if err != nil { + if t.rateLimiter != nil { + t.rateLimiter.fail(limiterKey) + } logger.Error("failed to decode account keys", "account", item.AccountName, "error", err) return true } @@ -103,20 +253,104 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ provided := []byte(strings.TrimSpace(publicToken)) expected := []byte(strings.TrimSpace(token.PublicKey)) if subtle.ConstantTimeCompare(provided, expected) != 1 { + if t.rateLimiter != nil { + t.rateLimiter.fail(limiterKey) + } logger.Warn("public token mismatch", "account", item.AccountName) return true } - // Compute local signature and compare in constant time - localSignature := auth.CreateSignatureFrom(token.AccountName, token.SecretKey) + // Nonce replay protection: check if already used for this account + if t.nonceCache != nil && t.nonceCache.used(item.AccountName, nonce) { + if t.rateLimiter != nil { + t.rateLimiter.fail(limiterKey) + } + logger.Warn("replay detected: nonce already used", "account", item.AccountName) + return true + } + + // Compute local signature over canonical request and compare in constant time + localSignature := auth.CreateSignatureFrom(canonical, token.SecretKey) if subtle.ConstantTimeCompare([]byte(signature), []byte(localSignature)) != 1 { + if t.rateLimiter != nil { + t.rateLimiter.fail(limiterKey) + } logger.Warn("signature mismatch", "account", item.AccountName) return true } + // Mark nonce as used within the TTL + if t.nonceCache != nil { + t.nonceCache.mark(item.AccountName, nonce, t.nonceTTL) + } + return false } +// Helpers +func sha256Hex(b []byte) string { + h := sha256.Sum256(b) + return hex.EncodeToString(h[:]) +} + +func sortedQuery(u *url.URL) string { + if u == nil { + return "" + } + q := u.Query() + if len(q) == 0 { + return "" + } + keys := make([]string, 0, len(q)) + for k := range q { + keys = append(keys, k) + } + sort.Strings(keys) + pairs := make([]string, 0, len(keys)) + for _, k := range keys { + vals := q[k] + sort.Strings(vals) + for _, v := range vals { + pairs = append(pairs, url.QueryEscape(k)+"="+url.QueryEscape(v)) + } + } + return strings.Join(pairs, "&") +} + +func buildCanonical(method string, u *url.URL, username, public, ts, nonce, bodyHash string) string { + path := "/" + if u != nil && u.Path != "" { + path = u.EscapedPath() + } + query := sortedQuery(u) + parts := []string{ + strings.ToUpper(method), + path, + query, + username, + public, + ts, + nonce, + bodyHash, + } + return strings.Join(parts, "\n") +} + +func parseClientIP(r *baseHttp.Request) string { + // prefer X-Forwarded-For if present + xff := strings.TrimSpace(r.Header.Get("X-Forwarded-For")) + if xff != "" { + // take first IP + parts := strings.Split(xff, ",") + return strings.TrimSpace(parts[0]) + } + host, _, err := net.SplitHostPort(strings.TrimSpace(r.RemoteAddr)) + if err == nil && host != "" { + return host + } + return strings.TrimSpace(r.RemoteAddr) +} + func (t TokenCheckMiddleware) getInvalidRequestError() *http.ApiError { return &http.ApiError{ Message: "Invalid authentication headers", From 8ae6ed6896156caa7e7e410540143ff97ab0d374 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 13:41:56 +0800 Subject: [PATCH 04/26] extract ttl_cache --- pkg/cache/ttl_cache.go | 49 +++++++++++ pkg/cache/ttl_cache_test.go | 103 ++++++++++++++++++++++++ pkg/http/middleware/token_middleware.go | 54 ++++--------- 3 files changed, 166 insertions(+), 40 deletions(-) create mode 100644 pkg/cache/ttl_cache.go create mode 100644 pkg/cache/ttl_cache_test.go diff --git a/pkg/cache/ttl_cache.go b/pkg/cache/ttl_cache.go new file mode 100644 index 00000000..c8ddce42 --- /dev/null +++ b/pkg/cache/ttl_cache.go @@ -0,0 +1,49 @@ +package cache + +import ( + "sync" + "time" +) + +// TTLCache is a tiny in-memory TTL key store. +// Values are not stored; only key existence within TTL is tracked. +// It is process-local and NOT suitable for distributed deployments. +// Use a shared cache (e.g., Redis) for multi-instance setups. +type TTLCache struct { + mu sync.Mutex + data map[string]time.Time // key -> expiry time +} + +// NewTTLCache creates a new empty TTL cache. +func NewTTLCache() *TTLCache { + return &TTLCache{ + data: make(map[string]time.Time), + } +} + +// Used reports whether key is present and not expired. +// It lazily prunes expired entries on access. +func (c *TTLCache) Used(key string) bool { + c.mu.Lock() + defer c.mu.Unlock() + + exp, ok := c.data[key] + if !ok { + return false + } + + if time.Now().After(exp) { + delete(c.data, key) + return false + } + + return true +} + +// Mark stores the key with a time-to-live. +func (c *TTLCache) Mark(key string, ttl time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + + c.data[key] = time.Now().Add(ttl) +} diff --git a/pkg/cache/ttl_cache_test.go b/pkg/cache/ttl_cache_test.go new file mode 100644 index 00000000..d89f4798 --- /dev/null +++ b/pkg/cache/ttl_cache_test.go @@ -0,0 +1,103 @@ +package cache + +import ( + "sync" + "testing" + "time" +) + +func TestTTLCache_BasicMarkAndUsed(t *testing.T) { + c := NewTTLCache() + + // Unknown key should be false + if c.Used("missing") { + t.Fatalf("expected missing key to be false") + } + + // Mark a key with a short TTL and verify it's true before expiry + c.Mark("k1", 50*time.Millisecond) + if !c.Used("k1") { + t.Fatalf("expected k1 to be true before expiry") + } + + // Sleep past TTL and verify it expires + time.Sleep(60 * time.Millisecond) + if c.Used("k1") { + t.Fatalf("expected k1 to be false after expiry") + } + + // After lazy pruning, subsequent calls should also be false + if c.Used("k1") { + t.Fatalf("expected k1 to remain false after pruning") + } +} + +func TestTTLCache_IndependentKeys(t *testing.T) { + c := NewTTLCache() + + c.Mark("short", 20*time.Millisecond) + c.Mark("long", 200*time.Millisecond) + + // Immediately both should be usable + if !c.Used("short") || !c.Used("long") { + t.Fatalf("expected both keys to be true initially") + } + + // Wait for the short to expire but not the long + time.Sleep(50 * time.Millisecond) + + if c.Used("short") { + t.Fatalf("expected short to be expired") + } + if !c.Used("long") { + t.Fatalf("expected long to still be valid") + } +} + +func TestTTLCache_RefreshTTL(t *testing.T) { + c := NewTTLCache() + + c.Mark("k", 20*time.Millisecond) + // Refresh before expiry + time.Sleep(10 * time.Millisecond) + c.Mark("k", 40*time.Millisecond) + + // Wait past the first TTL but before the refreshed TTL + time.Sleep(15 * time.Millisecond) // total 25ms from first mark + + if !c.Used("k") { + t.Fatalf("expected k to be valid due to refreshed TTL") + } + + // Now wait past the refreshed TTL + time.Sleep(30 * time.Millisecond) // total ~55ms from first mark + if c.Used("k") { + t.Fatalf("expected k to be expired after refreshed TTL") + } +} + +func TestTTLCache_Concurrency(t *testing.T) { + c := NewTTLCache() + + const goroutines = 20 + var wg sync.WaitGroup + wg.Add(goroutines) + + for i := 0; i < goroutines; i++ { + go func(i int) { + defer wg.Done() + key := "k" // all share same key to contend on lock + c.Mark(key, 200*time.Millisecond) + if !c.Used(key) { + t.Errorf("expected key to be usable; goroutine %d", i) + } + }(i) + } + + wg.Wait() + + // After all goroutines, key should still be valid + if !c.Used("k") { + t.Fatalf("expected key to still be valid after concurrent access") + } +} diff --git a/pkg/http/middleware/token_middleware.go b/pkg/http/middleware/token_middleware.go index 1003059f..9862653a 100644 --- a/pkg/http/middleware/token_middleware.go +++ b/pkg/http/middleware/token_middleware.go @@ -8,8 +8,8 @@ import ( "encoding/hex" "io" "log/slog" - baseHttp "net/http" "net" + baseHttp "net/http" "net/url" "sort" "strings" @@ -19,6 +19,7 @@ import ( "github.com/oullin/database" "github.com/oullin/database/repository" "github.com/oullin/pkg/auth" + "github.com/oullin/pkg/cache" "github.com/oullin/pkg/http" ) @@ -41,37 +42,6 @@ const ( // --- Phase 2 support types -type nonceCache struct { - mu sync.Mutex - data map[string]time.Time // key: accountName+"|"+nonce -> expiry time -} - -func newNonceCache() *nonceCache { - return &nonceCache{data: make(map[string]time.Time)} -} - -func (c *nonceCache) used(account, nonce string) bool { - c.mu.Lock() - defer c.mu.Unlock() - key := account + "|" + nonce - exp, ok := c.data[key] - if !ok { - return false - } - if time.Now().After(exp) { - delete(c.data, key) - return false - } - return true -} - -func (c *nonceCache) mark(account, nonce string, ttl time.Duration) { - c.mu.Lock() - defer c.mu.Unlock() - key := account + "|" + nonce - c.data[key] = time.Now().Add(ttl) -} - type rateLimiter struct { mu sync.Mutex history map[string][]time.Time // key: ip+"|"+account -> failures timestamps @@ -111,7 +81,7 @@ type TokenCheckMiddleware struct { TokenHandler *auth.TokenHandler // Phase 2 additions - nonceCache *nonceCache + nonceCache *cache.TTLCache rateLimiter *rateLimiter // Configurable parameters @@ -125,7 +95,7 @@ func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.Ap return TokenCheckMiddleware{ ApiKeys: apiKeys, TokenHandler: tokenHandler, - nonceCache: newNonceCache(), + nonceCache: cache.NewTTLCache(), rateLimiter: newRateLimiter(1*time.Minute, 10), clockSkew: 5 * time.Minute, nonceTTL: 5 * time.Minute, @@ -261,12 +231,15 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ } // Nonce replay protection: check if already used for this account - if t.nonceCache != nil && t.nonceCache.used(item.AccountName, nonce) { - if t.rateLimiter != nil { - t.rateLimiter.fail(limiterKey) + if t.nonceCache != nil { + key := item.AccountName + "|" + nonce + if t.nonceCache.Used(key) { + if t.rateLimiter != nil { + t.rateLimiter.fail(limiterKey) + } + logger.Warn("replay detected: nonce already used", "account", item.AccountName) + return true } - logger.Warn("replay detected: nonce already used", "account", item.AccountName) - return true } // Compute local signature over canonical request and compare in constant time @@ -281,7 +254,8 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ // Mark nonce as used within the TTL if t.nonceCache != nil { - t.nonceCache.mark(item.AccountName, nonce, t.nonceTTL) + key := item.AccountName + "|" + nonce + t.nonceCache.Mark(key, t.nonceTTL) } return false From 81d21a74d00efa2f1208d16821049c72464a1280 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 14:15:57 +0800 Subject: [PATCH 05/26] extract portal pkg --- database/repository/queries/posts_filters.go | 5 +++-- database/seeder/main.go | 4 ++-- database/seeder/seeds/users.go | 6 ++++-- handler/education.go | 5 +++-- handler/experience.go | 5 +++-- handler/payload/posts.go | 5 +++-- handler/posts.go | 6 ++++-- handler/profile.go | 5 +++-- handler/projects.go | 5 +++-- handler/recommendations.go | 5 +++-- handler/social.go | 5 +++-- handler/talks.go | 5 +++-- main.go | 11 ++++++----- metal/cli/main.go | 4 +++- metal/cli/panel/menu.go | 15 ++++++++------- metal/cli/panel/menu_test.go | 6 +++--- metal/kernel/app.go | 8 ++++---- metal/kernel/factory.go | 8 ++++---- metal/kernel/ignite.go | 4 ++-- metal/kernel/kernel_test.go | 20 ++++++++++---------- pkg/markdown/schema.go | 6 ++++-- pkg/{ => portal}/parser.go | 2 +- pkg/{ => portal}/parser_test.go | 2 +- pkg/{ => portal}/password.go | 2 +- pkg/{ => portal}/password_test.go | 2 +- pkg/{ => portal}/sentry.go | 2 +- pkg/{ => portal}/stringable.go | 7 ++++--- pkg/{ => portal}/stringable_test.go | 2 +- pkg/{ => portal}/support.go | 2 +- pkg/{ => portal}/support_test.go | 2 +- pkg/{ => portal}/validator.go | 2 +- pkg/{ => portal}/validator_test.go | 2 +- 32 files changed, 95 insertions(+), 75 deletions(-) rename pkg/{ => portal}/parser.go (98%) rename pkg/{ => portal}/parser_test.go (97%) rename pkg/{ => portal}/password.go (97%) rename pkg/{ => portal}/password_test.go (95%) rename pkg/{ => portal}/sentry.go (93%) rename pkg/{ => portal}/stringable.go (98%) rename pkg/{ => portal}/stringable_test.go (98%) rename pkg/{ => portal}/support.go (91%) rename pkg/{ => portal}/support_test.go (96%) rename pkg/{ => portal}/validator.go (99%) rename pkg/{ => portal}/validator_test.go (98%) diff --git a/database/repository/queries/posts_filters.go b/database/repository/queries/posts_filters.go index 20ef2e9e..ca9e4f11 100644 --- a/database/repository/queries/posts_filters.go +++ b/database/repository/queries/posts_filters.go @@ -1,7 +1,8 @@ package queries import ( - "github.com/oullin/pkg" + "github.com/oullin/pkg/portal" + "strings" ) @@ -34,7 +35,7 @@ func (f PostFilters) GetTag() string { } func (f PostFilters) sanitiseString(seed string) string { - str := pkg.MakeStringable(seed) + str := portal.MakeStringable(seed) return strings.TrimSpace(str.ToLower()) } diff --git a/database/seeder/main.go b/database/seeder/main.go index 154ea5b7..fd30a292 100644 --- a/database/seeder/main.go +++ b/database/seeder/main.go @@ -8,14 +8,14 @@ import ( "github.com/oullin/database/seeder/seeds" "github.com/oullin/metal/env" "github.com/oullin/metal/kernel" - "github.com/oullin/pkg" "github.com/oullin/pkg/cli" + "github.com/oullin/pkg/portal" ) var environment *env.Environment func init() { - secrets := kernel.Ignite("./.env", pkg.GetDefaultValidator()) + secrets := kernel.Ignite("./.env", portal.GetDefaultValidator()) environment = secrets } diff --git a/database/seeder/seeds/users.go b/database/seeder/seeds/users.go index 6e1b6e7b..cdcf2f65 100644 --- a/database/seeder/seeds/users.go +++ b/database/seeder/seeds/users.go @@ -2,10 +2,12 @@ package seeds import ( "fmt" + "github.com/google/uuid" "github.com/oullin/database" - "github.com/oullin/pkg" "github.com/oullin/pkg/gorm" + "github.com/oullin/pkg/portal" + "strings" "time" ) @@ -21,7 +23,7 @@ func MakeUsersSeed(db *database.Connection) *UsersSeed { } func (s UsersSeed) Create(attrs database.UsersAttrs) (database.User, error) { - pass, _ := pkg.MakePassword("password") + pass, _ := portal.MakePassword("password") fake := database.User{ UUID: uuid.NewString(), diff --git a/handler/education.go b/handler/education.go index 51767738..fabad168 100644 --- a/handler/education.go +++ b/handler/education.go @@ -2,8 +2,9 @@ package handler import ( "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -19,7 +20,7 @@ func MakeEducationHandler(filePath string) EducationHandler { } func (h EducationHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - data, err := pkg.ParseJsonFile[payload.EducationResponse](h.filePath) + data, err := portal.ParseJsonFile[payload.EducationResponse](h.filePath) if err != nil { slog.Error("Error reading education file", "error", err) diff --git a/handler/experience.go b/handler/experience.go index 8b9acc7b..95bc5253 100644 --- a/handler/experience.go +++ b/handler/experience.go @@ -2,8 +2,9 @@ package handler import ( "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -19,7 +20,7 @@ func MakeExperienceHandler(filePath string) ExperienceHandler { } func (h ExperienceHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - data, err := pkg.ParseJsonFile[payload.ExperienceResponse](h.filePath) + data, err := portal.ParseJsonFile[payload.ExperienceResponse](h.filePath) if err != nil { slog.Error("Error reading experience file", "error", err) diff --git a/handler/payload/posts.go b/handler/payload/posts.go index 29381850..51e12141 100644 --- a/handler/payload/posts.go +++ b/handler/payload/posts.go @@ -3,7 +3,8 @@ package payload import ( "github.com/oullin/database" "github.com/oullin/database/repository/queries" - "github.com/oullin/pkg" + "github.com/oullin/pkg/portal" + baseHttp "net/http" "strings" "time" @@ -45,7 +46,7 @@ func GetPostsFiltersFrom(request IndexRequestBody) queries.PostFilters { } func GetSlugFrom(r *baseHttp.Request) string { - str := pkg.MakeStringable(r.PathValue("slug")) + str := portal.MakeStringable(r.PathValue("slug")) return strings.TrimSpace(str.ToLower()) } diff --git a/handler/posts.go b/handler/posts.go index e6866f45..84d626bc 100644 --- a/handler/posts.go +++ b/handler/posts.go @@ -3,12 +3,14 @@ package handler import ( "encoding/json" "fmt" + "github.com/oullin/database/repository" "github.com/oullin/database/repository/pagination" "github.com/oullin/handler/paginate" "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -22,7 +24,7 @@ func MakePostsHandler(repo *repository.Posts) PostsHandler { } func (h *PostsHandler) Index(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - defer pkg.CloseWithLog(r.Body) + defer portal.CloseWithLog(r.Body) requestBody, err := http.ParseRequestBody[payload.IndexRequestBody](r) diff --git a/handler/profile.go b/handler/profile.go index bca0bbe9..396e177e 100644 --- a/handler/profile.go +++ b/handler/profile.go @@ -2,8 +2,9 @@ package handler import ( "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -19,7 +20,7 @@ func MakeProfileHandler(filePath string) ProfileHandler { } func (h ProfileHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - data, err := pkg.ParseJsonFile[payload.ProfileResponse](h.filePath) + data, err := portal.ParseJsonFile[payload.ProfileResponse](h.filePath) if err != nil { slog.Error("Error reading profile file", "error", err) diff --git a/handler/projects.go b/handler/projects.go index 9ea65e5b..6cb7c8c9 100644 --- a/handler/projects.go +++ b/handler/projects.go @@ -2,8 +2,9 @@ package handler import ( "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -19,7 +20,7 @@ func MakeProjectsHandler(filePath string) ProjectsHandler { } func (h ProjectsHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - data, err := pkg.ParseJsonFile[payload.ProjectsResponse](h.filePath) + data, err := portal.ParseJsonFile[payload.ProjectsResponse](h.filePath) if err != nil { slog.Error("Error reading projects file", "error", err) diff --git a/handler/recommendations.go b/handler/recommendations.go index 670e466e..dbb80d15 100644 --- a/handler/recommendations.go +++ b/handler/recommendations.go @@ -2,8 +2,9 @@ package handler import ( "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -19,7 +20,7 @@ func MakeRecommendationsHandler(filePath string) RecommendationsHandler { } func (h RecommendationsHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - data, err := pkg.ParseJsonFile[payload.RecommendationsResponse](h.filePath) + data, err := portal.ParseJsonFile[payload.RecommendationsResponse](h.filePath) if err != nil { slog.Error("Error reading recommendations file", "error", err) diff --git a/handler/social.go b/handler/social.go index 4c9e6d96..158e48f4 100644 --- a/handler/social.go +++ b/handler/social.go @@ -2,8 +2,9 @@ package handler import ( "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -19,7 +20,7 @@ func MakeSocialHandler(filePath string) SocialHandler { } func (h SocialHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - data, err := pkg.ParseJsonFile[payload.SocialResponse](h.filePath) + data, err := portal.ParseJsonFile[payload.SocialResponse](h.filePath) if err != nil { slog.Error("Error reading social file", "error", err) diff --git a/handler/talks.go b/handler/talks.go index de8a3e91..8d44e89d 100644 --- a/handler/talks.go +++ b/handler/talks.go @@ -2,8 +2,9 @@ package handler import ( "github.com/oullin/handler/payload" - "github.com/oullin/pkg" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" + "log/slog" baseHttp "net/http" ) @@ -19,7 +20,7 @@ func MakeTalksHandler(filePath string) TalksHandler { } func (h TalksHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - data, err := pkg.ParseJsonFile[payload.TalksResponse](h.filePath) + data, err := portal.ParseJsonFile[payload.TalksResponse](h.filePath) if err != nil { slog.Error("Error reading talks file", "error", err) diff --git a/main.go b/main.go index 7a3596cd..9f50444b 100644 --- a/main.go +++ b/main.go @@ -2,20 +2,21 @@ package main import ( "fmt" + "log/slog" + baseHttp "net/http" + "time" + "github.com/getsentry/sentry-go" _ "github.com/lib/pq" "github.com/oullin/metal/kernel" - "github.com/oullin/pkg" + "github.com/oullin/pkg/portal" "github.com/rs/cors" - "log/slog" - baseHttp "net/http" - "time" ) var app *kernel.App func init() { - validate := pkg.GetDefaultValidator() + validate := portal.GetDefaultValidator() secrets := kernel.Ignite("./.env", validate) application, err := kernel.MakeApp(secrets, validate) diff --git a/metal/cli/main.go b/metal/cli/main.go index 1bd6e34a..109035dc 100644 --- a/metal/cli/main.go +++ b/metal/cli/main.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "github.com/oullin/database" "github.com/oullin/metal/cli/accounts" "github.com/oullin/metal/cli/panel" @@ -11,13 +12,14 @@ import ( "github.com/oullin/pkg" "github.com/oullin/pkg/auth" "github.com/oullin/pkg/cli" + "github.com/oullin/pkg/portal" ) var environment *env.Environment var dbConn *database.Connection func init() { - secrets := kernel.Ignite("./.env", pkg.GetDefaultValidator()) + secrets := kernel.Ignite("./.env", portal.GetDefaultValidator()) environment = secrets dbConn = kernel.MakeDbConnection(environment) diff --git a/metal/cli/panel/menu.go b/metal/cli/panel/menu.go index 43c12315..61302e9e 100644 --- a/metal/cli/panel/menu.go +++ b/metal/cli/panel/menu.go @@ -3,27 +3,28 @@ package panel import ( "bufio" "fmt" - "github.com/oullin/metal/cli/posts" - "github.com/oullin/pkg" - "github.com/oullin/pkg/auth" - "github.com/oullin/pkg/cli" - "golang.org/x/term" "net/url" "os" "strconv" "strings" + + "github.com/oullin/metal/cli/posts" + "github.com/oullin/pkg/auth" + "github.com/oullin/pkg/cli" + "github.com/oullin/pkg/portal" + "golang.org/x/term" ) type Menu struct { Choice *int Reader *bufio.Reader - Validator *pkg.Validator + Validator *portal.Validator } func MakeMenu() Menu { menu := Menu{ Reader: bufio.NewReader(os.Stdin), - Validator: pkg.GetDefaultValidator(), + Validator: portal.GetDefaultValidator(), } menu.Print() diff --git a/metal/cli/panel/menu_test.go b/metal/cli/panel/menu_test.go index efd978cb..b741d49c 100644 --- a/metal/cli/panel/menu_test.go +++ b/metal/cli/panel/menu_test.go @@ -7,7 +7,7 @@ import ( "strings" "testing" - "github.com/oullin/pkg" + "github.com/oullin/pkg/portal" ) func captureOutput(fn func()) string { @@ -110,7 +110,7 @@ func TestCapturePostURL(t *testing.T) { goodURL := "https://raw.githubusercontent.com/user/repo/file.md" m := Menu{ Reader: bufio.NewReader(strings.NewReader(goodURL + "\n")), - Validator: pkg.GetDefaultValidator(), + Validator: portal.GetDefaultValidator(), } in, err := m.CapturePostURL() @@ -121,7 +121,7 @@ func TestCapturePostURL(t *testing.T) { m2 := Menu{ Reader: bufio.NewReader(strings.NewReader("http://example.com\n")), - Validator: pkg.GetDefaultValidator(), + Validator: portal.GetDefaultValidator(), } if _, err := m2.CapturePostURL(); err == nil { diff --git a/metal/kernel/app.go b/metal/kernel/app.go index b3d20138..2355be7b 100644 --- a/metal/kernel/app.go +++ b/metal/kernel/app.go @@ -7,22 +7,22 @@ import ( "github.com/oullin/database" "github.com/oullin/database/repository" "github.com/oullin/metal/env" - "github.com/oullin/pkg" "github.com/oullin/pkg/auth" "github.com/oullin/pkg/http/middleware" "github.com/oullin/pkg/llogs" + "github.com/oullin/pkg/portal" ) type App struct { router *Router - sentry *pkg.Sentry + sentry *portal.Sentry logs llogs.Driver - validator *pkg.Validator + validator *portal.Validator env *env.Environment db *database.Connection } -func MakeApp(env *env.Environment, validator *pkg.Validator) (*App, error) { +func MakeApp(env *env.Environment, validator *portal.Validator) (*App, error) { tokenHandler, err := auth.MakeTokensHandler( []byte(env.App.MasterKey), ) diff --git a/metal/kernel/factory.go b/metal/kernel/factory.go index 34ed096e..3a92956d 100644 --- a/metal/kernel/factory.go +++ b/metal/kernel/factory.go @@ -8,11 +8,11 @@ import ( sentryhttp "github.com/getsentry/sentry-go/http" "github.com/oullin/database" "github.com/oullin/metal/env" - "github.com/oullin/pkg" "github.com/oullin/pkg/llogs" + "github.com/oullin/pkg/portal" ) -func MakeSentry(env *env.Environment) *pkg.Sentry { +func MakeSentry(env *env.Environment) *portal.Sentry { cOptions := sentry.ClientOptions{ Dsn: env.Sentry.DSN, Debug: true, @@ -25,7 +25,7 @@ func MakeSentry(env *env.Environment) *pkg.Sentry { options := sentryhttp.Options{} handler := sentryhttp.New(options) - return &pkg.Sentry{ + return &portal.Sentry{ Handler: handler, Options: &options, Env: env, @@ -52,7 +52,7 @@ func MakeLogs(env *env.Environment) llogs.Driver { return lDriver } -func MakeEnv(validate *pkg.Validator) *env.Environment { +func MakeEnv(validate *portal.Validator) *env.Environment { errorSuffix := "Environment: " port, err := strconv.Atoi(env.GetEnvVar("ENV_DB_PORT")) diff --git a/metal/kernel/ignite.go b/metal/kernel/ignite.go index 6bf2f8a8..09294e8d 100644 --- a/metal/kernel/ignite.go +++ b/metal/kernel/ignite.go @@ -3,10 +3,10 @@ package kernel import ( "github.com/joho/godotenv" "github.com/oullin/metal/env" - "github.com/oullin/pkg" + "github.com/oullin/pkg/portal" ) -func Ignite(envPath string, validate *pkg.Validator) *env.Environment { +func Ignite(envPath string, validate *portal.Validator) *env.Environment { if err := godotenv.Load(envPath); err != nil { panic("failed to read the .env file/values: " + err.Error()) } diff --git a/metal/kernel/kernel_test.go b/metal/kernel/kernel_test.go index 43b8f71c..5501ece7 100644 --- a/metal/kernel/kernel_test.go +++ b/metal/kernel/kernel_test.go @@ -9,10 +9,10 @@ import ( "github.com/oullin/database" "github.com/oullin/database/repository" - "github.com/oullin/pkg" "github.com/oullin/pkg/auth" "github.com/oullin/pkg/http/middleware" "github.com/oullin/pkg/llogs" + "github.com/oullin/pkg/portal" ) func validEnvVars(t *testing.T) { @@ -38,7 +38,7 @@ func validEnvVars(t *testing.T) { func TestMakeEnv(t *testing.T) { validEnvVars(t) - env := MakeEnv(pkg.GetDefaultValidator()) + env := MakeEnv(portal.GetDefaultValidator()) if env.App.Name != "guss" { t.Fatalf("env not loaded") @@ -74,7 +74,7 @@ func TestIgnite(t *testing.T) { f.WriteString(content) f.Close() - env := Ignite(f.Name(), pkg.GetDefaultValidator()) + env := Ignite(f.Name(), portal.GetDefaultValidator()) if env.Network.HttpPort != "8080" { t.Fatalf("env not loaded") @@ -119,7 +119,7 @@ func TestAppHelpers(t *testing.T) { func TestAppBootRoutes(t *testing.T) { validEnvVars(t) - env := MakeEnv(pkg.GetDefaultValidator()) + env := MakeEnv(portal.GetDefaultValidator()) key, err := auth.GenerateAESKey() @@ -190,7 +190,7 @@ func TestMakeLogs(t *testing.T) { validEnvVars(t) t.Setenv("ENV_APP_LOGS_DIR", tempDir+"/log-%s.txt") - env := MakeEnv(pkg.GetDefaultValidator()) + env := MakeEnv(portal.GetDefaultValidator()) driver := MakeLogs(env) fl := driver.(llogs.FilesLogs) @@ -209,7 +209,7 @@ func TestMakeDbConnectionPanic(t *testing.T) { t.Setenv("ENV_DB_PORT", "1") t.Setenv("ENV_SENTRY_DSN", "https://public@o0.ingest.sentry.io/0") - env := MakeEnv(pkg.GetDefaultValidator()) + env := MakeEnv(portal.GetDefaultValidator()) defer func() { if r := recover(); r == nil { @@ -236,7 +236,7 @@ func TestMakeAppPanic(t *testing.T) { t.Setenv("ENV_APP_LOGS_DIR", tempDir+"/log-%s.txt") t.Setenv("ENV_SENTRY_DSN", "https://public@o0.ingest.sentry.io/0") - env := MakeEnv(pkg.GetDefaultValidator()) + env := MakeEnv(portal.GetDefaultValidator()) defer func() { if r := recover(); r == nil { @@ -244,14 +244,14 @@ func TestMakeAppPanic(t *testing.T) { } }() - MakeApp(env, pkg.GetDefaultValidator()) + MakeApp(env, portal.GetDefaultValidator()) } func TestMakeSentry(t *testing.T) { validEnvVars(t) t.Setenv("ENV_SENTRY_DSN", "https://public@o0.ingest.sentry.io/0") - env := MakeEnv(pkg.GetDefaultValidator()) + env := MakeEnv(portal.GetDefaultValidator()) s := MakeSentry(env) @@ -281,7 +281,7 @@ func TestCloseLogs(t *testing.T) { t.Setenv("ENV_APP_LOGS_DIR", tempDir+"/log-%s.txt") t.Setenv("ENV_SENTRY_DSN", "https://public@o0.ingest.sentry.io/0") - env := MakeEnv(pkg.GetDefaultValidator()) + env := MakeEnv(portal.GetDefaultValidator()) logs := MakeLogs(env) app := &App{logs: logs} diff --git a/pkg/markdown/schema.go b/pkg/markdown/schema.go index 2d14f39e..4856a640 100644 --- a/pkg/markdown/schema.go +++ b/pkg/markdown/schema.go @@ -2,7 +2,9 @@ package markdown import ( "fmt" - "github.com/oullin/pkg" + + "github.com/oullin/pkg/portal" + "time" ) @@ -28,7 +30,7 @@ type Parser struct { } func (f FrontMatter) GetPublishedAt() (*time.Time, error) { - stringable := pkg.MakeStringable(f.PublishedAt) + stringable := portal.MakeStringable(f.PublishedAt) publishedAt, err := stringable.ToDatetime() if err != nil { diff --git a/pkg/parser.go b/pkg/portal/parser.go similarity index 98% rename from pkg/parser.go rename to pkg/portal/parser.go index ddfed34b..5860133a 100644 --- a/pkg/parser.go +++ b/pkg/portal/parser.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "encoding/json" diff --git a/pkg/parser_test.go b/pkg/portal/parser_test.go similarity index 97% rename from pkg/parser_test.go rename to pkg/portal/parser_test.go index 7c337d1f..060cf322 100644 --- a/pkg/parser_test.go +++ b/pkg/portal/parser_test.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "os" diff --git a/pkg/password.go b/pkg/portal/password.go similarity index 97% rename from pkg/password.go rename to pkg/portal/password.go index 15accf5f..bb1637df 100644 --- a/pkg/password.go +++ b/pkg/portal/password.go @@ -1,4 +1,4 @@ -package pkg +package portal import "golang.org/x/crypto/bcrypt" diff --git a/pkg/password_test.go b/pkg/portal/password_test.go similarity index 95% rename from pkg/password_test.go rename to pkg/portal/password_test.go index ed11e8f0..d5b874b0 100644 --- a/pkg/password_test.go +++ b/pkg/portal/password_test.go @@ -1,4 +1,4 @@ -package pkg +package portal import "testing" diff --git a/pkg/sentry.go b/pkg/portal/sentry.go similarity index 93% rename from pkg/sentry.go rename to pkg/portal/sentry.go index f083486c..a355f6c1 100644 --- a/pkg/sentry.go +++ b/pkg/portal/sentry.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( sentryhttp "github.com/getsentry/sentry-go/http" diff --git a/pkg/stringable.go b/pkg/portal/stringable.go similarity index 98% rename from pkg/stringable.go rename to pkg/portal/stringable.go index 429a4d52..3ecc04d3 100644 --- a/pkg/stringable.go +++ b/pkg/portal/stringable.go @@ -1,12 +1,13 @@ -package pkg +package portal import ( "fmt" - "golang.org/x/text/cases" - "golang.org/x/text/language" "strings" "time" "unicode" + + "golang.org/x/text/cases" + "golang.org/x/text/language" ) type Stringable struct { diff --git a/pkg/stringable_test.go b/pkg/portal/stringable_test.go similarity index 98% rename from pkg/stringable_test.go rename to pkg/portal/stringable_test.go index 9e09b947..33361d76 100644 --- a/pkg/stringable_test.go +++ b/pkg/portal/stringable_test.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "testing" diff --git a/pkg/support.go b/pkg/portal/support.go similarity index 91% rename from pkg/support.go rename to pkg/portal/support.go index b1f1bf47..62591edd 100644 --- a/pkg/support.go +++ b/pkg/portal/support.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "io" diff --git a/pkg/support_test.go b/pkg/portal/support_test.go similarity index 96% rename from pkg/support_test.go rename to pkg/portal/support_test.go index 74e33330..de445d4b 100644 --- a/pkg/support_test.go +++ b/pkg/portal/support_test.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "errors" diff --git a/pkg/validator.go b/pkg/portal/validator.go similarity index 99% rename from pkg/validator.go rename to pkg/portal/validator.go index 8feeb0ef..f7475235 100644 --- a/pkg/validator.go +++ b/pkg/portal/validator.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "encoding/json" diff --git a/pkg/validator_test.go b/pkg/portal/validator_test.go similarity index 98% rename from pkg/validator_test.go rename to pkg/portal/validator_test.go index 321a6801..4be9b8ce 100644 --- a/pkg/validator_test.go +++ b/pkg/portal/validator_test.go @@ -1,4 +1,4 @@ -package pkg +package portal import "testing" From 3109ca20c0bec69036f467495be300929bb2e5cc Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 14:29:23 +0800 Subject: [PATCH 06/26] extract limiter --- pkg/http/middleware/token_middleware.go | 81 +++++++++-------------- pkg/limiter/limiter.go | 58 ++++++++++++++++ pkg/limiter/limiter_test.go | 88 +++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 50 deletions(-) create mode 100644 pkg/limiter/limiter.go create mode 100644 pkg/limiter/limiter_test.go diff --git a/pkg/http/middleware/token_middleware.go b/pkg/http/middleware/token_middleware.go index 9862653a..7879c20e 100644 --- a/pkg/http/middleware/token_middleware.go +++ b/pkg/http/middleware/token_middleware.go @@ -13,7 +13,6 @@ import ( "net/url" "sort" "strings" - "sync" "time" "github.com/oullin/database" @@ -21,6 +20,7 @@ import ( "github.com/oullin/pkg/auth" "github.com/oullin/pkg/cache" "github.com/oullin/pkg/http" + "github.com/oullin/pkg/limiter" ) const tokenHeader = "X-API-Key" @@ -32,7 +32,6 @@ const requestIDHeader = "X-Request-ID" // Context keys for propagating auth info downstream // Use unexported custom type to avoid collisions - type contextKey string const ( @@ -40,54 +39,36 @@ const ( requestIdKey contextKey = "request.id" ) -// --- Phase 2 support types +// TokenCheckMiddleware authenticates signed API requests using account tokens. +// It validates required headers, enforces a timestamp skew window, prevents +// replay attacks via nonce tracking, compares tokens/signatures in constant time, +// and applies a basic failure-based rate limiter per client scope. +type TokenCheckMiddleware struct { + // ApiKeys provides access to persisted API key records used to resolve + // account credentials (account name, public key, and secret key). + ApiKeys *repository.ApiKeys -type rateLimiter struct { - mu sync.Mutex - history map[string][]time.Time // key: ip+"|"+account -> failures timestamps - window time.Duration - maxFails int -} + // TokenHandler performs encoding/decoding of tokens and signature creation/verification. + TokenHandler *auth.TokenHandler -func newRateLimiter(window time.Duration, maxFails int) *rateLimiter { - return &rateLimiter{history: make(map[string][]time.Time), window: window, maxFails: maxFails} -} + // nonceCache stores recently seen nonce's to prevent replaying the same request + // within the configured TTL window. + nonceCache *cache.TTLCache -func (r *rateLimiter) tooMany(key string) bool { - r.mu.Lock() - defer r.mu.Unlock() - now := time.Now() - slice := r.history[key] - // prune - pruned := slice[:0] - for _, t := range slice { - if now.Sub(t) <= r.window { - pruned = append(pruned, t) - } - } - r.history[key] = pruned - return len(pruned) >= r.maxFails -} + // rateLimiter throttles repeated authentication failures per "clientIP|account" scope. + rateLimiter *limiter.MemoryLimiter -func (r *rateLimiter) fail(key string) { - r.mu.Lock() - defer r.mu.Unlock() - now := time.Now() - r.history[key] = append(r.history[key], now) -} + // clockSkew defines the allowed difference between client and server time when + // validating the request timestamp. + clockSkew time.Duration -type TokenCheckMiddleware struct { - ApiKeys *repository.ApiKeys - TokenHandler *auth.TokenHandler + // nonceTTL is how long a nonce remains invalid after its first use (replay-protection window). + nonceTTL time.Duration - // Phase 2 additions - nonceCache *cache.TTLCache - rateLimiter *rateLimiter + // failWindow indicates the sliding time window used to evaluate authentication failures. + failWindow time.Duration - // Configurable parameters - clockSkew time.Duration - nonceTTL time.Duration - failWindow time.Duration + // maxFailPerScope is the maximum number of failures allowed within failWindow for a given scope. maxFailPerScope int } @@ -96,7 +77,7 @@ func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.Ap ApiKeys: apiKeys, TokenHandler: tokenHandler, nonceCache: cache.NewTTLCache(), - rateLimiter: newRateLimiter(1*time.Minute, 10), + rateLimiter: limiter.NewMemoryLimiter(1*time.Minute, 10), clockSkew: 5 * time.Minute, nonceTTL: 5 * time.Minute, failWindow: 1 * time.Minute, @@ -192,7 +173,7 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature, canonical, nonce, clientIP string) bool { // Basic rate limiting on failures per IP/account limiterKey := clientIP + "|" + strings.ToLower(accountName) - if t.rateLimiter != nil && t.rateLimiter.tooMany(limiterKey) { + if t.rateLimiter != nil && t.rateLimiter.TooMany(limiterKey) { logger.Warn("too many authentication failures", "ip", clientIP) return true } @@ -200,7 +181,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ var item *database.APIKey if item = t.ApiKeys.FindBy(accountName); item == nil { if t.rateLimiter != nil { - t.rateLimiter.fail(limiterKey) + t.rateLimiter.Fail(limiterKey) } logger.Warn("account not found") return true @@ -213,7 +194,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ ) if err != nil { if t.rateLimiter != nil { - t.rateLimiter.fail(limiterKey) + t.rateLimiter.Fail(limiterKey) } logger.Error("failed to decode account keys", "account", item.AccountName, "error", err) return true @@ -224,7 +205,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ expected := []byte(strings.TrimSpace(token.PublicKey)) if subtle.ConstantTimeCompare(provided, expected) != 1 { if t.rateLimiter != nil { - t.rateLimiter.fail(limiterKey) + t.rateLimiter.Fail(limiterKey) } logger.Warn("public token mismatch", "account", item.AccountName) return true @@ -235,7 +216,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ key := item.AccountName + "|" + nonce if t.nonceCache.Used(key) { if t.rateLimiter != nil { - t.rateLimiter.fail(limiterKey) + t.rateLimiter.Fail(limiterKey) } logger.Warn("replay detected: nonce already used", "account", item.AccountName) return true @@ -246,7 +227,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ localSignature := auth.CreateSignatureFrom(canonical, token.SecretKey) if subtle.ConstantTimeCompare([]byte(signature), []byte(localSignature)) != 1 { if t.rateLimiter != nil { - t.rateLimiter.fail(limiterKey) + t.rateLimiter.Fail(limiterKey) } logger.Warn("signature mismatch", "account", item.AccountName) return true diff --git a/pkg/limiter/limiter.go b/pkg/limiter/limiter.go new file mode 100644 index 00000000..ab6db3a6 --- /dev/null +++ b/pkg/limiter/limiter.go @@ -0,0 +1,58 @@ +package limiter + +import ( + "sync" + "time" +) + +// MemoryLimiter provides a simple in-memory failure-based rate limiter. +// It tracks failure timestamps per arbitrary key (e.g., ip|account) +// and decides whether the number of failures within a sliding window +// exceeds a configured threshold. +type MemoryLimiter struct { + mu sync.Mutex + history map[string][]time.Time // key -> failure timestamps + window time.Duration + maxFails int +} + +// NewMemoryLimiter constructs a MemoryLimiter with the specified sliding window duration +// and the maximum number of failures allowed within that window. +func NewMemoryLimiter(window time.Duration, maxFails int) *MemoryLimiter { + return &MemoryLimiter{ + history: make(map[string][]time.Time), + window: window, + maxFails: maxFails, + } +} + +// TooMany reports whether the given key has reached or exceeded the +// maximum number of failures within the configured window. +func (r *MemoryLimiter) TooMany(key string) bool { + r.mu.Lock() + defer r.mu.Unlock() + + now := time.Now() + slice := r.history[key] + + // prune old entries outside the window + pruned := slice[:0] + for _, t := range slice { + if now.Sub(t) <= r.window { + pruned = append(pruned, t) + } + } + + r.history[key] = pruned + + return len(pruned) >= r.maxFails +} + +// Fail records a failure occurrence for the given key. +func (r *MemoryLimiter) Fail(key string) { + r.mu.Lock() + defer r.mu.Unlock() + + now := time.Now() + r.history[key] = append(r.history[key], now) +} diff --git a/pkg/limiter/limiter_test.go b/pkg/limiter/limiter_test.go new file mode 100644 index 00000000..8488c6bd --- /dev/null +++ b/pkg/limiter/limiter_test.go @@ -0,0 +1,88 @@ +package limiter + +import ( + "testing" + "time" +) + +// TestTooManyThreshold verifies that TooMany returns true only after +// maxFails failures have been recorded within the window. +func TestTooManyThreshold(t *testing.T) { + r := NewMemoryLimiter(1*time.Minute, 3) + key := "ip|account" + + if r.TooMany(key) { + t.Fatalf("expected TooMany to be false before any failures") + } + + r.Fail(key) + if r.TooMany(key) { + t.Fatalf("expected TooMany to be false after 1 failure (< maxFails)") + } + + r.Fail(key) + if r.TooMany(key) { + t.Fatalf("expected TooMany to be false after 2 failures (< maxFails)") + } + + r.Fail(key) + if !r.TooMany(key) { + t.Fatalf("expected TooMany to be true after reaching maxFails") + } +} + +// TestWindowPruning verifies that failures older than the window are pruned +// and do not contribute to the TooMany decision. +func TestWindowPruning(t *testing.T) { + // Use a short window to make the test fast and deterministic + window := 50 * time.Millisecond + r := NewMemoryLimiter(window, 2) + key := "client|user" + + // Record one failure and wait for it to expire + r.Fail(key) + time.Sleep(window + 10*time.Millisecond) + + // Calling TooMany triggers pruning of the older entry + if r.TooMany(key) { + t.Fatalf("expected TooMany to be false after window expiration and pruning") + } + + // Now add two quick failures within the window + r.Fail(key) + r.Fail(key) + + if !r.TooMany(key) { + t.Fatalf("expected TooMany to be true after 2 failures within window (maxFails=2)") + } +} + +// TestIndependentKeys checks that limiter maintains separate counters per key. +func TestIndependentKeys(t *testing.T) { + r := NewMemoryLimiter(1*time.Second, 2) + keyA := "ipA|acct" + keyB := "ipB|acct" + + r.Fail(keyA) + if r.TooMany(keyA) { + t.Fatalf("TooMany should be false for keyA with 1 failure") + } + + // keyB should be unaffected by keyA's failures + if r.TooMany(keyB) { // triggers prune/check but no failures recorded for keyB + t.Fatalf("TooMany should be false for keyB with 0 failures") + } + + // Push keyB over the threshold independently + r.Fail(keyB) + r.Fail(keyB) + + if !r.TooMany(keyB) { + t.Fatalf("TooMany should be true for keyB after reaching maxFails") + } + + // keyA still below threshold + if r.TooMany(keyA) { + t.Fatalf("TooMany should still be false for keyA (only 1 failure)") + } +} From 8c6a03543814a900e6ba2117dce7da92e6cc231e Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 14:35:39 +0800 Subject: [PATCH 07/26] format --- .github/workflows/tests.yml | 3 +- pkg/http/middleware/token_middleware.go | 133 ++++++++++++++---------- 2 files changed, 81 insertions(+), 55 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7eb67d3c..964853dc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,8 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go-version: ['1.24.6', '1.24.5'] + # , '1.24.5' + go-version: ['1.24.6'] steps: - uses: actions/setup-go@v5 diff --git a/pkg/http/middleware/token_middleware.go b/pkg/http/middleware/token_middleware.go index 7879c20e..62fa479a 100644 --- a/pkg/http/middleware/token_middleware.go +++ b/pkg/http/middleware/token_middleware.go @@ -88,81 +88,40 @@ func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.Ap func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { return func(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { reqID := strings.TrimSpace(r.Header.Get(requestIDHeader)) - if reqID == "" { return t.getInvalidRequestError() } logger := slog.With("request_id", reqID, "path", r.URL.Path, "method", r.Method) - accountName := strings.TrimSpace(r.Header.Get(usernameHeader)) - publicToken := strings.TrimSpace(r.Header.Get(tokenHeader)) - signature := strings.TrimSpace(r.Header.Get(signatureHeader)) - ts := strings.TrimSpace(r.Header.Get(timestampHeader)) - nonce := strings.TrimSpace(r.Header.Get(nonceHeader)) - - if accountName == "" || publicToken == "" || signature == "" || ts == "" || nonce == "" { - logger.Warn("missing authentication headers") - return t.getInvalidRequestError() + // Extract and validate required headers + accountName, publicToken, signature, ts, nonce, hdrErr := t.validateAndGetHeaders(r, logger) + if hdrErr != nil { + return hdrErr } - if err := auth.ValidateTokenFormat(publicToken); err != nil { - logger.Warn("invalid token format") - return t.getInvalidTokenFormatError() + // Validate timestamp within allowed window + if tsErr := t.validateTimestamp(ts, logger); tsErr != nil { + return tsErr } - // Validate timestamp skew window - parsed, err := time.ParseDuration(ts + "s") - if err != nil { - // try interpreting as epoch seconds (int) - var epoch int64 - for _, ch := range ts { - if ch < '0' || ch > '9' { - logger.Warn("invalid timestamp format") - return t.getInvalidRequestError() - } - } - // safe to parse as int64 now - // custom parse to avoid strconv import - for _, ch := range ts { - epoch = epoch*10 + int64(ch-'0') - } - now := time.Now().Unix() - if epoch < now-int64(t.clockSkew.Seconds()) || epoch > now+int64(t.clockSkew.Seconds()) { - logger.Warn("timestamp outside allowed window") - return t.getUnauthenticatedError() - } - } else { - _ = parsed // ignore if duration parsed (not expected), kept for completeness + // Read body and compute hash + bodyHash, bodyErr := t.readBodyHash(r, logger) + if bodyErr != nil { + return bodyErr } - // Read and hash body, then restore it for downstream - var bodyBytes []byte - if r.Body != nil { - b, readErr := io.ReadAll(r.Body) - if readErr != nil { - logger.Warn("unable to read body for signing") - return t.getInvalidRequestError() - } - bodyBytes = b - r.Body = io.NopCloser(bytes.NewReader(b)) - } - bodyHash := sha256Hex(bodyBytes) - // Build canonical request string canonical := buildCanonical(r.Method, r.URL, accountName, publicToken, ts, nonce, bodyHash) clientIP := parseClientIP(r) - reject := t.shallReject(logger, accountName, publicToken, signature, canonical, nonce, clientIP) - if reject { + if t.shallReject(logger, accountName, publicToken, signature, canonical, nonce, clientIP) { return t.getUnauthenticatedError() } // Update the request context - ctx := context.WithValue(r.Context(), authAccountNameKey, accountName) - ctx = context.WithValue(r.Context(), requestIdKey, reqID) - r = r.WithContext(ctx) + r = t.attachAuthContext(r, accountName, reqID) logger.Info("authentication successful") @@ -170,6 +129,72 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { } } +// validateAndGetHeaders extracts and validates required auth headers, logging and returning +// appropriate ApiError on failure. +func (t TokenCheckMiddleware) validateAndGetHeaders(r *baseHttp.Request, logger *slog.Logger) (accountName, publicToken, signature, ts, nonce string, apiErr *http.ApiError) { + accountName = strings.TrimSpace(r.Header.Get(usernameHeader)) + publicToken = strings.TrimSpace(r.Header.Get(tokenHeader)) + signature = strings.TrimSpace(r.Header.Get(signatureHeader)) + ts = strings.TrimSpace(r.Header.Get(timestampHeader)) + nonce = strings.TrimSpace(r.Header.Get(nonceHeader)) + + if accountName == "" || publicToken == "" || signature == "" || ts == "" || nonce == "" { + logger.Warn("missing authentication headers") + return "", "", "", "", "", t.getInvalidRequestError() + } + + if err := auth.ValidateTokenFormat(publicToken); err != nil { + logger.Warn("invalid token format") + return "", "", "", "", "", t.getInvalidTokenFormatError() + } + + return accountName, publicToken, signature, ts, nonce, nil +} + +// validateTimestamp ensures the provided timestamp is numeric and within skew. +func (t TokenCheckMiddleware) validateTimestamp(ts string, logger *slog.Logger) *http.ApiError { + if ts == "" { + logger.Warn("missing timestamp") + return t.getInvalidRequestError() + } + var epoch int64 + for _, ch := range ts { + if ch < '0' || ch > '9' { + logger.Warn("invalid timestamp format") + return t.getInvalidRequestError() + } + epoch = epoch*10 + int64(ch-'0') + } + now := time.Now().Unix() + if epoch < now-int64(t.clockSkew.Seconds()) || epoch > now+int64(t.clockSkew.Seconds()) { + logger.Warn("timestamp outside allowed window") + return t.getUnauthenticatedError() + } + return nil +} + +// readBodyHash reads and restores the request body and returns its SHA256 hex. +func (t TokenCheckMiddleware) readBodyHash(r *baseHttp.Request, logger *slog.Logger) (string, *http.ApiError) { + if r.Body == nil { + return sha256Hex(nil), nil + } + b, err := io.ReadAll(r.Body) + if err != nil { + logger.Warn("unable to read body for signing") + return "", t.getInvalidRequestError() + } + // restore for downstream handlers + r.Body = io.NopCloser(bytes.NewReader(b)) + return sha256Hex(b), nil +} + +// attachAuthContext adds auth/account data and request id to the request context. +func (t TokenCheckMiddleware) attachAuthContext(r *baseHttp.Request, accountName, reqID string) *baseHttp.Request { + ctx := context.WithValue(r.Context(), authAccountNameKey, accountName) + ctx = context.WithValue(r.Context(), requestIdKey, reqID) + return r.WithContext(ctx) +} + func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature, canonical, nonce, clientIP string) bool { // Basic rate limiting on failures per IP/account limiterKey := clientIP + "|" + strings.ToLower(accountName) From 72c3101e35484fe2ddaf7f10acf57fb189af7d49 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 14:39:56 +0800 Subject: [PATCH 08/26] remove nested package --- .github/workflows/gofmt.yml | 1 + metal/kernel/app.go | 2 +- metal/kernel/kernel_test.go | 2 +- metal/kernel/router.go | 2 +- pkg/{http => }/middleware/pipeline.go | 0 pkg/{http => }/middleware/pipeline_test.go | 0 pkg/{http => }/middleware/token_middleware.go | 92 ++++--------------- .../middleware/token_middleware_test.go | 0 pkg/portal/support.go | 75 +++++++++++++++ 9 files changed, 96 insertions(+), 78 deletions(-) rename pkg/{http => }/middleware/pipeline.go (100%) rename pkg/{http => }/middleware/pipeline_test.go (100%) rename pkg/{http => }/middleware/token_middleware.go (84%) rename pkg/{http => }/middleware/token_middleware_test.go (100%) diff --git a/.github/workflows/gofmt.yml b/.github/workflows/gofmt.yml index 4c297bf2..9c9b9b7d 100644 --- a/.github/workflows/gofmt.yml +++ b/.github/workflows/gofmt.yml @@ -13,6 +13,7 @@ permissions: jobs: gofmt: + if: false strategy: matrix: go-version: [1.24.5] diff --git a/metal/kernel/app.go b/metal/kernel/app.go index 2355be7b..868ac8ea 100644 --- a/metal/kernel/app.go +++ b/metal/kernel/app.go @@ -8,7 +8,7 @@ import ( "github.com/oullin/database/repository" "github.com/oullin/metal/env" "github.com/oullin/pkg/auth" - "github.com/oullin/pkg/http/middleware" + "github.com/oullin/pkg/middleware" "github.com/oullin/pkg/llogs" "github.com/oullin/pkg/portal" ) diff --git a/metal/kernel/kernel_test.go b/metal/kernel/kernel_test.go index 5501ece7..f2521a1c 100644 --- a/metal/kernel/kernel_test.go +++ b/metal/kernel/kernel_test.go @@ -10,7 +10,7 @@ import ( "github.com/oullin/database" "github.com/oullin/database/repository" "github.com/oullin/pkg/auth" - "github.com/oullin/pkg/http/middleware" + "github.com/oullin/pkg/middleware" "github.com/oullin/pkg/llogs" "github.com/oullin/pkg/portal" ) diff --git a/metal/kernel/router.go b/metal/kernel/router.go index 93778ac0..ca13d6b9 100644 --- a/metal/kernel/router.go +++ b/metal/kernel/router.go @@ -8,7 +8,7 @@ import ( "github.com/oullin/handler" "github.com/oullin/metal/env" "github.com/oullin/pkg/http" - "github.com/oullin/pkg/http/middleware" + "github.com/oullin/pkg/middleware" ) type StaticRouteResource interface { diff --git a/pkg/http/middleware/pipeline.go b/pkg/middleware/pipeline.go similarity index 100% rename from pkg/http/middleware/pipeline.go rename to pkg/middleware/pipeline.go diff --git a/pkg/http/middleware/pipeline_test.go b/pkg/middleware/pipeline_test.go similarity index 100% rename from pkg/http/middleware/pipeline_test.go rename to pkg/middleware/pipeline_test.go diff --git a/pkg/http/middleware/token_middleware.go b/pkg/middleware/token_middleware.go similarity index 84% rename from pkg/http/middleware/token_middleware.go rename to pkg/middleware/token_middleware.go index 62fa479a..7d8fb321 100644 --- a/pkg/http/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -3,15 +3,10 @@ package middleware import ( "bytes" "context" - "crypto/sha256" "crypto/subtle" - "encoding/hex" "io" "log/slog" - "net" baseHttp "net/http" - "net/url" - "sort" "strings" "time" @@ -21,6 +16,7 @@ import ( "github.com/oullin/pkg/cache" "github.com/oullin/pkg/http" "github.com/oullin/pkg/limiter" + "github.com/oullin/pkg/portal" ) const tokenHeader = "X-API-Key" @@ -100,7 +96,7 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { return hdrErr } - // Validate timestamp within allowed window + // Validate timestamp within allowed skew if tsErr := t.validateTimestamp(ts, logger); tsErr != nil { return tsErr } @@ -112,9 +108,9 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { } // Build canonical request string - canonical := buildCanonical(r.Method, r.URL, accountName, publicToken, ts, nonce, bodyHash) + canonical := portal.BuildCanonical(r.Method, r.URL, accountName, publicToken, ts, nonce, bodyHash) - clientIP := parseClientIP(r) + clientIP := portal.ParseClientIP(r) if t.shallReject(logger, accountName, publicToken, signature, canonical, nonce, clientIP) { return t.getUnauthenticatedError() @@ -157,35 +153,42 @@ func (t TokenCheckMiddleware) validateTimestamp(ts string, logger *slog.Logger) logger.Warn("missing timestamp") return t.getInvalidRequestError() } + var epoch int64 for _, ch := range ts { if ch < '0' || ch > '9' { logger.Warn("invalid timestamp format") return t.getInvalidRequestError() } + epoch = epoch*10 + int64(ch-'0') } + now := time.Now().Unix() if epoch < now-int64(t.clockSkew.Seconds()) || epoch > now+int64(t.clockSkew.Seconds()) { logger.Warn("timestamp outside allowed window") return t.getUnauthenticatedError() } + return nil } // readBodyHash reads and restores the request body and returns its SHA256 hex. func (t TokenCheckMiddleware) readBodyHash(r *baseHttp.Request, logger *slog.Logger) (string, *http.ApiError) { if r.Body == nil { - return sha256Hex(nil), nil + return portal.Sha256Hex(nil), nil } + b, err := io.ReadAll(r.Body) if err != nil { logger.Warn("unable to read body for signing") return "", t.getInvalidRequestError() } + // restore for downstream handlers r.Body = io.NopCloser(bytes.NewReader(b)) - return sha256Hex(b), nil + + return portal.Sha256Hex(b), nil } // attachAuthContext adds auth/account data and request id to the request context. @@ -196,8 +199,8 @@ func (t TokenCheckMiddleware) attachAuthContext(r *baseHttp.Request, accountName } func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature, canonical, nonce, clientIP string) bool { - // Basic rate limiting on failures per IP/account limiterKey := clientIP + "|" + strings.ToLower(accountName) + if t.rateLimiter != nil && t.rateLimiter.TooMany(limiterKey) { logger.Warn("too many authentication failures", "ip", clientIP) return true @@ -208,6 +211,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ if t.rateLimiter != nil { t.rateLimiter.Fail(limiterKey) } + logger.Warn("account not found") return true } @@ -217,6 +221,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ item.SecretKey, item.PublicKey, ) + if err != nil { if t.rateLimiter != nil { t.rateLimiter.Fail(limiterKey) @@ -243,6 +248,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ if t.rateLimiter != nil { t.rateLimiter.Fail(limiterKey) } + logger.Warn("replay detected: nonce already used", "account", item.AccountName) return true } @@ -267,70 +273,6 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ return false } -// Helpers -func sha256Hex(b []byte) string { - h := sha256.Sum256(b) - return hex.EncodeToString(h[:]) -} - -func sortedQuery(u *url.URL) string { - if u == nil { - return "" - } - q := u.Query() - if len(q) == 0 { - return "" - } - keys := make([]string, 0, len(q)) - for k := range q { - keys = append(keys, k) - } - sort.Strings(keys) - pairs := make([]string, 0, len(keys)) - for _, k := range keys { - vals := q[k] - sort.Strings(vals) - for _, v := range vals { - pairs = append(pairs, url.QueryEscape(k)+"="+url.QueryEscape(v)) - } - } - return strings.Join(pairs, "&") -} - -func buildCanonical(method string, u *url.URL, username, public, ts, nonce, bodyHash string) string { - path := "/" - if u != nil && u.Path != "" { - path = u.EscapedPath() - } - query := sortedQuery(u) - parts := []string{ - strings.ToUpper(method), - path, - query, - username, - public, - ts, - nonce, - bodyHash, - } - return strings.Join(parts, "\n") -} - -func parseClientIP(r *baseHttp.Request) string { - // prefer X-Forwarded-For if present - xff := strings.TrimSpace(r.Header.Get("X-Forwarded-For")) - if xff != "" { - // take first IP - parts := strings.Split(xff, ",") - return strings.TrimSpace(parts[0]) - } - host, _, err := net.SplitHostPort(strings.TrimSpace(r.RemoteAddr)) - if err == nil && host != "" { - return host - } - return strings.TrimSpace(r.RemoteAddr) -} - func (t TokenCheckMiddleware) getInvalidRequestError() *http.ApiError { return &http.ApiError{ Message: "Invalid authentication headers", diff --git a/pkg/http/middleware/token_middleware_test.go b/pkg/middleware/token_middleware_test.go similarity index 100% rename from pkg/http/middleware/token_middleware_test.go rename to pkg/middleware/token_middleware_test.go diff --git a/pkg/portal/support.go b/pkg/portal/support.go index 62591edd..b5569779 100644 --- a/pkg/portal/support.go +++ b/pkg/portal/support.go @@ -1,8 +1,15 @@ package portal import ( + "crypto/sha256" + "encoding/hex" "io" "log/slog" + "net" + baseHttp "net/http" + "net/url" + "sort" + "strings" ) func CloseWithLog(c io.Closer) { @@ -10,3 +17,71 @@ func CloseWithLog(c io.Closer) { slog.Error("failed to close resource", "err", err) } } + +func Sha256Hex(b []byte) string { + h := sha256.Sum256(b) + return hex.EncodeToString(h[:]) +} + +func SortedQuery(u *url.URL) string { + if u == nil { + return "" + } + q := u.Query() + if len(q) == 0 { + return "" + } + keys := make([]string, 0, len(q)) + for k := range q { + keys = append(keys, k) + } + sort.Strings(keys) + pairs := make([]string, 0, len(keys)) + for _, k := range keys { + vals := q[k] + sort.Strings(vals) + for _, v := range vals { + pairs = append(pairs, url.QueryEscape(k)+"="+url.QueryEscape(v)) + } + } + return strings.Join(pairs, "&") +} + +func BuildCanonical(method string, u *url.URL, username, public, ts, nonce, bodyHash string) string { + path := "/" + + if u != nil && u.Path != "" { + path = u.EscapedPath() + } + + query := SortedQuery(u) + parts := []string{ + strings.ToUpper(method), + path, + query, + username, + public, + ts, + nonce, + bodyHash, + } + + return strings.Join(parts, "\n") +} + +func ParseClientIP(r *baseHttp.Request) string { + // prefer X-Forwarded-For if present + xff := strings.TrimSpace(r.Header.Get("X-Forwarded-For")) + if xff != "" { + // take first IP + parts := strings.Split(xff, ",") + return strings.TrimSpace(parts[0]) + } + + host, _, err := net.SplitHostPort(strings.TrimSpace(r.RemoteAddr)) + if err == nil && host != "" { + return host + } + + return strings.TrimSpace(r.RemoteAddr) +} From b62d7db16078ecc3791cdfc2b96f5b0627a85e84 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 15:11:11 +0800 Subject: [PATCH 09/26] extract valid_timestamp --- metal/kernel/app.go | 2 +- metal/kernel/kernel_test.go | 2 +- pkg/middleware/token_middleware.go | 40 +++------- pkg/middleware/valid_timestamp.go | 74 +++++++++++++++++ pkg/middleware/valid_timestamp_test.go | 105 +++++++++++++++++++++++++ 5 files changed, 193 insertions(+), 30 deletions(-) create mode 100644 pkg/middleware/valid_timestamp.go create mode 100644 pkg/middleware/valid_timestamp_test.go diff --git a/metal/kernel/app.go b/metal/kernel/app.go index 868ac8ea..679c1c72 100644 --- a/metal/kernel/app.go +++ b/metal/kernel/app.go @@ -8,8 +8,8 @@ import ( "github.com/oullin/database/repository" "github.com/oullin/metal/env" "github.com/oullin/pkg/auth" - "github.com/oullin/pkg/middleware" "github.com/oullin/pkg/llogs" + "github.com/oullin/pkg/middleware" "github.com/oullin/pkg/portal" ) diff --git a/metal/kernel/kernel_test.go b/metal/kernel/kernel_test.go index f2521a1c..8ac19909 100644 --- a/metal/kernel/kernel_test.go +++ b/metal/kernel/kernel_test.go @@ -10,8 +10,8 @@ import ( "github.com/oullin/database" "github.com/oullin/database/repository" "github.com/oullin/pkg/auth" - "github.com/oullin/pkg/middleware" "github.com/oullin/pkg/llogs" + "github.com/oullin/pkg/middleware" "github.com/oullin/pkg/portal" ) diff --git a/pkg/middleware/token_middleware.go b/pkg/middleware/token_middleware.go index 7d8fb321..dccecc79 100644 --- a/pkg/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -58,6 +58,13 @@ type TokenCheckMiddleware struct { // validating the request timestamp. clockSkew time.Duration + // now is an injectable time source for deterministic tests. If nil, time.Now is used. + now func() time.Time + + // disallowFuture, if true, rejects timestamps greater than the current server time, + // even if they are within the positive skew window. + disallowFuture bool + // nonceTTL is how long a nonce remains invalid after its first use (replay-protection window). nonceTTL time.Duration @@ -75,6 +82,8 @@ func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.Ap nonceCache: cache.NewTTLCache(), rateLimiter: limiter.NewMemoryLimiter(1*time.Minute, 10), clockSkew: 5 * time.Minute, + now: time.Now, + disallowFuture: false, nonceTTL: 5 * time.Minute, failWindow: 1 * time.Minute, maxFailPerScope: 10, @@ -96,8 +105,9 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { return hdrErr } - // Validate timestamp within allowed skew - if tsErr := t.validateTimestamp(ts, logger); tsErr != nil { + // Validate timestamp within allowed skew using ValidTimestamp helper + vt := NewValidTimestamp(ts, logger, t.now) + if tsErr := vt.Validate(t.clockSkew, t.disallowFuture); tsErr != nil { return tsErr } @@ -147,32 +157,6 @@ func (t TokenCheckMiddleware) validateAndGetHeaders(r *baseHttp.Request, logger return accountName, publicToken, signature, ts, nonce, nil } -// validateTimestamp ensures the provided timestamp is numeric and within skew. -func (t TokenCheckMiddleware) validateTimestamp(ts string, logger *slog.Logger) *http.ApiError { - if ts == "" { - logger.Warn("missing timestamp") - return t.getInvalidRequestError() - } - - var epoch int64 - for _, ch := range ts { - if ch < '0' || ch > '9' { - logger.Warn("invalid timestamp format") - return t.getInvalidRequestError() - } - - epoch = epoch*10 + int64(ch-'0') - } - - now := time.Now().Unix() - if epoch < now-int64(t.clockSkew.Seconds()) || epoch > now+int64(t.clockSkew.Seconds()) { - logger.Warn("timestamp outside allowed window") - return t.getUnauthenticatedError() - } - - return nil -} - // readBodyHash reads and restores the request body and returns its SHA256 hex. func (t TokenCheckMiddleware) readBodyHash(r *baseHttp.Request, logger *slog.Logger) (string, *http.ApiError) { if r.Body == nil { diff --git a/pkg/middleware/valid_timestamp.go b/pkg/middleware/valid_timestamp.go new file mode 100644 index 00000000..44f06f7c --- /dev/null +++ b/pkg/middleware/valid_timestamp.go @@ -0,0 +1,74 @@ +package middleware + +import ( + "log/slog" + "strconv" + "time" + + "github.com/oullin/pkg/http" +) + +// ValidTimestamp encapsulates timestamp validation context. +// It accepts: the raw timestamp string (ts), a logger, and a clock (now) function. +// Use Validate to check against a provided skew window and future-time policy. +type ValidTimestamp struct { + // ts is the timestamp string (expected Unix epoch in seconds). + ts string + // logger is used to record validation details. + logger *slog.Logger + // now returns the current time; useful to inject a deterministic clock in tests. + now func() time.Time +} + +// NewValidTimestamp constructs a ValidTimestamp with the provided inputs. +// Use a nil logger or now function to fall back to defaults. +func NewValidTimestamp(ts string, logger *slog.Logger, now func() time.Time) ValidTimestamp { + return ValidTimestamp{ts: ts, logger: logger, now: now} +} + +// Validate ensures the timestamp is numeric (Unix seconds) and within the allowed skew window. +// If disallowFuture is true, timestamps after "now" are rejected even if within positive skew. +func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http.ApiError { + if v.ts == "" { + if v.logger != nil { + v.logger.Warn("missing timestamp") + } + return &http.ApiError{Message: "Invalid authentication headers", Status: 401} + } + + epoch, err := strconv.ParseInt(v.ts, 10, 64) + if err != nil { + if v.logger != nil { + v.logger.Warn("invalid timestamp format") + } + return &http.ApiError{Message: "Invalid authentication headers", Status: 401} + } + + nowFn := v.now + if nowFn == nil { + nowFn = time.Now + } + now := nowFn().Unix() + skewSecs := int64(skew.Seconds()) + minValue := now - skewSecs + maxValue := now + skewSecs + if disallowFuture { + maxValue = now + } + + if epoch < minValue { + if v.logger != nil { + v.logger.Warn("timestamp outside allowed window: too old") + } + return &http.ApiError{Message: "Invalid credentials", Status: 401} + } + + if epoch > maxValue { + if v.logger != nil { + v.logger.Warn("timestamp outside allowed window: in the future") + } + return &http.ApiError{Message: "Invalid credentials", Status: 401} + } + + return nil +} diff --git a/pkg/middleware/valid_timestamp_test.go b/pkg/middleware/valid_timestamp_test.go new file mode 100644 index 00000000..e7f65e08 --- /dev/null +++ b/pkg/middleware/valid_timestamp_test.go @@ -0,0 +1,105 @@ +package middleware + +import ( + "log/slog" + "strconv" + "testing" + "time" +) + +func fixedClock(t time.Time) func() time.Time { return func() time.Time { return t } } + +func TestNewValidTimestampConstructor(t *testing.T) { + base := time.Unix(1_700_000_000, 0) + logger := slogNoop() + vt := NewValidTimestamp("123", logger, fixedClock(base)) + + if vt.ts != "123" { + t.Fatalf("expected ts to be set by constructor") + } + if vt.logger != logger { + t.Fatalf("expected logger to be set by constructor") + } + if vt.now == nil || vt.now().Unix() != base.Unix() { + t.Fatalf("expected now clock to be set by constructor") + } +} + +func TestValidate_EmptyTimestamp(t *testing.T) { + vt := NewValidTimestamp("", nil, fixedClock(time.Unix(1_700_000_000, 0))) + err := vt.Validate(5*time.Minute, false) + if err == nil || err.Status != 401 || err.Message != "Invalid authentication headers" { + t.Fatalf("expected invalid request error for empty timestamp, got %#v", err) + } +} + +func TestValidate_NonNumericTimestamp(t *testing.T) { + vt := NewValidTimestamp("abc", nil, fixedClock(time.Unix(1_700_000_000, 0))) + err := vt.Validate(5*time.Minute, false) + if err == nil || err.Status != 401 || err.Message != "Invalid authentication headers" { + t.Fatalf("expected invalid request error for non-numeric timestamp, got %#v", err) + } +} + +func TestValidate_TooOldTimestamp(t *testing.T) { + base := time.Unix(1_700_000_000, 0) + skew := 60 * time.Second + oldTs := strconv.FormatInt(base.Add(-skew).Add(-1*time.Second).Unix(), 10) + vt := NewValidTimestamp(oldTs, nil, fixedClock(base)) + err := vt.Validate(skew, false) + if err == nil || err.Status != 401 || err.Message != "Invalid credentials" { + t.Fatalf("expected unauthenticated for too old timestamp, got %#v", err) + } +} + +func TestValidate_FutureWithinSkew_Behavior(t *testing.T) { + base := time.Unix(1_700_000_000, 0) + skew := 60 * time.Second + futureWithin := strconv.FormatInt(base.Add(30*time.Second).Unix(), 10) + + // Allowed when disallowFuture=false + vt := NewValidTimestamp(futureWithin, nil, fixedClock(base)) + if err := vt.Validate(skew, false); err != nil { + t.Fatalf("expected future timestamp within skew to be allowed when disallowFuture=false, got %#v", err) + } + + // Rejected when disallowFuture=true + vt = NewValidTimestamp(futureWithin, nil, fixedClock(base)) + err := vt.Validate(skew, true) + if err == nil || err.Status != 401 || err.Message != "Invalid credentials" { + t.Fatalf("expected unauthenticated for future timestamp when disallowFuture=true, got %#v", err) + } +} + +func TestValidate_Boundaries(t *testing.T) { + base := time.Unix(1_700_000_000, 0) + skew := 60 * time.Second + minExact := strconv.FormatInt(base.Add(-skew).Unix(), 10) + maxExact := strconv.FormatInt(base.Add(skew).Unix(), 10) + nowExact := strconv.FormatInt(base.Unix(), 10) + + // Lower boundary inclusive + vt := NewValidTimestamp(minExact, nil, fixedClock(base)) + if err := vt.Validate(skew, false); err != nil { + t.Fatalf("expected min boundary to pass, got %#v", err) + } + + // Upper boundary inclusive when disallowFuture=false + vt = NewValidTimestamp(maxExact, nil, fixedClock(base)) + if err := vt.Validate(skew, false); err != nil { + t.Fatalf("expected max boundary to pass when disallowFuture=false, got %#v", err) + } + + // When disallowFuture=true, upper boundary becomes 'now' + vt = NewValidTimestamp(nowExact, nil, fixedClock(base)) + if err := vt.Validate(skew, true); err != nil { + t.Fatalf("expected 'now' to pass when disallowFuture=true, got %#v", err) + } +} + +// slogNoop provides a minimal no-op logger compatible with *slog.Logger without requiring configuration in tests. +func slogNoop() *slog.Logger { + // slog.New requires a Handler; use a Discard handler by sending to a disabled level. + // For simplicity and to avoid extra deps, return nil to keep logging optional in validation. + return nil +} From 3cc9bb837cc9c91fdf5e4f224170ebc26c6bf5ce Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 15:22:22 +0800 Subject: [PATCH 10/26] format --- pkg/middleware/valid_timestamp.go | 34 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/pkg/middleware/valid_timestamp.go b/pkg/middleware/valid_timestamp.go index 44f06f7c..c837f2cb 100644 --- a/pkg/middleware/valid_timestamp.go +++ b/pkg/middleware/valid_timestamp.go @@ -14,33 +14,35 @@ import ( type ValidTimestamp struct { // ts is the timestamp string (expected Unix epoch in seconds). ts string + // logger is used to record validation details. logger *slog.Logger + // now returns the current time; useful to inject a deterministic clock in tests. now func() time.Time } -// NewValidTimestamp constructs a ValidTimestamp with the provided inputs. -// Use a nil logger or now function to fall back to defaults. func NewValidTimestamp(ts string, logger *slog.Logger, now func() time.Time) ValidTimestamp { - return ValidTimestamp{ts: ts, logger: logger, now: now} + return ValidTimestamp{ + ts: ts, + logger: logger, + now: now, + } } -// Validate ensures the timestamp is numeric (Unix seconds) and within the allowed skew window. -// If disallowFuture is true, timestamps after "now" are rejected even if within positive skew. func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http.ApiError { + if v.logger == nil { + return &http.ApiError{Message: "Invalid authentication headers internal formation", Status: 401} + } + if v.ts == "" { - if v.logger != nil { - v.logger.Warn("missing timestamp") - } + v.logger.Warn("missing timestamp") return &http.ApiError{Message: "Invalid authentication headers", Status: 401} } epoch, err := strconv.ParseInt(v.ts, 10, 64) if err != nil { - if v.logger != nil { - v.logger.Warn("invalid timestamp format") - } + v.logger.Warn("invalid timestamp format") return &http.ApiError{Message: "Invalid authentication headers", Status: 401} } @@ -48,25 +50,23 @@ func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http. if nowFn == nil { nowFn = time.Now } + now := nowFn().Unix() skewSecs := int64(skew.Seconds()) minValue := now - skewSecs maxValue := now + skewSecs + if disallowFuture { maxValue = now } if epoch < minValue { - if v.logger != nil { - v.logger.Warn("timestamp outside allowed window: too old") - } + v.logger.Warn("timestamp outside allowed window: too old") return &http.ApiError{Message: "Invalid credentials", Status: 401} } if epoch > maxValue { - if v.logger != nil { - v.logger.Warn("timestamp outside allowed window: in the future") - } + v.logger.Warn("timestamp outside allowed window: in the future") return &http.ApiError{Message: "Invalid credentials", Status: 401} } From 02b6f8def7b7f81151e1e44b31028b7b0013f69a Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:02:35 +0800 Subject: [PATCH 11/26] tests --- pkg/middleware/token_middleware.go | 73 ++++++++++++++++++++---------- pkg/middleware/valid_timestamp.go | 20 ++++---- 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/pkg/middleware/token_middleware.go b/pkg/middleware/token_middleware.go index dccecc79..17324d80 100644 --- a/pkg/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -93,11 +93,15 @@ func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.Ap func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { return func(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { reqID := strings.TrimSpace(r.Header.Get(requestIDHeader)) - if reqID == "" { + logger := slog.With("request_id", reqID, "path", r.URL.Path, "method", r.Method) + + if reqID == "" || logger == nil { return t.getInvalidRequestError() } - logger := slog.With("request_id", reqID, "path", r.URL.Path, "method", r.Method) + if depErr := t.guardDependencies(logger); depErr != nil { + return depErr + } // Extract and validate required headers accountName, publicToken, signature, ts, nonce, hdrErr := t.validateAndGetHeaders(r, logger) @@ -127,7 +131,7 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { } // Update the request context - r = t.attachAuthContext(r, accountName, reqID) + r = t.attachContext(r, accountName, reqID) logger.Info("authentication successful") @@ -135,6 +139,33 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { } } +func (t TokenCheckMiddleware) guardDependencies(logger *slog.Logger) *http.ApiError { + missing := make([]string, 0, 4) + + if t.ApiKeys == nil { + missing = append(missing, "ApiKeys") + } + + if t.TokenHandler == nil { + missing = append(missing, "TokenHandler") + } + + if t.nonceCache == nil { + missing = append(missing, "nonceCache") + } + + if t.rateLimiter == nil { + missing = append(missing, "rateLimiter") + } + + if len(missing) > 0 { + logger.Error("token middleware missing dependencies", "missing", strings.Join(missing, ",")) + return t.getUnauthenticatedError() + } + + return nil +} + // validateAndGetHeaders extracts and validates required auth headers, logging and returning // appropriate ApiError on failure. func (t TokenCheckMiddleware) validateAndGetHeaders(r *baseHttp.Request, logger *slog.Logger) (accountName, publicToken, signature, ts, nonce string, apiErr *http.ApiError) { @@ -157,7 +188,6 @@ func (t TokenCheckMiddleware) validateAndGetHeaders(r *baseHttp.Request, logger return accountName, publicToken, signature, ts, nonce, nil } -// readBodyHash reads and restores the request body and returns its SHA256 hex. func (t TokenCheckMiddleware) readBodyHash(r *baseHttp.Request, logger *slog.Logger) (string, *http.ApiError) { if r.Body == nil { return portal.Sha256Hex(nil), nil @@ -175,31 +205,30 @@ func (t TokenCheckMiddleware) readBodyHash(r *baseHttp.Request, logger *slog.Log return portal.Sha256Hex(b), nil } -// attachAuthContext adds auth/account data and request id to the request context. -func (t TokenCheckMiddleware) attachAuthContext(r *baseHttp.Request, accountName, reqID string) *baseHttp.Request { +func (t TokenCheckMiddleware) attachContext(r *baseHttp.Request, accountName, reqID string) *baseHttp.Request { ctx := context.WithValue(r.Context(), authAccountNameKey, accountName) ctx = context.WithValue(r.Context(), requestIdKey, reqID) + return r.WithContext(ctx) } func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature, canonical, nonce, clientIP string) bool { limiterKey := clientIP + "|" + strings.ToLower(accountName) - if t.rateLimiter != nil && t.rateLimiter.TooMany(limiterKey) { + if t.rateLimiter.TooMany(limiterKey) { logger.Warn("too many authentication failures", "ip", clientIP) return true } var item *database.APIKey if item = t.ApiKeys.FindBy(accountName); item == nil { - if t.rateLimiter != nil { - t.rateLimiter.Fail(limiterKey) - } - + t.rateLimiter.Fail(limiterKey) logger.Warn("account not found") + return true } + // Fetch account to understand its keys token, err := t.TokenHandler.DecodeTokensFor( item.AccountName, item.SecretKey, @@ -207,21 +236,20 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ ) if err != nil { - if t.rateLimiter != nil { - t.rateLimiter.Fail(limiterKey) - } + t.rateLimiter.Fail(limiterKey) logger.Error("failed to decode account keys", "account", item.AccountName, "error", err) + return true } // Constant-time compare of provided public token vs stored one provided := []byte(strings.TrimSpace(publicToken)) expected := []byte(strings.TrimSpace(token.PublicKey)) + if subtle.ConstantTimeCompare(provided, expected) != 1 { - if t.rateLimiter != nil { - t.rateLimiter.Fail(limiterKey) - } + t.rateLimiter.Fail(limiterKey) logger.Warn("public token mismatch", "account", item.AccountName) + return true } @@ -229,11 +257,9 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ if t.nonceCache != nil { key := item.AccountName + "|" + nonce if t.nonceCache.Used(key) { - if t.rateLimiter != nil { - t.rateLimiter.Fail(limiterKey) - } - + t.rateLimiter.Fail(limiterKey) logger.Warn("replay detected: nonce already used", "account", item.AccountName) + return true } } @@ -241,10 +267,9 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ // Compute local signature over canonical request and compare in constant time localSignature := auth.CreateSignatureFrom(canonical, token.SecretKey) if subtle.ConstantTimeCompare([]byte(signature), []byte(localSignature)) != 1 { - if t.rateLimiter != nil { - t.rateLimiter.Fail(limiterKey) - } + t.rateLimiter.Fail(limiterKey) logger.Warn("signature mismatch", "account", item.AccountName) + return true } diff --git a/pkg/middleware/valid_timestamp.go b/pkg/middleware/valid_timestamp.go index c837f2cb..c18d62d9 100644 --- a/pkg/middleware/valid_timestamp.go +++ b/pkg/middleware/valid_timestamp.go @@ -31,18 +31,18 @@ func NewValidTimestamp(ts string, logger *slog.Logger, now func() time.Time) Val } func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http.ApiError { - if v.logger == nil { - return &http.ApiError{Message: "Invalid authentication headers internal formation", Status: 401} - } - if v.ts == "" { - v.logger.Warn("missing timestamp") + if v.logger != nil { + v.logger.Warn("missing timestamp") + } return &http.ApiError{Message: "Invalid authentication headers", Status: 401} } epoch, err := strconv.ParseInt(v.ts, 10, 64) if err != nil { - v.logger.Warn("invalid timestamp format") + if v.logger != nil { + v.logger.Warn("invalid timestamp format") + } return &http.ApiError{Message: "Invalid authentication headers", Status: 401} } @@ -61,12 +61,16 @@ func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http. } if epoch < minValue { - v.logger.Warn("timestamp outside allowed window: too old") + if v.logger != nil { + v.logger.Warn("timestamp outside allowed window: too old") + } return &http.ApiError{Message: "Invalid credentials", Status: 401} } if epoch > maxValue { - v.logger.Warn("timestamp outside allowed window: in the future") + if v.logger != nil { + v.logger.Warn("timestamp outside allowed window: in the future") + } return &http.ApiError{Message: "Invalid credentials", Status: 401} } From fd1f6a7f01aac8fa409a31fbdd1e20521403250c Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:12:11 +0800 Subject: [PATCH 12/26] format --- pkg/middleware/valid_timestamp.go | 29 +++++++++----------- pkg/middleware/valid_timestamp_test.go | 38 ++++++++++++++++---------- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/pkg/middleware/valid_timestamp.go b/pkg/middleware/valid_timestamp.go index c18d62d9..8bc578ef 100644 --- a/pkg/middleware/valid_timestamp.go +++ b/pkg/middleware/valid_timestamp.go @@ -2,6 +2,7 @@ package middleware import ( "log/slog" + baseHttp "net/http" "strconv" "time" @@ -31,19 +32,19 @@ func NewValidTimestamp(ts string, logger *slog.Logger, now func() time.Time) Val } func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http.ApiError { + if v.logger == nil { + return &http.ApiError{Message: "Invalid timestamp headers tracker", Status: baseHttp.StatusUnauthorized} + } + if v.ts == "" { - if v.logger != nil { - v.logger.Warn("missing timestamp") - } - return &http.ApiError{Message: "Invalid authentication headers", Status: 401} + v.logger.Warn("missing timestamp") + return &http.ApiError{Message: "Invalid authentication headers", Status: baseHttp.StatusUnauthorized} } epoch, err := strconv.ParseInt(v.ts, 10, 64) if err != nil { - if v.logger != nil { - v.logger.Warn("invalid timestamp format") - } - return &http.ApiError{Message: "Invalid authentication headers", Status: 401} + v.logger.Warn("invalid timestamp format") + return &http.ApiError{Message: "Invalid authentication headers", Status: baseHttp.StatusUnauthorized} } nowFn := v.now @@ -61,17 +62,13 @@ func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http. } if epoch < minValue { - if v.logger != nil { - v.logger.Warn("timestamp outside allowed window: too old") - } - return &http.ApiError{Message: "Invalid credentials", Status: 401} + v.logger.Warn("timestamp outside allowed window: too old") + return &http.ApiError{Message: "Invalid credentials", Status: baseHttp.StatusUnauthorized} } if epoch > maxValue { - if v.logger != nil { - v.logger.Warn("timestamp outside allowed window: in the future") - } - return &http.ApiError{Message: "Invalid credentials", Status: 401} + v.logger.Warn("timestamp outside allowed window: in the future") + return &http.ApiError{Message: "Invalid credentials", Status: baseHttp.StatusUnauthorized} } return nil diff --git a/pkg/middleware/valid_timestamp_test.go b/pkg/middleware/valid_timestamp_test.go index e7f65e08..24c57c37 100644 --- a/pkg/middleware/valid_timestamp_test.go +++ b/pkg/middleware/valid_timestamp_test.go @@ -1,7 +1,9 @@ package middleware import ( + "io" "log/slog" + baseHttp "net/http" "strconv" "testing" "time" @@ -26,17 +28,17 @@ func TestNewValidTimestampConstructor(t *testing.T) { } func TestValidate_EmptyTimestamp(t *testing.T) { - vt := NewValidTimestamp("", nil, fixedClock(time.Unix(1_700_000_000, 0))) + vt := NewValidTimestamp("", slogNoop(), fixedClock(time.Unix(1_700_000_000, 0))) err := vt.Validate(5*time.Minute, false) - if err == nil || err.Status != 401 || err.Message != "Invalid authentication headers" { + if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Invalid authentication headers" { t.Fatalf("expected invalid request error for empty timestamp, got %#v", err) } } func TestValidate_NonNumericTimestamp(t *testing.T) { - vt := NewValidTimestamp("abc", nil, fixedClock(time.Unix(1_700_000_000, 0))) + vt := NewValidTimestamp("abc", slogNoop(), fixedClock(time.Unix(1_700_000_000, 0))) err := vt.Validate(5*time.Minute, false) - if err == nil || err.Status != 401 || err.Message != "Invalid authentication headers" { + if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Invalid authentication headers" { t.Fatalf("expected invalid request error for non-numeric timestamp, got %#v", err) } } @@ -45,9 +47,9 @@ func TestValidate_TooOldTimestamp(t *testing.T) { base := time.Unix(1_700_000_000, 0) skew := 60 * time.Second oldTs := strconv.FormatInt(base.Add(-skew).Add(-1*time.Second).Unix(), 10) - vt := NewValidTimestamp(oldTs, nil, fixedClock(base)) + vt := NewValidTimestamp(oldTs, slogNoop(), fixedClock(base)) err := vt.Validate(skew, false) - if err == nil || err.Status != 401 || err.Message != "Invalid credentials" { + if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Invalid credentials" { t.Fatalf("expected unauthenticated for too old timestamp, got %#v", err) } } @@ -58,15 +60,15 @@ func TestValidate_FutureWithinSkew_Behavior(t *testing.T) { futureWithin := strconv.FormatInt(base.Add(30*time.Second).Unix(), 10) // Allowed when disallowFuture=false - vt := NewValidTimestamp(futureWithin, nil, fixedClock(base)) + vt := NewValidTimestamp(futureWithin, slogNoop(), fixedClock(base)) if err := vt.Validate(skew, false); err != nil { t.Fatalf("expected future timestamp within skew to be allowed when disallowFuture=false, got %#v", err) } // Rejected when disallowFuture=true - vt = NewValidTimestamp(futureWithin, nil, fixedClock(base)) + vt = NewValidTimestamp(futureWithin, slogNoop(), fixedClock(base)) err := vt.Validate(skew, true) - if err == nil || err.Status != 401 || err.Message != "Invalid credentials" { + if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Invalid credentials" { t.Fatalf("expected unauthenticated for future timestamp when disallowFuture=true, got %#v", err) } } @@ -79,27 +81,33 @@ func TestValidate_Boundaries(t *testing.T) { nowExact := strconv.FormatInt(base.Unix(), 10) // Lower boundary inclusive - vt := NewValidTimestamp(minExact, nil, fixedClock(base)) + vt := NewValidTimestamp(minExact, slogNoop(), fixedClock(base)) if err := vt.Validate(skew, false); err != nil { t.Fatalf("expected min boundary to pass, got %#v", err) } // Upper boundary inclusive when disallowFuture=false - vt = NewValidTimestamp(maxExact, nil, fixedClock(base)) + vt = NewValidTimestamp(maxExact, slogNoop(), fixedClock(base)) if err := vt.Validate(skew, false); err != nil { t.Fatalf("expected max boundary to pass when disallowFuture=false, got %#v", err) } // When disallowFuture=true, upper boundary becomes 'now' - vt = NewValidTimestamp(nowExact, nil, fixedClock(base)) + vt = NewValidTimestamp(nowExact, slogNoop(), fixedClock(base)) if err := vt.Validate(skew, true); err != nil { t.Fatalf("expected 'now' to pass when disallowFuture=true, got %#v", err) } } +func TestValidate_NilLogger(t *testing.T) { + vt := NewValidTimestamp("", nil, fixedClock(time.Unix(1_700_000_000, 0))) + err := vt.Validate(5*time.Minute, false) + if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Invalid timestamp headers tracker" { + t.Fatalf("expected unauthorized for nil logger, got %#v", err) + } +} + // slogNoop provides a minimal no-op logger compatible with *slog.Logger without requiring configuration in tests. func slogNoop() *slog.Logger { - // slog.New requires a Handler; use a Discard handler by sending to a disabled level. - // For simplicity and to avoid extra deps, return nil to keep logging optional in validation. - return nil + return slog.New(slog.NewTextHandler(io.Discard, nil)) } From 31845430f0a91eadccb794dd9f8072f4f318d976 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:26:15 +0800 Subject: [PATCH 13/26] more tests --- pkg/cache/ttl_cache_test.go | 99 +++---------------------- pkg/limiter/limiter_test.go | 88 ++++------------------ pkg/middleware/token_middleware.go | 2 - pkg/middleware/token_middleware_test.go | 80 +++++++++++++++++--- pkg/portal/support_test.go | 49 ++++++------ 5 files changed, 123 insertions(+), 195 deletions(-) diff --git a/pkg/cache/ttl_cache_test.go b/pkg/cache/ttl_cache_test.go index d89f4798..d0bdfb23 100644 --- a/pkg/cache/ttl_cache_test.go +++ b/pkg/cache/ttl_cache_test.go @@ -1,103 +1,22 @@ package cache import ( - "sync" "testing" "time" ) -func TestTTLCache_BasicMarkAndUsed(t *testing.T) { +func TestTTLCache_UsedAndMark(t *testing.T) { c := NewTTLCache() - - // Unknown key should be false - if c.Used("missing") { - t.Fatalf("expected missing key to be false") + key := "acct|nonce1" + if c.Used(key) { + t.Fatalf("key should not be used initially") } - - // Mark a key with a short TTL and verify it's true before expiry - c.Mark("k1", 50*time.Millisecond) - if !c.Used("k1") { - t.Fatalf("expected k1 to be true before expiry") + c.Mark(key, 50*time.Millisecond) + if !c.Used(key) { + t.Fatalf("key should be marked as used within TTL") } - - // Sleep past TTL and verify it expires time.Sleep(60 * time.Millisecond) - if c.Used("k1") { - t.Fatalf("expected k1 to be false after expiry") - } - - // After lazy pruning, subsequent calls should also be false - if c.Used("k1") { - t.Fatalf("expected k1 to remain false after pruning") - } -} - -func TestTTLCache_IndependentKeys(t *testing.T) { - c := NewTTLCache() - - c.Mark("short", 20*time.Millisecond) - c.Mark("long", 200*time.Millisecond) - - // Immediately both should be usable - if !c.Used("short") || !c.Used("long") { - t.Fatalf("expected both keys to be true initially") - } - - // Wait for the short to expire but not the long - time.Sleep(50 * time.Millisecond) - - if c.Used("short") { - t.Fatalf("expected short to be expired") - } - if !c.Used("long") { - t.Fatalf("expected long to still be valid") - } -} - -func TestTTLCache_RefreshTTL(t *testing.T) { - c := NewTTLCache() - - c.Mark("k", 20*time.Millisecond) - // Refresh before expiry - time.Sleep(10 * time.Millisecond) - c.Mark("k", 40*time.Millisecond) - - // Wait past the first TTL but before the refreshed TTL - time.Sleep(15 * time.Millisecond) // total 25ms from first mark - - if !c.Used("k") { - t.Fatalf("expected k to be valid due to refreshed TTL") - } - - // Now wait past the refreshed TTL - time.Sleep(30 * time.Millisecond) // total ~55ms from first mark - if c.Used("k") { - t.Fatalf("expected k to be expired after refreshed TTL") - } -} - -func TestTTLCache_Concurrency(t *testing.T) { - c := NewTTLCache() - - const goroutines = 20 - var wg sync.WaitGroup - wg.Add(goroutines) - - for i := 0; i < goroutines; i++ { - go func(i int) { - defer wg.Done() - key := "k" // all share same key to contend on lock - c.Mark(key, 200*time.Millisecond) - if !c.Used(key) { - t.Errorf("expected key to be usable; goroutine %d", i) - } - }(i) - } - - wg.Wait() - - // After all goroutines, key should still be valid - if !c.Used("k") { - t.Fatalf("expected key to still be valid after concurrent access") + if c.Used(key) { + t.Fatalf("key should expire after TTL") } } diff --git a/pkg/limiter/limiter_test.go b/pkg/limiter/limiter_test.go index 8488c6bd..f1f22f28 100644 --- a/pkg/limiter/limiter_test.go +++ b/pkg/limiter/limiter_test.go @@ -5,84 +5,28 @@ import ( "time" ) -// TestTooManyThreshold verifies that TooMany returns true only after -// maxFails failures have been recorded within the window. -func TestTooManyThreshold(t *testing.T) { - r := NewMemoryLimiter(1*time.Minute, 3) - key := "ip|account" +func TestMemoryLimiter_BasicFlow(t *testing.T) { + lim := NewMemoryLimiter(50*time.Millisecond, 3) + key := "ip|acct" - if r.TooMany(key) { - t.Fatalf("expected TooMany to be false before any failures") + if lim.TooMany(key) { + t.Fatalf("should not be limited initially") } - r.Fail(key) - if r.TooMany(key) { - t.Fatalf("expected TooMany to be false after 1 failure (< maxFails)") + lim.Fail(key) + lim.Fail(key) + if lim.TooMany(key) { + t.Fatalf("should not be limited before reaching threshold") } - r.Fail(key) - if r.TooMany(key) { - t.Fatalf("expected TooMany to be false after 2 failures (< maxFails)") + lim.Fail(key) + if !lim.TooMany(key) { + t.Fatalf("should be limited after reaching threshold") } - r.Fail(key) - if !r.TooMany(key) { - t.Fatalf("expected TooMany to be true after reaching maxFails") - } -} - -// TestWindowPruning verifies that failures older than the window are pruned -// and do not contribute to the TooMany decision. -func TestWindowPruning(t *testing.T) { - // Use a short window to make the test fast and deterministic - window := 50 * time.Millisecond - r := NewMemoryLimiter(window, 2) - key := "client|user" - - // Record one failure and wait for it to expire - r.Fail(key) - time.Sleep(window + 10*time.Millisecond) - - // Calling TooMany triggers pruning of the older entry - if r.TooMany(key) { - t.Fatalf("expected TooMany to be false after window expiration and pruning") - } - - // Now add two quick failures within the window - r.Fail(key) - r.Fail(key) - - if !r.TooMany(key) { - t.Fatalf("expected TooMany to be true after 2 failures within window (maxFails=2)") - } -} - -// TestIndependentKeys checks that limiter maintains separate counters per key. -func TestIndependentKeys(t *testing.T) { - r := NewMemoryLimiter(1*time.Second, 2) - keyA := "ipA|acct" - keyB := "ipB|acct" - - r.Fail(keyA) - if r.TooMany(keyA) { - t.Fatalf("TooMany should be false for keyA with 1 failure") - } - - // keyB should be unaffected by keyA's failures - if r.TooMany(keyB) { // triggers prune/check but no failures recorded for keyB - t.Fatalf("TooMany should be false for keyB with 0 failures") - } - - // Push keyB over the threshold independently - r.Fail(keyB) - r.Fail(keyB) - - if !r.TooMany(keyB) { - t.Fatalf("TooMany should be true for keyB after reaching maxFails") - } - - // keyA still below threshold - if r.TooMany(keyA) { - t.Fatalf("TooMany should still be false for keyA (only 1 failure)") + // Wait for window to slide and prune + time.Sleep(60 * time.Millisecond) + if lim.TooMany(key) { + t.Fatalf("should not be limited after window passes") } } diff --git a/pkg/middleware/token_middleware.go b/pkg/middleware/token_middleware.go index 17324d80..244c3b78 100644 --- a/pkg/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -166,8 +166,6 @@ func (t TokenCheckMiddleware) guardDependencies(logger *slog.Logger) *http.ApiEr return nil } -// validateAndGetHeaders extracts and validates required auth headers, logging and returning -// appropriate ApiError on failure. func (t TokenCheckMiddleware) validateAndGetHeaders(r *baseHttp.Request, logger *slog.Logger) (accountName, publicToken, signature, ts, nonce string, apiErr *http.ApiError) { accountName = strings.TrimSpace(r.Header.Get(usernameHeader)) publicToken = strings.TrimSpace(r.Header.Get(tokenHeader)) diff --git a/pkg/middleware/token_middleware_test.go b/pkg/middleware/token_middleware_test.go index 8aa28587..83d83f2e 100644 --- a/pkg/middleware/token_middleware_test.go +++ b/pkg/middleware/token_middleware_test.go @@ -1,6 +1,8 @@ package middleware import ( + "bytes" + "io" "net/http" "net/http/httptest" "testing" @@ -13,34 +15,94 @@ func TestTokenMiddlewareErrors(t *testing.T) { e := tm.getInvalidRequestError() - if e.Status != 401 || e.Message == "" { + if e.Status != http.StatusUnauthorized || e.Message == "" { t.Fatalf("invalid request error") } e = tm.getInvalidTokenFormatError() - if e.Status != 401 { + if e.Status != http.StatusUnauthorized { t.Fatalf("invalid token error") } e = tm.getUnauthenticatedError() - if e.Status != 401 { + if e.Status != http.StatusUnauthorized { t.Fatalf("unauthenticated error") } } +func TestTokenMiddlewareHandle_RequiresRequestID(t *testing.T) { + tm := MakeTokenMiddleware(nil, nil) + + handler := tm.Handle(func(w http.ResponseWriter, r *http.Request) *pkgHttp.ApiError { return nil }) + + rec := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/", nil) + // No X-Request-ID present + if err := handler(rec, req); err == nil || err.Status != http.StatusUnauthorized { + t.Fatalf("expected 401 when X-Request-ID is missing, got %#v", err) + } +} + func TestTokenMiddlewareHandleInvalid(t *testing.T) { tm := MakeTokenMiddleware(nil, nil) - handler := tm.Handle(func(w http.ResponseWriter, r *http.Request) *pkgHttp.ApiError { - return nil - }) + handler := tm.Handle(func(w http.ResponseWriter, r *http.Request) *pkgHttp.ApiError { return nil }) rec := httptest.NewRecorder() - err := handler(rec, httptest.NewRequest("GET", "/", nil)) + req := httptest.NewRequest("GET", "/", nil) + req.Header.Set("X-Request-ID", "req-1") + // Missing other auth headers triggers invalid request + if err := handler(rec, req); err == nil || err.Status != http.StatusUnauthorized { + t.Fatalf("expected unauthorized for missing auth headers, got %#v", err) + } +} + +func TestValidateAndGetHeaders_MissingAndInvalidFormat(t *testing.T) { + tm := MakeTokenMiddleware(nil, nil) + logger := slogNoop() + req := httptest.NewRequest("GET", "/", nil) + // All empty + if _, _, _, _, _, apiErr := tm.validateAndGetHeaders(req, logger); apiErr == nil || apiErr.Status != http.StatusUnauthorized { + t.Fatalf("expected error for missing headers") + } + + // Set minimal headers but invalid token format (not pk_/sk_ prefix or too short) + req.Header.Set("X-API-Username", "alice") + req.Header.Set("X-API-Key", "badtoken") + req.Header.Set("X-API-Signature", "sig") + req.Header.Set("X-API-Timestamp", "1700000000") + req.Header.Set("X-API-Nonce", "n1") + if _, _, _, _, _, apiErr := tm.validateAndGetHeaders(req, logger); apiErr == nil || apiErr.Status != http.StatusUnauthorized { + t.Fatalf("expected error for invalid token format") + } +} - if err == nil || err.Status != 401 { - t.Fatalf("expected unauthorized") +func TestReadBodyHash_RestoresBody(t *testing.T) { + tm := MakeTokenMiddleware(nil, nil) + logger := slogNoop() + body := "{\"a\":1}" + req := httptest.NewRequest("POST", "/x", bytes.NewBufferString(body)) + hash, apiErr := tm.readBodyHash(req, logger) + if apiErr != nil || hash == "" { + t.Fatalf("expected body hash, got err=%v hash=%q", apiErr, hash) + } + // Now the body should be readable again for downstream + b, _ := io.ReadAll(req.Body) + if string(b) != body { + t.Fatalf("expected body to be restored, got %q", string(b)) + } +} + +func TestAttachContext(t *testing.T) { + tm := MakeTokenMiddleware(nil, nil) + req := httptest.NewRequest("GET", "/", nil) + r := tm.attachContext(req, "Alice", "RID-123") + if r == req { + t.Fatalf("expected a new request with updated context") + } + if r.Context() == nil { + t.Fatalf("expected non-nil context") } } diff --git a/pkg/portal/support_test.go b/pkg/portal/support_test.go index de445d4b..5ab01a32 100644 --- a/pkg/portal/support_test.go +++ b/pkg/portal/support_test.go @@ -1,35 +1,40 @@ package portal import ( - "errors" + "net/url" "testing" ) -type fakeCloser struct { - closed bool - err error -} - -func (f *fakeCloser) Close() error { - f.closed = true +func TestSortedQuery(t *testing.T) { + u, _ := url.Parse("https://x.test/api?b=2&a=1&a=0") + got := SortedQuery(u) + expected := "a=0&a=1&b=2" + if got != expected { + t.Fatalf("expected sorted query %q, got %q", expected, got) + } - return f.err + // Empty / nil cases + if SortedQuery(nil) != "" { + t.Fatalf("expected empty for nil URL") + } + u2, _ := url.Parse("https://x.test/api") + if SortedQuery(u2) != "" { + t.Fatalf("expected empty for no query params") + } } -func TestCloseWithLog(t *testing.T) { - c := &fakeCloser{} - - CloseWithLog(c) - - if !c.closed { - t.Fatalf("close not called") +func TestBuildCanonical(t *testing.T) { + u, _ := url.Parse("https://x.test/api/v1/resource?z=9&a=1&a=0") + bodyHash := "abc123" + got := BuildCanonical("post", u, "Alice", "pk_123", "1700000000", "nonce-1", bodyHash) + expected := "POST\n/api/v1/resource\na=0&a=1&z=9\nAlice\npk_123\n1700000000\nnonce-1\nabc123" + if got != expected { + t.Fatalf("unexpected canonical string:\nexpected: %q\n got: %q", expected, got) } - c2 := &fakeCloser{err: errors.New("fail")} - - CloseWithLog(c2) - - if !c2.closed { - t.Fatalf("close not called with error") + // Default path handling when URL is nil or empty + got = BuildCanonical("GET", nil, "u", "p", "1", "n", "h") + if got != "GET\n/\n\nu\np\n1\nn\nh" { + t.Fatalf("unexpected canonical for nil URL: %q", got) } } From 6c1adf1bacf5a2b7f187f20bd701d712b7473297 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:39:31 +0800 Subject: [PATCH 14/26] token integration tests --- pkg/middleware/token_middleware_test.go | 185 ++++++++++++++++++++++++ 1 file changed, 185 insertions(+) diff --git a/pkg/middleware/token_middleware_test.go b/pkg/middleware/token_middleware_test.go index 83d83f2e..ed5a6097 100644 --- a/pkg/middleware/token_middleware_test.go +++ b/pkg/middleware/token_middleware_test.go @@ -2,12 +2,25 @@ package middleware import ( "bytes" + "context" + "crypto/rand" "io" "net/http" "net/http/httptest" + "os/exec" + "strconv" "testing" + "time" + "github.com/testcontainers/testcontainers-go" + "github.com/testcontainers/testcontainers-go/modules/postgres" + + "github.com/oullin/database" + "github.com/oullin/database/repository" + "github.com/oullin/metal/env" + "github.com/oullin/pkg/auth" pkgHttp "github.com/oullin/pkg/http" + "github.com/oullin/pkg/portal" ) func TestTokenMiddlewareErrors(t *testing.T) { @@ -106,3 +119,175 @@ func TestAttachContext(t *testing.T) { t.Fatalf("expected non-nil context") } } + +// --- Integration test helpers (copied/adjusted from repository_test.go) --- + +// setupDB starts a Postgres testcontainer and returns a live DB connection. +func setupDB(t *testing.T) *database.Connection { + if _, err := exec.LookPath("docker"); err != nil { + t.Skip("docker not installed") + } + if err := exec.Command("docker", "ps").Run(); err != nil { + t.Skip("docker not running") + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + pg, err := postgres.RunContainer(ctx, + testcontainers.WithImage("postgres:16-alpine"), + postgres.WithDatabase("testdb"), + postgres.WithUsername("test"), + postgres.WithPassword("secret"), + postgres.BasicWaitStrategies(), + ) + if err != nil { + t.Fatalf("container run err: %v", err) + } + t.Cleanup(func() { _ = pg.Terminate(context.Background()) }) + + host, err := pg.Host(ctx) + if err != nil { + t.Fatalf("host err: %v", err) + } + port, err := pg.MappedPort(ctx, "5432/tcp") + if err != nil { + t.Fatalf("port err: %v", err) + } + + e := &env.Environment{ + DB: env.DBEnvironment{ + UserName: "test", + UserPassword: "secret", + DatabaseName: "testdb", + Port: port.Int(), + Host: host, + DriverName: database.DriverName, + SSLMode: "disable", + TimeZone: "UTC", + }, + } + + conn, err := database.MakeConnection(e) + if err != nil { + t.Fatalf("make connection: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + if err := conn.Sql().AutoMigrate(&database.APIKey{}); err != nil { + t.Fatalf("migrate err: %v", err) + } + + return conn +} + +// generate32 returns a 32-byte key for TokenHandler. +func generate32(t *testing.T) []byte { + t.Helper() + buf := make([]byte, 32) + if _, err := rand.Read(buf); err != nil { + return []byte("0123456789abcdef0123456789abcdef") + } + return buf +} + +// makeSignedRequest builds a request with required headers and a valid HMAC signature over the canonical string. +func makeSignedRequest(t *testing.T, method, rawURL, body, account, public, secret string, ts time.Time, nonce, reqID string) *http.Request { + t.Helper() + var bodyBuf *bytes.Buffer + if body != "" { + bodyBuf = bytes.NewBufferString(body) + } else { + bodyBuf = bytes.NewBuffer(nil) + } + req := httptest.NewRequest(method, rawURL, bodyBuf) + req.Header.Set("X-Request-ID", reqID) + req.Header.Set("X-API-Username", account) + req.Header.Set("X-API-Key", public) + req.Header.Set("X-API-Timestamp", strconv.FormatInt(ts.Unix(), 10)) + req.Header.Set("X-API-Nonce", nonce) + + bodyHash := portal.Sha256Hex([]byte(body)) + canonical := portal.BuildCanonical(method, req.URL, account, public, req.Header.Get("X-API-Timestamp"), nonce, bodyHash) + sig := auth.CreateSignatureFrom(canonical, secret) + req.Header.Set("X-API-Signature", sig) + return req +} + +func TestTokenMiddleware_DB_Integration(t *testing.T) { + conn := setupDB(t) + + // Prepare TokenHandler and seed an account with encrypted keys + th, err := auth.MakeTokensHandler(generate32(t)) + if err != nil { + t.Fatalf("MakeTokensHandler: %v", err) + } + seed, err := th.SetupNewAccount("acme-user") + if err != nil { + t.Fatalf("SetupNewAccount: %v", err) + } + + repo := &repository.ApiKeys{DB: conn} + if _, err := repo.Create(database.APIKeyAttr{ + AccountName: seed.AccountName, + PublicKey: seed.EncryptedPublicKey, + SecretKey: seed.EncryptedSecretKey, + }); err != nil { + t.Fatalf("repo.Create: %v", err) + } + + // Build middleware + tm := MakeTokenMiddleware(th, repo) + // make it tolerant and fast for test + tm.clockSkew = 2 * time.Minute + tm.nonceTTL = 1 * time.Minute + + nextCalled := false + next := func(w http.ResponseWriter, r *http.Request) *pkgHttp.ApiError { + nextCalled = true + return nil + } + handler := tm.Handle(next) + + // Positive case + now := time.Now() + req := makeSignedRequest(t, + http.MethodPost, + "https://api.test.local/v1/posts?z=9&a=1", + "{\"title\":\"ok\"}", + seed.AccountName, + seed.PublicKey, + seed.SecretKey, + now, + "nonce-1", + "req-001", + ) + rec := httptest.NewRecorder() + if err := handler(rec, req); err != nil { + t.Fatalf("expected success, got error: %#v", err) + } + if !nextCalled { + t.Fatalf("expected next to be called on success") + } + + // Negative case: unknown account + nextCalled = false + reqUnknown := makeSignedRequest(t, + http.MethodGet, + "https://api.test.local/v1/ping", + "", + "no-such-user", + seed.PublicKey, + seed.SecretKey, + now, + "nonce-2", + "req-002", + ) + rec = httptest.NewRecorder() + if err := handler(rec, reqUnknown); err == nil || err.Status != http.StatusUnauthorized { + t.Fatalf("expected 401 for unknown account, got %#v", err) + } + if nextCalled { + t.Fatalf("next should not be called on auth failure") + } +} From 1b1c938020adc48410b9a40cfab5411a244049b1 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:49:14 +0800 Subject: [PATCH 15/26] wip --- .github/workflows/tests.yml | 3 +- pkg/middleware/token_middleware_test.go | 56 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 964853dc..7eb67d3c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,8 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - # , '1.24.5' - go-version: ['1.24.6'] + go-version: ['1.24.6', '1.24.5'] steps: - uses: actions/setup-go@v5 diff --git a/pkg/middleware/token_middleware_test.go b/pkg/middleware/token_middleware_test.go index ed5a6097..6edb5bea 100644 --- a/pkg/middleware/token_middleware_test.go +++ b/pkg/middleware/token_middleware_test.go @@ -291,3 +291,59 @@ func TestTokenMiddleware_DB_Integration(t *testing.T) { t.Fatalf("next should not be called on auth failure") } } + +// New happy path only test +func TestTokenMiddleware_DB_Integration_HappyPath(t *testing.T) { + conn := setupDB(t) + + // Prepare TokenHandler and seed an account with encrypted keys + th, err := auth.MakeTokensHandler(generate32(t)) + if err != nil { + t.Fatalf("MakeTokensHandler: %v", err) + } + seed, err := th.SetupNewAccount("acme-user-happy") + if err != nil { + t.Fatalf("SetupNewAccount: %v", err) + } + + repo := &repository.ApiKeys{DB: conn} + if _, err := repo.Create(database.APIKeyAttr{ + AccountName: seed.AccountName, + PublicKey: seed.EncryptedPublicKey, + SecretKey: seed.EncryptedSecretKey, + }); err != nil { + t.Fatalf("repo.Create: %v", err) + } + + // Build middleware + tm := MakeTokenMiddleware(th, repo) + // Relax window for test + tm.clockSkew = 2 * time.Minute + tm.nonceTTL = 1 * time.Minute + + nextCalled := false + next := func(w http.ResponseWriter, r *http.Request) *pkgHttp.ApiError { + nextCalled = true + return nil + } + handler := tm.Handle(next) + + req := makeSignedRequest(t, + http.MethodPost, + "https://api.test.local/v1/resource?b=2&a=1", + "{\"x\":123}", + seed.AccountName, + seed.PublicKey, + seed.SecretKey, + time.Now(), + "n-happy-1", + "rid-happy-1", + ) + rec := httptest.NewRecorder() + if err := handler(rec, req); err != nil { + t.Fatalf("happy path failed: %#v", err) + } + if !nextCalled { + t.Fatalf("next was not called on happy path") + } +} From 6846502f28faa37b57b0ed2b0743afa9ff72b63b Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:54:42 +0800 Subject: [PATCH 16/26] wip --- .github/workflows/gofmt.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/gofmt.yml b/.github/workflows/gofmt.yml index 9c9b9b7d..b8c0846b 100644 --- a/.github/workflows/gofmt.yml +++ b/.github/workflows/gofmt.yml @@ -13,7 +13,7 @@ permissions: jobs: gofmt: - if: false + if: github.event.action == 'labeled' && github.event.label.name == 'style' strategy: matrix: go-version: [1.24.5] From d59250f7830dc049a34e6b5929a045d73cab7323 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:57:16 +0800 Subject: [PATCH 17/26] wip --- .github/workflows/gofmt.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/gofmt.yml b/.github/workflows/gofmt.yml index b8c0846b..31f47272 100644 --- a/.github/workflows/gofmt.yml +++ b/.github/workflows/gofmt.yml @@ -13,7 +13,7 @@ permissions: jobs: gofmt: - if: github.event.action == 'labeled' && github.event.label.name == 'style' + if: github.event.action == 'labeled' && contains(github.event.pull_request.labels.*.name, 'style') strategy: matrix: go-version: [1.24.5] From 2e61b426ae634acf9a2592c3572e3ef22f152c0c Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 16:58:09 +0800 Subject: [PATCH 18/26] wip --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7eb67d3c..d3b60ba7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -10,7 +10,7 @@ jobs: test: if: (github.event_name == 'push') || (github.event_name == 'pull_request' && (github.event.pull_request.draft == false || - (github.event.action == 'labeled' && github.event.label.name == 'testing'))) + (github.event.action == 'labeled' && contains(github.event.pull_request.labels.*.name, 'testing')))) runs-on: ubuntu-latest strategy: matrix: From 942b14bd0a296a3eef9d6533f1b446ae8b22f374 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 17:27:10 +0800 Subject: [PATCH 19/26] add progress --- .../token-local.postman_collection.json | 97 ++++++ .../token-prod.postman_collection.json | 97 ++++++ .../token_analysis_v1.md} | 46 ++- docs/middleware/token_analysis_v2.md | 189 ++++++++++++ docs/middleware/token_examples.md | 281 ++++++++++++++++++ 5 files changed, 704 insertions(+), 6 deletions(-) create mode 100644 docs/middleware/postman/token-local.postman_collection.json create mode 100644 docs/middleware/postman/token-prod.postman_collection.json rename docs/{token_middleware_analysis.md => middleware/token_analysis_v1.md} (76%) create mode 100644 docs/middleware/token_analysis_v2.md create mode 100644 docs/middleware/token_examples.md diff --git a/docs/middleware/postman/token-local.postman_collection.json b/docs/middleware/postman/token-local.postman_collection.json new file mode 100644 index 00000000..a5807d1c --- /dev/null +++ b/docs/middleware/postman/token-local.postman_collection.json @@ -0,0 +1,97 @@ +{ + "info": { + "name": "Oullin API — Token Auth (Local)", + "description": "Postman collection for calling protected endpoints locally via Caddy (http://localhost:8080). It uses a collection-level pre-request script to compute the required X-API-* headers and signature.", + "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json", + "_postman_id": "5d8d0a1a-7a1e-4f6e-b2f2-9d2a0a8f0c11" + }, + "item": [ + { + "name": "List posts (POST /posts)", + "request": { + "method": "POST", + "header": [ + { "key": "Content-Type", "value": "application/json" } + ], + "url": { + "raw": "{{baseUrl}}/posts", + "host": ["{{baseUrl}}"], + "path": ["posts"] + }, + "body": { + "mode": "raw", + "raw": "{}" + }, + "description": "List or filter posts. Requires signed headers generated by the collection pre-request script." + }, + "response": [] + }, + { + "name": "Show post (GET /posts/hello)", + "request": { + "method": "GET", + "header": [], + "url": { + "raw": "{{baseUrl}}/posts/hello", + "host": ["{{baseUrl}}"], + "path": ["posts", "hello"] + }, + "description": "Fetch a single post by slug (example: hello)." + }, + "response": [] + } + ], + "event": [ + { + "listen": "prerequest", + "script": { + "type": "text/javascript", + "exec": [ + "(function() {", + " // CryptoJS available in Postman sandbox", + " const crypto = require('crypto-js');", + " function sha256Hex(str) { return crypto.SHA256(str || '').toString(crypto.enc.Hex); }", + " function sortedQuery(u) {", + " const url = new URL(u);", + " const keys = Array.from(url.searchParams.keys());", + " keys.sort();", + " const parts = [];", + " for (const k of keys) {", + " const vs = url.searchParams.getAll(k).sort();", + " for (const v of vs) parts.push(encodeURIComponent(k) + '=' + encodeURIComponent(v));", + " }", + " return parts.join('&');", + " }", + " const method = pm.request.method.toUpperCase();", + " const urlStr = pm.environment.get('baseUrl') + pm.request.url.getPathWithQuery();", + " const urlObj = new URL(urlStr);", + " const path = urlObj.pathname;", + " const query = sortedQuery(urlStr);", + " const username = pm.environment.get('username');", + " const publicKey = pm.environment.get('publicKey');", + " const secretKey = pm.environment.get('secretKey');", + " const timestamp = Math.floor(Date.now() / 1000).toString();", + " const nonce = crypto.lib.WordArray.random(16).toString();", + " const body = (method === 'GET' || method === 'DELETE') ? '' : (pm.request.body && pm.request.body.raw || '');", + " const bodyHash = sha256Hex(body);", + " const canonical = [method, path, query, username, publicKey, timestamp, nonce, bodyHash].join('\n');", + " const signature = crypto.HmacSHA256(canonical, secretKey).toString();", + " pm.request.headers.upsert({ key: 'X-Request-ID', value: pm.environment.get('requestId') || nonce });", + " pm.request.headers.upsert({ key: 'X-API-Username', value: username });", + " pm.request.headers.upsert({ key: 'X-API-Key', value: publicKey });", + " pm.request.headers.upsert({ key: 'X-API-Timestamp', value: timestamp });", + " pm.request.headers.upsert({ key: 'X-API-Nonce', value: nonce });", + " pm.request.headers.upsert({ key: 'X-API-Signature', value: signature });", + "})();" + ] + } + } + ], + "variable": [ + { "key": "baseUrl", "value": "http://localhost:8080", "type": "string" }, + { "key": "username", "value": "", "type": "string" }, + { "key": "publicKey", "value": "", "type": "string" }, + { "key": "secretKey", "value": "", "type": "string" }, + { "key": "requestId", "value": "", "type": "string" } + ] +} diff --git a/docs/middleware/postman/token-prod.postman_collection.json b/docs/middleware/postman/token-prod.postman_collection.json new file mode 100644 index 00000000..8af6292f --- /dev/null +++ b/docs/middleware/postman/token-prod.postman_collection.json @@ -0,0 +1,97 @@ +{ + "info": { + "name": "Oullin API — Token Auth (Production)", + "description": "Postman collection for calling protected endpoints in production via Caddy (https://oullin.io/api). It uses a collection-level pre-request script to compute the required X-API-* headers and signature.", + "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json", + "_postman_id": "9a1b6e3a-2c24-4e24-8a8c-2f1ca5a7cb21" + }, + "item": [ + { + "name": "List posts (POST /api/posts)", + "request": { + "method": "POST", + "header": [ + { "key": "Content-Type", "value": "application/json" } + ], + "url": { + "raw": "{{baseUrl}}/posts", + "host": ["{{baseUrl}}"], + "path": ["posts"] + }, + "body": { + "mode": "raw", + "raw": "{}" + }, + "description": "List or filter posts. Requires signed headers generated by the collection pre-request script." + }, + "response": [] + }, + { + "name": "Show post (GET /api/posts/hello)", + "request": { + "method": "GET", + "header": [], + "url": { + "raw": "{{baseUrl}}/posts/hello", + "host": ["{{baseUrl}}"], + "path": ["posts", "hello"] + }, + "description": "Fetch a single post by slug (example: hello)." + }, + "response": [] + } + ], + "event": [ + { + "listen": "prerequest", + "script": { + "type": "text/javascript", + "exec": [ + "(function() {", + " // CryptoJS available in Postman sandbox", + " const crypto = require('crypto-js');", + " function sha256Hex(str) { return crypto.SHA256(str || '').toString(crypto.enc.Hex); }", + " function sortedQuery(u) {", + " const url = new URL(u);", + " const keys = Array.from(url.searchParams.keys());", + " keys.sort();", + " const parts = [];", + " for (const k of keys) {", + " const vs = url.searchParams.getAll(k).sort();", + " for (const v of vs) parts.push(encodeURIComponent(k) + '=' + encodeURIComponent(v));", + " }", + " return parts.join('&');", + " }", + " const method = pm.request.method.toUpperCase();", + " const urlStr = pm.environment.get('baseUrl') + pm.request.url.getPathWithQuery();", + " const urlObj = new URL(urlStr);", + " const path = urlObj.pathname;", + " const query = sortedQuery(urlStr);", + " const username = pm.environment.get('username');", + " const publicKey = pm.environment.get('publicKey');", + " const secretKey = pm.environment.get('secretKey');", + " const timestamp = Math.floor(Date.now() / 1000).toString();", + " const nonce = crypto.lib.WordArray.random(16).toString();", + " const body = (method === 'GET' || method === 'DELETE') ? '' : (pm.request.body && pm.request.body.raw || '');", + " const bodyHash = sha256Hex(body);", + " const canonical = [method, path, query, username, publicKey, timestamp, nonce, bodyHash].join('\n');", + " const signature = crypto.HmacSHA256(canonical, secretKey).toString();", + " pm.request.headers.upsert({ key: 'X-Request-ID', value: pm.environment.get('requestId') || nonce });", + " pm.request.headers.upsert({ key: 'X-API-Username', value: username });", + " pm.request.headers.upsert({ key: 'X-API-Key', value: publicKey });", + " pm.request.headers.upsert({ key: 'X-API-Timestamp', value: timestamp });", + " pm.request.headers.upsert({ key: 'X-API-Nonce', value: nonce });", + " pm.request.headers.upsert({ key: 'X-API-Signature', value: signature });", + "})();" + ] + } + } + ], + "variable": [ + { "key": "baseUrl", "value": "https://oullin.io/api", "type": "string" }, + { "key": "username", "value": "", "type": "string" }, + { "key": "publicKey", "value": "", "type": "string" }, + { "key": "secretKey", "value": "", "type": "string" }, + { "key": "requestId", "value": "", "type": "string" } + ] +} diff --git a/docs/token_middleware_analysis.md b/docs/middleware/token_analysis_v1.md similarity index 76% rename from docs/token_middleware_analysis.md rename to docs/middleware/token_analysis_v1.md index daa68a9f..0412f566 100644 --- a/docs/token_middleware_analysis.md +++ b/docs/middleware/token_analysis_v1.md @@ -258,9 +258,43 @@ Validation rules: --- -## 10) References - -- RFC 6749 (OAuth 2.0), OAuth 2.1 draft best practices. -- RFC 7636 (PKCE). -- NIST SP 800-63B (Digital Identity Guidelines). -- Timing attack mitigations (use constant-time comparisons). +## 10) Deployment and runtime context (docker-compose, Caddy, Makefile) + +Date: 2025-08-08 16:52 local + +- Containers and networks (docker-compose.yml): + - Services: + - api: Go API built from docker/dockerfile-api; exposes ENV_HTTP_PORT (default 8080) to the caddy_net and oullin_net networks. DB host is api-db via Docker DNS. Secrets are injected using Docker secrets (pg_username, pg_password, pg_dbname). + - api-db: Postgres 17.3-alpine. Port bound to 127.0.0.1:${ENV_DB_PORT:-5432} (not exposed publicly). Uses Docker secrets for credentials. Includes healthcheck and SSL files mounted read-only. + - api-db-migrate: Runs migrations from database/infra/migrations via a wrapper script. + - api-runner: Convenience container to run Go commands (e.g., seeders) with the code mounted at /app, sharing the network with api-db. + - caddy_local (profile local): Reverse proxy for local development. Host ports 8080->80 and 8443->443. Caddyfile: caddy/Caddyfile.local. + - caddy_prod (profile prod): Public reverse proxy/terminates TLS via Let’s Encrypt. Host ports 80/443 exposed. Caddyfile: caddy/Caddyfile.prod. + - Networks: + - caddy_net: Fronting proxy <-> API network. + - oullin_net: Internal network for API <-> DB and runner. + - Volumes: + - caddy_data, caddy_config, oullin_db_data for persistence; go_mod_cache for cached modules in api-runner. + +- Caddy local proxy (caddy/Caddyfile.local): + - auto_https off (HTTP only locally). + - Listens on :80 in the container (published as http://localhost:8080 on the host). + - CORS: Allows Origin http://localhost:5173 and headers X-API-Username, X-API-Key, X-API-Signature; handles OPTIONS preflight. + - reverse_proxy api:8080 — all paths are forwarded to API without an "/api" prefix. + +- Caddy production proxy (caddy/Caddyfile.prod): + - Site: oullin.io (automatic HTTPS). + - API is routed under /api/* and proxied to api:8080. That means production API path = https://oullin.io/api/... while local is http://localhost:8080/.... + - CORS configured for https://oullin.io within the /api handler. For preflight, echoes Access-Control-Allow-Origin back. + - Forwards key auth headers upstream (header_up Host, X-API-Username, X-API-Key, X-API-Signature). X-Forwarded-For is also set by Caddy; the middleware’s ParseClientIP will prefer the first X-Forwarded-For entry. + +- Makefile (metal/makefile): + - build-local: docker compose --profile local up --build -d (starts api, api-db, caddy_local). After this, the API is reachable at http://localhost:8080. + - db:up, db:seed, db:migrate: Manage DB lifecycle and schema. + - validate-caddy: Format/validate local and prod Caddyfiles. + - env:init, env:check: Manage .env from .env.example. + +- API routes (metal/kernel/router.go): + - POST /posts (list/filter posts) and GET /posts/{slug} (show post) are protected by TokenCheckMiddleware. + - Other public static routes include /profile, /experience, /projects, /social, /talks, /education, /recommendations. + - In production behind Caddy, the protected routes are under /api (e.g., POST https://oullin.io/api/posts). Locally through caddy_local they are at http://localhost:8080/posts. diff --git a/docs/middleware/token_analysis_v2.md b/docs/middleware/token_analysis_v2.md new file mode 100644 index 00000000..51ceb836 --- /dev/null +++ b/docs/middleware/token_analysis_v2.md @@ -0,0 +1,189 @@ +# Token middleware analysis (v2) + +Date: 2025-08-08 +Scope: pkg/middleware/token_middleware.go and related helpers (valid_timestamp.go, pkg/portal/support.go) + +--- + +## 1) What the token middleware does today + +The TokenCheckMiddleware authenticates signed API requests using per‑account API keys. It enforces a signed-request protocol that binds each request to: +- HTTP method and path +- Sorted query string +- Account username and public token +- Unix timestamp and nonce +- Body content hash + +Main steps: +1) Required headers + - X-Request-ID (required for logging/tracing) + - X-API-Username (account identifier) + - X-API-Key (public token) + - X-API-Signature (HMAC over canonical request) + - X-API-Timestamp (Unix seconds) + - X-API-Nonce (unique per request) + +2) Dependency guard + - Ensures ApiKeys repo, TokenHandler, nonce cache, and rate limiter exist. If missing, fails with 401. + +3) Header validation + - Rejects if any required header is missing (401: "Invalid authentication headers"). + - Validates public token format via auth.ValidateTokenFormat (e.g., prefix/length conventions); on failure returns generic 401 ("Invalid credentials"). + +4) Timestamp validation + - Uses ValidTimestamp.Validate with a configurable skew (default 5 minutes) and an optional disallowFuture flag (default false). + - Rejects if timestamp older than now - skew or (if disallowFuture) newer than now; otherwise allows within [now - skew, now + skew]. + +5) Body hashing + - Reads the request body to compute SHA-256 hex via portal.Sha256Hex, then restores the body so downstream handlers can read it again. + +6) Canonical request construction + - portal.BuildCanonical(method, url, username, public, ts, nonce, bodyHash) + - Includes: uppercased method, escaped path (default "/"), sorted & url-escaped query string, username, public token, timestamp, nonce, body hash, joined by newlines. + +7) Client IP parsing + - portal.ParseClientIP prefers X-Forwarded-For first IP, otherwise uses RemoteAddr host. + +8) Rejection checks (shallReject) + - Failure-based rate limiting per scope clientIP|account (MemoryLimiter with 1-minute window and max 10 fails): + - If TooMany, reject early. + - Account lookup via repository.ApiKeys.FindBy(username); if not found, mark failure and reject. + - Key decoding via TokenHandler.DecodeTokensFor(account, encSecret, encPublic); on error, mark failure and reject. + - Constant-time compare (crypto/subtle.ConstantTimeCompare) of provided public token vs decoded public. + - Nonce replay protection with TTL cache: if nonce for account already used within TTL, reject; otherwise mark it as used after successful signature. + - HMAC signature verification: localSignature = auth.CreateSignatureFrom(canonical, token.SecretKey); constant-time compare vs provided signature. + +9) Context propagation + - Attaches auth.account_name and request.id to context for downstream handlers. + +10) Logging and errors + - Uses structured slog logger with request_id, method, path. + - Logs warnings for missing headers, invalid format, too many failures, account not found, replay detected, signature mismatch. + - Errors are generic to clients (HTTP 401 with neutral messages) to avoid leaking details. + +Defaults (from MakeTokenMiddleware): +- clockSkew: 5m; disallowFuture: false +- nonceTTL: 5m; nonceCache: in-memory TTL +- rateLimiter: in-memory with 1m window, 10 failures threshold +- now: time.Now (injectable for tests) + +--- + +## 2) What “Version 1” achieved + +From the implementation and tests, v1 delivered the following: +- Request binding and signing + - Canonical request includes method, path, sorted query, timestamp, nonce, and body hash. + - HMAC signature over canonical string using per-account secret key. +- Input hardening + - Strict header presence checks. + - Token format validation. + - Constant-time comparisons for public token and signature to prevent timing leaks. +- Replay and freshness controls + - Timestamp skew window enforcement with configurable policy. + - Nonce replay cache with TTL per account. +- Operational safeguards + - Failure-based rate limiting per clientIP|account scope. + - Structured logging with request correlation id (X-Request-ID) and neutral client-facing errors. +- DX and correctness + - Body is re-usable after hashing. + - Context carries account name and request id for downstream. + - Thorough unit/integration tests (including canonicalization and DB-backed key lookup/decoding). + +These map to the earlier docs’ Phases 1–2 checkboxes: constant-time compare, generic 401s, structured logging with request-id, context propagation, timestamp & nonce controls, canonical signing, failure rate limiting. + +--- + +## 3) Gaps and risks + +- In-memory state + - Nonce cache and rate limiter are in-memory; not horizontally scalable. Replays could work across nodes, and failure limits won’t coordinate cluster-wide. +- Future timestamp policy + - disallowFuture is false by default, allowing future timestamps within skew; opens small window for limited replay across clocks. +- Key rotation and key identification + - Canonical/signature doesn’t include a key identifier (kid). Rotations require coordination and lookup; currently only username+public token are provided. +- Observability and audit + - Logs are present but no explicit audit event for successful/failed auth with stable event schema or metrics emission. +- Error handling surfaces + - All errors map to 401; may want 429 for rate-limited scopes and distinct codes for clock skew vs. formatting for better client remediation (while still generic). +- DoS controls + - Rate limiter is failure-triggered only; no general request token bucket per account/IP for auth endpoints. +- Request size / body hashing + - Large bodies are fully read into memory to hash; could be abused. No explicit max size before rejection. + +--- + +## 4) Phase 3: Recommended next steps + +1) Distributed nonce and rate limiting + - Replace in-memory TTL nonce cache with Redis (SETNX with TTL) keyed by account|nonce. + - Use Redis or a shared backend for failure-based limiter; emit 429 Too Many Requests when threshold exceeded. + +2) Tighten time policy and drift support + - Set disallowFuture = true by default; document policy for clients. + - Reduce skew to 2 minutes (configurable), and expose a time sync endpoint returning server time (and maybe signed) to help clients adjust. + +3) Key management and rotation + - Introduce key IDs (kid) and include it in headers and canonical string. + - Support multiple active keys per account with activation/expiration and server-side rotation policy. + - Add a deprecation window and telemetry to observe usage of old keys before revocation. + +4) Stronger request binding and algorithm agility + - Version the signing scheme (alg/version) in headers; allow upgrading to stronger algorithms if needed. + - Ensure canonicalization is frozen per version and formally documented; add conformance tests for edge cases (empty query, repeated params, unicode path). + +5) Resource and abuse protections + - Enforce a maximum body size for requests that must be signed (e.g., 2–5MB) before hashing; reject larger with 413. + - Add general-purpose rate limiting (token bucket) per IP/account for auth attempts and overall requests. + +6) Observability and audit + - Emit structured audit events (success/failure) with fields: request_id, account, client_ip (anonymized), reason, kid, alg, and skew/nonce metrics. + - Add metrics (Prometheus): auth_success_total, auth_failure_total by reason, replay_detected_total, skew_violations_total, limiter_block_total. + +7) Security posture improvements + - Consider mTLS for server-to-server clients; retain HMAC as app-level assurance. + - Add IP allow/deny lists for sensitive routes. + - Ensure all secrets are stored encrypted at rest and rotate encryption keys for stored API keys. + +8) Developer ergonomics and docs + - Publish exact canonicalization rules and client libraries/examples for multiple languages. + - Provide a sandbox endpoint to verify signatures and surface detailed diagnostics to authenticated developers. + +9) Backward compatibility and rollout + - Introduce phase-3 features behind config flags. + - Add dual-signing period when introducing kid/versioned algorithms so clients can switch gradually. + +--- + +## 5) Concrete implementation tasks + +- Replace nonceCache with interface and add RedisTTLCache implementation; wire by config. Use key "acct|nonce" with PX TTL and SET NX. +- Replace limiter.MemoryLimiter with a pluggable Limiter interface and add Redis-based sliding window or token bucket implementation. Return 429 on scope saturation. +- Default disallowFuture = true; make skew configurable via env. Add /time endpoint that returns server unix time. +- Extend headers: + - X-API-Key-ID (kid) + - X-API-Alg (e.g., hmac-sha256;v=1) + Include these in canonical string and signature verification path. Update tests. +- Add MaxBodyBytes wrapper before hashing; reject with 413 if exceeded; document limits. +- Emit metrics and structured audit logs; create middleware counters and reason tags. +- Documentation updates and client examples for canonicalization and signing including new headers. + +--- + +## 6) What remains unchanged (for clarity) + +- The overall request flow and canonicalization approach remains; we are enriching it with distributed state, policy tightening, and key management features. +- Error messages remain generic to clients; only status codes and headers may change for rate-limiting (429) and possibly clock skew guidance headers. + +--- + +## 7) Testing strategy for Phase 3 + +- Unit tests + - Canonicalization invariants, header parsing with kid/alg, disallowFuture behavior, max body enforcement. +- Integration tests + - Redis-backed nonce and limiter behavior across multiple requests and simulated nodes. + - Key rotation: old/new key acceptance within window, rejection after revocation. +- Load tests + - Validate limiter correctness under concurrency and replay attempts. + diff --git a/docs/middleware/token_examples.md b/docs/middleware/token_examples.md new file mode 100644 index 00000000..6e266e30 --- /dev/null +++ b/docs/middleware/token_examples.md @@ -0,0 +1,281 @@ +# Token Examples + +Date: 2025-08-08 17:01 local + +This guide shows how to call the protected API endpoints using: +- Postman (with a pre-request script that builds the signature) +- JavaScript fetch (Node.js and Browser) + +It complements the middleware analysis by giving copy‑pasteable, working examples. + +--- + +### Overview + +Protected routes (see metal/kernel/router.go): +- POST /posts — list/filter posts +- GET /posts/{slug} — show a post by slug + +Gateway paths differ by environment: +- Local (via Caddy local): http://localhost:8080/posts +- Production (via Caddy prod): https://oullin.io/api/posts + +Required headers on every protected request: +- X-Request-ID: A unique ID per request (string) +- X-API-Username: Your account name (case-insensitive lookup) +- X-API-Key: Your public token (pk_...) +- X-API-Timestamp: Unix epoch seconds +- X-API-Nonce: Unique per request within TTL window +- X-API-Signature: hex(HMAC-SHA256(secret, canonical_request)) + +Canonical request string (exact order): +METHOD + "\n" + PATH + "\n" + SORTED_QUERY + "\n" + username + "\n" + public + "\n" + timestamp + "\n" + nonce + "\n" + sha256_hex(body) + +Notes: +- METHOD must be uppercase. +- PATH must be escaped path (e.g., /posts/%7Bslug%7D if encoded). +- SORTED_QUERY: sort keys then values; join as key=value&... +- sha256_hex(body): hex SHA-256 of raw request body ("" for GET/DELETE by convention here). +- Time skew default ±5m, and nonce replay is blocked during TTL. + +--- + +### Postman + +Environment variables to set: +- baseUrl: Local: http://localhost:8080; Prod: https://oullin.io/api +- username: Your account name +- publicKey: Your pk_... value (plaintext) +- secretKey: Your sk_... value (plaintext) +- requestId: Optional; if absent, we’ll reuse the nonce + +Pre-request Script (copy/paste into your request or collection): + +pm.sendRequest = pm.sendRequest; // keep reference + +```js +(function() { + // Postman sandbox: CryptoJS is available via require + const crypto = require('crypto-js'); + function sha256Hex(str: string): string { return crypto.SHA256(str || '').toString(crypto.enc.Hex); } + function sortedQuery(u: string): string { + const url = new URL(u); + const keys = Array.from(url.searchParams.keys()); + keys.sort(); + const parts: string[] = []; + for (const k of keys) { + const vs = url.searchParams.getAll(k).sort(); + for (const v of vs) parts.push(encodeURIComponent(k) + '=' + encodeURIComponent(v)); + } + return parts.join('&'); + } + + const method = pm.request.method.toUpperCase(); + const urlStr = pm.environment.get('baseUrl') + pm.request.url.getPathWithQuery(); + const urlObj = new URL(urlStr); + const path = urlObj.pathname; + const query = sortedQuery(urlStr); + const username = pm.environment.get('username'); + const publicKey = pm.environment.get('publicKey'); + const secretKey = pm.environment.get('secretKey'); + const timestamp = Math.floor(Date.now() / 1000).toString(); + const nonce = crypto.lib.WordArray.random(16).toString(); + const body = (method === 'GET' || method === 'DELETE') ? '' : (pm.request.body?.raw || ''); + const bodyHash = sha256Hex(body); + const canonical = [method, path, query, username, publicKey, timestamp, nonce, bodyHash].join('\n'); + const signature = crypto.HmacSHA256(canonical, secretKey).toString(); + + pm.request.headers.upsert({ key: 'X-Request-ID', value: pm.environment.get('requestId') || nonce }); + pm.request.headers.upsert({ key: 'X-API-Username', value: username }); + pm.request.headers.upsert({ key: 'X-API-Key', value: publicKey }); + pm.request.headers.upsert({ key: 'X-API-Timestamp', value: timestamp }); + pm.request.headers.upsert({ key: 'X-API-Nonce', value: nonce }); + pm.request.headers.upsert({ key: 'X-API-Signature', value: signature }); +})(); +``` + +Example requests: +- Local list posts + - Method: POST + - URL: {{baseUrl}}/posts + - Body: raw JSON {} +- Local show post + - Method: GET + - URL: {{baseUrl}}/posts/hello +- Production list posts + - Method: POST + - URL: https://oullin.io/api/posts + +Tips: +- Ensure your system clock is correct (NTP); skew is typically ±5 minutes. +- Do not reuse the same Nonce within the TTL window. +- Always include X-Request-ID. + +--- + +### JavaScript fetch — Node.js (TypeScript + ESM) + +```ts +import { createHash, createHmac, randomBytes } from 'node:crypto'; + +function sha256Hex(text: string): string { + return createHash('sha256').update(text || '').digest('hex'); +} + +function sortedQuery(u: string): string { + const url = new URL(u); + const keys = Array.from(url.searchParams.keys()).sort(); + const pairs: string[] = []; + for (const k of keys) { + const vs = url.searchParams.getAll(k).sort(); + for (const v of vs) pairs.push(encodeURIComponent(k) + '=' + encodeURIComponent(v)); + } + return pairs.join('&'); +} + +type HttpMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; + +type SignedFetchParams = { + baseUrl: string; + path: string; + method: HttpMethod; + body?: unknown; + username: string; + publicKey: string; + secretKey: string; +}; + +export async function signedFetch(params: SignedFetchParams): Promise { + const { baseUrl, path, method, body, username, publicKey, secretKey } = params; + const url = new URL(path, baseUrl).toString(); + const u = new URL(url); + const ts = Math.floor(Date.now() / 1000).toString(); + const nonce = randomBytes(16).toString('hex'); + const payload = body && (method === 'POST' || method === 'PUT') ? JSON.stringify(body) : ''; + const bodyHash = sha256Hex(payload); + const canonical = [ + method.toUpperCase(), + u.pathname, + sortedQuery(u.toString()), + username, + publicKey, + ts, + nonce, + bodyHash, + ].join('\n'); + const signature = createHmac('sha256', secretKey).update(canonical).digest('hex'); + const headers: Record = { + 'Content-Type': 'application/json', + 'X-Request-ID': nonce, + 'X-API-Username': username, + 'X-API-Key': publicKey, + 'X-API-Timestamp': ts, + 'X-API-Nonce': nonce, + 'X-API-Signature': signature, + }; + + const init: RequestInit = { method, headers, body: payload || undefined }; + const res = await fetch(u.toString(), init); + if (!res.ok) throw new Error(`HTTP ${res.status}`); + return res.json(); +} + +// Example usage (Node 18+ has global fetch; run with: node --env-file=.env example.ts after transpile) +(async () => { + const data = await signedFetch({ + baseUrl: 'http://localhost:8080', // local via Caddy + path: '/posts', + method: 'POST', + body: {}, + username: process.env.API_USER as string, + publicKey: process.env.API_PUBLIC as string, + secretKey: process.env.API_SECRET as string, + }); + console.log('posts:', data); +})(); +``` +--- + +### JavaScript fetch — Browser (Web Crypto, TypeScript) + +```ts +async function sha256Hex(text: string): Promise { + const enc = new TextEncoder(); + const buf = await crypto.subtle.digest('SHA-256', enc.encode(text || '')); + return Array.from(new Uint8Array(buf)).map(b => b.toString(16).padStart(2, '0')).join(''); +} + +function sortedQuery(u: string): string { + const url = new URL(u); + const keys = Array.from(url.searchParams.keys()).sort(); + const pairs: string[] = []; + for (const k of keys) { + const vs = url.searchParams.getAll(k).sort(); + for (const v of vs) pairs.push(encodeURIComponent(k) + '=' + encodeURIComponent(v)); + } + return pairs.join('&'); +} + +async function hmacSha256Hex(secret: string, message: string): Promise { + const enc = new TextEncoder(); + const key = await crypto.subtle.importKey('raw', enc.encode(secret), { name: 'HMAC', hash: 'SHA-256' }, false, ['sign']); + const sig = await crypto.subtle.sign('HMAC', key, enc.encode(message)); + return Array.from(new Uint8Array(sig)).map(b => b.toString(16).padStart(2, '0')).join(''); +} + +type HttpMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; + +type SignedFetchParams = { + baseUrl: string; + path: string; + method: HttpMethod; + body?: unknown; + username: string; + publicKey: string; + secretKey: string; +}; + +async function signedFetch(params: SignedFetchParams): Promise { + const { baseUrl, path, method, body, username, publicKey, secretKey } = params; + const url = new URL(path, baseUrl).toString(); + const u = new URL(url); + const ts = Math.floor(Date.now() / 1000).toString(); + const nonce = crypto.getRandomValues(new Uint8Array(16)).reduce((s, b) => s + b.toString(16).padStart(2, '0'), ''); + const payload = body && (method === 'POST' || method === 'PUT') ? JSON.stringify(body) : ''; + const bodyHash = await sha256Hex(payload); + const canonical = [method.toUpperCase(), u.pathname, sortedQuery(u.toString()), username, publicKey, ts, nonce, bodyHash].join('\n'); + const signature = await hmacSha256Hex(secretKey, canonical); + const headers: Record = { + 'Content-Type': 'application/json', + 'X-Request-ID': nonce, + 'X-API-Username': username, + 'X-API-Key': publicKey, + 'X-API-Timestamp': ts, + 'X-API-Nonce': nonce, + 'X-API-Signature': signature, + }; + const res = await fetch(u.toString(), { method, headers, body: payload || undefined, mode: 'cors' }); + if (!res.ok) throw new Error(`HTTP ${res.status}`); + return res.json(); +} + +// Local example (Vite dev server at http://localhost:5173) +(async () => { + const data = await signedFetch({ + baseUrl: 'http://localhost:8080', + path: '/posts', + method: 'POST', + body: {}, + username: 'your-account', + publicKey: 'pk_...', + secretKey: 'sk_...', + }); + console.log('posts', data); +})(); +``` + +### Notes: +- In production, prefix routes with /api (e.g., https://oullin.io/api/posts). +- Do not reuse Nonces within TTL; replay requests are rejected. +- X-Request-ID is required for tracing. +- Your canonicalization must match the server (method upper-casing, path escaping, SortedQuery, body hash). From 2baa42fbb6b47d04aeef3827e58a20194607e2e0 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 17:37:40 +0800 Subject: [PATCH 20/26] wip --- database/seeder/seeds/users.go | 5 ++++- pkg/cache/ttl_cache.go | 28 +++++++++++++++++++++++++++- pkg/middleware/token_middleware.go | 29 +++++++++++++++-------------- pkg/portal/support.go | 4 ++++ pkg/portal/validator.go | 10 ++++++---- 5 files changed, 56 insertions(+), 20 deletions(-) diff --git a/database/seeder/seeds/users.go b/database/seeder/seeds/users.go index cdcf2f65..ee626ca5 100644 --- a/database/seeder/seeds/users.go +++ b/database/seeder/seeds/users.go @@ -23,7 +23,10 @@ func MakeUsersSeed(db *database.Connection) *UsersSeed { } func (s UsersSeed) Create(attrs database.UsersAttrs) (database.User, error) { - pass, _ := portal.MakePassword("password") + pass, err := portal.MakePassword("password") + if err != nil { + return database.User{}, fmt.Errorf("failed to generate seed password: %w", err) + } fake := database.User{ UUID: uuid.NewString(), diff --git a/pkg/cache/ttl_cache.go b/pkg/cache/ttl_cache.go index c8ddce42..711c6b7c 100644 --- a/pkg/cache/ttl_cache.go +++ b/pkg/cache/ttl_cache.go @@ -45,5 +45,31 @@ func (c *TTLCache) Mark(key string, ttl time.Duration) { c.mu.Lock() defer c.mu.Unlock() - c.data[key] = time.Now().Add(ttl) + now := time.Now() + // Opportunistic prune of expired entries to bound memory growth + + for k, exp := range c.data { + if now.After(exp) { + delete(c.data, k) + } + } + + c.data[key] = now.Add(ttl) +} + +// UseOnce atomically checks whether the key has already been used and, if not, +// marks it as used with the provided ttl. Returns true if the key was already +// present (and not expired), false if it was newly marked. +func (c *TTLCache) UseOnce(key string, ttl time.Duration) bool { + c.mu.Lock() + defer c.mu.Unlock() + + now := time.Now() + if exp, ok := c.data[key]; ok && now.Before(exp) { + return true // already used + } + + c.data[key] = now.Add(ttl) + + return false } diff --git a/pkg/middleware/token_middleware.go b/pkg/middleware/token_middleware.go index 244c3b78..1f0a465a 100644 --- a/pkg/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -3,6 +3,7 @@ package middleware import ( "bytes" "context" + "crypto/sha256" "crypto/subtle" "io" "log/slog" @@ -240,21 +241,24 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ return true } - // Constant-time compare of provided public token vs stored one - provided := []byte(strings.TrimSpace(publicToken)) - expected := []byte(strings.TrimSpace(token.PublicKey)) + // Constant-time compare (fixed-length by hashing) of provided public token vs stored one + pBytes := []byte(strings.TrimSpace(publicToken)) + eBytes := []byte(strings.TrimSpace(token.PublicKey)) + hP := sha256.Sum256(pBytes) + hE := sha256.Sum256(eBytes) - if subtle.ConstantTimeCompare(provided, expected) != 1 { + if subtle.ConstantTimeCompare(hP[:], hE[:]) != 1 { t.rateLimiter.Fail(limiterKey) logger.Warn("public token mismatch", "account", item.AccountName) return true } - // Nonce replay protection: check if already used for this account + // Nonce replay protection: atomically check-and-mark (UseOnce) if t.nonceCache != nil { key := item.AccountName + "|" + nonce - if t.nonceCache.Used(key) { + + if t.nonceCache.UseOnce(key, t.nonceTTL) { t.rateLimiter.Fail(limiterKey) logger.Warn("replay detected: nonce already used", "account", item.AccountName) @@ -262,21 +266,18 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ } } - // Compute local signature over canonical request and compare in constant time + // Compute local signature over canonical request and compare in constant time (hash to fixed-length first) localSignature := auth.CreateSignatureFrom(canonical, token.SecretKey) - if subtle.ConstantTimeCompare([]byte(signature), []byte(localSignature)) != 1 { + hSig := sha256.Sum256([]byte(strings.TrimSpace(signature))) + hLocal := sha256.Sum256([]byte(localSignature)) + + if subtle.ConstantTimeCompare(hSig[:], hLocal[:]) != 1 { t.rateLimiter.Fail(limiterKey) logger.Warn("signature mismatch", "account", item.AccountName) return true } - // Mark nonce as used within the TTL - if t.nonceCache != nil { - key := item.AccountName + "|" + nonce - t.nonceCache.Mark(key, t.nonceTTL) - } - return false } diff --git a/pkg/portal/support.go b/pkg/portal/support.go index b5569779..95a02c3c 100644 --- a/pkg/portal/support.go +++ b/pkg/portal/support.go @@ -13,6 +13,10 @@ import ( ) func CloseWithLog(c io.Closer) { + if c == nil { + return + } + if err := c.Close(); err != nil { slog.Error("failed to close resource", "err", err) } diff --git a/pkg/portal/validator.go b/pkg/portal/validator.go index f7475235..db3ca9fc 100644 --- a/pkg/portal/validator.go +++ b/pkg/portal/validator.go @@ -15,11 +15,13 @@ type Validator struct { Errors map[string]interface{} } -func GetDefaultValidator() *Validator { - var once sync.Once - var defaultValidator *Validator +var ( + defaultOnce sync.Once + defaultValidator *Validator +) - once.Do(func() { +func GetDefaultValidator() *Validator { + defaultOnce.Do(func() { defaultValidator = MakeValidatorFrom( validator.New( validator.WithRequiredStructEnabled(), From 441208996116c1c9b163ffe4c725f47cad4a4d3a Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Fri, 8 Aug 2025 17:48:35 +0800 Subject: [PATCH 21/26] wip --- metal/cli/main.go | 3 +-- metal/cli/posts/factory.go | 6 +++--- metal/cli/posts/handler_test.go | 4 ++-- pkg/{ => portal}/client.go | 2 +- pkg/{ => portal}/client_test.go | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) rename pkg/{ => portal}/client.go (99%) rename pkg/{ => portal}/client_test.go (98%) diff --git a/metal/cli/main.go b/metal/cli/main.go index 109035dc..a8cbbd84 100644 --- a/metal/cli/main.go +++ b/metal/cli/main.go @@ -9,7 +9,6 @@ import ( "github.com/oullin/metal/cli/posts" "github.com/oullin/metal/env" "github.com/oullin/metal/kernel" - "github.com/oullin/pkg" "github.com/oullin/pkg/auth" "github.com/oullin/pkg/cli" "github.com/oullin/pkg/portal" @@ -94,7 +93,7 @@ func createBlogPost(menu panel.Menu) error { return err } - httpClient := pkg.MakeDefaultClient(nil) + httpClient := portal.MakeDefaultClient(nil) handler := posts.MakeHandler(input, httpClient, dbConn) if _, err = handler.NotParsed(); err != nil { diff --git a/metal/cli/posts/factory.go b/metal/cli/posts/factory.go index 8d986234..18b4ed79 100644 --- a/metal/cli/posts/factory.go +++ b/metal/cli/posts/factory.go @@ -5,21 +5,21 @@ import ( "fmt" "github.com/oullin/database" "github.com/oullin/database/repository" - "github.com/oullin/pkg" "github.com/oullin/pkg/markdown" + "github.com/oullin/pkg/portal" "net/http" "time" ) type Handler struct { Input *Input - Client *pkg.Client + Client *portal.Client Posts *repository.Posts Users *repository.Users IsDebugging bool } -func MakeHandler(input *Input, client *pkg.Client, db *database.Connection) Handler { +func MakeHandler(input *Input, client *portal.Client, db *database.Connection) Handler { tags := &repository.Tags{DB: db} categories := &repository.Categories{DB: db} diff --git a/metal/cli/posts/handler_test.go b/metal/cli/posts/handler_test.go index 27dd024b..7c76935a 100644 --- a/metal/cli/posts/handler_test.go +++ b/metal/cli/posts/handler_test.go @@ -11,8 +11,8 @@ import ( "github.com/google/uuid" "github.com/oullin/database" "github.com/oullin/metal/cli/clitest" - "github.com/oullin/pkg" "github.com/oullin/pkg/markdown" + "github.com/oullin/pkg/portal" ) func captureOutput(fn func()) string { @@ -56,7 +56,7 @@ func setupPostsHandler(t *testing.T) (*Handler, *database.Connection) { Url: "http://example", } - h := MakeHandler(input, pkg.MakeDefaultClient(nil), conn) + h := MakeHandler(input, portal.MakeDefaultClient(nil), conn) return &h, conn } diff --git a/pkg/client.go b/pkg/portal/client.go similarity index 99% rename from pkg/client.go rename to pkg/portal/client.go index 4c0bd722..87a18ffb 100644 --- a/pkg/client.go +++ b/pkg/portal/client.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "context" diff --git a/pkg/client_test.go b/pkg/portal/client_test.go similarity index 98% rename from pkg/client_test.go rename to pkg/portal/client_test.go index 514ed78e..64140a4e 100644 --- a/pkg/client_test.go +++ b/pkg/portal/client_test.go @@ -1,4 +1,4 @@ -package pkg +package portal import ( "context" From b4ce019d31f3685b89ee14bbf3d06e05b7a7f191 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Mon, 11 Aug 2025 11:01:29 +0800 Subject: [PATCH 22/26] extract ReadWithSizeLimit --- pkg/markdown/handler.go | 8 ++++---- pkg/middleware/token_middleware.go | 2 +- pkg/portal/client.go | 5 +---- pkg/portal/support.go | 17 +++++++++++++++++ 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/pkg/markdown/handler.go b/pkg/markdown/handler.go index 1a488ef2..4a34d830 100644 --- a/pkg/markdown/handler.go +++ b/pkg/markdown/handler.go @@ -2,11 +2,12 @@ package markdown import ( "fmt" - "gopkg.in/yaml.v3" - "io" "net/http" "regexp" "strings" + + "github.com/oullin/pkg/portal" + "gopkg.in/yaml.v3" ) func (p Parser) Fetch() (string, error) { @@ -31,8 +32,7 @@ func (p Parser) Fetch() (string, error) { return "", fmt.Errorf("failed to fetch markdown: status %d", resp.StatusCode) } - body, err := io.ReadAll(resp.Body) - + body, err := portal.ReadWithSizeLimit(resp.Body) if err != nil { return "", err } diff --git a/pkg/middleware/token_middleware.go b/pkg/middleware/token_middleware.go index 1f0a465a..866324f4 100644 --- a/pkg/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -192,7 +192,7 @@ func (t TokenCheckMiddleware) readBodyHash(r *baseHttp.Request, logger *slog.Log return portal.Sha256Hex(nil), nil } - b, err := io.ReadAll(r.Body) + b, err := portal.ReadWithSizeLimit(r.Body) if err != nil { logger.Warn("unable to read body for signing") return "", t.getInvalidRequestError() diff --git a/pkg/portal/client.go b/pkg/portal/client.go index 87a18ffb..48e8d41a 100644 --- a/pkg/portal/client.go +++ b/pkg/portal/client.go @@ -3,7 +3,6 @@ package portal import ( "context" "fmt" - "io" "net/http" "time" ) @@ -71,9 +70,7 @@ func (f *Client) Get(ctx context.Context, url string) (string, error) { return "", fmt.Errorf("received non-2xx status code: %d", resp.StatusCode) } - // To avoid allocating a massive buffer for a potentially huge response, we could use io.Copy with a limited reader - // if we need to process the body. However, if we must return a string, reading all is necessary. - body, err := io.ReadAll(resp.Body) + body, err := ReadWithSizeLimit(resp.Body) if err != nil { return "", fmt.Errorf("failed to read response body: %w", err) } diff --git a/pkg/portal/support.go b/pkg/portal/support.go index 95a02c3c..37902aac 100644 --- a/pkg/portal/support.go +++ b/pkg/portal/support.go @@ -89,3 +89,20 @@ func ParseClientIP(r *baseHttp.Request) string { return strings.TrimSpace(r.RemoteAddr) } + +// ReadWithSizeLimit reads from an io.Reader with a size limit to prevent DoS attacks. +// It returns the read bytes and any error encountered. +// The default size limit is 5MB. +func ReadWithSizeLimit(r io.Reader, maxSize ...int64) ([]byte, error) { + // Default size limit is 5MB + const defaultMaxSize int64 = 5 * 1024 * 1024 // 5MB + + limit := defaultMaxSize + if len(maxSize) > 0 && maxSize[0] > 0 { + limit = maxSize[0] + } + + limitedReader := io.LimitReader(r, limit) + + return io.ReadAll(limitedReader) +} From bbaf94c772fb4ba855a53e94cd372d51ef31a777 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Mon, 11 Aug 2025 11:13:19 +0800 Subject: [PATCH 23/26] format --- caddy/Caddyfile.local | 6 +++--- caddy/Caddyfile.prod | 9 +++++--- docs/middleware/token_analysis_v1.md | 32 +++++++++++++++++++--------- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/caddy/Caddyfile.local b/caddy/Caddyfile.local index df3fe4b7..94bbf7eb 100644 --- a/caddy/Caddyfile.local +++ b/caddy/Caddyfile.local @@ -16,8 +16,8 @@ header { Access-Control-Allow-Origin "http://localhost:5173" # allows the Vue app (running on localhost:5173) to make requests. Access-Control-Allow-Methods "GET, POST, PUT, DELETE, OPTIONS" # Specifies which methods are allowed. - Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, Content-Type, User-Agent, If-None-Match" # allows the custom headers needed by the API. - Access-Control-Expose-Headers "ETag" + Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, X-API-Timestamp, X-API-Nonce, X-Request-ID, Content-Type, User-Agent, If-None-Match" # allows the custom headers needed by the API. + Access-Control-Expose-Headers "ETag, X-Request-ID" } # This handles the browser's "preflight" OPTIONS request. @@ -30,7 +30,7 @@ # Reflect the Origin back so it's always allowed header Access-Control-Allow-Origin "{http.request.header.Origin}" header Access-Control-Allow-Methods "GET, POST, PUT, DELETE, OPTIONS" - header Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, Content-Type, User-Agent, If-None-Match" + header Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, X-API-Timestamp, X-API-Nonce, X-Request-ID, Content-Type, User-Agent, If-None-Match" header Access-Control-Max-Age "86400" respond 204 } diff --git a/caddy/Caddyfile.prod b/caddy/Caddyfile.prod index 70c09835..3ff7665d 100644 --- a/caddy/Caddyfile.prod +++ b/caddy/Caddyfile.prod @@ -34,8 +34,8 @@ oullin.io { header { Access-Control-Allow-Origin "https://oullin.io" Access-Control-Allow-Methods "GET, POST, PUT, DELETE, OPTIONS" - Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, Content-Type, User-Agent, If-None-Match" - Access-Control-Expose-Headers "ETag" + Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, X-API-Timestamp, X-API-Nonce, X-Request-ID, Content-Type, User-Agent, If-None-Match" + Access-Control-Expose-Headers "ETag, X-Request-ID" } @preflight { @@ -47,7 +47,7 @@ oullin.io { # Reflect the Origin back so it's always allowed header Access-Control-Allow-Origin "{http.request.header.Origin}" header Access-Control-Allow-Methods "GET, POST, PUT, DELETE, OPTIONS" - header Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, Content-Type, User-Agent, If-None-Match" + header Access-Control-Allow-Headers "X-API-Key, X-API-Username, X-API-Signature, X-API-Timestamp, X-API-Nonce, X-Request-ID, Content-Type, User-Agent, If-None-Match" header Access-Control-Max-Age "86400" respond 204 } @@ -57,6 +57,9 @@ oullin.io { header_up X-API-Username {http.request.header.X-API-Username} header_up X-API-Key {http.request.header.X-API-Key} header_up X-API-Signature {http.request.header.X-API-Signature} + header_up X-API-Timestamp {http.request.header.X-API-Timestamp} + header_up X-API-Nonce {http.request.header.X-API-Nonce} + header_up X-Request-ID {http.request.header.X-Request-ID} transport http { dial_timeout 10s diff --git a/docs/middleware/token_analysis_v1.md b/docs/middleware/token_analysis_v1.md index 0412f566..96528b4b 100644 --- a/docs/middleware/token_analysis_v1.md +++ b/docs/middleware/token_analysis_v1.md @@ -1,11 +1,13 @@ # Token Middleware Analysis and Recommendations -File: pkg/http/middleware/token_middleware.go +Files: +- Legacy (pre-PR #77): pkg/http/middleware/token_middleware.go (removed) +- Current (v1): pkg/middleware/token_middleware.go Date: 2025-08-08 --- -## 1) What it does +## 1) What the legacy middleware did (pre-PR #77) The TokenCheckMiddleware enforces a simple HMAC-based request authentication using three custom HTTP headers: @@ -102,6 +104,14 @@ Security hardening (medium impact): - Input normalization: - Canonicalize header casing, path, and query param encoding consistently. +- Canonicalization rules (to prevent signature drift): + - METHOD uppercased; PATH must be URI-normalized without dot-segments. + - Percent-encode using RFC 3986 unreserved set; do not double-encode. + - SORTED_QUERY_STRING sorts by key, then by value, both byte-wise ascending; multi-value params preserved in sorted order. + - Collapse duplicate query separators; omit keys with empty names. + - BODY hash is the SHA-256 of the exact bytes sent; for empty body use the hash of the empty string. + - Header names are case-insensitive; trim surrounding whitespace on all header values. + - Rate limiting: - Rate limit auth failures per IP/account. @@ -279,20 +289,22 @@ Date: 2025-08-08 16:52 local - Caddy local proxy (caddy/Caddyfile.local): - auto_https off (HTTP only locally). - Listens on :80 in the container (published as http://localhost:8080 on the host). - - CORS: Allows Origin http://localhost:5173 and headers X-API-Username, X-API-Key, X-API-Signature; handles OPTIONS preflight. + - CORS: Allows Origin http://localhost:5173 and headers X-API-Username, X-API-Key, X-API-Signature, X-API-Timestamp, X-API-Nonce, X-Request-ID; handles OPTIONS preflight. + - CORS: Exposes header X-Request-ID to clients. - reverse_proxy api:8080 — all paths are forwarded to API without an "/api" prefix. - Caddy production proxy (caddy/Caddyfile.prod): - Site: oullin.io (automatic HTTPS). - API is routed under /api/* and proxied to api:8080. That means production API path = https://oullin.io/api/... while local is http://localhost:8080/.... - CORS configured for https://oullin.io within the /api handler. For preflight, echoes Access-Control-Allow-Origin back. - - Forwards key auth headers upstream (header_up Host, X-API-Username, X-API-Key, X-API-Signature). X-Forwarded-For is also set by Caddy; the middleware’s ParseClientIP will prefer the first X-Forwarded-For entry. - -- Makefile (metal/makefile): - - build-local: docker compose --profile local up --build -d (starts api, api-db, caddy_local). After this, the API is reachable at http://localhost:8080. - - db:up, db:seed, db:migrate: Manage DB lifecycle and schema. - - validate-caddy: Format/validate local and prod Caddyfiles. - - env:init, env:check: Manage .env from .env.example. + - Forwards key auth headers upstream (header_up Host, X-API-Username, X-API-Key, X-API-Signature, X-API-Timestamp, X-API-Nonce, X-Request-ID). X-Forwarded-For is also set by Caddy; the middleware’s ParseClientIP will prefer the first X-Forwarded-For entry. + - CORS: Exposes header X-Request-ID to clients. + +- Makefiles (metal/makefile/*.mk): + - build-local (build.mk): docker compose --profile local up --build -d (starts api, api-db, caddy_local). After this, the API is reachable at http://localhost:8080. + - db:up, db:seed, db:migrate (db.mk): Manage DB lifecycle and schema. + - validate-caddy (app.mk): Format/validate local and production Caddyfiles. + - env:init, env:check (env.mk): Initialize and verify .env from .env.example. - API routes (metal/kernel/router.go): - POST /posts (list/filter posts) and GET /posts/{slug} (show post) are protected by TokenCheckMiddleware. From 9121f8ab0955ae076c2ed1ca4a1661384af6787c Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Mon, 11 Aug 2025 11:17:17 +0800 Subject: [PATCH 24/26] headers --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index 9f50444b..08a5ee3d 100644 --- a/main.go +++ b/main.go @@ -55,7 +55,7 @@ func serverHandler() baseHttp.Handler { c := cors.New(cors.Options{ AllowedOrigins: []string{localhost, "http://localhost:5173"}, AllowedMethods: []string{baseHttp.MethodGet, baseHttp.MethodPost, baseHttp.MethodPut, baseHttp.MethodDelete, baseHttp.MethodOptions}, - AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token", "User-Agent", "X-API-Key", "X-API-Username", "X-API-Signature", "If-None-Match"}, + AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token", "User-Agent", "X-API-Key", "X-API-Username", "X-API-Signature", "X-API-Timestamp", "X-API-Nonce", "X-Request-ID", "If-None-Match"}, AllowCredentials: true, Debug: true, }) From 5049a4f368b38c67f0ff09180ad40956ef7ce596 Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Mon, 11 Aug 2025 11:59:22 +0800 Subject: [PATCH 25/26] better abstraction --- caddy/Caddyfile.prod | 3 + docs/middleware/token_analysis_v2.md | 19 ++--- pkg/markdown/handler.go | 3 +- pkg/middleware/token_middleware.go | 48 ++++++++++--- pkg/middleware/token_middleware_test.go | 89 +++++++++++++++++++++++ pkg/middleware/valid_timestamp.go | 4 +- pkg/middleware/valid_timestamp_test.go | 4 +- pkg/portal/support.go | 16 ++++- pkg/portal/support_test.go | 96 +++++++++++++++++++++++++ 9 files changed, 254 insertions(+), 28 deletions(-) diff --git a/caddy/Caddyfile.prod b/caddy/Caddyfile.prod index 3ff7665d..10507694 100644 --- a/caddy/Caddyfile.prod +++ b/caddy/Caddyfile.prod @@ -60,6 +60,9 @@ oullin.io { header_up X-API-Timestamp {http.request.header.X-API-Timestamp} header_up X-API-Nonce {http.request.header.X-API-Nonce} header_up X-Request-ID {http.request.header.X-Request-ID} + header_up Content-Type {http.request.header.Content-Type} + header_up User-Agent {http.request.header.User-Agent} + header_up If-None-Match {http.request.header.If-None-Match} transport http { dial_timeout 10s diff --git a/docs/middleware/token_analysis_v2.md b/docs/middleware/token_analysis_v2.md index 51ceb836..1f07500a 100644 --- a/docs/middleware/token_analysis_v2.md +++ b/docs/middleware/token_analysis_v2.md @@ -1,6 +1,6 @@ # Token middleware analysis (v2) -Date: 2025-08-08 +Date: 2025-08-11 Scope: pkg/middleware/token_middleware.go and related helpers (valid_timestamp.go, pkg/portal/support.go) --- @@ -35,7 +35,7 @@ Main steps: - Rejects if timestamp older than now - skew or (if disallowFuture) newer than now; otherwise allows within [now - skew, now + skew]. 5) Body hashing - - Reads the request body to compute SHA-256 hex via portal.Sha256Hex, then restores the body so downstream handlers can read it again. + - Reads the request body with size limit (default 5MB) via portal.ReadWithSizeLimit to prevent DoS attacks, computes SHA-256 hex via portal.Sha256Hex, then restores the body so downstream handlers can read it again. 6) Canonical request construction - portal.BuildCanonical(method, url, username, public, ts, nonce, bodyHash) @@ -99,17 +99,18 @@ These map to the earlier docs’ Phases 1–2 checkboxes: constant-time compare, - In-memory state - Nonce cache and rate limiter are in-memory; not horizontally scalable. Replays could work across nodes, and failure limits won’t coordinate cluster-wide. - Future timestamp policy - - disallowFuture is false by default, allowing future timestamps within skew; opens small window for limited replay across clocks. + - [x] disallowFuture is false by default, allowing future timestamps within skew; opens small window for limited replay across clocks. - Key rotation and key identification - Canonical/signature doesn’t include a key identifier (kid). Rotations require coordination and lookup; currently only username+public token are provided. - Observability and audit - Logs are present but no explicit audit event for successful/failed auth with stable event schema or metrics emission. - Error handling surfaces - - All errors map to 401; may want 429 for rate-limited scopes and distinct codes for clock skew vs. formatting for better client remediation (while still generic). + - [x] Implemented appropriate status codes: 429 for rate-limited scopes and distinct error messages for clock skew vs. formatting errors for better client remediation (while still keeping messages generic). - DoS controls - Rate limiter is failure-triggered only; no general request token bucket per account/IP for auth endpoints. - Request size / body hashing - - Large bodies are fully read into memory to hash; could be abused. No explicit max size before rejection. + - [x] Implemented ReadWithSizeLimit with default 5MB limit to prevent DoS attacks from large request bodies. + - [x] Used in token_middleware.go, client.go, and markdown/handler.go for consistent protection. --- @@ -133,7 +134,7 @@ These map to the earlier docs’ Phases 1–2 checkboxes: constant-time compare, - Ensure canonicalization is frozen per version and formally documented; add conformance tests for edge cases (empty query, repeated params, unicode path). 5) Resource and abuse protections - - Enforce a maximum body size for requests that must be signed (e.g., 2–5MB) before hashing; reject larger with 413. + - ✓ Enforce a maximum body size for requests that must be signed (default 5MB) before hashing using ReadWithSizeLimit. - Add general-purpose rate limiting (token bucket) per IP/account for auth attempts and overall requests. 6) Observability and audit @@ -158,13 +159,13 @@ These map to the earlier docs’ Phases 1–2 checkboxes: constant-time compare, ## 5) Concrete implementation tasks - Replace nonceCache with interface and add RedisTTLCache implementation; wire by config. Use key "acct|nonce" with PX TTL and SET NX. -- Replace limiter.MemoryLimiter with a pluggable Limiter interface and add Redis-based sliding window or token bucket implementation. Return 429 on scope saturation. +- Replace limiter.MemoryLimiter with a pluggable Limiter interface and add Redis-based sliding window or token bucket implementation. ✓ Return 429 on scope saturation. - Default disallowFuture = true; make skew configurable via env. Add /time endpoint that returns server unix time. - Extend headers: - X-API-Key-ID (kid) - X-API-Alg (e.g., hmac-sha256;v=1) Include these in canonical string and signature verification path. Update tests. -- Add MaxBodyBytes wrapper before hashing; reject with 413 if exceeded; document limits. +- ✓ Added ReadWithSizeLimit function with default 5MB limit before hashing; rejects with generic error if exceeded. - Emit metrics and structured audit logs; create middleware counters and reason tags. - Documentation updates and client examples for canonicalization and signing including new headers. @@ -173,7 +174,7 @@ These map to the earlier docs’ Phases 1–2 checkboxes: constant-time compare, ## 6) What remains unchanged (for clarity) - The overall request flow and canonicalization approach remains; we are enriching it with distributed state, policy tightening, and key management features. -- Error messages remain generic to clients; only status codes and headers may change for rate-limiting (429) and possibly clock skew guidance headers. +- Error messages remain generic to clients; status codes now include 429 for rate-limiting and more specific (but still generic) error messages for timestamp validation. --- diff --git a/pkg/markdown/handler.go b/pkg/markdown/handler.go index 4a34d830..14f7983d 100644 --- a/pkg/markdown/handler.go +++ b/pkg/markdown/handler.go @@ -5,6 +5,7 @@ import ( "net/http" "regexp" "strings" + "time" "github.com/oullin/pkg/portal" "gopkg.in/yaml.v3" @@ -20,7 +21,7 @@ func (p Parser) Fetch() (string, error) { req.Header.Set("Cache-Control", "no-cache") req.Header.Set("Pragma", "no-cache") - client := &http.Client{} + client := &http.Client{Timeout: 10 * time.Second} resp, err := client.Do(req) if err != nil { return "", err diff --git a/pkg/middleware/token_middleware.go b/pkg/middleware/token_middleware.go index 866324f4..6749d305 100644 --- a/pkg/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -40,6 +40,11 @@ const ( // It validates required headers, enforces a timestamp skew window, prevents // replay attacks via nonce tracking, compares tokens/signatures in constant time, // and applies a basic failure-based rate limiter per client scope. +// +// Error handling: +// - Rate limiting errors return 429 Too Many Requests +// - Timestamp errors return 401 with specific messages for expired or future timestamps +// - Other authentication errors return 401 with generic messages type TokenCheckMiddleware struct { // ApiKeys provides access to persisted API key records used to resolve // account credentials (account name, public key, and secret key). @@ -84,7 +89,7 @@ func MakeTokenMiddleware(tokenHandler *auth.TokenHandler, apiKeys *repository.Ap rateLimiter: limiter.NewMemoryLimiter(1*time.Minute, 10), clockSkew: 5 * time.Minute, now: time.Now, - disallowFuture: false, + disallowFuture: true, nonceTTL: 5 * time.Minute, failWindow: 1 * time.Minute, maxFailPerScope: 10, @@ -127,8 +132,8 @@ func (t TokenCheckMiddleware) Handle(next http.ApiHandler) http.ApiHandler { clientIP := portal.ParseClientIP(r) - if t.shallReject(logger, accountName, publicToken, signature, canonical, nonce, clientIP) { - return t.getUnauthenticatedError() + if err := t.shallReject(logger, accountName, publicToken, signature, canonical, nonce, clientIP); err != nil { + return err } // Update the request context @@ -211,12 +216,12 @@ func (t TokenCheckMiddleware) attachContext(r *baseHttp.Request, accountName, re return r.WithContext(ctx) } -func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature, canonical, nonce, clientIP string) bool { +func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publicToken, signature, canonical, nonce, clientIP string) *http.ApiError { limiterKey := clientIP + "|" + strings.ToLower(accountName) if t.rateLimiter.TooMany(limiterKey) { logger.Warn("too many authentication failures", "ip", clientIP) - return true + return t.getRateLimitedError() } var item *database.APIKey @@ -224,7 +229,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ t.rateLimiter.Fail(limiterKey) logger.Warn("account not found") - return true + return t.getUnauthenticatedError() } // Fetch account to understand its keys @@ -238,7 +243,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ t.rateLimiter.Fail(limiterKey) logger.Error("failed to decode account keys", "account", item.AccountName, "error", err) - return true + return t.getUnauthenticatedError() } // Constant-time compare (fixed-length by hashing) of provided public token vs stored one @@ -251,7 +256,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ t.rateLimiter.Fail(limiterKey) logger.Warn("public token mismatch", "account", item.AccountName) - return true + return t.getUnauthenticatedError() } // Nonce replay protection: atomically check-and-mark (UseOnce) @@ -262,7 +267,7 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ t.rateLimiter.Fail(limiterKey) logger.Warn("replay detected: nonce already used", "account", item.AccountName) - return true + return t.getUnauthenticatedError() } } @@ -275,10 +280,10 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ t.rateLimiter.Fail(limiterKey) logger.Warn("signature mismatch", "account", item.AccountName) - return true + return t.getUnauthenticatedError() } - return false + return nil } func (t TokenCheckMiddleware) getInvalidRequestError() *http.ApiError { @@ -301,3 +306,24 @@ func (t TokenCheckMiddleware) getUnauthenticatedError() *http.ApiError { Status: baseHttp.StatusUnauthorized, } } + +func (t TokenCheckMiddleware) getRateLimitedError() *http.ApiError { + return &http.ApiError{ + Message: "Too many authentication attempts", + Status: baseHttp.StatusTooManyRequests, + } +} + +func (t TokenCheckMiddleware) getTimestampTooOldError() *http.ApiError { + return &http.ApiError{ + Message: "Request timestamp expired", + Status: baseHttp.StatusUnauthorized, + } +} + +func (t TokenCheckMiddleware) getTimestampTooNewError() *http.ApiError { + return &http.ApiError{ + Message: "Request timestamp invalid", + Status: baseHttp.StatusUnauthorized, + } +} diff --git a/pkg/middleware/token_middleware_test.go b/pkg/middleware/token_middleware_test.go index 6edb5bea..18b00b57 100644 --- a/pkg/middleware/token_middleware_test.go +++ b/pkg/middleware/token_middleware_test.go @@ -43,6 +43,24 @@ func TestTokenMiddlewareErrors(t *testing.T) { if e.Status != http.StatusUnauthorized { t.Fatalf("unauthenticated error") } + + e = tm.getRateLimitedError() + + if e.Status != http.StatusTooManyRequests || e.Message == "" { + t.Fatalf("rate limited error should return 429 status code") + } + + e = tm.getTimestampTooOldError() + + if e.Status != http.StatusUnauthorized || e.Message != "Request timestamp expired" { + t.Fatalf("timestamp too old error") + } + + e = tm.getTimestampTooNewError() + + if e.Status != http.StatusUnauthorized || e.Message != "Request timestamp invalid" { + t.Fatalf("timestamp too new error") + } } func TestTokenMiddlewareHandle_RequiresRequestID(t *testing.T) { @@ -347,3 +365,74 @@ func TestTokenMiddleware_DB_Integration_HappyPath(t *testing.T) { t.Fatalf("next was not called on happy path") } } + +// TestMakeTokenMiddleware_DefaultDisallowFuture verifies that disallowFuture is true by default +func TestMakeTokenMiddleware_DefaultDisallowFuture(t *testing.T) { + // Create middleware with default settings + tm := MakeTokenMiddleware(nil, nil) + + // Verify disallowFuture is true by default + if !tm.disallowFuture { + t.Fatalf("expected disallowFuture to be true by default, got false") + } +} + +// TestTokenMiddleware_RejectsFutureTimestamps verifies that future timestamps are rejected +func TestTokenMiddleware_RejectsFutureTimestamps(t *testing.T) { + conn := setupDB(t) + + // Prepare TokenHandler and seed an account with encrypted keys + th, err := auth.MakeTokensHandler(generate32(t)) + if err != nil { + t.Fatalf("MakeTokensHandler: %v", err) + } + seed, err := th.SetupNewAccount("acme-user-future") + if err != nil { + t.Fatalf("SetupNewAccount: %v", err) + } + + repo := &repository.ApiKeys{DB: conn} + if _, err := repo.Create(database.APIKeyAttr{ + AccountName: seed.AccountName, + PublicKey: seed.EncryptedPublicKey, + SecretKey: seed.EncryptedSecretKey, + }); err != nil { + t.Fatalf("repo.Create: %v", err) + } + + // Build middleware with default settings (disallowFuture = true) + tm := MakeTokenMiddleware(th, repo) + + nextCalled := false + next := func(w http.ResponseWriter, r *http.Request) *pkgHttp.ApiError { + nextCalled = true + return nil + } + handler := tm.Handle(next) + + // Create a request with a future timestamp (30 seconds in the future) + futureTime := time.Now().Add(30 * time.Second) + req := makeSignedRequest(t, + http.MethodGet, + "https://api.test.local/v1/test", + "", + seed.AccountName, + seed.PublicKey, + seed.SecretKey, + futureTime, + "n-future-1", + "rid-future-1", + ) + rec := httptest.NewRecorder() + + // The request should be rejected with a 401 Unauthorized + apiErr := handler(rec, req) + if apiErr == nil || apiErr.Status != http.StatusUnauthorized { + t.Fatalf("expected 401 for future timestamp, got %#v", apiErr) + } + + // Next handler should not be called + if nextCalled { + t.Fatalf("next should not be called when future timestamp is rejected") + } +} diff --git a/pkg/middleware/valid_timestamp.go b/pkg/middleware/valid_timestamp.go index 8bc578ef..eeb9367e 100644 --- a/pkg/middleware/valid_timestamp.go +++ b/pkg/middleware/valid_timestamp.go @@ -63,12 +63,12 @@ func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http. if epoch < minValue { v.logger.Warn("timestamp outside allowed window: too old") - return &http.ApiError{Message: "Invalid credentials", Status: baseHttp.StatusUnauthorized} + return &http.ApiError{Message: "Request timestamp expired", Status: baseHttp.StatusUnauthorized} } if epoch > maxValue { v.logger.Warn("timestamp outside allowed window: in the future") - return &http.ApiError{Message: "Invalid credentials", Status: baseHttp.StatusUnauthorized} + return &http.ApiError{Message: "Request timestamp invalid", Status: baseHttp.StatusUnauthorized} } return nil diff --git a/pkg/middleware/valid_timestamp_test.go b/pkg/middleware/valid_timestamp_test.go index 24c57c37..04f31ef7 100644 --- a/pkg/middleware/valid_timestamp_test.go +++ b/pkg/middleware/valid_timestamp_test.go @@ -49,7 +49,7 @@ func TestValidate_TooOldTimestamp(t *testing.T) { oldTs := strconv.FormatInt(base.Add(-skew).Add(-1*time.Second).Unix(), 10) vt := NewValidTimestamp(oldTs, slogNoop(), fixedClock(base)) err := vt.Validate(skew, false) - if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Invalid credentials" { + if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Request timestamp expired" { t.Fatalf("expected unauthenticated for too old timestamp, got %#v", err) } } @@ -68,7 +68,7 @@ func TestValidate_FutureWithinSkew_Behavior(t *testing.T) { // Rejected when disallowFuture=true vt = NewValidTimestamp(futureWithin, slogNoop(), fixedClock(base)) err := vt.Validate(skew, true) - if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Invalid credentials" { + if err == nil || err.Status != baseHttp.StatusUnauthorized || err.Message != "Request timestamp invalid" { t.Fatalf("expected unauthenticated for future timestamp when disallowFuture=true, got %#v", err) } } diff --git a/pkg/portal/support.go b/pkg/portal/support.go index 37902aac..44fbd374 100644 --- a/pkg/portal/support.go +++ b/pkg/portal/support.go @@ -3,6 +3,7 @@ package portal import ( "crypto/sha256" "encoding/hex" + "fmt" "io" "log/slog" "net" @@ -93,7 +94,11 @@ func ParseClientIP(r *baseHttp.Request) string { // ReadWithSizeLimit reads from an io.Reader with a size limit to prevent DoS attacks. // It returns the read bytes and any error encountered. // The default size limit is 5MB. -func ReadWithSizeLimit(r io.Reader, maxSize ...int64) ([]byte, error) { +func ReadWithSizeLimit(reader io.Reader, maxSize ...int64) ([]byte, error) { + if reader == nil { + return nil, io.ErrUnexpectedEOF + } + // Default size limit is 5MB const defaultMaxSize int64 = 5 * 1024 * 1024 // 5MB @@ -102,7 +107,12 @@ func ReadWithSizeLimit(r io.Reader, maxSize ...int64) ([]byte, error) { limit = maxSize[0] } - limitedReader := io.LimitReader(r, limit) + limitedReader := &io.LimitedReader{R: reader, N: limit + 1} + data, err := io.ReadAll(limitedReader) + + if int64(len(data)) > limit || err != nil { + return nil, fmt.Errorf("read exceeds size limit: %d, error: %w", limit, err) + } - return io.ReadAll(limitedReader) + return data, nil } diff --git a/pkg/portal/support_test.go b/pkg/portal/support_test.go index 5ab01a32..a113d049 100644 --- a/pkg/portal/support_test.go +++ b/pkg/portal/support_test.go @@ -1,7 +1,10 @@ package portal import ( + "errors" + "io" "net/url" + "strings" "testing" ) @@ -38,3 +41,96 @@ func TestBuildCanonical(t *testing.T) { t.Fatalf("unexpected canonical for nil URL: %q", got) } } + +// TestReadWithSizeLimit_NilReader tests that ReadWithSizeLimit returns an error when given a nil reader +func TestReadWithSizeLimit_NilReader(t *testing.T) { + data, err := ReadWithSizeLimit(nil) + + if data != nil { + t.Errorf("expected nil data for nil reader, got %v", data) + } + + if err != io.ErrUnexpectedEOF { + t.Errorf("expected io.ErrUnexpectedEOF for nil reader, got %v", err) + } +} + +// TestReadWithSizeLimit_DefaultLimit tests reading data within and exceeding the default size limit +func TestReadWithSizeLimit_DefaultLimit(t *testing.T) { + // Test reading data within the default limit + smallData := strings.Repeat("a", 1024) // 1KB of data + reader := strings.NewReader(smallData) + + data, err := ReadWithSizeLimit(reader) + + if err != nil { + t.Errorf("unexpected error for small data: %v", err) + } + + if string(data) != smallData { + t.Errorf("data mismatch for small read") + } + + // We can't easily test the default 5MB limit in a unit test, + // but we can test the logic by using a smaller custom limit +} + +// TestReadWithSizeLimit_CustomLimit tests reading data with a custom size limit +func TestReadWithSizeLimit_CustomLimit(t *testing.T) { + // Set a small custom limit for testing + customLimit := int64(100) + + // Test reading data within the custom limit + smallData := strings.Repeat("a", 50) + reader := strings.NewReader(smallData) + + data, err := ReadWithSizeLimit(reader, customLimit) + + if err != nil { + t.Errorf("unexpected error for data within custom limit: %v", err) + } + + if string(data) != smallData { + t.Errorf("data mismatch for read within custom limit") + } + + // Test reading data exceeding the custom limit + largeData := strings.Repeat("b", 200) // Exceeds our 100 byte limit + reader = strings.NewReader(largeData) + + data, err = ReadWithSizeLimit(reader, customLimit) + + if err == nil { + t.Error("expected error for data exceeding custom limit, got nil") + } + + if data != nil { + t.Errorf("expected nil data for exceeded limit, got %v", data) + } +} + +// TestReadWithSizeLimit_ErrorPropagation tests that ReadWithSizeLimit properly propagates errors +func TestReadWithSizeLimit_ErrorPropagation(t *testing.T) { + // Create a reader that returns an error + expectedErr := errors.New("read error") + errorReader := &ErrorReader{Err: expectedErr} + + data, err := ReadWithSizeLimit(errorReader) + + if data != nil { + t.Errorf("expected nil data for error reader, got %v", data) + } + + if err == nil || !strings.Contains(err.Error(), expectedErr.Error()) { + t.Errorf("expected error containing %q, got %v", expectedErr, err) + } +} + +// ErrorReader is a mock reader that always returns an error +type ErrorReader struct { + Err error +} + +func (r *ErrorReader) Read(p []byte) (n int, err error) { + return 0, r.Err +} From 29923d651da5ec79645ae5b0cf6cc99918564b5e Mon Sep 17 00:00:00 2001 From: Gustavo Ocanto Date: Mon, 11 Aug 2025 12:26:25 +0800 Subject: [PATCH 26/26] wip --- pkg/middleware/token_middleware.go | 24 ++++++++++++------------ pkg/middleware/valid_timestamp.go | 6 +++++- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/pkg/middleware/token_middleware.go b/pkg/middleware/token_middleware.go index 6749d305..0c6c7e3e 100644 --- a/pkg/middleware/token_middleware.go +++ b/pkg/middleware/token_middleware.go @@ -259,18 +259,6 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ return t.getUnauthenticatedError() } - // Nonce replay protection: atomically check-and-mark (UseOnce) - if t.nonceCache != nil { - key := item.AccountName + "|" + nonce - - if t.nonceCache.UseOnce(key, t.nonceTTL) { - t.rateLimiter.Fail(limiterKey) - logger.Warn("replay detected: nonce already used", "account", item.AccountName) - - return t.getUnauthenticatedError() - } - } - // Compute local signature over canonical request and compare in constant time (hash to fixed-length first) localSignature := auth.CreateSignatureFrom(canonical, token.SecretKey) hSig := sha256.Sum256([]byte(strings.TrimSpace(signature))) @@ -283,6 +271,18 @@ func (t TokenCheckMiddleware) shallReject(logger *slog.Logger, accountName, publ return t.getUnauthenticatedError() } + // Nonce replay protection: atomically check-and-mark (UseOnce) + if t.nonceCache != nil { + key := item.AccountName + "|" + nonce + + if t.nonceCache.UseOnce(key, t.nonceTTL) { + t.rateLimiter.Fail(limiterKey) + logger.Warn("replay detected: nonce already used", "account", item.AccountName) + + return t.getUnauthenticatedError() + } + } + return nil } diff --git a/pkg/middleware/valid_timestamp.go b/pkg/middleware/valid_timestamp.go index eeb9367e..88cb2dbc 100644 --- a/pkg/middleware/valid_timestamp.go +++ b/pkg/middleware/valid_timestamp.go @@ -53,7 +53,11 @@ func (v ValidTimestamp) Validate(skew time.Duration, disallowFuture bool) *http. } now := nowFn().Unix() - skewSecs := int64(skew.Seconds()) + if skew < 0 { + skew = -skew + } + + skewSecs := int64(skew / time.Second) minValue := now - skewSecs maxValue := now + skewSecs