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

[Bug] Incorrect suggestedCache key for removeAsync(account) in confidential client apps #2643

Closed
abhidnya13 opened this issue May 19, 2021 · 6 comments · Fixed by #2745
Closed
Assignees
Milestone

Comments

@abhidnya13
Copy link
Contributor

RemoveAsync(account) in confidential client apps returns an empty cache key.

The cache key, I think should be home_accout_id

Reference:
https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/master/src/client/Microsoft.Identity.Client/Cache/SuggestedWebCacheKeyFactory.cs#L23-L26

@abhidnya13 abhidnya13 changed the title [Bug] Incorrect suggestedCache key for removeAccount() in confidential client apps [Bug] Incorrect suggestedCache key for removeAsync(account) in confidential client apps May 19, 2021
@jennyf19
Copy link
Collaborator

closing as duplicate

@bgavrilMS
Copy link
Member

Not sure if it is a duplicate. When a call to RemoveAccount is made, MSAL does not generate any suggested_cacke_key so we're unable to load the cache at all. So this affects Web app scenarios, not just web api calls downstream web api.

In any case, the fix is trivial, the key is account.HomeAccountId

@jennyf19
Copy link
Collaborator

@abhidnya13 and I synced off-line on this. It's not an exact duplicate but related.

@bgavrilMS
Copy link
Member

Thx @jennyf19 - I'm a bit surprised that nobody using M.I.W. didn't complain about this, since TokenAcquisition.RemoveAccountAsync is broken. I guess ppl don't call this too often since it doesn't affect cookie state?

https://github.com/AzureAD/microsoft-identity-web/blob/b106d9a9250522d0bf9ed0e78e0e3dbd376d8170/src/Microsoft.Identity.Web/TokenAcquisition.cs#L514

@bgavrilMS bgavrilMS added this to the 4.32.0 milestone May 20, 2021
@jennyf19
Copy link
Collaborator

@bgavrilMS It does work. We get the account info from the httpContext for the currently signed-in user. @abhidnya13 and i were discussing the OBO use case, which doesn't remove an account because we don't have a cache key, but rather rely on eviction policies from the cache itself.

How does oid_tid get into the httpContext? It's because we call the user info endpoint, which gives that info, which is needed for guest scenarios.

@bgavrilMS
Copy link
Member

bgavrilMS commented May 20, 2021

@bgavrilMS bgavrilMS modified the milestones: 4.32.0, 4.33.0 Jun 3, 2021
@pmaytak pmaytak modified the milestones: 4.33.0, 4.34.0 Jun 23, 2021
@neha-bhargava neha-bhargava self-assigned this Jun 30, 2021
@neha-bhargava neha-bhargava linked a pull request Jul 6, 2021 that will close this issue
@trwalke trwalke modified the milestones: 4.34.0, 4.35.0 Jul 8, 2021
@bgavrilMS bgavrilMS self-assigned this Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants