Skip to content

Commit a41d683

Browse files
mattdhollowaymattdholloway
committed
token extraction middleware 401 change
1 parent a56a5b5 commit a41d683

File tree

3 files changed

+236
-1
lines changed

3 files changed

+236
-1
lines changed

‎pkg/http/middleware/token.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ func ExtractUserToken(oauthCfg *oauth.Config) func(next http.Handler) http.Handl
5050
sendAuthChallenge(w, r, oauthCfg)
5151
return
5252
}
53-
// For other auth errors (bad format, unsupported), return 400
53+
// For invalid token format, behavior depends on ValidateTokenFormat config:
54+
// - When true: return 401 with WWW-Authenticate header (local server validates)
55+
// - When false: return 400 Bad Request (CAPI/remote validates)
56+
if oauthCfg != nil && oauthCfg.ValidateTokenFormat {
57+
sendAuthChallenge(w, r, oauthCfg)
58+
return
59+
}
5460
http.Error(w, err.Error(), http.StatusBadRequest)
5561
return
5662
}

‎pkg/http/middleware/token_test.go‎

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
package middleware
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"strings"
7+
"testing"
8+
9+
"github.com/github/github-mcp-server/pkg/http/oauth"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestExtractUserToken(t *testing.T) {
15+
dummyHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
16+
w.WriteHeader(http.StatusOK)
17+
})
18+
19+
tests := []struct {
20+
name string
21+
authHeader string
22+
validateTokenFormat bool
23+
expectedStatus int
24+
expectWWWAuth bool
25+
}{
26+
{
27+
name: "valid PAT token passes through",
28+
authHeader: "Bearer ghp_test1234567890123456789012345678901234",
29+
validateTokenFormat: false,
30+
expectedStatus: http.StatusOK,
31+
expectWWWAuth: false,
32+
},
33+
{
34+
name: "valid fine-grained PAT passes through",
35+
authHeader: "Bearer github_pat_test1234567890123456789012345678901234",
36+
validateTokenFormat: false,
37+
expectedStatus: http.StatusOK,
38+
expectWWWAuth: false,
39+
},
40+
{
41+
name: "valid OAuth token passes through",
42+
authHeader: "Bearer gho_test1234567890123456789012345678901234",
43+
validateTokenFormat: false,
44+
expectedStatus: http.StatusOK,
45+
expectWWWAuth: false,
46+
},
47+
{
48+
name: "valid old-style token passes through",
49+
authHeader: "Bearer 1234567890abcdef1234567890abcdef12345678",
50+
validateTokenFormat: false,
51+
expectedStatus: http.StatusOK,
52+
expectWWWAuth: false,
53+
},
54+
{
55+
name: "IDE token passes through",
56+
authHeader: "Bearer tid=1;exp=25145314523;chat=1:hmac",
57+
validateTokenFormat: false,
58+
expectedStatus: http.StatusOK,
59+
expectWWWAuth: false,
60+
},
61+
{
62+
name: "missing auth header returns 401 with WWW-Authenticate",
63+
authHeader: "",
64+
validateTokenFormat: false,
65+
expectedStatus: http.StatusUnauthorized,
66+
expectWWWAuth: true,
67+
},
68+
{
69+
name: "invalid token format returns 400 when ValidateTokenFormat is false",
70+
authHeader: "Bearer invalid_token",
71+
validateTokenFormat: false,
72+
expectedStatus: http.StatusBadRequest,
73+
expectWWWAuth: false,
74+
},
75+
{
76+
name: "invalid token format returns 401 with WWW-Authenticate when ValidateTokenFormat is true",
77+
authHeader: "Bearer invalid_token",
78+
validateTokenFormat: true,
79+
expectedStatus: http.StatusUnauthorized,
80+
expectWWWAuth: true,
81+
},
82+
{
83+
name: "GitHub-Bearer prefix returns 400 when ValidateTokenFormat is false",
84+
authHeader: "GitHub-Bearer some_token",
85+
validateTokenFormat: false,
86+
expectedStatus: http.StatusBadRequest,
87+
expectWWWAuth: false,
88+
},
89+
{
90+
name: "GitHub-Bearer prefix returns 401 when ValidateTokenFormat is true",
91+
authHeader: "GitHub-Bearer some_token",
92+
validateTokenFormat: true,
93+
expectedStatus: http.StatusUnauthorized,
94+
expectWWWAuth: true,
95+
},
96+
}
97+
98+
for _, tc := range tests {
99+
t.Run(tc.name, func(t *testing.T) {
100+
oauthCfg := &oauth.Config{
101+
BaseURL: "https://api.github.com",
102+
ValidateTokenFormat: tc.validateTokenFormat,
103+
}
104+
105+
middleware := ExtractUserToken(oauthCfg)
106+
handler := middleware(dummyHandler)
107+
108+
req := httptest.NewRequest(http.MethodGet, "/test", nil)
109+
if tc.authHeader != "" {
110+
req.Header.Set("Authorization", tc.authHeader)
111+
}
112+
113+
rr := httptest.NewRecorder()
114+
handler.ServeHTTP(rr, req)
115+
116+
assert.Equal(t, tc.expectedStatus, rr.Code)
117+
118+
if tc.expectWWWAuth {
119+
wwwAuth := rr.Header().Get("WWW-Authenticate")
120+
require.NotEmpty(t, wwwAuth, "expected WWW-Authenticate header to be set")
121+
assert.True(t, strings.HasPrefix(wwwAuth, "Bearer resource_metadata="),
122+
"WWW-Authenticate header should have Bearer resource_metadata format")
123+
} else {
124+
assert.Empty(t, rr.Header().Get("WWW-Authenticate"),
125+
"WWW-Authenticate header should not be set")
126+
}
127+
})
128+
}
129+
}
130+
131+
func TestParseAuthorizationHeader(t *testing.T) {
132+
tests := []struct {
133+
name string
134+
authHeader string
135+
expectedAuth authType
136+
expectError bool
137+
}{
138+
{
139+
name: "classic PAT",
140+
authHeader: "Bearer ghp_abcdef1234567890",
141+
expectedAuth: authTypeGhToken,
142+
expectError: false,
143+
},
144+
{
145+
name: "fine-grained PAT",
146+
authHeader: "Bearer github_pat_abcdef1234567890",
147+
expectedAuth: authTypeGhToken,
148+
expectError: false,
149+
},
150+
{
151+
name: "OAuth token",
152+
authHeader: "Bearer gho_abcdef1234567890",
153+
expectedAuth: authTypeGhToken,
154+
expectError: false,
155+
},
156+
{
157+
name: "user access token",
158+
authHeader: "Bearer ghu_abcdef1234567890",
159+
expectedAuth: authTypeGhToken,
160+
expectError: false,
161+
},
162+
{
163+
name: "installation token",
164+
authHeader: "Bearer ghs_abcdef1234567890",
165+
expectedAuth: authTypeGhToken,
166+
expectError: false,
167+
},
168+
{
169+
name: "old-style 40 char hex token",
170+
authHeader: "Bearer 1234567890abcdef1234567890abcdef12345678",
171+
expectedAuth: authTypeGhToken,
172+
expectError: false,
173+
},
174+
{
175+
name: "IDE token with colon",
176+
authHeader: "Bearer tid=1;exp=25145314523;chat=1:hmac",
177+
expectedAuth: authTypeIDE,
178+
expectError: false,
179+
},
180+
{
181+
name: "lowercase bearer",
182+
authHeader: "bearer ghp_abcdef1234567890",
183+
expectedAuth: authTypeGhToken,
184+
expectError: false,
185+
},
186+
{
187+
name: "missing header",
188+
authHeader: "",
189+
expectedAuth: 0,
190+
expectError: true,
191+
},
192+
{
193+
name: "unsupported prefix",
194+
authHeader: "GitHub-Bearer token",
195+
expectedAuth: 0,
196+
expectError: true,
197+
},
198+
{
199+
name: "invalid token format",
200+
authHeader: "Bearer invalid",
201+
expectedAuth: 0,
202+
expectError: true,
203+
},
204+
}
205+
206+
for _, tc := range tests {
207+
t.Run(tc.name, func(t *testing.T) {
208+
req := httptest.NewRequest(http.MethodGet, "/test", nil)
209+
if tc.authHeader != "" {
210+
req.Header.Set("Authorization", tc.authHeader)
211+
}
212+
213+
authType, _, err := parseAuthorizationHeader(req)
214+
215+
if tc.expectError {
216+
require.Error(t, err)
217+
} else {
218+
require.NoError(t, err)
219+
assert.Equal(t, tc.expectedAuth, authType)
220+
}
221+
})
222+
}
223+
}

‎pkg/http/oauth/oauth.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ type Config struct {
5151
// This is used to restore the original path when a proxy strips a base path before forwarding.
5252
// If empty, requests are treated as already using the external path.
5353
ResourcePath string
54+
55+
// ValidateTokenFormat controls whether the token extraction middleware validates
56+
// the token format and returns a 401 with WWW-Authenticate header for invalid tokens.
57+
// When false (default), invalid tokens pass through for validation elsewhere (e.g., CAPI).
58+
// When true, invalid tokens receive a 401 response with OAuth challenge headers.
59+
ValidateTokenFormat bool
5460
}
5561

5662
// AuthHandler handles OAuth-related HTTP endpoints.

0 commit comments

Comments
 (0)