Skip to content

Comments

Generate authorization server urls during API resolution.#2070

Open
omgitsads wants to merge 10 commits intomainfrom
oauth-authorization-server
Open

Generate authorization server urls during API resolution.#2070
omgitsads wants to merge 10 commits intomainfrom
oauth-authorization-server

Conversation

@omgitsads
Copy link
Member

@omgitsads omgitsads commented Feb 23, 2026

Summary

Handle generation of the OAuth Authorization Server URL when we're parsing URLs for the underlying clients.

Why

As highlighted in #2046, we're not generating the OAuth Authorization Server URLs correctly.

This PR moves the generation into the APIHostResolver implementation and uses that in the OAuthHandler.

What changed

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Prompts tested (tool changes only)

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Note: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed
  • Updated (README / docs / examples)

@omgitsads omgitsads marked this pull request as ready for review February 23, 2026 16:59
@omgitsads omgitsads requested a review from a team as a code owner February 23, 2026 16:59
Copilot AI review requested due to automatic review settings February 23, 2026 16:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes the OAuth Authorization Server URL generation by moving it from the OAuth handler into the API host resolution logic. This addresses issue #2046 where the authorization server URLs were not being correctly generated for different GitHub host types (github.com, GHEC, and GHES).

Changes:

  • Added AuthorizationServerURL method to the APIHostResolver interface and its implementations in pkg/utils/api.go
  • Updated NewAuthHandler to accept context.Context and APIHostResolver parameters, using the resolver to get the authorization server URL instead of using a hardcoded constant
  • Added comprehensive test coverage for the new authorization server URL resolution across different host types

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/utils/api.go Added AuthorizationServerURL method to interface and implementation; generates appropriate OAuth URLs for dotcom, GHEC, and GHES hosts
pkg/http/oauth/oauth.go Modified NewAuthHandler to accept APIHostResolver and use it to dynamically resolve authorization server URLs; removed hardcoded DefaultAuthorizationServer constant
pkg/http/server.go Updated call to NewAuthHandler to pass context and API host resolver
pkg/http/oauth/oauth_test.go Updated all test calls with new parameters; added comprehensive test suite for authorization server URL resolution across host types
pkg/scopes/fetcher_test.go Updated mock APIHostResolver implementation to include new AuthorizationServerURL method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant