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

B2C token does not contain tid in the token #87

Closed
orcharddweller opened this issue Jun 29, 2022 · 17 comments · Fixed by #96
Closed

B2C token does not contain tid in the token #87

orcharddweller opened this issue Jun 29, 2022 · 17 comments · Fixed by #96
Labels
question Further information is requested

Comments

@orcharddweller
Copy link
Contributor

I've managed to set up B2C, but it doesn't seem to return tid in the token. Everything else is working.

I'm not sure how to confirm if everything is working well (I might have to contact microsoft), but if B2C doesn't return tid, would it make sense to make it optional in the User model?

@orcharddweller orcharddweller added the question Further information is requested label Jun 29, 2022
@JonasKs JonasKs changed the title [Question] B2C token does not contain tid in the token Jun 29, 2022
@JonasKs
Copy link
Member

JonasKs commented Jun 29, 2022

Hi. Weird, I thought it would 🤔 @robteeuwen, could you confirm?

@Bulga-xD
Copy link
Contributor

We just implement B2C and tid is there.

@JonasKs
Copy link
Member

JonasKs commented Jun 30, 2022

Thank you! I assume something else is up, I've never heard of the tid key missing.

I'd love if someone would contribute with a B2C tutorial, since it's hard to troubleshoot these things without a "best practice base".

@orcharddweller
Copy link
Contributor Author

This is interesting, because everything seems to be working on my side, besides this.

My setup is basically the same as the Single Tenant one, but:

azure_scheme = AzureAuthorizationCodeBearerBase(
    openapi_authorization_url=f"https://{tenant}.b2clogin.com/{tenant}.onmicrosoft.com/oauth2/v2.0/authorize?p={policy}&nonce=defaultNonce&prompt=login",
    app_client_id=settings.APP_CLIENT_ID,
    openapi_token_url=f"https://{tenant}.b2clogin.com/{tenant}.onmicrosoft.com/{policy}/oauth2/v2.0/token",
    openid_config_url=f"https://{tenant}.b2clogin.com/{tenant}.onmicrosoft.com/{policy}/v2.0/.well-known/openid-configuration",
    scopes={
        f'https://{tenant}.onmicrosoft.com/{settings.APP_CLIENT_ID}/user_impersonation': 'user_impersonation',
    },
    
)

azure_scheme.scheme_name = "B2C"

and I've picked this option when setting up app registrations:

Screenshot 2022-06-30 at 19 25 11

@Bulga-xD
Copy link
Contributor

Sure @JonasKs I will try to add a tutorial this week!

@JonasKs
Copy link
Member

JonasKs commented Jul 1, 2022

That would be awesome! 👏 👏

I am on summer vacation from the 9th, so if you're able to make a PR before then I'll be able to give feedback pretty much immediately 😊

@JonasKs
Copy link
Member

JonasKs commented Jul 2, 2022

I've never used Azure B2C, but I suspect the lack of tid is because of the fact that you're using a B2C tenant, and the appreg is within that tenant. These other guys are using a combination of multi-tenant and B2C tenants, where the app is registered in the multi-tenant tenant, and then exposed to the B2C tenant?

I'm happy to accept a PR where the tid field is optional.

@Bulga-xD
Copy link
Contributor

Bulga-xD commented Jul 2, 2022

We also using B2C tenant and tid is there. From my point of view, this is a configuration problem in Azure.

@orcharddweller
Copy link
Contributor Author

orcharddweller commented Jul 2, 2022

I've investigated a bit, and it looks like it's normal for B2C to miss tid:

MSAL.Net supports a token cache. The token caching key is based on the claims returned by the Identity Provider. Currently MSAL.Net needs two claims to build a token cache key: :

tid which is the Azure AD Tenant Id and
preferred_username
Both these claims are missing in many of the Azure AD B2C scenarios.

https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/AAD-B2C-specifics#known-issue-with-azure-b2c

also:

I read the B2C access token documentation, and I noticed that the tenantId claim (tid) is not in the list. This is different from the normal Azure Active Directory access token claims.

https://stackoverflow.com/questions/55978290/azure-b2c-access-token-missing-tenantid

There seem to be workarounds for this, but I think it would be best if the lib worked without those.

@Bulga-xD
Copy link
Contributor

Bulga-xD commented Jul 2, 2022

This is still strange for me because in our case we don't have any problems with tid. @marcinplatek, can you test with my PR? This is a solution that worked in our project.

@orcharddweller
Copy link
Contributor Author

I'll have a look, but the difference is I'm not using multi-tenant, so I think that would explain the difference.

I think the posts I linked clearly show, that tid might be missing in many B2C scenarios.

@orcharddweller
Copy link
Contributor Author

Still, it's really cool that you contributed, @kristiqntashev! I'll try to have a look at it this week, thanks!

@Bulga-xD
Copy link
Contributor

Bulga-xD commented Jul 4, 2022

I'll have a look, but the difference is I'm not using multi-tenant, so I think that would explain the difference.

I think the posts I linked clearly show, that tid might be missing in many B2C scenarios.

That's possible! We only try with Multitenant!

@JonasKs
Copy link
Member

JonasKs commented Jul 5, 2022

I'm happy to accept a PR where tid is a optional field. 😊 I'm going on vacation on Friday, so I'll fix it before then if no PR is created. 😊

@orcharddweller
Copy link
Contributor Author

I'll try to sit on this tomorrow.

@Bulga-xD
Copy link
Contributor

Bulga-xD commented Jul 5, 2022

I'll try to sit on this tomorrow.

Ping me if you want we can work together. Happy to help!

@orcharddweller
Copy link
Contributor Author

@kristiqntashev, sorry, I was busy and got to do this a bit late. Thanks for the offer, anyway!

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

Successfully merging a pull request may close this issue.

3 participants