Skip to content

Commit e198060

Browse files
JAORMXclaude
andauthored
Remove GetClaimsFromContext backward compatibility helper (#2443)
Remove GetClaimsFromContext function and migrate all usages to IdentityFromContext pattern following the authentication unification completed in d32f2bf. Changes: - Remove GetClaimsFromContext() from pkg/auth/context.go - Update all test files to use IdentityFromContext() and access identity.Claims directly when needed - Remove dedicated GetClaimsFromContext test functions - Remove unused jwt imports from test files Rationale: GetClaimsFromContext was added as a backward-compatibility helper during the Identity struct unification. All production code has migrated to using IdentityFromContext directly, with zero production usages remaining. Tests should verify the actual production API contract. Impact: - Removes ~130 lines of code (function + tests) - Simplifies the auth API to a single pattern - All tests pass with improved clarity 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9e5924e commit e198060

File tree

6 files changed

+50
-227
lines changed

6 files changed

+50
-227
lines changed

pkg/auth/anonymous_test.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,32 @@ import (
1212

1313
func TestAnonymousMiddleware(t *testing.T) {
1414
t.Parallel()
15-
// Create a test handler that checks for claims in the context
15+
// Create a test handler that checks for identity in the context
1616
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
17-
claims, ok := GetClaimsFromContext(r.Context())
18-
require.True(t, ok, "Expected claims to be present in context")
17+
identity, ok := IdentityFromContext(r.Context())
18+
require.True(t, ok, "Expected identity to be present in context")
19+
require.NotNil(t, identity, "Expected identity to be non-nil")
20+
21+
// Verify the identity fields
22+
assert.Equal(t, "anonymous", identity.Subject)
23+
assert.Equal(t, "Anonymous User", identity.Name)
24+
assert.Equal(t, "anonymous@localhost", identity.Email)
1925

2026
// Verify the anonymous claims
21-
assert.Equal(t, "anonymous", claims["sub"])
22-
assert.Equal(t, "toolhive-local", claims["iss"])
23-
assert.Equal(t, "toolhive", claims["aud"])
24-
assert.Equal(t, "anonymous@localhost", claims["email"])
25-
assert.Equal(t, "Anonymous User", claims["name"])
27+
require.NotNil(t, identity.Claims)
28+
assert.Equal(t, "anonymous", identity.Claims["sub"])
29+
assert.Equal(t, "toolhive-local", identity.Claims["iss"])
30+
assert.Equal(t, "toolhive", identity.Claims["aud"])
31+
assert.Equal(t, "anonymous@localhost", identity.Claims["email"])
32+
assert.Equal(t, "Anonymous User", identity.Claims["name"])
2633

2734
// Verify timestamps are reasonable
2835
now := time.Now().Unix()
29-
exp, ok := claims["exp"].(int64)
36+
exp, ok := identity.Claims["exp"].(int64)
3037
require.True(t, ok, "Expected exp to be present and be an int64")
3138
assert.Greater(t, exp, now, "Expected exp to be in the future")
3239

33-
iat, ok := claims["iat"].(int64)
40+
iat, ok := identity.Claims["iat"].(int64)
3441
require.True(t, ok, "Expected iat to be present and be an int64")
3542
assert.LessOrEqual(t, iat, now+1, "Expected iat to be current time or earlier (with 1 second tolerance)")
3643

pkg/auth/context.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,24 +50,6 @@ func IdentityFromContext(ctx context.Context) (*Identity, bool) {
5050
return identity, ok
5151
}
5252

53-
// GetClaimsFromContext retrieves the claims from Identity in the request context.
54-
// This is a helper function for backward compatibility with code that expects MapClaims.
55-
// New code should use IdentityFromContext and access the Claims field directly.
56-
func GetClaimsFromContext(ctx context.Context) (jwt.MapClaims, bool) {
57-
if ctx == nil {
58-
return nil, false
59-
}
60-
61-
// Get Identity and return its Claims
62-
if identity, ok := IdentityFromContext(ctx); ok && identity != nil {
63-
if identity.Claims != nil {
64-
return jwt.MapClaims(identity.Claims), true
65-
}
66-
}
67-
68-
return nil, false
69-
}
70-
7153
// claimsToIdentity converts JWT claims to Identity struct.
7254
// It requires the 'sub' claim per OIDC Core 1.0 spec § 5.1.
7355
// The original token can be provided for passthrough scenarios.

pkg/auth/context_test.go

Lines changed: 0 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"testing"
66

7-
"github.com/golang-jwt/jwt/v5"
87
"github.com/stretchr/testify/assert"
98
"github.com/stretchr/testify/require"
109
)
@@ -92,89 +91,6 @@ func TestIdentityContext_ExplicitNilValue(t *testing.T) {
9291
assert.Nil(t, identity, "expected nil identity")
9392
}
9493

95-
// TestGetClaimsFromContext_EdgeCases verifies backward-compatible claims retrieval edge cases.
96-
func TestGetClaimsFromContext_EdgeCases(t *testing.T) {
97-
t.Parallel()
98-
99-
tests := []struct {
100-
name string
101-
setupCtx func() context.Context
102-
wantOk bool
103-
checkFunc func(t *testing.T, claims jwt.MapClaims)
104-
}{
105-
{
106-
name: "identity_with_claims",
107-
setupCtx: func() context.Context {
108-
identity := &Identity{
109-
Subject: "user123",
110-
Claims: map[string]any{
111-
"sub": "user123",
112-
"org_id": "org456",
113-
},
114-
}
115-
return WithIdentity(context.Background(), identity)
116-
},
117-
wantOk: true,
118-
checkFunc: func(t *testing.T, claims jwt.MapClaims) {
119-
t.Helper()
120-
assert.Equal(t, "user123", claims["sub"])
121-
assert.Equal(t, "org456", claims["org_id"])
122-
},
123-
},
124-
{
125-
name: "identity_with_nil_claims",
126-
setupCtx: func() context.Context {
127-
identity := &Identity{
128-
Subject: "user123",
129-
Claims: nil,
130-
}
131-
return WithIdentity(context.Background(), identity)
132-
},
133-
wantOk: false,
134-
},
135-
{
136-
name: "no_identity",
137-
setupCtx: func() context.Context {
138-
return context.Background()
139-
},
140-
wantOk: false,
141-
},
142-
{
143-
name: "nil_context",
144-
setupCtx: func() context.Context {
145-
return nil
146-
},
147-
wantOk: false,
148-
},
149-
{
150-
name: "explicitly_nil_identity",
151-
setupCtx: func() context.Context {
152-
return context.WithValue(context.Background(), IdentityContextKey{}, (*Identity)(nil))
153-
},
154-
wantOk: false,
155-
},
156-
}
157-
158-
for _, tt := range tests {
159-
t.Run(tt.name, func(t *testing.T) {
160-
t.Parallel()
161-
162-
ctx := tt.setupCtx()
163-
claims, ok := GetClaimsFromContext(ctx)
164-
165-
assert.Equal(t, tt.wantOk, ok)
166-
if tt.wantOk {
167-
require.NotNil(t, claims)
168-
if tt.checkFunc != nil {
169-
tt.checkFunc(t, claims)
170-
}
171-
} else {
172-
assert.Nil(t, claims)
173-
}
174-
})
175-
}
176-
}
177-
17894
// TestIdentityContext_Overwrite verifies that storing a new identity replaces the old one.
17995
func TestIdentityContext_Overwrite(t *testing.T) {
18096
t.Parallel()

pkg/auth/local_test.go

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,32 @@ func TestLocalUserMiddleware(t *testing.T) {
1414
t.Parallel()
1515
username := "testuser"
1616

17-
// Create a test handler that checks for claims in the context
17+
// Create a test handler that checks for identity in the context
1818
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
19-
claims, ok := GetClaimsFromContext(r.Context())
20-
require.True(t, ok, "Expected claims to be present in context")
19+
identity, ok := IdentityFromContext(r.Context())
20+
require.True(t, ok, "Expected identity to be present in context")
21+
require.NotNil(t, identity, "Expected identity to be non-nil")
22+
23+
// Verify the identity fields
24+
assert.Equal(t, username, identity.Subject)
25+
assert.Equal(t, "Local User: "+username, identity.Name)
26+
assert.Equal(t, username+"@localhost", identity.Email)
2127

2228
// Verify the local user claims
23-
assert.Equal(t, username, claims["sub"])
24-
assert.Equal(t, "toolhive-local", claims["iss"])
25-
assert.Equal(t, "toolhive", claims["aud"])
26-
assert.Equal(t, username+"@localhost", claims["email"])
27-
assert.Equal(t, "Local User: "+username, claims["name"])
29+
require.NotNil(t, identity.Claims)
30+
assert.Equal(t, username, identity.Claims["sub"])
31+
assert.Equal(t, "toolhive-local", identity.Claims["iss"])
32+
assert.Equal(t, "toolhive", identity.Claims["aud"])
33+
assert.Equal(t, username+"@localhost", identity.Claims["email"])
34+
assert.Equal(t, "Local User: "+username, identity.Claims["name"])
2835

2936
// Verify timestamps are reasonable
3037
now := time.Now().Unix()
31-
exp, ok := claims["exp"].(int64)
38+
exp, ok := identity.Claims["exp"].(int64)
3239
require.True(t, ok, "Expected exp to be present and be an int64")
3340
assert.Greater(t, exp, now, "Expected exp to be in the future")
3441

35-
iat, ok := claims["iat"].(int64)
42+
iat, ok := identity.Claims["iat"].(int64)
3643
require.True(t, ok, "Expected iat to be present and be an int64")
3744
assert.LessOrEqual(t, iat, now+1, "Expected iat to be current time or earlier (with 1 second tolerance)")
3845

@@ -63,11 +70,12 @@ func TestLocalUserMiddlewareWithDifferentUsernames(t *testing.T) {
6370
t.Run("username_"+username, func(t *testing.T) {
6471
t.Parallel()
6572
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
66-
claims, ok := GetClaimsFromContext(r.Context())
67-
require.True(t, ok, "Expected claims to be present in context")
73+
identity, ok := IdentityFromContext(r.Context())
74+
require.True(t, ok, "Expected identity to be present in context")
75+
require.NotNil(t, identity, "Expected identity to be non-nil")
6876

69-
assert.Equal(t, username, claims["sub"])
70-
assert.Equal(t, username+"@localhost", claims["email"])
77+
assert.Equal(t, username, identity.Subject)
78+
assert.Equal(t, username+"@localhost", identity.Email)
7179

7280
w.WriteHeader(http.StatusOK)
7381
})

pkg/auth/token_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,17 +242,17 @@ func TestTokenValidatorMiddleware(t *testing.T) {
242242

243243
// Create a test handler
244244
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
245-
// Get the claims from the context using the helper function
246-
claims, ok := GetClaimsFromContext(r.Context())
247-
if !ok {
248-
t.Errorf("Failed to get claims from context")
249-
http.Error(w, "Failed to get claims from context", http.StatusInternalServerError)
245+
// Get the identity from the context
246+
identity, ok := IdentityFromContext(r.Context())
247+
if !ok || identity == nil {
248+
t.Errorf("Failed to get identity from context")
249+
http.Error(w, "Failed to get identity from context", http.StatusInternalServerError)
250250
return
251251
}
252252

253253
// Write the claims as the response
254254
w.Header().Set("Content-Type", "application/json")
255-
if err := json.NewEncoder(w).Encode(claims); err != nil {
255+
if err := json.NewEncoder(w).Encode(identity.Claims); err != nil {
256256
t.Errorf("Failed to encode claims: %v", err)
257257
http.Error(w, fmt.Sprintf("Failed to encode claims: %v", err), http.StatusInternalServerError)
258258
return

pkg/auth/utils_test.go

Lines changed: 5 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"net/http/httptest"
88
"testing"
99

10-
"github.com/golang-jwt/jwt/v5"
1110
"github.com/stretchr/testify/assert"
1211
"github.com/stretchr/testify/require"
1312

@@ -105,97 +104,6 @@ func TestExtractBearerToken(t *testing.T) {
105104
}
106105
}
107106

108-
func TestGetClaimsFromContext(t *testing.T) {
109-
t.Parallel()
110-
// Test with claims in context
111-
claims := jwt.MapClaims{
112-
"sub": "testuser",
113-
"iss": "test-issuer",
114-
"aud": "test-audience",
115-
}
116-
identity := &Identity{Subject: "testuser", Claims: claims}
117-
ctx := WithIdentity(context.Background(), identity)
118-
119-
retrievedClaims, ok := GetClaimsFromContext(ctx)
120-
require.True(t, ok, "Expected to retrieve claims from context")
121-
assert.Equal(t, "testuser", retrievedClaims["sub"])
122-
assert.Equal(t, "test-issuer", retrievedClaims["iss"])
123-
124-
// Test with no identity in context
125-
emptyCtx := context.Background()
126-
_, ok = GetClaimsFromContext(emptyCtx)
127-
assert.False(t, ok, "Expected no claims to be found in empty context")
128-
129-
// Test with identity that has nil claims
130-
identityWithNilClaims := &Identity{Subject: "testuser", Claims: nil}
131-
ctxWithNilClaims := WithIdentity(context.Background(), identityWithNilClaims)
132-
_, ok = GetClaimsFromContext(ctxWithNilClaims)
133-
assert.False(t, ok, "Expected no claims to be found when identity has nil claims")
134-
135-
// Test with nil context - we intentionally pass nil to test the nil check
136-
//nolint:staticcheck // SA1012: Testing nil context handling is intentional
137-
_, ok = GetClaimsFromContext(nil)
138-
assert.False(t, ok, "Expected no claims to be found with nil context")
139-
}
140-
141-
func TestGetClaimsFromContextWithDifferentClaimTypes(t *testing.T) {
142-
t.Parallel()
143-
testCases := []struct {
144-
name string
145-
claims jwt.MapClaims
146-
expected map[string]interface{}
147-
}{
148-
{
149-
name: "string_claims",
150-
claims: jwt.MapClaims{
151-
"sub": "user123",
152-
"email": "user@example.com",
153-
"name": "Test User",
154-
},
155-
expected: map[string]interface{}{
156-
"sub": "user123",
157-
"email": "user@example.com",
158-
"name": "Test User",
159-
},
160-
},
161-
{
162-
name: "mixed_claims",
163-
claims: jwt.MapClaims{
164-
"sub": "user123",
165-
"exp": int64(1234567890),
166-
"iat": int64(1234567800),
167-
"admin": true,
168-
},
169-
expected: map[string]interface{}{
170-
"sub": "user123",
171-
"exp": int64(1234567890),
172-
"iat": int64(1234567800),
173-
"admin": true,
174-
},
175-
},
176-
{
177-
name: "empty_claims",
178-
claims: jwt.MapClaims{},
179-
expected: map[string]interface{}{},
180-
},
181-
}
182-
183-
for _, tc := range testCases {
184-
t.Run(tc.name, func(t *testing.T) {
185-
t.Parallel()
186-
identity := &Identity{Subject: "test-user", Claims: tc.claims}
187-
ctx := WithIdentity(context.Background(), identity)
188-
retrievedClaims, ok := GetClaimsFromContext(ctx)
189-
190-
require.True(t, ok, "Expected to retrieve claims from context")
191-
192-
for key, expectedValue := range tc.expected {
193-
assert.Equal(t, expectedValue, retrievedClaims[key], "Expected %s to be %v, got %v", key, expectedValue, retrievedClaims[key])
194-
}
195-
})
196-
}
197-
}
198-
199107
func TestGetAuthenticationMiddleware(t *testing.T) {
200108
t.Parallel()
201109
// Initialize logger for testing
@@ -210,9 +118,11 @@ func TestGetAuthenticationMiddleware(t *testing.T) {
210118

211119
// Test that the middleware works by creating a test handler
212120
testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
213-
claims, ok := GetClaimsFromContext(r.Context())
214-
require.True(t, ok, "Expected claims to be present in context")
215-
assert.Equal(t, "toolhive-local", claims["iss"])
121+
identity, ok := IdentityFromContext(r.Context())
122+
require.True(t, ok, "Expected identity to be present in context")
123+
require.NotNil(t, identity, "Expected identity to be non-nil")
124+
require.NotNil(t, identity.Claims, "Expected claims to be present")
125+
assert.Equal(t, "toolhive-local", identity.Claims["iss"])
216126
w.WriteHeader(http.StatusOK)
217127
})
218128

0 commit comments

Comments
 (0)