CODEWITHSUNDEEP
c#asp.netasp.net-web-apimembership-providersimplemembership
rosalindwills
rosalindwills

Reputation: 1484

SimpleMembershipProvider intermittently returning wrong user

I am administrator of a small practice project web application, AngularJS front-end pulling its back-end data from a C#/.NET WebAPI, and I'm handling security using the SimpleMembershipProvider.

I suspect that the way I implemented said security is not the best (I'm told ASP.NET Identity is now the way to go?) but that's another question altogether.

The issue that I'm very bewilderingly running into is that I get occasional reports that on a given page load to display a particular user's data, it returns somebody else's. Reloading the page fixes the issue (evidently) and I haven't been able to duplicate the scenario myself, or figure out anything particularly consistent in the users to which this happens.

None of the information being displayed is at all sensitive in nature (the app's just a friendly front end for an already public third-party API) so I'm not in panic mode about this, but I am both concerned and confused and want it fixed.

Here is what one of my API controller endpoints looks like:

[Authorize]
public class UserController : ApiController
{
    private static int _userId;
    private readonly IUserProfileRepository _userProfileRepository;

    public UserController()
    {
        _userProfileRepository = new UserProfileRepository(new DatabaseContext());
        _userId = WebSecurity.GetUserId(User.Identity.Name);
    }

    public UserProfileDto Get()
    {
        return _userProfileRepository.GetUserProfileById(_userId).ToDto();
    }
}

Any feedback on where I might be going wrong here or what might be causing the intermittant inconsistency would be very much appreciated. (Laughter also acceptable if the way I handled this is just really bad. :P )

Upvotes: 0

Views: 55

Answers (2)

Igor
Igor

Reputation: 15893

Static class fields are shared by all instances/threads of the same AppDomain (in your case - process). Different http requests are processed by threads running in parallel. Any two threads running [almost] at the same time may (will) change the value of _userId. You are assigning _userId in the constructor of your controller, and a new instance of this controller is created for each http request that is to be responded to by UserController. Therefore, this assignment will happen multiple times.

You will have hard time replicating this problem, since you are a single user testing the code, hence there are no overlapping request threads.

Remove static specifier from the _userId field declaration of the controller class.

Note: make sure that DatabaseContext is disposed of. One place that can be used for this is the overriden Controller.Dispose.

Upvotes: 2

Brendan Green
Brendan Green

Reputation: 11924

Change the Get to retrieve the user id rather than from a static variable:

public UserProfileDto Get()
{
    return _userProfileRepository.GetUserProfileById(WebSecurity.GetUserId(User.Identity.Name)).ToDto();
}

Upvotes: 2

Related Questions