Skip to content

Commit 2871761

Browse files
omgitsadsomgitsads
committed
Remove IDE token support, these will never work. Add tests.
1 parent cb957d0 commit 2871761

File tree

2 files changed

+320
-7
lines changed

2 files changed

+320
-7
lines changed

‎pkg/http/middleware/token_test.go‎

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
package middleware
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
ghcontext "github.com/github/github-mcp-server/pkg/context"
9+
"github.com/github/github-mcp-server/pkg/http/oauth"
10+
"github.com/github/github-mcp-server/pkg/utils"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestExtractUserToken(t *testing.T) {
16+
oauthCfg := &oauth.Config{
17+
BaseURL: "https://example.com",
18+
AuthorizationServer: "https://github.com/login/oauth",
19+
}
20+
21+
tests := []struct {
22+
name string
23+
authHeader string
24+
expectedStatusCode int
25+
expectedTokenType utils.TokenType
26+
expectedToken string
27+
expectTokenInfo bool
28+
expectWWWAuth bool
29+
}{
30+
// Missing authorization header
31+
{
32+
name: "missing Authorization header returns 401 with WWW-Authenticate",
33+
authHeader: "",
34+
expectedStatusCode: http.StatusUnauthorized,
35+
expectTokenInfo: false,
36+
expectWWWAuth: true,
37+
},
38+
// Personal Access Token (classic) - ghp_ prefix
39+
{
40+
name: "personal access token (classic) with Bearer prefix",
41+
authHeader: "Bearer ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
42+
expectedStatusCode: http.StatusOK,
43+
expectedTokenType: utils.TokenTypePersonalAccessToken,
44+
expectedToken: "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
45+
expectTokenInfo: true,
46+
},
47+
{
48+
name: "personal access token (classic) with bearer lowercase",
49+
authHeader: "bearer ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
50+
expectedStatusCode: http.StatusOK,
51+
expectedTokenType: utils.TokenTypePersonalAccessToken,
52+
expectedToken: "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
53+
expectTokenInfo: true,
54+
},
55+
{
56+
name: "personal access token (classic) without Bearer prefix",
57+
authHeader: "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
58+
expectedStatusCode: http.StatusOK,
59+
expectedTokenType: utils.TokenTypePersonalAccessToken,
60+
expectedToken: "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
61+
expectTokenInfo: true,
62+
},
63+
// Fine-grained Personal Access Token - github_pat_ prefix
64+
{
65+
name: "fine-grained personal access token with Bearer prefix",
66+
authHeader: "Bearer github_pat_xxxxxxxxxxxxxxxxxxxxxxx",
67+
expectedStatusCode: http.StatusOK,
68+
expectedTokenType: utils.TokenTypeFineGrainedPersonalAccessToken,
69+
expectedToken: "github_pat_xxxxxxxxxxxxxxxxxxxxxxx",
70+
expectTokenInfo: true,
71+
},
72+
{
73+
name: "fine-grained personal access token without Bearer prefix",
74+
authHeader: "github_pat_xxxxxxxxxxxxxxxxxxxxxxx",
75+
expectedStatusCode: http.StatusOK,
76+
expectedTokenType: utils.TokenTypeFineGrainedPersonalAccessToken,
77+
expectedToken: "github_pat_xxxxxxxxxxxxxxxxxxxxxxx",
78+
expectTokenInfo: true,
79+
},
80+
// OAuth Access Token - gho_ prefix
81+
{
82+
name: "OAuth access token with Bearer prefix",
83+
authHeader: "Bearer gho_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
84+
expectedStatusCode: http.StatusOK,
85+
expectedTokenType: utils.TokenTypeOAuthAccessToken,
86+
expectedToken: "gho_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
87+
expectTokenInfo: true,
88+
},
89+
{
90+
name: "OAuth access token without Bearer prefix",
91+
authHeader: "gho_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
92+
expectedStatusCode: http.StatusOK,
93+
expectedTokenType: utils.TokenTypeOAuthAccessToken,
94+
expectedToken: "gho_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
95+
expectTokenInfo: true,
96+
},
97+
// User-to-Server GitHub App Token - ghu_ prefix
98+
{
99+
name: "user-to-server GitHub App token with Bearer prefix",
100+
authHeader: "Bearer ghu_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
101+
expectedStatusCode: http.StatusOK,
102+
expectedTokenType: utils.TokenTypeUserToServerGitHubAppToken,
103+
expectedToken: "ghu_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
104+
expectTokenInfo: true,
105+
},
106+
{
107+
name: "user-to-server GitHub App token without Bearer prefix",
108+
authHeader: "ghu_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
109+
expectedStatusCode: http.StatusOK,
110+
expectedTokenType: utils.TokenTypeUserToServerGitHubAppToken,
111+
expectedToken: "ghu_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
112+
expectTokenInfo: true,
113+
},
114+
// Server-to-Server GitHub App Token (installation token) - ghs_ prefix
115+
{
116+
name: "server-to-server GitHub App token with Bearer prefix",
117+
authHeader: "Bearer ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
118+
expectedStatusCode: http.StatusOK,
119+
expectedTokenType: utils.TokenTypeServerToServerGitHubAppToken,
120+
expectedToken: "ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
121+
expectTokenInfo: true,
122+
},
123+
{
124+
name: "server-to-server GitHub App token without Bearer prefix",
125+
authHeader: "ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
126+
expectedStatusCode: http.StatusOK,
127+
expectedTokenType: utils.TokenTypeServerToServerGitHubAppToken,
128+
expectedToken: "ghs_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
129+
expectTokenInfo: true,
130+
},
131+
// Old-style Personal Access Token (40 hex characters, pre-2021)
132+
{
133+
name: "old-style personal access token (40 hex chars) with Bearer prefix",
134+
authHeader: "Bearer 0123456789abcdef0123456789abcdef01234567",
135+
expectedStatusCode: http.StatusOK,
136+
expectedTokenType: utils.TokenTypePersonalAccessToken,
137+
expectedToken: "0123456789abcdef0123456789abcdef01234567",
138+
expectTokenInfo: true,
139+
},
140+
{
141+
name: "old-style personal access token (40 hex chars) without Bearer prefix",
142+
authHeader: "0123456789abcdef0123456789abcdef01234567",
143+
expectedStatusCode: http.StatusOK,
144+
expectedTokenType: utils.TokenTypePersonalAccessToken,
145+
expectedToken: "0123456789abcdef0123456789abcdef01234567",
146+
expectTokenInfo: true,
147+
},
148+
// Error cases
149+
{
150+
name: "unsupported GitHub-Bearer header returns 400",
151+
authHeader: "GitHub-Bearer some_encrypted_token",
152+
expectedStatusCode: http.StatusBadRequest,
153+
expectTokenInfo: false,
154+
},
155+
{
156+
name: "invalid token format returns 400",
157+
authHeader: "Bearer invalid_token_format",
158+
expectedStatusCode: http.StatusBadRequest,
159+
expectTokenInfo: false,
160+
},
161+
{
162+
name: "unrecognized prefix returns 400",
163+
authHeader: "Bearer xyz_notavalidprefix",
164+
expectedStatusCode: http.StatusBadRequest,
165+
expectTokenInfo: false,
166+
},
167+
}
168+
169+
for _, tt := range tests {
170+
t.Run(tt.name, func(t *testing.T) {
171+
var capturedTokenInfo *ghcontext.TokenInfo
172+
var tokenInfoCaptured bool
173+
174+
nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
175+
capturedTokenInfo, tokenInfoCaptured = ghcontext.GetTokenInfo(r.Context())
176+
w.WriteHeader(http.StatusOK)
177+
})
178+
179+
middleware := ExtractUserToken(oauthCfg)
180+
handler := middleware(nextHandler)
181+
182+
req := httptest.NewRequest(http.MethodGet, "/test", nil)
183+
if tt.authHeader != "" {
184+
req.Header.Set("Authorization", tt.authHeader)
185+
}
186+
rr := httptest.NewRecorder()
187+
188+
handler.ServeHTTP(rr, req)
189+
190+
assert.Equal(t, tt.expectedStatusCode, rr.Code)
191+
192+
if tt.expectWWWAuth {
193+
wwwAuth := rr.Header().Get("WWW-Authenticate")
194+
assert.NotEmpty(t, wwwAuth, "expected WWW-Authenticate header")
195+
assert.Contains(t, wwwAuth, "Bearer resource_metadata=")
196+
}
197+
198+
if tt.expectTokenInfo {
199+
require.True(t, tokenInfoCaptured, "expected TokenInfo to be present in context")
200+
require.NotNil(t, capturedTokenInfo)
201+
assert.Equal(t, tt.expectedTokenType, capturedTokenInfo.TokenType)
202+
assert.Equal(t, tt.expectedToken, capturedTokenInfo.Token)
203+
} else {
204+
assert.False(t, tokenInfoCaptured, "expected no TokenInfo in context")
205+
}
206+
})
207+
}
208+
}
209+
210+
func TestExtractUserToken_NilOAuthConfig(t *testing.T) {
211+
var capturedTokenInfo *ghcontext.TokenInfo
212+
var tokenInfoCaptured bool
213+
214+
nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
215+
capturedTokenInfo, tokenInfoCaptured = ghcontext.GetTokenInfo(r.Context())
216+
w.WriteHeader(http.StatusOK)
217+
})
218+
219+
middleware := ExtractUserToken(nil)
220+
handler := middleware(nextHandler)
221+
222+
req := httptest.NewRequest(http.MethodGet, "/test", nil)
223+
req.Header.Set("Authorization", "Bearer ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
224+
rr := httptest.NewRecorder()
225+
226+
handler.ServeHTTP(rr, req)
227+
228+
assert.Equal(t, http.StatusOK, rr.Code)
229+
require.True(t, tokenInfoCaptured)
230+
require.NotNil(t, capturedTokenInfo)
231+
assert.Equal(t, utils.TokenTypePersonalAccessToken, capturedTokenInfo.TokenType)
232+
}
233+
234+
func TestExtractUserToken_MissingAuthHeader_WWWAuthenticateFormat(t *testing.T) {
235+
oauthCfg := &oauth.Config{
236+
BaseURL: "https://api.example.com",
237+
AuthorizationServer: "https://github.com/login/oauth",
238+
ResourcePath: "/mcp",
239+
}
240+
241+
nextHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
242+
w.WriteHeader(http.StatusOK)
243+
})
244+
245+
middleware := ExtractUserToken(oauthCfg)
246+
handler := middleware(nextHandler)
247+
248+
req := httptest.NewRequest(http.MethodGet, "/test", nil)
249+
// No Authorization header
250+
rr := httptest.NewRecorder()
251+
252+
handler.ServeHTTP(rr, req)
253+
254+
assert.Equal(t, http.StatusUnauthorized, rr.Code)
255+
wwwAuth := rr.Header().Get("WWW-Authenticate")
256+
assert.NotEmpty(t, wwwAuth)
257+
assert.Contains(t, wwwAuth, "Bearer")
258+
assert.Contains(t, wwwAuth, "resource_metadata=")
259+
assert.Contains(t, wwwAuth, "/.well-known/oauth-protected-resource")
260+
}
261+
262+
func TestSendAuthChallenge(t *testing.T) {
263+
tests := []struct {
264+
name string
265+
oauthCfg *oauth.Config
266+
requestPath string
267+
expectedContains []string
268+
}{
269+
{
270+
name: "with base URL configured",
271+
oauthCfg: &oauth.Config{
272+
BaseURL: "https://mcp.example.com",
273+
},
274+
requestPath: "/api/test",
275+
expectedContains: []string{
276+
"Bearer",
277+
"resource_metadata=",
278+
"https://mcp.example.com/.well-known/oauth-protected-resource",
279+
},
280+
},
281+
{
282+
name: "with nil config uses request host",
283+
oauthCfg: nil,
284+
requestPath: "/api/test",
285+
expectedContains: []string{
286+
"Bearer",
287+
"resource_metadata=",
288+
"/.well-known/oauth-protected-resource",
289+
},
290+
},
291+
{
292+
name: "with resource path configured",
293+
oauthCfg: &oauth.Config{
294+
BaseURL: "https://mcp.example.com",
295+
ResourcePath: "/mcp",
296+
},
297+
requestPath: "/api/test",
298+
expectedContains: []string{
299+
"Bearer",
300+
"resource_metadata=",
301+
"/mcp",
302+
},
303+
},
304+
}
305+
306+
for _, tt := range tests {
307+
t.Run(tt.name, func(t *testing.T) {
308+
rr := httptest.NewRecorder()
309+
req := httptest.NewRequest(http.MethodGet, tt.requestPath, nil)
310+
311+
sendAuthChallenge(rr, req, tt.oauthCfg)
312+
313+
assert.Equal(t, http.StatusUnauthorized, rr.Code)
314+
wwwAuth := rr.Header().Get("WWW-Authenticate")
315+
for _, expected := range tt.expectedContains {
316+
assert.Contains(t, wwwAuth, expected)
317+
}
318+
})
319+
}
320+
}

‎pkg/utils/token.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ const (
1919
TokenTypeOAuthAccessToken
2020
TokenTypeUserToServerGitHubAppToken
2121
TokenTypeServerToServerGitHubAppToken
22-
TokenTypeIDEToken
2322
)
2423

2524
var supportedGitHubPrefixes = map[string]TokenType{
@@ -61,12 +60,6 @@ func ParseAuthorizationHeader(req *http.Request) (tokenType TokenType, token str
6160
}
6261
}
6362

64-
// Do a naïve check for a colon in the token - currently, only the IDE token has a colon in it.
65-
// ex: tid=1;exp=25145314523;chat=1:<hmac>
66-
if strings.Contains(token, ":") {
67-
return TokenTypeIDEToken, token, nil
68-
}
69-
7063
for prefix, tokenType := range supportedGitHubPrefixes {
7164
if strings.HasPrefix(token, prefix) {
7265
return tokenType, token, nil

0 commit comments

Comments
 (0)