Aspnetcore: Default Values for IdentityUser

[davidrevoledo]

I've realized that the configuration for the default entity IdentityUser
https://github.com/aspnet/Identity/blob/master/src/EF/IdentityUserContext.cs#L134 doesn't have any configuration for the PhoneNumber and other values.

I faced a few performance lacking using varchar(max) sizing.

Reference : https://sqlperformance.com/2017/06/sql-plan/performance-myths-oversizing-strings

I think it'd be a good idea use some standard as default to avoid users that don't care or don't know about it have to deal with those kind of throughput issues and every once can configure like they prefer (as it is possible now) but IMO is a quite dangerous letting this as default

Acording this with For full international support, you'd need a VARCHAR of 15 digits.
https://en.wikipedia.org/wiki/E.164

Thanks.

[davidrevoledo]

any news ?

[KevMorelli]

I ran a few tests and it seems that these length values could work without issues:
PhoneNumber => 15
PasswordHash => 84
SecurityStamp => 32
ConcurrencyStamp => 36

[davidrevoledo]

@KevMorelli Yeah I'm aware of that.

[KevMorelli]

I will check the other tables before making a pull request for 3.0 with the fixed migration for the templates because there are many fields with maximum length.

[davidrevoledo]

@KevMorelli I think we should wait until some one of the team answer this Issue, the PR will be a quite complicated as will be a breaking change.

[ajcvickers]

@HaoK @blowdart I think there is value here, and for 3.0 we should probably bite the bullet and do schema cleanup, but clearly not before 3.0 given where we currently are. Suggest we triage this to 3.0.

[davidrevoledo]

@ajcvickers yes I think the same if not it will force everyonce to run a migration.

[Ponant]

@ajcvickers , I have a new project and would like to know what would be some better defaults for the length of the PasswordHash, Concurrency and Security stamps, knowing that I have usernames not exceeding 32 chars, same for phone numbers and emails must be below 128. So far I have the defauls values for PasswordHash, Concurrency and Security stamps (nvarchar(max)) and would like some fixed upper limit (256?), if possible.

[ajcvickers]

@HaoK Do you know if their are well-known maximum lengths for "PasswordHash, Concurrency and Security stamps"?

[HaoK]

@blowdart for his thoughts as well, the stamps are just guids right now by default, the password hash looks like its always 49 bytes

[alexk8]

please consider this too

problem

Default IdentityUser is declared as IdentityUser<string>
That generates column of type "nvarchar(450)" is MsSql and unlimited "text" in postgres which is a bit strange
If users are supposed to explicitly declare maxlength
services.AddDefaultIdentity<IdentityUser>(opts => {opts.Stores.MaxLengthForKeys = 36; })
-anyway, it doesnt help (#7568)

solution

since it internaly uses Guid.ToString(), column should be explicitly declared as 'char(36)' or similar.
Thanks

[jadarnel27]

I still think this would be a useful change - the framework and templates should be setting users up for success, and part of that is putting appropriate constraints on columns.

I spent a little time looking into the current state of the code, and I think that these would be very reasonable settings:

builder.Entity<IdentityUser>(u =>
{                                                                                                                            
    u.Property(user => user.PhoneNumber)
        .IsUnicode(false)
        .IsFixedLength(false)
        .HasMaxLength(15);

    u.Property(user => user.PasswordHash)
        .IsUnicode(false)
        .IsFixedLength(true)
        .HasMaxLength(84);

    u.Property(user => user.ConcurrencyStamp)
        .IsUnicode(false)
        .IsFixedLength(true)
        .HasMaxLength(36)
        .IsRequired(true);

    u.Property(user => user.SecurityStamp)
        .IsUnicode(false)
        .IsFixedLength(false)
        .HasMaxLength(36)
        .IsRequired(true);
});

To highlight some of those items:

• none of these columns needs unicode (saves on storage size, at least in SQL Server)
• PasswordHash and ConcurrencyStamp have fixed length
• ConcurrencyStamp and SecurityStamp are never null

It would be nice if ConcurrencyStamp were a Guid on the .NET side, so that a proper uniqueidentifier could be used in SQL Server. However, I don't know if all the other supported data platforms have a comparable data type to map to.