-
Notifications
You must be signed in to change notification settings - Fork 38
✨ Add authlib/ — shared token exchange building blocks (Phase 1) #310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
4603cdb
✨ Add authlib/ — shared token exchange building blocks (Phase 1)
huang195 ead07d9
🐛 Fix review findings: context propagation, response limits, tests
huang195 ed00dab
⬆️ Bump golang.org/x/crypto to v0.48.0 (fix CVE in dependency review)
huang195 a5623ea
Fix PR #310 review: enforce audience, remove nil Header, add JWKS tests
huang195 080f19f
Fix review findings: actor cache TTL, t.Fatal in handlers, JWKS deriv…
huang195 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Package bypass provides path pattern matching for skipping JWT validation | ||
| // on public endpoints (e.g., health probes, agent card discovery). | ||
| package bypass | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "path" | ||
| "strings" | ||
| ) | ||
|
|
||
| // DefaultPatterns are paths that skip inbound JWT validation by default. | ||
| var DefaultPatterns = []string{"/.well-known/*", "/healthz", "/readyz", "/livez"} | ||
|
|
||
| // Matcher checks request paths against a set of bypass patterns. | ||
| // Patterns use Go's path.Match syntax (e.g., "/.well-known/*"). | ||
| // Note: path.Match's "*" does NOT cross "/" separators, so "/.well-known/*" | ||
| // matches "/.well-known/agent.json" but NOT "/.well-known/foo/bar". | ||
| type Matcher struct { | ||
| patterns []string | ||
| } | ||
|
|
||
| // NewMatcher creates a Matcher from the given patterns. | ||
| // Returns an error if any pattern has invalid path.Match syntax. | ||
| func NewMatcher(patterns []string) (*Matcher, error) { | ||
| for _, p := range patterns { | ||
| if _, err := path.Match(p, "/"); err != nil { | ||
| return nil, fmt.Errorf("invalid bypass pattern %q: %w", p, err) | ||
| } | ||
| } | ||
| return &Matcher{patterns: patterns}, nil | ||
| } | ||
|
|
||
| // Match checks if the given request path matches any bypass pattern. | ||
| // Query strings are stripped and the path is normalized via path.Clean | ||
| // to prevent bypass via non-canonical forms (e.g., //healthz, /./healthz). | ||
| func (m *Matcher) Match(requestPath string) bool { | ||
| if idx := strings.IndexByte(requestPath, '?'); idx >= 0 { | ||
| requestPath = requestPath[:idx] | ||
| } | ||
| requestPath = path.Clean(requestPath) | ||
| for _, pattern := range m.patterns { | ||
| if matched, _ := path.Match(pattern, requestPath); matched { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| package bypass | ||
|
|
||
| import ( | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestNewMatcher_ValidPatterns(t *testing.T) { | ||
| _, err := NewMatcher(DefaultPatterns) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestNewMatcher_InvalidPattern(t *testing.T) { | ||
| _, err := NewMatcher([]string{"[invalid"}) | ||
| if err == nil { | ||
| t.Fatal("expected error for invalid pattern") | ||
| } | ||
| } | ||
|
|
||
| func TestMatch(t *testing.T) { | ||
| m, err := NewMatcher(DefaultPatterns) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| tests := []struct { | ||
| path string | ||
| match bool | ||
| }{ | ||
| // Default bypass paths | ||
| {"/.well-known/agent.json", true}, | ||
| {"/.well-known/openid-configuration", true}, | ||
| {"/healthz", true}, | ||
| {"/readyz", true}, | ||
| {"/livez", true}, | ||
|
|
||
| // Non-bypass paths | ||
| {"/test", false}, | ||
| {"/api/v1/agents", false}, | ||
| {"/", false}, | ||
|
|
||
| // Query string stripping | ||
| {"/healthz?verbose=true", true}, | ||
| {"/.well-known/agent.json?format=json", true}, | ||
|
|
||
| // Path normalization (security) | ||
| {"//healthz", true}, | ||
| {"/./healthz", true}, | ||
| {"/foo/../healthz", true}, | ||
|
|
||
| // Well-known does NOT match nested paths (path.Match * doesn't cross /) | ||
| {"/.well-known/foo/bar", false}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| got := m.Match(tt.path) | ||
| if got != tt.match { | ||
| t.Errorf("Match(%q) = %v, want %v", tt.path, got, tt.match) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func TestMatch_EmptyPatterns(t *testing.T) { | ||
| m, _ := NewMatcher(nil) | ||
| if m.Match("/healthz") { | ||
| t.Error("empty matcher should not match anything") | ||
| } | ||
| } | ||
|
|
||
| func TestMatch_CustomPatterns(t *testing.T) { | ||
| m, err := NewMatcher([]string{"/public/*", "/status"}) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
| if !m.Match("/public/index.html") { | ||
| t.Error("expected /public/index.html to match") | ||
| } | ||
| if !m.Match("/status") { | ||
| t.Error("expected /status to match") | ||
| } | ||
| if m.Match("/private/data") { | ||
| t.Error("expected /private/data to not match") | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| // Package cache provides a SHA-256 keyed token cache with TTL-based eviction. | ||
| package cache | ||
|
|
||
| import ( | ||
| "crypto/sha256" | ||
| "encoding/hex" | ||
| "sync" | ||
| "time" | ||
| ) | ||
|
|
||
| type entry struct { | ||
| token string | ||
| expiresAt time.Time | ||
| } | ||
|
|
||
| // Cache is a thread-safe token cache keyed by SHA-256 hash of (subjectToken, audience). | ||
| type Cache struct { | ||
| mu sync.RWMutex | ||
| entries map[string]entry | ||
| maxSize int | ||
| } | ||
|
|
||
| // Option configures cache behavior. | ||
| type Option func(*Cache) | ||
|
|
||
| // WithMaxSize sets the maximum number of cache entries. | ||
| // When exceeded, expired entries are evicted first; if still full, all entries are cleared. | ||
| func WithMaxSize(n int) Option { | ||
| return func(c *Cache) { c.maxSize = n } | ||
| } | ||
|
|
||
| // New creates a token cache. | ||
| func New(opts ...Option) *Cache { | ||
| c := &Cache{ | ||
| entries: make(map[string]entry), | ||
| maxSize: 10000, | ||
| } | ||
| for _, o := range opts { | ||
| o(c) | ||
| } | ||
| return c | ||
| } | ||
|
|
||
| // Get returns a cached token for the given subject token and audience. | ||
| // Returns ("", false) if not found or expired. Expired entries are left | ||
| // for eviction during Set() to avoid write-lock contention on reads. | ||
| func (c *Cache) Get(subjectToken, audience string) (string, bool) { | ||
| key := cacheKey(subjectToken, audience) | ||
| c.mu.RLock() | ||
| e, ok := c.entries[key] | ||
| c.mu.RUnlock() | ||
| if !ok || time.Now().After(e.expiresAt) { | ||
| return "", false | ||
| } | ||
| return e.token, true | ||
| } | ||
|
|
||
| // Set stores a token with the given TTL. A buffer of 30 seconds is subtracted | ||
| // from the TTL to ensure tokens are refreshed before they expire. | ||
| func (c *Cache) Set(subjectToken, audience, token string, ttl time.Duration) { | ||
| if ttl <= 30*time.Second { | ||
| return // too short to cache | ||
| } | ||
| key := cacheKey(subjectToken, audience) | ||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
| if len(c.entries) >= c.maxSize { | ||
| c.evictExpired() | ||
| if len(c.entries) >= c.maxSize { | ||
| // TODO: Consider LRU or random-sample eviction for high-cardinality | ||
| // traffic. Full clear can cause temporary cache-miss storms. | ||
| c.entries = make(map[string]entry) | ||
| } | ||
| } | ||
| c.entries[key] = entry{ | ||
| token: token, | ||
| expiresAt: time.Now().Add(ttl - 30*time.Second), | ||
| } | ||
| } | ||
|
|
||
| // Len returns the number of entries (including potentially expired ones). | ||
| func (c *Cache) Len() int { | ||
| c.mu.RLock() | ||
| defer c.mu.RUnlock() | ||
| return len(c.entries) | ||
| } | ||
|
|
||
| func (c *Cache) evictExpired() { | ||
| now := time.Now() | ||
| for k, e := range c.entries { | ||
| if now.After(e.expiresAt) { | ||
| delete(c.entries, k) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func cacheKey(subjectToken, audience string) string { | ||
| h := sha256.New() | ||
| h.Write([]byte(subjectToken)) | ||
| h.Write([]byte{0}) // separator | ||
| h.Write([]byte(audience)) | ||
| return hex.EncodeToString(h.Sum(nil)) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| package cache | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| func TestGetSet(t *testing.T) { | ||
| c := New() | ||
| c.Set("token-abc", "aud-1", "exchanged-xyz", 5*time.Minute) | ||
|
|
||
| got, ok := c.Get("token-abc", "aud-1") | ||
| if !ok || got != "exchanged-xyz" { | ||
| t.Errorf("Get() = (%q, %v), want (%q, true)", got, ok, "exchanged-xyz") | ||
| } | ||
| } | ||
|
|
||
| func TestGetMiss(t *testing.T) { | ||
| c := New() | ||
| _, ok := c.Get("nonexistent", "aud") | ||
| if ok { | ||
| t.Error("expected cache miss") | ||
| } | ||
| } | ||
|
|
||
| func TestGetDifferentAudience(t *testing.T) { | ||
| c := New() | ||
| c.Set("token-abc", "aud-1", "exchanged-1", 5*time.Minute) | ||
|
|
||
| _, ok := c.Get("token-abc", "aud-2") | ||
| if ok { | ||
| t.Error("expected cache miss for different audience") | ||
| } | ||
| } | ||
|
|
||
| func TestTTLTooShort(t *testing.T) { | ||
| c := New() | ||
| c.Set("token", "aud", "val", 20*time.Second) // below 30s threshold | ||
| if c.Len() != 0 { | ||
| t.Error("expected no entry for TTL below threshold") | ||
| } | ||
| } | ||
|
|
||
| func TestMaxSize(t *testing.T) { | ||
| c := New(WithMaxSize(2)) | ||
| c.Set("t1", "a", "v1", 5*time.Minute) | ||
| c.Set("t2", "a", "v2", 5*time.Minute) | ||
| c.Set("t3", "a", "v3", 5*time.Minute) // should trigger eviction | ||
|
|
||
| if c.Len() > 2 { | ||
| t.Errorf("cache has %d entries, want <= 2", c.Len()) | ||
| } | ||
| } | ||
|
|
||
| func TestCacheKeyCollisionResistance(t *testing.T) { | ||
| // Keys must differ when tokens or audiences differ | ||
| k1 := cacheKey("abc", "def") | ||
| k2 := cacheKey("ab", "cdef") | ||
| k3 := cacheKey("abc", "de") | ||
| if k1 == k2 { | ||
| t.Error("different token/audience combos produced same key") | ||
| } | ||
| if k1 == k3 { | ||
| t.Error("different audience produced same key") | ||
| } | ||
| } | ||
|
|
||
| func TestConcurrentAccess(t *testing.T) { | ||
| c := New() | ||
| done := make(chan struct{}) | ||
| for i := range 100 { | ||
| go func(n int) { | ||
| defer func() { done <- struct{}{} }() | ||
| token := string(rune('A' + n%26)) | ||
| c.Set(token, "aud", "val", 5*time.Minute) | ||
| c.Get(token, "aud") | ||
| }(i) | ||
| } | ||
| for range 100 { | ||
| <-done | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| package exchange | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/url" | ||
| ) | ||
|
|
||
| // ClientAuth applies client authentication to a token exchange request. | ||
| // Implementations modify the form values to include credentials. | ||
| type ClientAuth interface { | ||
| Apply(ctx context.Context, form url.Values) error | ||
| } | ||
|
|
||
| // ClientSecretAuth authenticates using client_id and client_secret in the form body. | ||
| type ClientSecretAuth struct { | ||
| ClientID string | ||
| ClientSecret string | ||
| } | ||
|
|
||
| func (a *ClientSecretAuth) Apply(_ context.Context, form url.Values) error { | ||
| form.Set("client_id", a.ClientID) | ||
| form.Set("client_secret", a.ClientSecret) | ||
| return nil | ||
| } | ||
|
|
||
| // JWTAssertionAuth authenticates using a JWT client assertion (e.g., SPIFFE JWT-SVID). | ||
| // The tokenSource is called on every request to support key rotation. | ||
| type JWTAssertionAuth struct { | ||
| ClientID string | ||
| AssertionType string // e.g., "urn:ietf:params:oauth:client-assertion-type:jwt-spiffe" | ||
| TokenSource func(ctx context.Context) (string, error) | ||
| } | ||
|
|
||
| func (a *JWTAssertionAuth) Apply(ctx context.Context, form url.Values) error { | ||
| token, err := a.TokenSource(ctx) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| form.Set("client_id", a.ClientID) | ||
| form.Set("client_assertion", token) | ||
| form.Set("client_assertion_type", a.AssertionType) | ||
| return nil | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The nuclear eviction (clear all entries) when the cache is full and no expired entries exist could cause temporary cache-miss storms under sustained high-cardinality traffic. Not a problem for Phase 1 volumes, but worth a
// TODO: consider LRU or random-sample eviction for Phase 2marker so it doesn't get forgotten when this is wired into the hot path.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added TODO comment in b2ca966. The nuclear eviction is acceptable for Phase 1 volumes. Will consider LRU or random-sample eviction when this is wired into the hot path.