-
Notifications
You must be signed in to change notification settings - Fork 208
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
Should clear session auth cookie if cache is missing account #13
Comments
@jennyf19 are you PR'ing this one? |
@onovotny do you recommend we put this into our own solution? WIll this go to PR? |
@jrmcdona we are still deciding what is the best thing to do. feel free to provide input. |
Any update on this on, is it being PR'd? |
hitting the same issue, do we have any update? |
@jmprieur can you respond? thx. |
Any update here as this seems to be a big blocker for my Blazor server side app atm |
@jmprieur fyi |
Same here, working on a server-side Blazor project with Microsoft Identity Web - is there an update or does anyone have a sample workflow of how to workaround this issue ? |
I had the same issue with blazor in the past but not in the last month. At the moment I don't have this issue and I don't know if this helps in it. ` // Token acquisition service based on MSAL.NET
// do something
` |
Clunky workaround for Blazor Server Apps. |
Any guidance update for Blazor server-side? |
Feature request: Request new access token instead of throwing exception See #588 for details of setup. It would really help if this (default in-memory cache with API requests) worked after application restarts. It is a really bad dev experience having to clear out the cache before you can test when you have a valid session but no token. Would it be possible to add a feature where if the access token is missing and the identity session is present and valid, then the application gets a new access token instead of throwing an exception. This would help loads Greetings Damien |
@damienbod. Unless I mis-understand, I'm confused. We already have this feature: The authorizeForScopes attribute and the MicrosoftIdentityConsentAndConditionalAccessHandler class. Did you read this: https://github.com/AzureAD/microsoft-identity-web/wiki/Managing-incremental-consent-and-conditional-access ? What else would you have in mind? would you think that we don't even need to give a chance to the the developer to handle the exception? |
@jmprieur Thanks for your answer :) Now Im confused. At the moment when I run the application for the first time everything works fine. I use in-memory cache. When I stop my application and then start it again, it no longer works and returns an exception. I would like that no exception is thrown and that on the second start with in-memory cache it just works. (ie gets a new access token) Is this possible? I don't see this configuration in the doc, maybe I missed something. With the default in-memory implementation, I have to delete the cookies after every start to test. This is in my opinion not good. I also think that as in-memory is the default, I should be able to test (stop-start, stop-start) without error. Greetings Damien |
This is behaviour we see with Blazor server as well if the connection is lost between the browser and server (eg: connectivity or user refreshes page after a while) |
I just ran into this and fixed it with the ConsentHandler, though it would be nice to be able to handle it in a service instead of every time the service is used in a blazor page. Is there a way to do that now? |
@jdaless : the service needs to communicate with the user, and that can only be through the app. Which is why the flow is a bit convoluted |
Here is the design I propose for this issue: Add a new method This would be an opt-in method, used like this: services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
.AddMicrosoftIdentityWebApp(Configuration.GetSection("AzureAd"))
.EnableTokenAcquisitionToCallDownstreamApi(initialScopes)
.WithForceLoginWhenEmptyCache()
.AddMicrosoftGraph(Configuration.GetSection("GraphBeta"))
.AddInMemoryTokenCaches(); It's implementation could be something like the following: public MicrosoftIdentityAppCallsWebApiAuthenticationBuilder WithForceLoginWhenEmptyCache()
{
Services.Configure<CookieAuthenticationOptions>(cookieScheme, options => options.Events = new RejectSessionCookieWhenAccountNotInCacheEvents());
return this;
} and with internal class RejectSessionCookieWhenAccountNotInCacheEvents : CookieAuthenticationEvents
{
public async override Task ValidatePrincipal(CookieValidatePrincipalContext context)
{
try
{
var tokenAcquisition = context.HttpContext.RequestServices.GetRequiredService<ITokenAcquisition>();
string token = await tokenAcquisition.GetAccessTokenForUserAsync(
scopes: new[] { "profile" },
user: context.Principal);
}
catch (MicrosoftIdentityWebChallengeUserException ex)
when (AccountDoesNotExitInTokenCache(ex))
{
context.RejectPrincipal();
}
}
/// <summary>
/// Is the exception thrown because there is no account in the token cache?
/// </summary>
/// <param name="ex">Exception thrown by <see cref="ITokenAcquisition"/>.GetTokenForXX methods.</param>
/// <returns>A boolean telling if the exception was about not having an account in the cache</returns>
private static bool AccountDoesNotExitInTokenCache(MicrosoftIdentityWebChallengeUserException ex)
{
return ex.InnerException is MsalUiRequiredException
&& (ex.InnerException as MsalUiRequiredException).ErrorCode == "user_null";
}
} Alternatively |
Hi @jmprieur, this error occurs when MSAL's access token filtering logic returns more than 1 result. We filter user tokens by:
Out of all of these, the scopes might be problematic, because there could be overlapping scopes etc. Note that we compare request scopes to response scopes. MSAL has logic to delete access tokens with overlapping scopes, but maybe we have a bug there? @h2df @sidediallo Is it possible to enable verbose MSAL logging with PII to understand this better? Feel free to send them directly to bogavril[at]microsoft.com Or to give exact repro steps, i.e. what scopes are you asking for the first time and what scopes are you asking for the second time when you get the error? And what user is logging in (Personal account or Work School account?). But logs would be better. |
Hi @bgavrilMS and thank you for your feedback.
I added it to manage the
only run if the account is not present in the token cache. |
@sidediallo doe you have any progress yet to implement the GetAccessTokenForUserAsync in the case of an "AccountDoesNotExitInTokenCache" exception? |
This is not constructive in any way, but I'm going to share anyway... This functionality led to a couple of days trying to figure out why my app was losing the token that I expected to be there. I'm building new Blazor Server/ASP.NET Core Web API projects and trying to secure them with Azure B2C. This is my first time doing this, so the documentation wasn't super helpful. I feel like you need to have a grasp on Microsoft Identity before you can decipher the information in the provided docs. I eventually stumbled upon https://github.com/Azure-Samples/ms-identity-blazor-server/blob/main/WebApp-OIDC/B2C/README.md, which got me 90% of the way there, but I screwed up by wrapping the call to 'GetAccessTokenForUserAsync()' in a try-catch to log whatever the exception was. Today, I finally realized that this exception needed to be left unhandled so that it would bubble up to the 'ConsentHandler.HandleException()' call. I would just like to echo the sentiments of others: this functionality is very unintuitive. It would be nice if the VS2022 Blazor Server template would use a different cache method for tokens (if that would indeed solve the issue). This functionality nearly pushed me away from Azure AD B2C all together in favor of something like an API key auth model. I'm glad I stuck with it and followed the rabbit hole to the bottom, but still. It shouldn't have been this hard to get Blazor Server querying an API that is protected with Microsoft Identity. I feel that I wouldn't have encountered this issue if I was using Azure AD (non-B2C). That development experience was always pretty intuitive and easy to build. I'll shut up. Thanks for listening. |
It's actually very constructive and helpful feedback @DerekFoulk |
Thanks @DerekFoulk for this feedback. |
This is the same problem i am facing with. How can we ensure that the token is generated only once ? |
@erhan355. |
It is a bit long reply, I tried to sum up my flow briefly. To prevent this , I used your workaround(RejectSessionCookieWhenAccountNotInCacheEvents ) but this time it causes another exception. My app calls both my protected azure api and graph.When app starts on debugging mode it first hits RejectSessionCookieWhenAccountNotInCacheEvents and context.RejectPrincipal(); is executed I think Gatewayscope comes to play because of being default scope of downstream api but I can't figure out that why profile scope doesn't present in token.That's weird. After passing RejectSessionCookieWhenAccountNotInCacheEvents without exception it hits index page with following attribute.(Because It is razor page not mvc, attribute is placed on the top of the file) When it is done with OnGet method on my Index page ,header component is called on my layout which fetches current user's photo when it finishes job(successfully obtains access token) RejectSessionCookieWhenAccountNotInCacheEvents is hit again and an exception is thrown. MsalClientException: The cache contains multiple tokens satisfying the requirements. Try to clear token cache. By the way despite demanding User.Read scope at GetPhotoAsBase64Async at HeaderViewComponent ,access token with following scopes "scp": "User.Read User.Read.All profile openid email" are received.I think this is because of configured app permissions. |
@jmprieur Indeed, I did. One important thing that I didn't mention is that I am using VS2022 Preview. Here is my VS2022 "Info" output: Microsoft Visual Studio Community 2022 Installed Version: Community .NET Core Debugging with WSL 1.0 ADL Tools Service Provider 1.0 ASA Service Provider 1.0 ASP.NET and Web Tools 2019 17.3.122.33185 Azure App Service Tools v3.0.0 17.3.122.33185 Azure Data Lake Tools for Visual Studio 2.6.5000.0 Azure Functions and Web Jobs Tools 17.3.122.33185 Azure Stream Analytics Tools for Visual Studio 2.6.5000.0 C# Tools 4.3.0-1.22254.1+9919d7e7bd753404a5d2328e5e3fb2de635169f3 Common Azure Tools 1.10 Microsoft Azure Hive Query Language Service 2.6.5000.0 Microsoft Azure Stream Analytics Language Service 2.6.5000.0 Microsoft Azure Tools for Visual Studio 2.9 Microsoft JVM Debugger 1.0 NuGet Package Manager 6.3.0 Razor (ASP.NET Core) 17.0.0.2222701+751db1ebea5e6a9ecc7fa57fe447180422afa610 SQL Server Data Tools 17.0.62204.01010 ToolWindowHostedEditor 1.0 TypeScript Tools 17.0.10420.2001 Visual Basic Tools 4.3.0-1.22254.1+9919d7e7bd753404a5d2328e5e3fb2de635169f3 Visual F# Tools 17.1.0-beta.22178.3+6da0245a7ce4bb8483b8d1f2993c8ecaea967ad9 Visual Studio IntelliCode 2.2 |
Thank you @jennyf19, I always feel like a whiner on here lol |
@jmprieur I actually used File | New (instead of dotnet) to create both the Blazor Server App and ASP.NET Core Web API projects, using the Connected Services window to setup Azure AD B2C. After I got both projects created, calls to the ASP.NET Core Web API didn't authenticate properly, if I remember correctly. I had to add these properties to the appsettings.json file to get things working as the VS template didn't include them:
I also had to blow away the auto-generated App Registrations in the Azure Portal and redo them step-by-step using the examples in https://github.com/Azure-Samples/ms-identity-blazor-server/blob/main/WebApp-OIDC/B2C/README.md as a guide on how to configure them properly. I never did figure out what was configured incorrectly in the auto-generated app registrations, I just know that after I deleted them and created new ones that mirror the repo in the link above- things started working properly. Just try to create a Blazor Server App and an ASP.NET Core Web API project, using the VS2022 interface to create the projects and configure Azure AD B2C to secure the Web API calls between the two projects and you'll see what I mean. Something is not right when you use this flow- the auth doesn't work. I wish I had more time available to provide a repro, but I have a couple family emergencies going on at the moment. Thanks for listening |
Thanks @DerekFoulk For the moment, I'd recommend you use the msidentity-app-sync dotnet tool. See https://github.com/AzureAD/microsoft-identity-web/tree/master/tools/app-provisioning-tool |
This issue stole 3-4 days of my week with great confusion and frustration. Please ensure it gets fixed! |
@jmprieur, I've implemented your work-around, which works great. However, I see two problems with this solution:
In order to avoid these problems, I came up with a different solution:
I share my code in case someone is interested: public class RejectSessionCookieWhenAccountNotInCacheEvents : CookieAuthenticationEvents
{
public async override Task ValidatePrincipal(CookieValidatePrincipalContext context)
{
var msalTokenCacheProvider = context.HttpContext.RequestServices.GetRequiredService<IMsalTokenCacheProvider>();
var configuration = context.HttpContext.RequestServices.GetRequiredService<IConfiguration>();
/* Build a confidential application to be able to access the token cache. */
ConfidentialClientApplicationOptions options = new();
configuration.GetSection(Constants.AzureAd).Bind(options);
var app = ConfidentialClientApplicationBuilder
.CreateWithApplicationOptions(options)
.Build();
/* Check in the cache if the current user is present. */
msalTokenCacheProvider.Initialize(app.UserTokenCache);
var accountId = (context.Principal ?? throw new InvalidOperationException("Missing cookie principal")).GetMsalAccountId();
var account = await app.GetAccountAsync(accountId);
if (account == null)
{
/* The current user is not present in the token cache, and needs to authenticate again in order to get security tokens. */
context.RejectPrincipal();
}
}
} |
@jmprieur is it correct to assume this scenario, on Blazor Server, should be resolved by catching the exception and let ConsentHandler.HandleException() resolve the situation? And is there a way to implement the [AuthorizeForScopes(Scopes = new[] { "scopes" } ] attribute in Blazor pages? |
Yes @MauRiEEZZZ The attribute works on MVC controllers, MVC actions, Razor pages and Razor actions. Did you try? |
I will build an example that uses the Web Api from https://github.com/Azure-Samples/active-directory-dotnet-native-aspnetcore-v2 and report the behaviour. |
Is there any update or solution for the "Multiple access tokens found for matching authority, client_id, user and scopes" issue mentioned in the comments below? |
I believe this is the issue I'm facing in my app, related to the above #2185. Can someone confirm, and provide an update as to if / when this will be fixed? It is a significant issue for some fairly high profile clients with tens of thousands of users. |
Are there any updates? I'm still facing this issue. |
Not sure if this belongs here, I reached this issue by googling, I speak Spanish and this is the first time ever writing here. please let me know if this could be a work around for the issue, trying to implement this for my MVC .NET Framework 4.7.2 Web App with Microsoft.Identity.Web and Microsoft Graph SDK:
app will try to get the token because the auth claims are still there (hence why user is still logged in), yet this will fail. public async Task<ActionResult> Index()
{
try
{
//Lets say user already logged in, authenticated = true
var isAuthenticatedInMicrosoft= ComunesModel.ObtenerEstaAutenticadoEnMicrosoft();
var homeVM = await HomeModel.CargarHomeViewModel(GraphClient, isAuthenticatedInMicrosoft);
return View(homeVM);
}
catch (Exception ex)
{
//Deleted token registry manually in order to trigger ex
var exception = ex.InnerException as MsalUiRequiredException;
if (exception != null && exception .ErrorCode.Contains(MsalError.UserNullError))
{
//Removing claims from auth ticket
HttpContext.GetOwinContext().Response.Cookies.Delete("EntraCookie");
//CargarHomeViewModel (has default bool value false)
//bool is false because the user is not authenticated anymore.
var homeVmSinAutenticacion = await HomeModel.CargarHomeViewModel(GraphClient);
return View(homeVmSinAutenticacion);
}
var homeVM = await HomeModel.CargarHomeViewModel(GraphClient);
return View(homeVM);
}
} internal static bool ObtenerEstaAutenticadoEnMicrosoft()
{
var authenticated = ClaimsPrincipal.Current.Identities.Any(i => i.AuthenticationType == "Cookies");
return authenticated;
} |
This Specifically, this exception happens because the "openid" scope is filtered out resulting in filtering for "no scope" in the cache. And that means it will return any and all tokens for that user, because every token matches those requirements. Thus this exception. One workaround for this is that you change "openid" to "User.Read" (or some other scope) and add that scope to the application in Azure. |
@edahlberg - MSAL should have logic to always disambiguate tokens. That exception is always a bug. If you can provide a repro, we will aim to fix it. |
Holy necromancer- I'm 2 years late to respond! I'm sure that I did. I almost never use the cli to create projects/solutions. After File > New, I most likely jumped straight into the docs and followed them to a "T". |
@bgavrilMS I created #2998 as it's not strictly related to this issue |
from @onovotny and copied from Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2#240
In the Microsoft.Identity.Web library, the system should automatically clear (
RejectPrincipal()
) the auth cookie if the corresponding account entry is missing from the token cache. This can happen if the cache expired, if it was a memory cache and the server bounced, etc.The issue is that the system is now in an inconsistent state, where the user is considered logged-in, but any operations to call API's won't succeed because there's no token cache, no refresh token, etc.
I've worked around this here: https://github.com/dotnet-foundation/membership
In order to do so, I had to make some changes to
ITokenAquisition
and the MsalAbstractCacheProvider's GetCacheKey method.ITokenAcquisition
needed a method to check for a user's token cache:https://github.com/dotnet-foundation/membership/blob/97b75e30e50aab76bfa5a21f1ab88bf31ae66da4/Microsoft.Identity.Web/TokenAcquisition.cs#L406-L426
In there, it takes the
CookieValidatePrincipalContext
to get the incoming ClaimsPrincpal as HtttpContext.User is not yet set at that point. It stores it in the HttpContext.Items via theStoreCookieValidatePrincipalContext
extension method (used later by the GetCacheKey method so it can derive the appropriate key):https://github.com/dotnet-foundation/membership/blob/97b75e30e50aab76bfa5a21f1ab88bf31ae66da4/Microsoft.Identity.Web/TokenCacheProviders/MsalAbstractTokenCacheProvider.cs#L68-L69
Finally, the
CookieAuthenticationOptions
needs to be configured to check for and reject the incoming principal (this could/should be moved into the main IdentityPlatform AddMsal extension methods):https://github.com/dotnet-foundation/membership/blob/97b75e30e50aab76bfa5a21f1ab88bf31ae66da4/Membership/Startup.cs#L110-L123
I can submit these changes as PR if you're in agreement with these changes.
The text was updated successfully, but these errors were encountered: