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

[Feature request] Re-Enable Azure Entra v1 token support #218

Open
OliverKleinBST opened this issue Nov 20, 2024 · 11 comments
Open

[Feature request] Re-Enable Azure Entra v1 token support #218

OliverKleinBST opened this issue Nov 20, 2024 · 11 comments
Labels
enhancement New feature or request

Comments

@OliverKleinBST
Copy link

Describe the feature you'd like
We are using Azure Entry ID and our central governance team allows us to use only v1 tokens.
Hence, we would kindly ask if the support for v1 tokens could be kept / re-enabled.

@OliverKleinBST OliverKleinBST added the enhancement New feature or request label Nov 20, 2024
@JonasKs
Copy link
Member

JonasKs commented Nov 20, 2024

I removed it in v5 #200, and then closed #215. I initially did this in order to remove a bunch of tests which were a pain to maintain and for others to understand. When i searched around, it seemed that v1 is no longer recommended by Azure, and Entra ID defaults to v2, so I removed it.

I'd like to understand why one wouldn't migrate to v2 tokens, especially why a governance team wouldn't allow applications to use them. This seems odd to me.

@OliverKleinBST
Copy link
Author

Hi Jonas, thanks for your quick reply. Let me know when I am on a wrong track eventually.

I have not found a real recommendation for v2 over v1, and as per my understanding v1 refers to
"Accounts in this organizational directory only (xxx Group only - Single tenant) if possible."
In the manifest this leads me to
"api": {
"requestedAccessTokenVersion": null,
where a requestedAccessTokenVersion of null equals v1.

Changing this to v2 would imply to set the account type to be extended to outside our group:
Accounts in any organizational directory (Any Microsoft Entra ID tenant - Multitenant) and personal Microsoft accounts (e.g. "api": {
"requestedAccessTokenVersion": 2,
Skype, Xbox), Manifest:

which conflicts with my governance guideline to stay within my companies tenant for internal applications.

@OliverKleinBST
Copy link
Author

....You are not allowed to build multi-tenant applications or enable the login with personal Microsoft accounts (Skype, Xbox) ....
Consequently, this configuration will always be reverted and reconsidering the authentication setup of your application is highly recommended....

@JonasKs
Copy link
Member

JonasKs commented Nov 20, 2024

This is not correct. The app reg multi tenancy is not connected to the token version.

See this article.

@JonasKs
Copy link
Member

JonasKs commented Nov 20, 2024

And this😊

@OliverKleinBST
Copy link
Author

Ok, thanks. So the v2 token should just work given that the code connects to that v2 endpoint.

Currently, when we upgrade from package 4.4.0 to 5.x and only remove that token_version=1 parameter in SingleTenantAzureAuthorizationCodeBearer we get http 401, so I eventually need to deep dive why.

The only special thing we may do is that we use a custom scope like "api://application_id/..." which was required to us to make use of roles in the jwt, which we dont get with the "normal" open id scopes.

@JonasKs
Copy link
Member

JonasKs commented Nov 21, 2024

Exactly!

If you give me the traceback/errors I’m sure I can help debug.

The only special thing we may do is that we use a custom scope like "api://application_id/..." which was required to us to make use of roles in the jwt, which we dont get with the "normal" open id scopes.

I don’t understand, could you provide an example?

If there’s some code that you can’t share online, please just email it to me and I’ll have a look.

@OliverKleinBST
Copy link
Author

After further analysis, I found what is happening to me.

My app registrations are all configured so far with their default, which is token version null, which is then interpreted as token version 1.
Now, when I call the v2 API, a v1 token is successfully returned, but the validate function fails to verify the iss field.

Reason is that from well known configurations API we get informed that for v2 it should be
https://login.microsoftonline.com/{tenant} but as my app registration is version 1 I get https://sts.windows.net/{tenant}.

Though I have now 3 options:

  1. change the manifest as you suggest to 2 for my app registrations, which should be ok as long as I dont touch the
    signInAudience": "AzureADMyOrg" setting.
    Actually when I selected this from Portal as AD+Private I got "2" on token version since AD+Private and Private require 2, so it was more a coincidence. But MyOrg actually supports 1 and 2 and just defaults to 1 (null).
  2. I could tweak my code by setting azure_scheme.validate_iss = False after instantiation, to disable iss verification
  3. Eventually, I could create a callable for iss validation where I provide the sts address manually and override what I got from well known API

@JonasKs
Copy link
Member

JonasKs commented Nov 21, 2024

Be aware - if you change the token setting from v1 to v2, it may take up to 48 hours for it to work.

So if you receive a v1 token after changing to v2, and fetching the token from a v2 endpoint, it's because of this. Please try again tomorrow.

@OliverKleinBST
Copy link
Author

OK thanks. For me it was much faster though.

With respect to workarounds I mentioned above, the validate_xxx = False approach is only working in version 4.x since in version 4.x only the validation of iss fails. However, in version 5.x also the validation of aud field fails due to the change in "api:" in that string, and unfortunately there is no flag to control that validation also.

Hence, my only option is now to change the manifest, hoping that the change in tokens does not have any impact on other implementations.

@JonasKs
Copy link
Member

JonasKs commented Nov 22, 2024

This is unfortunate, I agree, and I'm sorry about the impact!

As I said, the test suite was hard understand because of all the different versions and magic in how tests were run, and external/new contributors struggled.
Complexity in the test suite also makes it harder to ensure that all edge cases are caught which is essential for a security library like this. In other words, I had to get rid of complexity in order for me to continue to maintain this repository.

Since I personally have never used v1 tokens(just read docs), I'm not too sure on the upgrade path - but any steps you find, please write them here for others to follow!

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

No branches or pull requests

2 participants