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

Follow up to question to: Are items in TokenCache case insensitive? #578

Closed
lilgreenbird opened this issue Dec 22, 2022 · 7 comments
Closed

Comments

@lilgreenbird
Copy link

This is a follow up to #546 which was closed. Apologies for inactivity since we were focused on releases last few weeks. Please see original issue in https://youtrack.jetbrains.com/issue/DBE-13085.

Here is where we get the account cache from MSAL in the JDBC driver.

Looking through the MSAL code it looks like matches are not case sensitive here? and here? I also see that toLowerCase is called in AccountCacheEntity.

We're seeing that if the user string is JohnnyCash@folsom.org we're getting johnnycash@folsom.org back from getAccounts(). User is proposing that we do case insensitive match and consider that as a hit however linux/mac is case sensitive these could be 2 different users.

@siddhijain
Copy link
Contributor

Hello @lilgreenbird! Looking closely at your question and the MSAL Java implementation, you are correct that the keys used to put and retrieve tokens to and from the token cache are case insensitive as they are converted to lowercase throughout the code. Does that work for you if the user is looking for a case-insensitive match? Are you proposing any changes to the MSAL Java implementation?

@lilgreenbird
Copy link
Author

hi @siddhijain

This is a bug as in linux/mac user names are case sensitive they could be different users so could be a security issue. We have an issue opened on this (original external issue here ) As you can see JohnnyCash@folsom.org and johnnycash@folsom.org are different users but cache contains johnnycash@folsom.org this causes multiple browsers to be opened in interactive login.

So yes we would request this to be fixed, the MSAL library should use the username as provided and not do any conversions.

@siddhijain
Copy link
Contributor

@lilgreenbird Sure, we will discuss and work on fixing this.

@siddhijain
Copy link
Contributor

@lilgreenbird After discussing with our PM, this looks like an intended behavior and not something that can be fixed in msal4j. msal4j does not deal directly with the username. It uses Azure AD tokens to populate the cache ( and username is one of the values in the token). Let us know if there are more questions.

@lilgreenbird
Copy link
Author

lilgreenbird commented Jan 12, 2023

hi @siddhijain

I'm not sure what you mean that msal4j does not deal directly with the username? That is the account name in the cache that's passed by the JDBC driver (provided by the user) to MSAL. If I set the user name to V-Susanh@microsoft.com (i.e. with mixed case) and step into MSAL code I can see that MSAL converts it to lower case v-susanh@microsoft.com and store in the cache. This is a problem because the next time we call pca.getAccounts() it will return the lower case user name v-susanh@microsoft.com which is wrong and does not match the user name provided. Ok we could workaround this by doing a case insensitive match and assume a cache hit however this is wrong would only work for windows which is not case sensitive. In unix systems account names are case sensitive it could be a security issue as these could be 2 different users. Or perhaps it doesn't matter? Are AAD user names case insensitive? If that is the case then this would make sense however I could not find any docs to verify this other than here which states "Resource and resource group names are case-insensitive unless specifically noted in the valid characters column.". Not sure if this applies to AAD user names? Could you please verify if it is true that AAD user names are case insensitive? If so then we will change to a case insensitive match when retrieving account names from the cache. Thanks.

@siddhijain
Copy link
Contributor

@lilgreenbird MSAL Java does not convert the username to lowercase. The library passes the information received to ESTS for token acquisition. This conversion might happen at ESTS. You are correct that AAD usernames are not case-sensitive so abc@microsoft.com and Abc@microsoft.com are the same users. I will see if I can find any documentation around this. Hope that helps. Thanks.

@lilgreenbird
Copy link
Author

thank you, we will update our driver accordingly to account for case insensitivity then.

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

No branches or pull requests

2 participants