JudahGabriel / RavenDB.AspNet.Identity

ASP.NET Identity provider that uses RavenDB for storage of users and logins.
https://www.nuget.org/packages/RavenDB.AspNet.Identity/
Apache License 2.0
51 stars 24 forks source link

UserId vs UserName #2

Closed Bishbulb closed 7 years ago

Bishbulb commented 10 years ago

In my application, users register with an email address and I wanted to be able to have IdentityUser.UserName represent the email and IdentityUser.Id represent a GUID identifier. This approach isn't working, as I can see in the implementation of UserStore.CreateAsync() that the IdentityUser.Id property is always being generated based off the UserName.

It seems like the Microsoft.AspNet.Identity.IUser interface was intended to have two different pieces of information, a unique id for the user and the user name. In this implementation, I can only really store one piece of information, which is the username.

I know that supporting both Id and UserName as distinct fields on IdentityUser would require a RavenDB index on UserName (in order to support the FindByNameAsync() method). That seems like a reasonable tradeoff to me, but there might be other downsides that I'm not seeing.

I'm interested in your thoughts on this matter.

Bishbulb commented 10 years ago

To clarify, I know that I can extend the IdentityUser model to include the GUID identifier, and I could include a claim for the GUID id when generating a token so that this information would be available to my app. I wanted to understand the thought process behind the current design before I go and implement this workaround.

DavidBoike commented 10 years ago

I'm a few months removed from this, but what I remember is that the default MVC 5 templates really want to do login with a username and password, and a non-avoidable part of the Identity framework wouldn't allow a username that contained non-alphanumerics, like an email address. I wanted this library to be a quick drop-in for an MVC 5 project, so anything that would require massive changes to the basic template were out of the question.

What I have done in my projects is created an ApplicationUser model like this:

public class ApplicationUser : IdentityUser
{
    public string DisplayName { get; set; }
    public string EmailAddress { get; set; }
}

And then I largely ignore the user name, since most of the time I am going for OpenID logins anyway.

Open to suggestions on how to improve!

Bishbulb commented 10 years ago

I ran into that same problem with the non-alphanumeric characters. I did a bit of digging and found that you can disable this feature (see http://stackoverflow.com/questions/19481835/asp-net-username-to-email).

I'm working on some changes that would allow you to store the Id as a user-defined value.

In order to get around the problem of being able to find documents by both Id and UserName, I'm experimenting with an approach whereby you create a secondary mapping document that correlates the UserName to the UserId, like this:

public class IdentityUserByName
{
      public string Id { get; set; }
      public string UserId { get; set; }
}

This document id is in the format "IdentityUserByName/", so you can do a session.Load<IdentityUserByName>(userName) in the FindByNameAsync() method without requiring an index. The document creation and deletion is handled internally by the UserStore, so it doesn't change any of the external APIs.

The downside here is that you end up with extra documents floating around, so it may be better to just configure an index instead. If I get something working, I'll send over a pull request for you to look at.

DavidBoike commented 10 years ago

Definitely a document, not an index. Extra documents shouldn't be a concern as it's the only way to enforce consistency. That's what the Unique Contstraints bundles does, as well as the existing code in this project to manage external logins.