dotnet-cas-client
dotnet-cas-client copied to clipboard
Fixed static currentPrincipal (issue #32)
Avoids multiple users in ASP.NET application sharing a single identity.
I have tried to reproduce the issue using my own demo (with the .Net CAS client v1.0.2): https://github.com/leleuj/dotnet-cas-client-demo, but unsuccessfully.
I open two browsers and authenticates in each of them with different identities and I do have two different identities.
Can you check on your side on give me more details? Thanks
Since the backing variable is [ThreadStatic], it might be required for both requests to be served by the same webserver thread. This behaviour will depend on whether you are using VS's internal server, IIS, etc.
Honestly I'm not sure of the conditions to make this happen, only that it happened consistently in our staging environment (web app hosted in IIS7), and that this diff solved it.
OK. So if I want to reproduce it, I should simulate some load to make authentications on the same request, right?
Maybe... It may also be that the issue only appears on some web servers (such as IIS but not visual studio's integrated one).
Should user principal objects even be static?
Yes, I have the same feeling: I would have expected the principal to be stored in the web session like in the Java or PHP CAS client for example.
Thread.CurrentPrincipal
is the "official" method of authenticating a user in .NET.
This article was a good read : http://www.lhotka.net/weblog/ASPNETThreadSwitching.aspx
I feel like with this PR the code is OK, maybe it would be a good idea to set Thread.CurrentPrincipal = null
explicitly at the start of the method.
The purpose of that field is similar to that of AssertionThreadLocalFilter
in the Java CAS Client; it provides access to the current principal in other arbitrary components in the request pipeline. I'm open to other solutions, but session storage may not be appropriate since the session may not have been allocated yet. Authentication happens pretty early in request processing as I recall.
Looking at b345f1f, where the field was introduced, we should probably just remove the field and use HttpContext.Current.User
directly. Several references indicate that is the best way to determine the identity of the authenticated user; here's a good one: http://stackoverflow.com/questions/1843271/whats-the-difference-between-httpcontext-current-user-and-thread-currentprincip.