mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
feat(auth): make auth token TTL configurable via AUTH_TOKEN_TTL env var (MUL-2371) (#2713)
* feat(auth): make auth token TTL configurable via AUTH_TOKEN_TTL env var Add AUTH_TOKEN_TTL environment variable (in seconds) to override the hardcoded 30-day auth token lifetime. Self-hosted deployments on trusted networks can set a longer value to avoid frequent magic-link re-authentication. The value is read once at startup and cached. Invalid or missing values fall back to the 30-day default with a warning log. Closes #2685 * refactor(auth): extract parseAuthTokenTTL for testability Address review feedback: extract pure parse function from sync.Once wrapper so the parsing logic can be unit-tested independently. Add TestParseAuthTokenTTL with table-driven cases. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com> * refactor(auth): accept Go duration strings + hoist shared TTL in SetAuthCookies Address nice-to-have review feedback from Bohan-J: - parseAuthTokenTTL now tries time.ParseDuration first (e.g. '8760h'), falling back to ParseInt for integer seconds - Warn on unreasonable values (>10 years) but still accept them - Hoist AuthTokenTTL() and time.Now() in SetAuthCookies so both cookies share the exact same expiry - Add security trade-off note in .env.example - Add 5 new test cases for duration strings Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com> Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com> * fix: use AuthTokenTTL() in CloudFront middleware, guard ParseInt overflow Address review feedback from Bohan-J (round 2): 1. CloudFront refresh middleware (cloudfront.go:21) was hardcoding 30*24*time.Hour instead of using auth.AuthTokenTTL(). Now calls AuthTokenTTL() so the middleware respects AUTH_TOKEN_TTL env var. 2. parseAuthTokenTTL integer-seconds branch: very large values like 9999999999 would silently overflow int64 when multiplied by time.Second. Added overflow guard comparing against math.MaxInt64/int64(time.Second) before the multiplication. 3. Updated AuthTokenTTL() doc comment to reflect that it accepts Go duration strings or integer seconds (not just seconds). 4. Added middleware test (cloudfront_test.go) verifying short AUTH_TOKEN_TTL produces short cookie expiry, not 30-day hardcode. Also covers nil signer and existing-cookie-skip cases. 5. Added integer overflow test case to cookie_test.go. * style: run gofmt on cookie.go and cookie_test.go --------- Signed-off-by: kagura-agent <kagura.agent.ai@gmail.com> Co-authored-by: Claude Opus 4 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -112,6 +112,13 @@ CLOUDFRONT_DOMAIN=
|
||||
# attribute and browsers silently drop such cookies.
|
||||
COOKIE_DOMAIN=
|
||||
|
||||
# AUTH_TOKEN_TTL — auth token lifetime. Accepts Go duration strings (e.g.
|
||||
# "8760h", "720h30m") or plain integer seconds.
|
||||
# Default: 2592000 (30 days). Self-hosted deployments on trusted networks can
|
||||
# set a longer value to reduce re-authentication frequency.
|
||||
# Note: longer TTL = longer exposure window if a cookie is leaked.
|
||||
# AUTH_TOKEN_TTL=2592000
|
||||
|
||||
# Local file storage (fallback when S3_BUCKET is not set)
|
||||
LOCAL_UPLOAD_DIR=./data/uploads
|
||||
LOCAL_UPLOAD_BASE_URL=http://localhost:8080
|
||||
|
||||
@@ -6,22 +6,87 @@ import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"log/slog"
|
||||
"math"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
const (
|
||||
AuthCookieName = "multica_auth"
|
||||
CSRFCookieName = "multica_csrf"
|
||||
authCookieMaxAge = 30 * 24 * 60 * 60 // 30 days in seconds
|
||||
AuthCookieName = "multica_auth"
|
||||
CSRFCookieName = "multica_csrf"
|
||||
defaultAuthTokenTTL = 30 * 24 * time.Hour // 30 days
|
||||
)
|
||||
|
||||
var ipCookieDomainWarnOnce sync.Once
|
||||
var (
|
||||
ipCookieDomainWarnOnce sync.Once
|
||||
authTokenTTLOnce sync.Once
|
||||
authTokenTTLCached time.Duration
|
||||
)
|
||||
|
||||
// parseAuthTokenTTL parses a raw AUTH_TOKEN_TTL value into a duration.
|
||||
// It first tries time.ParseDuration (e.g. "8760h", "720h30m"), then falls
|
||||
// back to parsing as integer seconds. Returns the parsed duration and true
|
||||
// on success; zero and false when the input is empty or invalid.
|
||||
func parseAuthTokenTTL(raw string) (time.Duration, bool) {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
return 0, false
|
||||
}
|
||||
|
||||
// Try Go duration string first (e.g. "8760h", "720h30m").
|
||||
if d, err := time.ParseDuration(raw); err == nil {
|
||||
if d <= 0 {
|
||||
return 0, false
|
||||
}
|
||||
if d > 10*365*24*time.Hour {
|
||||
slog.Warn("AUTH_TOKEN_TTL exceeds 10 years; accepting but verify this is intentional",
|
||||
"value", raw, "hours", d.Hours())
|
||||
}
|
||||
return d, true
|
||||
}
|
||||
|
||||
// Fall back to plain integer seconds.
|
||||
secs, err := strconv.ParseInt(raw, 10, 64)
|
||||
if err != nil || secs <= 0 {
|
||||
return 0, false
|
||||
}
|
||||
if secs > int64(math.MaxInt64/int64(time.Second)) {
|
||||
return 0, false
|
||||
}
|
||||
d := time.Duration(secs) * time.Second
|
||||
if d > 10*365*24*time.Hour {
|
||||
slog.Warn("AUTH_TOKEN_TTL exceeds 10 years; accepting but verify this is intentional",
|
||||
"value", raw, "hours", d.Hours())
|
||||
}
|
||||
return d, true
|
||||
}
|
||||
|
||||
// AuthTokenTTL returns the configured auth token lifetime. It reads the
|
||||
// AUTH_TOKEN_TTL environment variable (Go duration string or integer seconds) on first call and caches
|
||||
// the result. When the variable is unset or invalid the default of 30 days
|
||||
// is used.
|
||||
func AuthTokenTTL() time.Duration {
|
||||
authTokenTTLOnce.Do(func() {
|
||||
raw := os.Getenv("AUTH_TOKEN_TTL")
|
||||
if ttl, ok := parseAuthTokenTTL(raw); ok {
|
||||
authTokenTTLCached = ttl
|
||||
slog.Info("auth token TTL configured", "seconds", int(ttl.Seconds()))
|
||||
return
|
||||
}
|
||||
authTokenTTLCached = defaultAuthTokenTTL
|
||||
if strings.TrimSpace(raw) != "" {
|
||||
slog.Warn("AUTH_TOKEN_TTL is not a valid duration or positive integer; using default",
|
||||
"value", raw, "default_seconds", int(defaultAuthTokenTTL.Seconds()))
|
||||
}
|
||||
})
|
||||
return authTokenTTLCached
|
||||
}
|
||||
|
||||
// cookieDomain returns the trimmed COOKIE_DOMAIN env value, or "" if it looks
|
||||
// like an IP address. RFC 6265 §4.1.2.3 forbids IP literals in the cookie
|
||||
@@ -84,14 +149,16 @@ func generateCSRFToken(authToken string) (string, error) {
|
||||
func SetAuthCookies(w http.ResponseWriter, token string) error {
|
||||
secure := isSecureCookie()
|
||||
domain := cookieDomain()
|
||||
ttl := AuthTokenTTL()
|
||||
now := time.Now()
|
||||
|
||||
http.SetCookie(w, &http.Cookie{
|
||||
Name: AuthCookieName,
|
||||
Value: token,
|
||||
Path: "/",
|
||||
Domain: domain,
|
||||
MaxAge: authCookieMaxAge,
|
||||
Expires: time.Now().Add(30 * 24 * time.Hour),
|
||||
MaxAge: int(ttl.Seconds()),
|
||||
Expires: now.Add(ttl),
|
||||
HttpOnly: true,
|
||||
Secure: secure,
|
||||
SameSite: http.SameSiteStrictMode,
|
||||
@@ -107,8 +174,8 @@ func SetAuthCookies(w http.ResponseWriter, token string) error {
|
||||
Value: csrfToken,
|
||||
Path: "/",
|
||||
Domain: domain,
|
||||
MaxAge: authCookieMaxAge,
|
||||
Expires: time.Now().Add(30 * 24 * time.Hour),
|
||||
MaxAge: int(ttl.Seconds()),
|
||||
Expires: now.Add(ttl),
|
||||
HttpOnly: false,
|
||||
Secure: secure,
|
||||
SameSite: http.SameSiteStrictMode,
|
||||
|
||||
@@ -3,6 +3,7 @@ package auth
|
||||
import (
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestIsSecureCookie(t *testing.T) {
|
||||
@@ -81,6 +82,37 @@ func TestSetAuthCookies_HTTPSelfHost(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseAuthTokenTTL(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
raw string
|
||||
wantDur time.Duration
|
||||
wantOK bool
|
||||
}{
|
||||
{"empty string", "", 0, false},
|
||||
{"valid 3600", "3600", time.Hour, true},
|
||||
{"valid 86400", "86400", 24 * time.Hour, true},
|
||||
{"negative", "-100", 0, false},
|
||||
{"zero", "0", 0, false},
|
||||
{"non-numeric", "abc", 0, false},
|
||||
{"whitespace trimmed", " 7200 ", 2 * time.Hour, true},
|
||||
{"duration hours", "8760h", 8760 * time.Hour, true},
|
||||
{"duration compound", "720h30m", 720*time.Hour + 30*time.Minute, true},
|
||||
{"duration minutes", "90m", 90 * time.Minute, true},
|
||||
{"duration negative", "-1h", 0, false},
|
||||
{"duration zero", "0s", 0, false},
|
||||
{"integer overflow", "9999999999", 0, false},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got, ok := parseAuthTokenTTL(tc.raw)
|
||||
if ok != tc.wantOK || got != tc.wantDur {
|
||||
t.Errorf("parseAuthTokenTTL(%q) = (%v, %v), want (%v, %v)", tc.raw, got, ok, tc.wantDur, tc.wantOK)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetAuthCookies_HTTPSProduction(t *testing.T) {
|
||||
t.Setenv("FRONTEND_ORIGIN", "https://app.example.com")
|
||||
t.Setenv("COOKIE_DOMAIN", "app.example.com")
|
||||
|
||||
@@ -139,7 +139,7 @@ func (h *Handler) issueJWT(user db.User) (string, error) {
|
||||
"sub": uuidToString(user.ID),
|
||||
"email": user.Email,
|
||||
"name": user.Name,
|
||||
"exp": time.Now().Add(30 * 24 * time.Hour).Unix(),
|
||||
"exp": time.Now().Add(auth.AuthTokenTTL()).Unix(),
|
||||
"iat": time.Now().Unix(),
|
||||
})
|
||||
return token.SignedString(auth.JWTSecret())
|
||||
@@ -393,7 +393,7 @@ func (h *Handler) VerifyCode(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// Set CloudFront signed cookies for CDN access.
|
||||
if h.CFSigner != nil {
|
||||
for _, cookie := range h.CFSigner.SignedCookies(time.Now().Add(30 * 24 * time.Hour)) {
|
||||
for _, cookie := range h.CFSigner.SignedCookies(time.Now().Add(auth.AuthTokenTTL())) {
|
||||
http.SetCookie(w, cookie)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,8 @@ func RefreshCloudFrontCookies(signer *auth.CloudFrontSigner) func(http.Handler)
|
||||
}
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if _, err := r.Cookie("CloudFront-Policy"); err != nil {
|
||||
for _, cookie := range signer.SignedCookies(time.Now().Add(30 * 24 * time.Hour)) {
|
||||
ttl := auth.AuthTokenTTL()
|
||||
for _, cookie := range signer.SignedCookies(time.Now().Add(ttl)) {
|
||||
http.SetCookie(w, cookie)
|
||||
}
|
||||
}
|
||||
|
||||
102
server/internal/middleware/cloudfront_test.go
Normal file
102
server/internal/middleware/cloudfront_test.go
Normal file
@@ -0,0 +1,102 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/rsa"
|
||||
"crypto/x509"
|
||||
"encoding/base64"
|
||||
"encoding/pem"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/multica-ai/multica/server/internal/auth"
|
||||
)
|
||||
|
||||
// testSigner sets up env vars and creates a CloudFrontSigner with a throwaway RSA key.
|
||||
func testSigner(t *testing.T) *auth.CloudFrontSigner {
|
||||
t.Helper()
|
||||
key, err := rsa.GenerateKey(rand.Reader, 2048)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
pkcs8Bytes, err := x509.MarshalPKCS8PrivateKey(key)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
pemBlock := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: pkcs8Bytes})
|
||||
b64Key := base64.StdEncoding.EncodeToString(pemBlock)
|
||||
|
||||
t.Setenv("CLOUDFRONT_KEY_PAIR_ID", "TESTKEY")
|
||||
t.Setenv("CLOUDFRONT_DOMAIN", "cdn.example.com")
|
||||
t.Setenv("COOKIE_DOMAIN", ".example.com")
|
||||
t.Setenv("CLOUDFRONT_PRIVATE_KEY", b64Key)
|
||||
|
||||
signer := auth.NewCloudFrontSignerFromEnv()
|
||||
if signer == nil {
|
||||
t.Fatal("failed to create test CloudFrontSigner")
|
||||
}
|
||||
return signer
|
||||
}
|
||||
|
||||
func TestRefreshCloudFrontCookies_UsesAuthTokenTTL(t *testing.T) {
|
||||
// Set a short TTL (1 hour) so we can verify the middleware does NOT use
|
||||
// the old hardcoded 30-day value.
|
||||
t.Setenv("AUTH_TOKEN_TTL", "1h")
|
||||
|
||||
signer := testSigner(t)
|
||||
handler := RefreshCloudFrontCookies(signer)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
cookies := rec.Result().Cookies()
|
||||
if len(cookies) == 0 {
|
||||
t.Fatal("expected CloudFront cookies to be set")
|
||||
}
|
||||
|
||||
for _, c := range cookies {
|
||||
// Cookie expiry should be ~1 hour from now, not ~30 days.
|
||||
untilExpiry := time.Until(c.Expires)
|
||||
if untilExpiry > 2*time.Hour {
|
||||
t.Errorf("cookie %q expires in %v; expected ~1h (AUTH_TOKEN_TTL), got what looks like 30-day hardcode", c.Name, untilExpiry)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRefreshCloudFrontCookies_NilSigner(t *testing.T) {
|
||||
handler := RefreshCloudFrontCookies(nil)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if len(rec.Result().Cookies()) != 0 {
|
||||
t.Error("nil signer should not set any cookies")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRefreshCloudFrontCookies_SkipsWhenCookiePresent(t *testing.T) {
|
||||
signer := testSigner(t)
|
||||
handler := RefreshCloudFrontCookies(signer)(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.AddCookie(&http.Cookie{Name: "CloudFront-Policy", Value: "existing"})
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if len(rec.Result().Cookies()) != 0 {
|
||||
t.Error("should not refresh cookies when CloudFront-Policy is already present")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user