django-allauth
django-allauth copied to clipboard
pennersr
Authorization Code Flow with PKCE should be integrated
Microsoft Identity Platform suggest using Authorization Code Flow instead of the Implicit Grant Flow, now for all applications. The reason for that begins with the plans for third-party cookies being removed from browsers. Without them, the Implicit Grant Flow won't work. See here: https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-implicit-grant-flow#prefer-the-auth-code-flow
PKCE as far as I know was firstly introduced for Native Apps. However, for Microsoft, it is necessary when creating this flow on Single-Page-Applications, but is recommended for all native and also confidential apps. See here: https://docs.microsoft.com/en-us/azure/active-directory/develop/reference-third-party-cookies-spas#overview-of-the-solution
In order for completing an Auth Code Flow with PKCE, the code_verifier and code_challenge must be used. Currently, django-allauth doesn't support it.
I am using django-allauth together with dj-rest-auth to handle social authentication on my app, which consist on a Django Rest back-end and a React front-end, but I found it impossible to integrate Microsoft Authentication in my React SPA, without having to fork and slightly change django-allauth.
For this reason, I think that the integration of this flow can be useful to have on django-allauth.
@pennersr If you consider this as something that might be good to have on django-allauth, since I am currently working on it I can make a Pull Request as soon as I have it ready.
@mateokurti - will your PR allow us to add new providers that use PKCE? If so I'd love to get a copy of your code since I'm currently trying to do exactly that.
@colin-fsa Yes, that was the idea I had. I haven't tested it with other providers, but in theory it is designed to work. I'll push the changes on my fork asap.
Any chance this PR can get merged? Since PKCE is required for public and recommended for confidential clients, and django-oauth-toolkit now defaults to PKCE, it would be great to get this merged! Thanks :)
Can an example of what this configuration looks like be added to the docs?
