Weird HttpContext.User.Identity issue

Question

I have an MVC 3 project which authenticates users by Forms Authentication.

To access user's different information when needed after authentication I have implemented a simple class:

public class ComplexUserData
{
   public static string complexAccountName;
   public static string complexRole;
   ...
   ...
}

On every page request the controller looks to this object and fills in the data to properties if needed.

On the other hand:

The login process goes as:

1. LogOn page is requested
2. Prior to returning LogOn View, the controller checks if HttpContext.User.Identity.Name is not null
3. If it is not null, find the user with that username and redirect to proper landing page
4. Else return the LogOn page

And the logout process goes as:

1. FormsAuthentication.SignOut();

   Session.Abandon();

   HttpCookie cookie1 = new HttpCookie(FormsAuthentication.FormsCookieName, "");

   cookie1.Expires = DateTime.Now.AddYears(-1); Response.Cookies.Add(cookie1);

   HttpCookie cookie2 = new HttpCookie("ASP.NET_SessionId", "");

   cookie2.Expires = DateTime.Now.AddYears(-1);

   Response.Cookies.Add(cookie2);

2. Redirect to LogOn

In my master page I use SessionData.complexAccountName in top bar. Everything seems fine until user testing.

User A says that after he left a page idle for a while and then logs out from the system the site returns to the main page with displaying User B on the top bar. At the same time User B did the same thing and saw User A on top bar.

So I assume that when the users logs out and redirected to the logon page, as the process indicates, the controller checks the HttpContext.User.Identity.Name and while it should be null, it is not! Actually it holds the name of another user!

What am I doing wrong, why is the system acting weirdly.

Thanks in advance and best regards.

Answer 1

Your code uses static fields:

public class ComplexUserData
{
  public static string complexAccountName;
  ...

Such a static field is shared by all threads, so all users are working on the same data(!) This is very dangerous.

static is probably the most dangerous keyword, because the code works but fails weirdly in a multi-threaded scenario and can leak information to other parties.

Solution: Don't use static unless you really understand what it does. Never store per-user, per-request or similar information in static variables. Store all user information in the session, in a database or a similar data store.