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

[Bug] Acquire token interactively with secure redirect URL #891

Open
ErRijcken opened this issue Dec 23, 2024 · 3 comments
Open

[Bug] Acquire token interactively with secure redirect URL #891

ErRijcken opened this issue Dec 23, 2024 · 3 comments
Labels
Bug Something isn't working, needs an investigation and a fix P2 Normal priority items, should be done after P1 public-client For questions/issues related to public client apps

Comments

@ErRijcken
Copy link

Library version used

1.16.2

Java version

17

Scenario

PublicClient (AcquireTokenInteractive, AcquireTokenByUsernamePassword)

Is this a new or an existing app?

This is a new app or experiment

Issue description and reproduction steps

We have a Java client where we would like to use MSAL4J for fetching a token (for authentication and authorization).
This worked well using http://localhost:<port> as a redirect URL, but our Windows people advised us that the security of this setup was not optimal (we have an on-premise authority which is configured by our people). They have talked to their MS contact, who recommended them to configure the redirect URL as http://localhost:<port>/<randomString>/.
I have been unable to make my MSAL4J setup work with this. Based on debugging the browser, it seems like the call to our authority does work, but the library doesn't properly treat the response.

Relevant code snippets

No response

Expected behavior

No response

Identity provider

Azure Active Directory Federation Services (ADFS)

Regression

No response

Solution and workarounds

No response

@ErRijcken ErRijcken added needs attention Automatically used when an issue is created through an issue template untriaged Automatically used when an issue is created through an issue template labels Dec 23, 2024
@ErRijcken
Copy link
Author

Ok, after a deep dive into the code (I'm by no means an expert), it seems like adding the random string in the redirect URL is not supported (see the if-block in AuthorizationResponseHandler#handle).
In my mind, it should not be hard to modify this to allow a specific redirect path. I will try to make the change.

@Avery-Dunn Avery-Dunn added Bug Something isn't working, needs an investigation and a fix P2 Normal priority items, should be done after P1 public-client For questions/issues related to public client apps and removed needs attention Automatically used when an issue is created through an issue template untriaged Automatically used when an issue is created through an issue template labels Dec 31, 2024
@rayluo
Copy link

rayluo commented Jan 6, 2025

FWIW, I don't think adding a random string path would work; and adding a specific/static path can work but that has nothing to do with security, but still useful as a feature to differentiate different redirect URIs.

@ErRijcken
Copy link
Author

ErRijcken commented Jan 7, 2025

I agree that the security-gain is probably negligible, some extra obscurity at most. I still agree that it would be a useful change, I made the change and created a pull request: #892. I have no experience in this library, but the code change seemed rather small. I'm not sure if I should close this issue and leave the rest of the discussion in the pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working, needs an investigation and a fix P2 Normal priority items, should be done after P1 public-client For questions/issues related to public client apps
Projects
None yet
Development

No branches or pull requests

3 participants