Feature - Extend GitHub OAuth to Support GitHub Enterprise

[devangtomar]

trafficstars

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: https://github.com/Chainlit/chainlit/issues/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.

[dokterbob]

@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.

[devangtomar]

@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! 🙂

[dokterbob]

@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! 🙂

Hi @devangtomar, any chance you could work on this? :)

[devangtomar]

@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! 🙂

Hi @devangtomar, any chance you could work on this? :)

@dokterbob Apologies for the delay! I’ve been busy lately but will pick this up over the weekend and update the PR. 🙂

[hayescode]

Months old, feedback not addressed