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

Support passing a user-defined state value to AuthCodeURL on public clients #491

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dcormier
Copy link

Related to #485, #227, and #407.

@bgavrilMS
Copy link
Member

Why do you need to pass in your own state on public client? MSAL should perform both legs (authorization call and token call) on public client.

I can understand this scenario for confidential client, where you need MSAL only to generate the auth code URL, redirect the user there and then MSAL calls AcquireTokenByAuthCode

@dcormier
Copy link
Author

Maybe it isn't needed. I'm using the confidential client and needed it. I started working on it before seeing #485 was sitting there. I had done both sides since the functionality appeared similar, so I figured I might as well submit it.

@rayluo
Copy link
Contributor

rayluo commented Jun 24, 2024

Why do you need to pass in your own state on public client? MSAL should perform both legs (authorization call and token call) on public client.

I can understand this scenario for confidential client, where you need MSAL only to generate the auth code URL, redirect the user there and then MSAL calls AcquireTokenByAuthCode

Agreed with what @bgavrilMS said.

Also want to add that some MSALs historically have the generate auth url function even in public client, and it looks like MSAL Go does. So, technically we could have added state support for that legacy function. But perhaps we shouldn't bother, and just deprecate that function and ask app developers to use public client's interactive function instead. If you would also agree, let's close this PR and close issues that are public-client-only as wontfix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants