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

Enable sign-out and tasks for all tenants #60

Merged
merged 1 commit into from
Sep 19, 2019

Conversation

mmacy
Copy link
Contributor

@mmacy mmacy commented Sep 18, 2019

This enables both sign-out and task list functionality when using a tenant in one's own B2C directory or the demo fabrikamb2c tenant.

The sample currently functions out of the box using the demo tenant, fabrikamb2c, but if one follows these B2C tutorials to configure it for their own B2C tenant, neither sign-out nor the task list work correctly:

There is also a follow-up update I'll be making to the API doc as it's missing a needed modification to the TaskService/Web.config (addresses the busted To-Do portion of MicrosoftDocs/azure-docs#38447).

Cc: @jennyf19 @yoelhor

@jennyf19
Copy link
Contributor

@TiagoBrenck and @jmprieur I know you have plans for this sample. putting this PR on your radar. thx.

Copy link
Contributor

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @mmacy thanks!
do you have the rights to merge?

@mmacy
Copy link
Contributor Author

mmacy commented Sep 19, 2019

@jmprier Excellent! And, no, I have no merge capability in this repo.

@jmprieur jmprieur merged commit c5556f4 into Azure-Samples:master Sep 19, 2019
@jmprieur
Copy link
Contributor

Thanks @mmacy . Merged!

@mmacy mmacy deleted the fix-logout branch September 19, 2019 14:57
@TiagoBrenck
Copy link

@mmacy if you navigate to Azure Portal > Azure AD B2C > User flows (policies) > Your sign up/in policy > Application claims, you will see the Object ID checkbox. By checking it would fix your logout issue.

I will evaluate if we need to revert the changes since we are using OID for the token cache as a standard in many samples.

@mmacy
Copy link
Contributor Author

mmacy commented Sep 19, 2019

Thanks, @TiagoBrenck. I think the issue stems from MSAL remapping (?) the claim name from sub, which B2C always returns, to the NameIdentifier claim type value. Normally we rely on sub as oid isn't always returned (unless, as you say, you enable the object ID claim), so this change helps mitigate the requirement to have all our docs advise enabling the oid claim when sub as a default would normally work "out of the box."

@TiagoBrenck
Copy link

@mmacy you have a valid point. I think we use oid for token cache because since its value is the same on multiple applications in the same tenant, we would be able to share cache across apps.

I will open a thread with MSAL team about your point.

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

Successfully merging this pull request may close these issues.

4 participants