Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Lockout doesn't work if email address is invalid. #1675

Closed
Tamaletjie opened this issue Mar 11, 2018 · 4 comments
Closed

Lockout doesn't work if email address is invalid. #1675

Tamaletjie opened this issue Mar 11, 2018 · 4 comments
Labels

Comments

@Tamaletjie
Copy link

I'm developing an ASP.NET MVC app using ASP.Net Identity 2.2.1 in Visual Studio 2017. The app is for internal use only, does not use 2FA or password recovery and will have accounts managed by the user's manager.

In light of this I changed the login page to use the UserName field instead of the Email address field to identify the user. When I create users in dbo.AspNetUsers I populated the Email field with the same value as the UserName field e.g. JoeB. Doing so does not affect the login process but when it comes to triggering Automatic Lockouts for failed login attempts, this feature fails to trigger.

The following email addresses won't trigger a Lockout:

NULL
JoeB
JoeB@

Anything of the form alphanumberic@alphanumberic seems to be OK (e.g. JoeB@example).

Taking a look at the ASP.Net identity source code I think this bug is related to the last method in this call chain:

  • SignInManager.CheckPasswordSignInAsync()
    
  •   UserManager.AccessFailedAsync(user)
    
  •     UserManager.UpdateUserAsync()
    
  •       UserManager.UpdateNormalizedEmailAsync(user)
    

Since the email address doesn't exist or is invalid (according to some internal check?), the UpdateNormalizedEmailAsync(user) fails in some way causing the following record store.IncrementAccessFailedCountAsync(user, CancellationToken) to not be committed to the database.

Running SQL Profiler confirms that the SQL to update dbo.AspNetUsers.AccessFailedCount is never sent to the database if the email address is invalid.

This behavior may be by design (for reasons not obvious to me) but I've wasted many hours wrestling with this issue so I though I would log it here in case anyone else gets tripped up by it.

P.S. The ApplicationManager.SupportsUserEmail is read-only so I can't use this to signal that Email address is not relevant.

@blowdart
Copy link
Member

When you say Identity 2.2.1 do you meant Identity Core, or the non-core versions?

@blowdart
Copy link
Member

@blowdart blowdart added this to the 2.1.0-rc1 milestone Mar 29, 2018
@Tamaletjie
Copy link
Author

I am working with the Identity non-core version.

@HaoK
Copy link
Member

HaoK commented Apr 10, 2018

Closing as this isn't an issue with Core, we won't be changing something like this with older versions of identity templates, but its possible we will revisit this for future Core versions of identity in the linked #1721

@HaoK HaoK closed this as completed Apr 10, 2018
@HaoK HaoK removed this from the 2.1.0-rc1 milestone Apr 10, 2018
@HaoK HaoK added the External label Apr 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants