Wednesday, 29 October 2014

The good, the bad and the ugly of ASP.NET Identity


Ok, here we go again… and if you don’t know what I’m talking about, then see this post.
With Visual Studio 2013 and .NET 4.5.1 we have a new framework called ASP.NET Identity. ASP.NET Identity is yet another identity management framework from Microsoft (recall that we also had two prior frameworks from Microsoft: Membership and SimpleMembership).  Let’s take a look at the good and the bad aspects of this new framework.
TLDR; Click here to get to the ugly conclusion.
One of the major complaints with the previous identity management frameworks was that it was either too cumbersome (with Membership) or too subtle (with SimpleMembership) to customize the storage. With this release, they’ve actually achieved a separation of the storage of the identity information (e.g. username, password, etc.) from the code that implements the security (e.g., password hashing, password validation, etc.). The way they’ve done this is by defining the account related data behind an interface, IUser and the storage operations behind another interface IUserStore. Here they are:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
public interface IUser
{
   string Id { get; }
   string UserName { get; set; }
}
 
public interface IUserStore<TUser> : IDisposable where TUser : IUser
{
   Task CreateAsync(TUser user);
   Task DeleteAsync(TUser user);
   Task<TUser> FindByIdAsync(string userId);
   Task<TUser> FindByNameAsync(string userName);
   Task UpdateAsync(TUser user);
}
The idea is that you then implement these interfaces and this puts you in control of how the account data is actually stored. This also makes it easier for developers to customize the user account data. If you want more data associated with your user you can add it on your custom user class that implements this interface. This extra data would then be stored by your implementation of the IUserStore.
There are default implementations of these interfaces that use Entity Framework 6. When you choose “Individual User Accounts” in the new ASP.NET templates in Visual Studio 2013 you will get an IUser implementation in a class called ApplicationUser. Any custom data you’d want stored on your user accounts would be added to this class. Given EF’s support for POCOs, this extra data on the user account class will simply be mapped into the database with little effort on your part. See this post for an example.
For the class that implements IUserStore, there is a class from the ASP.NET Identity EF assembly called UserStore. It requires an EF DbContext, which is provided by another class generated in your project called ApplicationDbContext. There’s even less to code or customize on the ApplicationDbContext because its base already defined the DbSet for the user accounts (in its base class). The ApplicationDbContext class is primary there for you to control connection strings to indicate the database to actually use.
With this design it should be very straightforward and obvious for a developer using this framework what data is stored and how it is stored. In this sense, this new identity framework is a success and quells one of the long standing complaints about the previous membership systems.
Sidebar – Identity vs Authentication
Keep in mind that (as always) the provider model is simply about the storage and management of account related data. In a running application, once the user’s password has been validated (against the persisted password) then the user is logged into the application (typically) with some sort of cookie based mechanism like ASP.NET’s Forms authentication, WIF’s Session Authentication Module, or now in Visual Studio 2013 OWIN cookie middleware. Far too often the line between these two different subsystems (storage vs. authentication) is blurred. See this post for more context.
Another nice addition I should point out is that most (if not all) of the APIs in the new ASP.NET Identity system are asynchronous. This is a nice addition to the API, and almost assumed these days. I give kudos more to the EF team than anyone else since EF6 now supports asynchronous APIs.
So this new design segregates the storage of identity information from the rest of the security code. Well, what’s left? In theory it’s all the hard and complicated stuff related to identity management and the idea is that Microsoft will implement it for us. This is achieved via the UserManager class:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
public class UserManager<TUser> : IDisposable where TUser : IUser
{
   public UserManager(IUserStore<TUser> store);
 
   public ClaimsIdentityFactory<TUser> ClaimsIdentityFactory { get; set; }
   public IPasswordHasher PasswordHasher { get; set; }
   public IIdentityValidator<string> PasswordValidator { get; set; }
   protected IUserStore<TUser> Store { get; }
   public virtual bool SupportsUserClaim { get; }
   public virtual bool SupportsUserLogin { get; }
   public virtual bool SupportsUserPassword { get; }
   public virtual bool SupportsUserRole { get; }
   public virtual bool SupportsUserSecurityStamp { get; }
   public IIdentityValidator<TUser> UserValidator { get; set; }
 
   public virtual Task<IdentityResult> AddClaimAsync(string userId, Claim claim);4
   public virtual Task<IdentityResult> AddLoginAsync(string userId, UserLoginInfo login);
   public virtual Task<IdentityResult> AddPasswordAsync(string userId, string password);
   public virtual Task<IdentityResult> AddToRoleAsync(string userId, string role);
   public virtual Task<IdentityResult> ChangePasswordAsync(string userId, string currentPassword, string newPassword);
   public virtual Task<IdentityResult> CreateAsync(TUser user);
   public virtual Task<IdentityResult> CreateAsync(TUser user, string password);
   public virtual Task<ClaimsIdentity> CreateIdentityAsync(TUser user, string authenticationType);
   public virtual Task<TUser> FindAsync(UserLoginInfo login);
   public virtual Task<TUser> FindAsync(string userName, string password);
   public virtual Task<TUser> FindByIdAsync(string userId);
   public virtual Task<TUser> FindByNameAsync(string userName);
   public virtual Task<Collections.Generic.IList<Claim>> GetClaimsAsync(string userId);
   public virtual Task<Collections.Generic.IList<UserLoginInfo>> GetLoginsAsync(string userId);
   public virtual Task<Collections.Generic.IList<string>> GetRolesAsync(string userId);
   public virtual Task<bool> HasPasswordAsync(string userId);
   public virtual Task<bool> IsInRoleAsync(string userId, string role);
   public virtual Task<IdentityResult> RemoveClaimAsync(string userId, Claim claim);
   public virtual Task<IdentityResult> RemoveFromRoleAsync(string userId, string role);
   public virtual Task<IdentityResult> RemoveLoginAsync(string userId, UserLoginInfo login);
   public virtual Task<IdentityResult> RemovePasswordAsync(string userId);
   public virtual Task<IdentityResult> UpdateAsync(TUser user);
   public virtual Task<IdentityResult> UpdateSecurityStampAsync(string userId);
}
Notice the constructor accepts the IUserStore. An application developer would instantiate the UserManger passing in their user store. The developer would then code to the UserManager’s APIs to do all of the account related functions, such as creating an account, validating a password, changing the password, etc. The APIs, as you can tell from the names, are fairly self-explanatory. This pattern is apparent in any of the new ASP.NET templates in Visual Studio 2013 when you choose “Individual user Accounts” for the authentication configuration.
Also, notice how most of the APIs are virtual; if the developer wishes to customize any of the built-in behavior then they would simply override the appropriate method. Hopefully the need for such a thing is rare.
One of the selling points about the new identity system is that it supports claims. Here’s an excerpt from the announcement:
“ASP.NET Identity supports claims-based authentication, where the user’s identity is represented as a set of claims.”
“Claims-based authentication” is a misnomer, and is akin to saying “role-based authentication”. I think what they mean is that the new identity system can model user identities with claims. They get around to that sentiment in the latter half of the sentence, but their terminology misuse confuses the point. For clarity, the new identity system uses password-based authentication (just like the prior systems). There’s also federation support, but, strictly speaking, it’s not a feature of the new ASP.NET Identity system.
Ok, back to claims: Notice on the IUser definition there are no claims (also notice that a Claim doesn’t appear anywhere in the code for the new ASP.NET templates in VS2013). Ok, so where are they? Well, I think Microsoft is still not confident enough in developers to understand how claims work so they made this piece optional. Depending on your perspective maybe this is good and maybe this is bad. I think it’s bad.
Anyway, to model claims there is another interface called IUserClaimsStore:
1
2
3
4
5
6
public interface IUserClaimStore<TUser> : IUserStore<TUser>, IDisposable where TUser : IUser
{
   Task AddClaimAsync(TUser user, Claim claim);
   Task<System.Collections.Generic.IList<Claim>> GetClaimsAsync(TUser user);
   Task RemoveClaimAsync(TUser user, Claim claim);
}
The approach here is that your user store would then also implement this API to be able to associate claims with the user account.
I find this approach distasteful. If you are building a custom user store, then you now have to implement an additional store interface to support claims. It’d be much easier and obvious if the user definition had a claims collection property.
Also, to confuse the situation, you now have two ways of modeling additional identity data on your user: one as custom properties on the user class and another as claims in the claims store. Which leads me to the next issue.
As I mentioned above, a driving feature in the design of ASP.NET Identity was that it’s easy to store custom properties on the user. The only problem is that if they’re custom, then once the user logs into your application these properties won’t automatically be made available in the claims on the ClaimsIdentity (which is the standard class in .NET 4.5 for modeling an identity, is what represents the logged in user in any .NET application, and is what people mean when they say claims-aware). The reason that custom properties on your user class won’t be part of the ClaimsIdentity’s Claims collection is that the built-in code only knows about the IClaimsStore and not your custom properties.
This leads us to the next extensibility point, which is the ClaimsIdentityFactory on the UserManager. This is the class that maps the data on the user class to a ClaimsIdentity (presumably when the user logs in). If you need your custom properties to your user class as part of the ClaimsIdentity then you need to implement a custom ClaimsIdentityFactory to do this mapping.
This makes me wonder if it makes sense to add custom identity data to the user class and instead store it in the claims store. Also, this makes me wonder if it makes sense to add custom non-identity data to the user class at all and instead store it elsewhere in the database. I don’t know the right answer.
Yep, to complicate things there’s also an interface to associate roles with users called IUserRoleStore:
1
2
3
4
5
6
7
public interface IUserRoleStore<TUser> : IUserStore<TUser>, IDisposable where TUser : IUser
{
   Task AddToRoleAsync(TUser user, string role);
   Task<System.Collections.Generic.IList<string>> GetRolesAsync(TUser user);
   Task<bool> IsInRoleAsync(TUser user, string role);
   Task RemoveFromRoleAsync(TUser user, string role);
}
This interface, like IUserClaimStore, is optional and if you want explicit role support you will need to implement this on your user store.
For those who are already familiar with claims, you know full well that claims are a superset of roles and this it’s unnecessary to treat roles special and separate from claims. That also make this IUserRoleStore interface superfluous. Microsoft knows this as well, but for some reason they continue to feel that there’s demand for roles separate from claims.
For what it’s worth, the previously mentioned ClaimsIdentityFactory will read from both the claim store and the role store and will map them both to the ClaimsIdentity Claim collection.
But this also leads to the potential (rather, likely) confusion as to where should a role be kept. Should you store roles in the claims store or in the role store? Well, you get to decide and ensure everyone else on your team knows the right answer, especially if you have your own code querying those stores directly.
Notice the lack of a password (or rather hashed password) on the user account? That’s right, passwords are optional. Just like the claims and role stores, there’s an optional store if your application needs to persist passwords (actually, hashed passwords) for the user called IUserPasswordStore:
1
2
3
4
5
6
public interface IUserPasswordStore<TUser> : IUserStore<TUser>, IDisposable where TUser : IUser
{
   Task<string> GetPasswordHashAsync(TUser user);
   Task<bool> HasPasswordAsync(TUser user);
   Task SetPasswordHashAsync(TUser user, string passwordHash);
}
So if you want your users to be able to use passwords to login then your user store must also implement this interface.
I’m on the fence about this – I can imagine scenarios where users would only login with an external identity provider and thus never have a local password. But if this is your scenario, I don’t see much point in the ASP.NET Identity system. You have innumerable ways to store data anyway you want to without tying yourself to the ASP.NET Identity system APIs.
So I guess this really is bad, because why else would you be using this API.
I’ve been complaining for a long time that the default password hashing (PBKDF2) from Microsoft only performs 1000 iterations. With the UserManager API we can now plug in a custom implementation (via IPasswordHasher) and do more iterations as recommended by OWASP and this is good.
Unfortunately, this means you have to write security code and this is bad. I thought this was the whole point of using a security framework; someone else who supposedly knows what they’re doing is has already has done all the complex security work. This should have been implemented by Microsoft with an iteration count property instead.
The new ASP.NET Identity system allows you to map an external login provider to a local user account. This feature was introduced with SimpleMembership and is similarly available with this new framework. To store this info, though, there’s yet another store interface called IUserLoginStore which relies upon a class called UserLoginInfo which maintains the provider name and the identifier for the user:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
public interface IUserLoginStore<TUser> : IUserStore<TUser>, IDisposable where TUser : IUser
{
   Task AddLoginAsync(TUser user, UserLoginInfo login);
   Task<TUser> FindAsync(UserLoginInfo login);
   Task<System.Collections.Generic.IList<UserLoginInfo>> GetLoginsAsync(TUser user);
   Task RemoveLoginAsync(TUser user, UserLoginInfo login);
}
 
public sealed class UserLoginInfo
{
   public UserLoginInfo(string loginProvider, string providerKey);
 
   public string LoginProvider { get; set; }
   public string ProviderKey { get; set; }
}
This is sounding like a broken record; if you want external login support, your user store must also implement this interface. Again, I feel like this additional interface is burdensome and it’d be simpler if the external logins were a collection on the user definition.
Notice there are no APIs for arbitrary queries. This means you’ll have to go around the interfaces to query your user data.
One of the complaints with the old membership system was the leaky abstractions. So many APIs might not be implemented and your application had to know which APIs it could and couldn’t invoke on the membership APIs. This meant you were forcing yourself to adhere to an interface contract without the benefit of the interface abstraction.
In the new system, I think it’s better designed, but it still feels like there’s a lot of leaky abstractions. You notice on the UserManager (beyond the simple user related APIs) there are APIs for managing claims, roles, passwords and external logins. These APIs only work if the user store implements the corresponding IUserClaimStore, IUserRoleStore, IUserPasswordStore, and IUserLoginStore. This seems like it’s the same problem as before, just in a different form.
I must admit, the story is slightly better with the new ASP.NET Identity; if an application wants to use these APIs, though, it can first query the UserManager and ask if it can use these APIs (via SupportsUserClaim, SupportsUserRole, SupportsUserPassword and SupportsUserLogin). Again this is better than before, but it still feels like a weak API design in that it’s trying to be too many things to too many people. If your application needs one of these features, it’s going to just use it. This then means any other user store you’d want to use must implement these same features.
It’d be much stronger of a design if the programming model had claims, passwords and external login support as properties on the user account class (and no roles*). If your application didn’t want to use them then it could ignore those APIs. This would simplify the design by eliminating all of these extra store interfaces and we’d be left with just the one user store. This design change would be better specifically for NoSql implementations (see below: Bad: Non-EF implementations).
* Specific support for roles is unnecessary given claims being a superset of roles, as previously mentioned. And if you really want a role specific API then just add an extension method that sits on top of the claims.
I suppose technically this design allows those aspects of the user to be stored separate from the user itself, but then this feels as if the framework is trying to be all things to all people. If I were designing a system where my user’s claims/roles needed to be loaded from a different system then I’d probably do that as a deliberate step in my authentication code (with something like a claims transformer, which is WIF’s pattern for modeling this type of operation).
As mentioned earlier, there is a default implementation of the user store that uses EF 6. This implementation is called UserStore and it implements IUserStore, IUserLoginStore, IUserClaimStore, IUserRoleStore and IUserPasswordStore. This means it’s implementing all the store interfaces I droned on about earlier. This also means you are getting all of these features with little or no effort and the EF-specific implementation does many of the things I suggested: it defines claims, passwords, roles and external logins as part of the user class. I suppose my only complaint here is that roles are unnecessary (as previously mentioned). Of course if I don’t want the possibility of roles in my application, I’d have to go implement my own user store that omits the role store interface, but implements all of the other store interfaces.
One other minor issue is that the EF implementation of IUserStore, the API to delete a user throws a NotSupportedException. This stinks and harkens back to some of the leaky abstractions from the previous membership APIs.
I feel for those who are using NoSql or anything not with EF. The reason is that given the multiple interface design, your user store will be responsible for implementing all of the aforementioned store interfaces. As I previously mentioned, it would be much simpler if there were one user store with all the pertinent identity information stored on the user definition.
Aside (and probably Bad): I wonder if Azure (or Live) is using this framework?
As we evaluate frameworks and products from Microsoft, a useful metric on the quality and longevity is if Microsoft is using them internally. I suspect they’re not using the new ASP.NET Identity anywhere internally, thus there’s little mileage and internal feedback on it.
I wonder what Azure’s (or Live’s) identity management looks like and why Microsoft didn’t just use that instead for ASP.NET Identity. Now that I think about it, I’d also be curious what they use for their password hashing iterations. I digress.
As I see it, the reason for using a security framework (and in this case, an identity management framework) is because there’s some hard security stuff that you want to ensure has been done properly by a security professional. Looking over the UserManager API above, my question is “where’s all the security functionality?” I don’t know.
What sort of features am I talking about? Well, here’s a list:
  • Email account verification : It’s not easy to ensure that a user is really in charge of an email account they claim to own.
  • Password reset : What happens when a user forgets their password?
  • Username reminder : What happens when a user forgets their username?
  • Account lockout : What happens when an admin needs to lock an account?
  • Password guessing prevention : How do you detect and prevent someone from attempting to brute force an account’s password?
  • Close/Delete account : How do we handle an account that the user wants to close or delete? (right now the EF implementation throws a NotSupportedException… nice)
  • Modern password storage (OWASP) : We should be able to ask the framework to do a better job of storing passwords.
  • Mobile phone verification : It’s not easy to ensure that a user is really in charge of a mobile phone they claim to own.
  • Two factor authentication : To improve security, we want the user to provide something they know and something they have to prove their identity.
  • Certificate based authentication : How do we allow users to authenticate with certificates?
  • Auditing : How do we audit changes to the user account information?
  • Tracing : How do we debug failures in the identity management and authentication system?
This list of features is the hard part of building an identity management library. If you want any of the above features, then you get to implement them yourself. Many of these are non-trivial to implement and in doing so you will want to ensure you don’t open up any vulnerabilities in your application. G’d luck.
Also, with the previous membership frameworks we used to have support for: account lockout, password guessing prevention, and close/delete account semantics. So with this version, we’ve lost features.
This glass is half empty
Really the main bulk of code that Microsoft has provided for us in this new framework is the persistence code (via the EF-specific implementation). Unfortunately, the persistence is the easy part. What we really needed was a framework that solves the hard and complicated problems related to identity management.
As it stands now, these missing features in ASP.NET Identity make it unusable for all but the most trivial demo applications. It’s just not ready for prime time today. Of course, with this redesign, I think Microsoft is in a much better place to add these features in a future release. But it ain’t there now.
MembershipReboot
Given my frustrations with the previous provider APIs, I decided to build and open source my own identity management library called MembershipReboot. With this version of ASP.NET Identity, I really wanted it to be great so that there would no longer be a need for MembershipReboot, but I guess that didn’t happen. So if you’re looking for an alternative you can get it here.

1 comment:

  1. Your site is wonderful and diverse. Being disabled and insomniac I am always busy thanks to you.
    Good continuation !

    Voyance mail

    ReplyDelete

Angular Tutorial (Update to Angular 7)

As Angular 7 has just been released a few days ago. This tutorial is updated to show you how to create an Angular 7 project and the new fe...