Skip to content
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

Feature - Extend GitHub OAuth to Support GitHub Enterprise #1486

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

devangtomar
Copy link

Feature Enhancement: Extend GitHub OAuth to Support GitHub Enterprise

This feature enhancement extends the GithubOAuthProvider class to support GitHub Enterprise environments, allowing for greater flexibility and integration with custom GitHub domains.

This is the implementation for the feature request I created yesterday: #1484

Key Changes:

  • Custom GitHub Domain Support:
    • Added new environment variables, OAUTH_GITHUB_CUSTOM_URL and OAUTH_GITHUB_CUSTOM_API_URL, enabling the setup of a custom GitHub domain and API. If not specified, This defaults to github.com if not specified.
  • API URL Customization:
    • The API URL is now dynamically constructed using the custom domain, ensuring compatibility with GitHub Enterprise instances.

Benefits:

  • Enterprise Compatibility:
    • Users can now seamlessly integrate with GitHub Enterprise, utilizing their specific domain configurations.
  • Backward Compatibility:
    • Maintains existing functionality for standard GitHub users by defaulting to the public GitHub domain and API URL.
  • Enhanced Flexibility:
    • Provides a more adaptable OAuth solution for diverse GitHub environments.

Please review the changes and provide feedback or approval. Let me know if there are any questions or further adjustments needed.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. auth Pertaining to authentication. backend Pertains to the Python backend. labels Oct 26, 2024
@dokterbob dokterbob added the enhancement New feature or request label Nov 6, 2024
@dokterbob
Copy link
Collaborator

@devangtomar Thanks for the contrib! However, as we're looking to remove our own oauth client implementation in favour of some third party library (#1240), we're not going to invest a lot of energy in adding additional auth backends/features. The ETA for this is weeks more than months.

That having been said, if you could add either E2E or unit tests to this PR, and the tests look sane (you probably have to mock GH OAuth!), we can merge this.

If not, I'll leave this one open for other users' benefit. Additionally, if we get feedback from them that this is working as expected, and not breaking pre-existing stuff, I will consider merging it.

@dokterbob dokterbob added the wontfix This will not be worked on label Nov 6, 2024
@devangtomar
Copy link
Author

@devangtomar Thanks for the contrib! However, as we're looking to remove our own oauth client implementation in favour of some third party library (#1240), we're not going to invest a lot of energy in adding additional auth backends/features. The ETA for this is weeks more than months.

That having been said, if you could add either E2E or unit tests to this PR, and the tests look sane (you probably have to mock GH OAuth!), we can merge this.

If not, I'll leave this one open for other users' benefit. Additionally, if we get feedback from them that this is working as expected, and not breaking pre-existing stuff, I will consider merging it.

Thanks @dokterbob for the review! I’ll add the test cases with GitHub OAuth mocks and commit the changes soon! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Pertaining to authentication. backend Pertains to the Python backend. enhancement New feature or request size:S This PR changes 10-29 lines, ignoring generated files. wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants