2

Something rather scary is happening on my ASP.NET Core 2.1.0 MVC site. While I was browsing, all of a sudden it shows I am logged in as a different user (who also happens to be browsing the site at that time).

I can't pinpoint whether there is a specific use case that triggers this, but this has happened twice now. Navigating to other pages still shows I am logged in as the other user. It even seems I take over the claims of the user I am incorrectly logged in as.

My question is: what could make this happen?

EDIT: I have since changed userManager and notificationService to 'scoped' and this issue occurred again, thus the potential issue reported here cannot be the cause.

Trying to look into this, I believe the culprit might be the following call in _Layout.cshtml:

@inject UserManager<ApplicationUser> userManager
@inject NotificationService notificationService
@inject CommunityService communityService
@{
    ApplicationUser user = await userManager.GetUserAsync( User );
}

The returned user is used to show user information and do calls to notificationService and communityService. These were also showing data pertaining to the incorrect (not me) user.

If it matters, this is how ApplicationDbContext is set up in Startup.cs:

// Add framework services.
services
    .AddDbContext<ApplicationDbContext>( options => options
        .UseLazyLoadingProxies()
        .UseSqlServer(_configuration.GetConnectionString( "DefaultConnection" ) ) );
services
    .AddIdentity<ApplicationUser, IdentityRole>()
    .AddEntityFrameworkStores<ApplicationDbContext>()
    .AddDefaultTokenProviders();

I recalled that 'scoped' is the recommended lifetime to use when registering Entity Framework for dependency injection. Both NotificationService and CommunityService, however, are registered as 'transient' and request ApplicationDbContext through constructor injection to access data.

services.AddTransient<CommunityService, CommunityService>();
services.AddTransient<NotificationService, NotificationService>();

Could this have anything to do with it? Currently, I do not understand whether this could make any difference. I cannot seem to replicate this issue.

Steven Jeuris
  • 18,274
  • 9
  • 70
  • 161
  • AFAIK, unless you get access to the Auth cookie of the other user, it is bit difficult to impersonate the users. How are you testing it, two instances on the same machine or from 2 different machines? I guess the 'AddTransient' and 'AddScoped' has nothing to do with this issue. – Thangadurai Jul 28 '18 at 13:49
  • @Thangadurai This happened on two different machines, in a live environment. – Steven Jeuris Jul 28 '18 at 13:54
  • 1
    Any cached parts which don’t realize there’s a change in data and don’t update or something? – Sami Kuhmonen Jul 28 '18 at 13:55
  • @SamiKuhmonen Not that I am aware of. `UserManager` is the default one provided by MVC, and I simply show `user.DisplayName` (which showed up as 'not me'). In addition, I cannot log in as the other user (as in, I do not have pwd, or Google Authentication, whichever he uses). – Steven Jeuris Jul 28 '18 at 13:57
  • Are you behind any CDN or proxy? – Grimm Jul 28 '18 at 14:03
  • @SamiKuhmonen Sorry. Apparently `DisplayName` is already a property I added myself, but it is a simple property with a getter and setter I added to the default `ApplicationUser : IdentityUser`. – Steven Jeuris Jul 28 '18 at 14:09
  • @Grimm When the error occurred I was on my mobile phone. 4G network by Oister in Denmark, if that tells you anything. :) The other user (my brother) was in Belgium. – Steven Jeuris Jul 28 '18 at 14:10
  • @Thangadurai To me, it seems like entity framework is the most likely culprit here, since the lifetime of the objects returned is managed by entity framework. If (for some reason) the lifetime/caching is extended between requests, this could happen. – Steven Jeuris Jul 28 '18 at 14:13
  • 1
    @StevenJeuris OK, I guess we can exclude this theory then ... – Grimm Jul 28 '18 at 14:32
  • "The returned user is used to show user information and do calls to notificationService and communityService" ... Can you show how these services use that User object? – Grimm Jul 28 '18 at 14:35
  • @Grimm, certainly. They primarily query `ApplicationDbContext` by doing ID matching, e.g., `_data.Notifications.Where( n => n.UserId == user.Id )`. However, I have one method that actually sets a property on the passed user object and calls `_data.SaveChangesAsync()` in `notificationService`. This is called from a different page than `Layout` (Notifications). Does any of this matter? – Steven Jeuris Jul 28 '18 at 14:48
  • Hm, looks all fine to me. I'm rather helpless at the moment ... – Grimm Jul 28 '18 at 15:23
  • Upgraded to ASP.NET 2.1.2 now and using 'scoped' for the two services instead of 'transient'. I hope nobody runs into this error, but I have no clue why these changes should change anything. I will update in case I get a report this happened again. – Steven Jeuris Jul 28 '18 at 15:57
  • `ApplicationUser user = await userManager.GetUserAsync( User );` - What is the User parameter here? Normally, I use the 'sub' claim which is the unique identifier of the currently logger in user, and the use the FindById to get the user details. Transient is more restricted, i.e. every place the object is required a new instance is supplied by the DI framework, but with 'scoped', the same object is shared but for that one request. – Thangadurai Jul 29 '18 at 05:23
  • @Thangadurai `User` is provided by MVC and is defined in `RazorPageBase` as `public virtual ClaimsPrincipal User { get; }`. – Steven Jeuris Jul 29 '18 at 10:10
  • This just happened again, this time with a different user. :O In fact, I was wrong, it seems like I also take over the claims of the user I am incorrectly logged in as. :O – Steven Jeuris Jul 30 '18 at 11:13

1 Answers1

2

Good news! I was causing this myself (I believe, help me figure this out by reading the details below). You can thus rest assured that unless you are making the same mistake as I am, the ASP MVC authentication mechanism is not to blame here (at least, that is my current understanding).

I will document what exactly I did wrong, and how to replicate, since others might possibly make the same mistake.

In short: I called SignInManager<TUser>.RefreshSignInAsync(TUser) with the 'wrong' user (the one I ended up being logged in as), causing me to be logged in as that user.

Why did I do this? In my specific use case, I wanted to hand out a claim to another user based on the action of the currently logged in user. I therefore called:

await _userManager.AddClaimAsync( userToGiveClaim, newClaim );
await _signInManager.RefreshSignInAsync( userToGiveClaim );

I called RefreshSignInAsync since I wanted to prevent the user who had been given the claim from having to log out and in for the new claim to go into effect. From the RefreshSignInAsync documentation I got the impression this should work:

Regenerates the user's application cookie, whilst preserving the existing AuthenticationProperties like rememberMe, as an asynchronous operation.

Parameters user The user whose sign-in cookie should be refreshed.

I'm still not entirely clear why the user that is currently logged in when this call is triggered gets the identity of the user passed to this call. That is still not how I understand the documentation (I filed this as a bug report), but since this is reproducible I am now more inclined to believe I simply misunderstood the documentation.

Steven Jeuris
  • 18,274
  • 9
  • 70
  • 161
  • [The user in this questions seems to have made the same mistake](https://stackoverflow.com/q/45347956/590790). – Steven Jeuris Jul 30 '18 at 11:59
  • [More users seem to wonder whether this should be possible](https://stackoverflow.com/questions/39026796/update-claims-values-in-asp-net-one-core/40815319#comment86262490_40815319). – Steven Jeuris Jul 30 '18 at 12:12
  • This makes perfect sense to me. https://github.com/aspnet/Identity/issues/1831 https://github.com/aspnet/Identity/blob/master/src/Identity/SignInManager.cs It regenerates the users cookie with updated claims. If you call this method with a different user it is going to sign you out and sign in the user passed in. – Alex Oct 09 '18 at 20:04