AzureAD / azure-activedirectory-library-for-java

MIT License
161 stars 126 forks source link

Tokens generated using adal 1.2.0 (python) works but those generated using adal4j 1.6.0 do not Azure Active Directory App Integration and Development #282

Closed yubhat closed 4 years ago

yubhat commented 4 years ago

Using adal 1.2.0(python) and adal4j 1.6.0(java) to generate access tokens using username and password. The resource ids and the client app ids are the same in both cases. However, while the token generated via the python sdk works perfectly, the token generated via the java sdk does not.

Python is giving audition as SPN:Client_ID Java is giving audition: Client_ID Kubernetes service expect audition as SPN:Client_ID

Python Adal Saml assertion:- https://github.com/AzureAD/azure-activedirectory-library-for-python/blob/dev/adal/authentication_context.py#L147 https://github.com/AzureAD/azure-activedirectory-library-for-python/blob/dev/adal/token_request.py#L186

Ask: Tokens generated using adal 1.2.0 (python) works but those generated using adal4j 1.6.0 do not Azure Active Directory App Integration and Development.

sangonzal commented 4 years ago

@yubhat The reason this is happening is because ADAL Python hardcodes the api-version in the endpoints they are hitting ("api-verseion=1.0"), while Java does not. ADAL Java is in maintenance mode and we are not making updates unless they are security related.

Do you own the Kubernetes service which you using the token for?

chatter92 commented 4 years ago

@sangonzal unfortunately, we dont own the Kubernetes service, hence we cannot make any changes there. If there is any way in which we can pass the api-version via the java sdk (adal or msal), it will solve this issue for us.

rayluo commented 4 years ago

@yubhat @chatter92 Out of curiosity, how do you initialize ADAL Python in your code? Did you explicitly use its api_version parameter? In their recent samples, that parameter is typically left undefined in order to trigger the default behavior (rather than using api_version="1.0").

chatter92 commented 4 years ago

@rayluo yes, we have explicitly set api_version to 1.0 in our script.

sangonzal commented 4 years ago

@yubhat @chatter92 ADAL is in maintenance mode and we are not planning on making changes unless they are security related.

The Kubernetes service should accept tokens with the new format. Have you tried contacting the owners and asking them to update?

rayluo commented 4 years ago

@chatter92 Thanks for sharing this info! Back then when I implemented that api_version parameter in ADAL Python so that developers could opt in for the old behavior for backward compatibility, we did not exactly know which service(s) would require such old behavior. Now I/we learn from you that "Kubernetes service expects audience as SPN:Client_ID".

As @sangonzal correctly pointed out, ideally the Kubernetes service would better accept new format of token.

By the way, @chatter92 have you folks even try using our MSAL library (either MSAL Python or MSAL Java)? Will the token acquired by those libraries work for Kubernetes service? If not, that will be another topic that we would like to figure out.

//CC our PMs @navyasric @jmprieur as a FYI.

chatter92 commented 4 years ago

@rayluo we did try using the MSAL java library (1.1.0), but it looks like even that is generating tokens without the "spn" prefix, so it didnt work with our cluster. It is possible that the k8s clusters that we use are on an older version, and as @yubhat had mentioned, a PR was raised on the Kubernetes repo for the same: https://github.com/kubernetes/kubernetes/pull/86412 But it was fairly recent and may not even have been released yet.

So till the time we get a k8s update, if we could get the token in the older format, it would great for us.

rayluo commented 4 years ago

@chatter92 Your research effort on this topic is amazing!

Given that the Kubernetes side has fixed this and is probably in the process of releasing/rolling out their new version, would you consider checking their release plan and see if you can wait? Because, even if you somehow find a workaround to add that legacy api_version=1.0 behavior into your code base, your code would likely still break when the aforementioned Kubernetes fix would be deployed to your environment.

chatter92 commented 4 years ago

Yeah that's the thing. We don't know when we will get an update on the cluster, so we dont know how long we have to wait. If you can expose this api_version parameter, we can keep it configurable in our service in, say, a properties file. So today, that property can say api_version=1.0. Tomorrow, when they do rollout the fix, we can just update the properties file to say api_version=2.0 or something

henrik-me commented 4 years ago

Closing. We will not be addressing this in adal4j.

Migrating to MSAL can be found here: https://docs.microsoft.com/en-us/azure/active-directory/develop/migrate-adal-msal-java