-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Scope challenge http #1925
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
Open
omgitsads
wants to merge
42
commits into
http-stack-2
Choose a base branch
from
scope-challenge-http
base: http-stack-2
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Scope challenge http #1925
Changes from all commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
370ebca
initial oauth metadata implementation
mattdholloway 0a1b701
add nolint for GetEffectiveHostAndScheme
mattdholloway afda19b
remove CAPI reference
mattdholloway 97859a1
remove nonsensical example URL
mattdholloway f8f109c
anonymize
mattdholloway 9f308b3
add oauth tests
mattdholloway 9b5c2fb
Merge branch 'http-stack-2' into oauth-handler-implementation
mattdholloway 50227bf
replace custom protected resource metadata handler with our own
mattdholloway a3135d9
remove unused header
mattdholloway 1ce01df
Update pkg/http/oauth/oauth.go
mattdholloway 4fc6c3a
pass oauth config to mcp handler for token extraction
mattdholloway b0bddbf
chore: retrigger ci
mattdholloway 6c5102a
align types with base branch
mattdholloway 3daa5c3
update more types
mattdholloway e3c565a
initial oauth metadata implementation
mattdholloway f768eda
add nolint for GetEffectiveHostAndScheme
mattdholloway 68e1f50
remove CAPI reference
mattdholloway 67b821c
remove nonsensical example URL
mattdholloway 7c90050
anonymize
mattdholloway 78f1a82
add oauth tests
mattdholloway e2699c8
replace custom protected resource metadata handler with our own
mattdholloway 9c21eed
Update pkg/http/oauth/oauth.go
mattdholloway 49191a9
chore: retrigger ci
mattdholloway 03a5082
update more types
mattdholloway 37c32c5
Merge branch 'oauth-handler-implementation' of https://github.com/git…
mattdholloway 97092a0
remove CAPI specific header
mattdholloway cfea762
restore mcp path specific logic
mattdholloway 199e62c
WIP
omgitsads 840b41e
implement better resource path handling for OAuth server
mattdholloway 203ebb3
return auth handler to lib version
mattdholloway 3990325
rename to base-path flag
mattdholloway 4b690f5
Add scopes challenge middleware to HTTP handler and initialize global…
omgitsads 7801dc5
Merge branch 'oauth-handler-implementation' into scope-challenge-http
omgitsads 4d679fd
Flags on the http command
omgitsads 9a338d7
Add tests for scope maps
omgitsads 7f6e0e8
Add scope challenge & pat filtering based on token scopes
omgitsads 45ff914
Merge branch 'http-stack-2' into scope-challenge-http
omgitsads 2b016e5
Add scope filtering if challenge is enabled
omgitsads f6d4337
Linter fixes and renamed scope challenge to PAT scope filter
omgitsads 2b1b9bb
Linter issues.
omgitsads dad1e71
Remove unsused methoodod FetchScopesFromGitHubAPI, store active scope…
omgitsads ce05b87
Require an API host when creating scope fetchers
omgitsads 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
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
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,39 @@ | ||
| package context | ||
|
|
||
| import "context" | ||
|
|
||
| type mcpMethodInfoCtx string | ||
|
|
||
| var mcpMethodInfoCtxKey mcpMethodInfoCtx = "mcpmethodinfo" | ||
|
|
||
| // MCPMethodInfo contains pre-parsed MCP method information extracted from the JSON-RPC request. | ||
| // This is populated early in the request lifecycle to enable: | ||
| // - Inventory filtering via ForMCPRequest (only register needed tools/resources/prompts) | ||
| // - Avoiding duplicate JSON parsing in middlewares (secret-scanning, scope-challenge) | ||
| // - Performance optimization for per-request server creation | ||
| type MCPMethodInfo struct { | ||
| // Method is the MCP method being called (e.g., "tools/call", "tools/list", "initialize") | ||
| Method string | ||
| // ItemName is the name of the specific item being accessed (tool name, resource URI, prompt name) | ||
| // Only populated for call/get methods (tools/call, prompts/get, resources/read) | ||
| ItemName string | ||
| // Owner is the repository owner from tool call arguments, if present | ||
| Owner string | ||
| // Repo is the repository name from tool call arguments, if present | ||
| Repo string | ||
| // Arguments contains the raw tool arguments for tools/call requests | ||
| Arguments map[string]any | ||
| } | ||
|
|
||
| // WithMCPMethodInfo stores the MCPMethodInfo in the context. | ||
| func WithMCPMethodInfo(ctx context.Context, info *MCPMethodInfo) context.Context { | ||
| return context.WithValue(ctx, mcpMethodInfoCtxKey, info) | ||
| } | ||
|
|
||
| // MCPMethod retrieves the MCPMethodInfo from the context. | ||
| func MCPMethod(ctx context.Context) (*MCPMethodInfo, bool) { | ||
| if info, ok := ctx.Value(mcpMethodInfoCtxKey).(*MCPMethodInfo); ok { | ||
| return info, true | ||
| } | ||
| return nil, 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 |
|---|---|---|
| @@ -1,19 +1,39 @@ | ||
| package context | ||
|
|
||
| import "context" | ||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/github/github-mcp-server/pkg/utils" | ||
| ) | ||
|
|
||
| // tokenCtxKey is a context key for authentication token information | ||
| type tokenCtxKey struct{} | ||
| type tokenCtx string | ||
|
|
||
| var tokenCtxKey tokenCtx = "tokenctx" | ||
|
|
||
| type TokenInfo struct { | ||
| Token string | ||
| TokenType utils.TokenType | ||
| ScopesFetched bool | ||
| Scopes []string | ||
| } | ||
|
|
||
| // WithTokenInfo adds TokenInfo to the context | ||
| func WithTokenInfo(ctx context.Context, token string) context.Context { | ||
| return context.WithValue(ctx, tokenCtxKey{}, token) | ||
| func WithTokenInfo(ctx context.Context, tokenInfo *TokenInfo) context.Context { | ||
| return context.WithValue(ctx, tokenCtxKey, tokenInfo) | ||
| } | ||
|
|
||
| func SetTokenScopes(ctx context.Context, scopes []string) { | ||
| if tokenInfo, ok := GetTokenInfo(ctx); ok { | ||
| tokenInfo.Scopes = scopes | ||
| tokenInfo.ScopesFetched = true | ||
| } | ||
| } | ||
|
|
||
| // GetTokenInfo retrieves the authentication token from the context | ||
| func GetTokenInfo(ctx context.Context) (string, bool) { | ||
| if token, ok := ctx.Value(tokenCtxKey{}).(string); ok { | ||
| return token, true | ||
| func GetTokenInfo(ctx context.Context) (*TokenInfo, bool) { | ||
| if tokenInfo, ok := ctx.Value(tokenCtxKey).(*TokenInfo); ok { | ||
| return tokenInfo, true | ||
| } | ||
| return "", false | ||
| return nil, 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
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
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
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.
The
WithMCPParsemiddleware is defined but never registered in the middleware chain. The scope challenge middleware at line 85 mentions that it tries to use pre-parsed MCP method info "if WithMCPParse middleware ran earlier", but this middleware is not being registered anywhere. This means the optimization path will never be taken and the request body will always be parsed twice (once inWithScopeChallengeand again in downstream handlers). Consider addingmiddleware.WithMCPParse()to the middleware chain beforeWithScopeChallengeat line 122.