microsoft-identity-web
microsoft-identity-web copied to clipboard
AzureAD
Should clear session auth cookie if cache is missing account
from @onovotny and copied from https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/issues/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 the StoreCookieValidatePrincipalContext 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.
@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 here as this seems to be a big blocker for my Blazor server side app atm
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. During development I could bypass this error by starting an inprivate browser session and close all inprivate sessions before starting a new one.
At the moment I don't have this issue and I don't know if this helps in it. This part of my code it stupid copy/paste without knowing why it works
` // Token acquisition service based on MSAL.NET // and chosen token cache implementation services.Configure<CookiePolicyOptions>(options => { // This lambda determines whether user consent for non-essential cookies is needed for a given request. options.CheckConsentNeeded = context => false; options.MinimumSameSitePolicy = SameSiteMode.Strict; // Handling SameSite cookie according to https://docs.microsoft.com/en-us/aspnet/core/security/samesite?view=aspnetcore-3.1 options.HandleSameSiteCookieCompatibility(); });
services.AddOptions();
services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
.AddSignIn(x =>
{
Configuration.Bind("AzureAD", x);
x.UseTokenLifetime = true;
x.GetClaimsFromUserInfoEndpoint = true;
x.Events.OnTokenValidated = async ctx =>
{
// do something }; }, y => { Configuration.Bind("AzureAD", y); });
// Token acquisition service based on MSAL.NET
// and chosen token cache implementation
services.AddWebAppCallsProtectedWebApi(Configuration,
new[] {Constants.ScopeUserRead})
.AddInMemoryTokenCaches();
`
Feature request: Request new access token instead of throwing exception
See https://github.com/AzureAD/microsoft-identity-web/issues/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)
@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?
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
@jennyf19 @clairernovotny
Here is the design I propose for this issue:
Add a new method .WithForceLoginWhenEmptyCache() on the MicrosoftIdentityAppCallsWebApiAuthenticationBuilder to force the users to login when there is a session cookie, but no account in the cache (for instance because the cache is an in memory token cache and the application was restarted).
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 AccountDoesNotExitInTokenCache could be a bool property surfaced on MicrosoftIdentityWebChallengeUserException
@jennyf19 how do you lose anything given that the token cache is empty? Either way, a redirect to the IDP is required, isn't it?
@clairernovotny do you control the tenant in which your app runs? or is it up to the customers? if the latter, then tenant admin can enable some conditional access policies that will trigger a multi-factor authentication even if you have pre-authorization. By not using the [AuthorizeForScopes] attribute you don't allow your app to process this kind of scenario. you'll have to handle it yourself, by challenging the user (if you are in the web app part of your app), or reply with a WWW-Authenticate header (if you are in the web API part of your app). Ms.Id.Web supports these scenarios for you, if you use the exception filter attribute. Otherwise you are on your own.
I control the tenant where the app runs, it's not up to the customers. There's no chance CA will take effect here.
@jmprieur i think we should document this specific scenario, but not take any product changes.
@jennyf19 there's no workaround today as the GetAccessTokenForUserAsync is internal. If that was external, I could code around it. I should also note that this is part of an ADAL migration and I don't have time to rewrite the app around Identity Web's new concepts. I'm sure I'm not the only one in this boat. For new apps, it's easier to work with the new model, but it needs to be easy to get off of ADAL given its imminent deprecation.
Adding things like support for incremental consent and CA are "nice to have," and important, but apps migrating from ADAL already don't support this, so they're no worse off. They need a path forward off of the deprecated library. They can then gradually add support for the missing functionality.
@clairernovotny : I provided the work around and tested it.
In Startup.cs:
services.AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
.AddMicrosoftIdentityWebApp(Configuration.GetSection("AzureAd"))
.EnableTokenAcquisitionToCallDownstreamApi(initialScopes)
.AddMicrosoftGraph(Configuration.GetSection("GraphBeta"))
.AddInMemoryTokenCaches();
Services.Configure<CookieAuthenticationOptions>(cookieScheme, options => options.Events = new RejectSessionCookieWhenAccountNotInCacheEvents());
And then (inspired by what you had)
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";
}
}
@clairernovotny GetAccessTokenForUserAsync is public. If this is a 1P migration, from ADAL to MSAL, we have a procedure in place to assist with migration. Please ping one of us and we'll send you the link to the Teams channel. ADAL did not support incremental consent, that's a v2 concept, but it does support CA.
I didn't realize that was public, thanks. Documenting it would be helpful, for sure somewhere in the wiki where it'll be seen.
This isn't 1P, it's for the .NET Foundation's code signing service, so like 1.5P. Not sure if that counts?
sounds good.
the migration is for web apps/web APIs right now, so if that makes sense, feel free to join.
That's what it is, it's a combination web app and api app. It could very much benefit from an updated security review too to ensure it's doing the right things with the new code. what's the right way to get started?