-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
OAuth2 PKCE authentication with git-gateway-backend #7439
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
base: main
Are you sure you want to change the base?
Conversation
…t-gateway backend
I have a couple of discussion points about this PR. These are not necessarily directly related to your PR, but to general decap architecture and it's limitations. Also to the way the oauth vendors mangle up the standard. So first this is that there's an almost exact copy of your Basicly we need to decide if we'll have one module to handle all the providers, or a module for each. A combination of the two aproaches will be a mess. I tried Azure B2C with your code and after some tweaking I came to the login page. However the Perhaps we should start with a list of providers that we want to support? |
True, I saw the existing file and re-used it. There are some changes added to mangle the claims into a format acceptable by git-gateway, but some deduplication could be done with the code between those two classes, if you prefer. I just didn't want to add a dependency on an undocumented backend file.
In theory, the actual authentication mechanism (PKCE flow) should be the same, right? If so, perhaps we just need a way to normalize the claims from each provider, hopefully with overrideable autodetection. Personally, as a user of the Decap app, I'd expect PKCE to work regardless of provider, and wouldn't want to be limited to a smaller set of providers when any OAuth2 should work, as long as the necessary data is somewhere in the claims. If you like, I can move the PKCE auth UI and associated claim-normalization into a separate module, and use that in both backends. |
In theory, yes. It's just that the vendors didn't get the message. Here are my notes for Azure B2C (the idea is to take these notes and add options to PkceAuthenticator):
That would be great. The idea is not so much that you need to support all of them, just that there's enough flexibility in the implementation so that others can be added without too much work and too much duplication. So split what's really generic to common module. Don't be afraid to rip out the old stuff. |
I updated the PR with some refactoring and some improvements:
I didn't touch any of the nonce/state logic, since I don't have an Azure B2C provider. If I have time this week, I can try firing up a basic test Azure App Registration and see if it works, but I don't know if that is significantly different from B2C. One comment about Azure and OAuth that I ran into a few weeks ago for something completely unrelated: by default, Azure App Registrations use OAuth v1, which has very different claims from OAuth v2. I wouldn't be surprised if B2C had the same default. To switch over to v2 for App Registrations, you need to edit the manifest manually, find the Do you know if your B2C testing was on v1 or v2? If you aren't sure, trying decoding the JWT, it should have a version or similar claim with |
…t-gateway backend
Sorry, I'm busy, will take a look during the weekend or sooner. |
Summary
With Netlify deprecating the Identity product, solutions need to be added to support additional authentication providers. This code adds support to the git-gateway backend for the OAuth2 PKCE authentication flow, without impacting any existing defaults.
Test plan
Test using the standard options with git-gateway and ensure that functionality is not affected.
Test with OAuth2 provider (I used Amazon Cognito). Example configuration that uses PKCE authentication (this is also documented in the decap-cms-backend-git-gateway README, for future use):
Checklist