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

[Identity] [InteractiveBrowserCredential] [Node] Enable PKCE #15168

Closed
sadasant opened this issue May 6, 2021 · 0 comments · Fixed by #15853
Closed

[Identity] [InteractiveBrowserCredential] [Node] Enable PKCE #15168

sadasant opened this issue May 6, 2021 · 0 comments · Fixed by #15853
Assignees
Milestone

Comments

@sadasant
Copy link
Contributor

sadasant commented May 6, 2021

While InteractiveBrowserCredential on Node and on Browsers both use the Auth Code Flow, and both use MSAL, MSAL only handles PKCE automatically on browsers. For Node, MSAL supports it, but it's not done automatically.

It should be done though,

From the OAuth docs for native apps (link):

Public native app clients MUST implement the Proof Key for Code
Exchange (PKCE [RFC7636]) extension to OAuth, and authorization
servers MUST support PKCE for such clients, for the reasons detailed
in Section 8.1.

MSAL's approach for PKE is very clear and we may be able to essentially copy how MSAL is using it here: https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/samples/msal-node-samples/auth-code-pkce/index.js#L87

@sadasant sadasant added this to the [2021] June milestone May 6, 2021
@sadasant sadasant self-assigned this May 6, 2021
@sadasant sadasant modified the milestones: [2021] June, [2021] July May 12, 2021
@ghost ghost closed this as completed in #15853 Jun 29, 2021
ghost pushed a commit that referenced this issue Jun 29, 2021
This PR enables PKCE on the InteractiveBrowserCredential.

We don’t have tests for this yet. I’ll make sure to test it manually, at least.

Fixes #15168
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant