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

Enable lock out by default #487

Closed
wants to merge 1 commit into from
Closed

Enable lock out by default #487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion samples/IdentitySample.Mvc/Controllers/AccountController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public async Task<IActionResult> Login(LoginViewModel model, string returnUrl =
ViewBag.ReturnUrl = returnUrl;
if (ModelState.IsValid)
{
var result = await SignInManager.PasswordSignInAsync(model.UserName, model.Password, model.RememberMe, shouldLockout: false);
var result = await SignInManager.PasswordSignInAsync(model.UserName, model.Password, model.RememberMe, lockoutOnFailure: false);
if (result.Succeeded)
{
return RedirectToLocal(returnUrl);
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.AspNet.Identity/LockoutOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ namespace Microsoft.AspNet.Identity
public class LockoutOptions
{
/// <summary>
/// Gets or sets a flag indicating whether users are locked out upon creation.
/// Gets or sets a flag indicating whether users can be locked out after creation.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blowdart The doc comments for this were totally wrong :)

/// </summary>
/// <value>
/// True if a newly created user is locked out, otherwise false.
/// True if a newly created user can be locked out, otherwise false.
/// </value>
/// <remarks>
/// Defaults to false.
/// Defaults to true.
/// </remarks>
public bool EnabledByDefault { get; set; } = false;
public bool AllowedForNewUsers { get; set; } = true;

/// <summary>
/// Gets or sets the number of failed access attempts allowed before a user is locked out,
Expand Down
13 changes: 6 additions & 7 deletions src/Microsoft.AspNet.Identity/SignInManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,10 @@ public virtual async Task<TUser> ValidateSecurityStampAsync(ClaimsPrincipal prin
/// <param name="user">The user to sign in.</param>
/// <param name="password">The password to attempt to sign in with.</param>
/// <param name="isPersistent">Flag indicating whether the sign-in cookie should persist after the browser is closed.</param>
/// <param name="shouldLockout">Flag indicating if the user account should be locked if the sign in fails.</param>
/// <param name="lockoutOnFailure">Flag indicating if the user account should be locked if the sign in fails too many times.</param>
/// <returns>The task object representing the asynchronous operation containing the <see name="SignInResult"/>
/// for the sign-in attempt.</returns>
public virtual async Task<SignInResult> PasswordSignInAsync(TUser user, string password,
bool isPersistent, bool shouldLockout)
public virtual async Task<SignInResult> PasswordSignInAsync(TUser user, string password, bool isPersistent, bool lockoutOnFailure)
{
if (user == null)
{
Expand All @@ -213,7 +212,7 @@ public virtual async Task<TUser> ValidateSecurityStampAsync(ClaimsPrincipal prin
}
Logger.LogWarning("User {userId} failed to provide the correct password.", await UserManager.GetUserIdAsync(user));

if (UserManager.SupportsUserLockout && shouldLockout)
if (UserManager.SupportsUserLockout && lockoutOnFailure)
{
// If lockout is requested, increment access failed count which might lock out the user
await UserManager.AccessFailedAsync(user);
Expand All @@ -232,19 +231,19 @@ public virtual async Task<TUser> ValidateSecurityStampAsync(ClaimsPrincipal prin
/// <param name="userName">The user name to sign in.</param>
/// <param name="password">The password to attempt to sign in with.</param>
/// <param name="isPersistent">Flag indicating whether the sign-in cookie should persist after the browser is closed.</param>
/// <param name="shouldLockout">Flag indicating if the user account should be locked if the sign in fails.</param>
/// <param name="lockoutOnFailure">Flag indicating if the user account should be locked if the sign in fails too many times.</param>
/// <returns>The task object representing the asynchronous operation containing the <see name="SignInResult"/>
/// for the sign-in attempt.</returns>
public virtual async Task<SignInResult> PasswordSignInAsync(string userName, string password,
bool isPersistent, bool shouldLockout)
bool isPersistent, bool lockoutOnFailure)
{
var user = await UserManager.FindByNameAsync(userName);
if (user == null)
{
return SignInResult.Failed;
}

return await PasswordSignInAsync(user, password, isPersistent, shouldLockout);
return await PasswordSignInAsync(user, password, isPersistent, lockoutOnFailure);
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.AspNet.Identity/UserManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ public virtual async Task<IdentityResult> CreateAsync(TUser user)
{
return result;
}
if (Options.Lockout.EnabledByDefault && SupportsUserLockout)
if (Options.Lockout.AllowedForNewUsers && SupportsUserLockout)
{
await GetUserLockoutStore().SetLockoutEnabledAsync(user, true, CancellationToken);
}
Expand Down
6 changes: 3 additions & 3 deletions test/Microsoft.AspNet.Identity.Test/IdentityOptionsTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class IdentityOptionsTest
public void VerifyDefaultOptions()
{
var options = new IdentityOptions();
Assert.False(options.Lockout.EnabledByDefault);
Assert.True(options.Lockout.AllowedForNewUsers);
Assert.Equal(TimeSpan.FromMinutes(5), options.Lockout.DefaultLockoutTimeSpan);
Assert.Equal(5, options.Lockout.MaxFailedAccessAttempts);

Expand Down Expand Up @@ -58,7 +58,7 @@ public void IdentityOptionsFromConfig()
{"identity:password:RequireUpperCase", "false"},
{"identity:password:RequireDigit", "false"},
{"identity:password:RequireLowerCase", "false"},
{"identity:lockout:EnabledByDefault", "TRUe"},
{"identity:lockout:AllowedForNewUsers", "FALSe"},
{"identity:lockout:MaxFailedAccessAttempts", "1000"}
};
var builder = new ConfigurationBuilder(new MemoryConfigurationSource(dic));
Expand All @@ -82,7 +82,7 @@ public void IdentityOptionsFromConfig()
Assert.False(options.Password.RequireNonLetterOrDigit);
Assert.False(options.Password.RequireUppercase);
Assert.Equal(10, options.Password.RequiredLength);
Assert.True(options.Lockout.EnabledByDefault);
Assert.False(options.Lockout.AllowedForNewUsers);
Assert.Equal(1000, options.Lockout.MaxFailedAccessAttempts);
}

Expand Down
1 change: 1 addition & 0 deletions test/Shared/MockHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public static class MockHelpers
store = store ?? new Mock<IUserStore<TUser>>().Object;
var options = new Mock<IOptions<IdentityOptions>>();
var idOptions = new IdentityOptions();
idOptions.Lockout.AllowedForNewUsers = false;
options.Setup(o => o.Options).Returns(idOptions);
var userValidators = new List<IUserValidator<TUser>>();
var validator = new Mock<IUserValidator<TUser>>();
Expand Down
10 changes: 2 additions & 8 deletions test/Shared/UserManagerTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,6 @@ public async Task SingleFailureLockout()
{
var mgr = CreateManager();
mgr.Options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromHours(1);
mgr.Options.Lockout.EnabledByDefault = true;
mgr.Options.Lockout.MaxFailedAccessAttempts = 0;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Expand All @@ -833,7 +832,6 @@ public async Task TwoFailureLockout()
{
var mgr = CreateManager();
mgr.Options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromHours(1);
mgr.Options.Lockout.EnabledByDefault = true;
mgr.Options.Lockout.MaxFailedAccessAttempts = 2;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Expand All @@ -855,7 +853,6 @@ public async Task ResetAccessCountPreventsLockout()
{
var mgr = CreateManager();
mgr.Options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromHours(1);
mgr.Options.Lockout.EnabledByDefault = true;
mgr.Options.Lockout.MaxFailedAccessAttempts = 2;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Expand All @@ -880,6 +877,7 @@ public async Task CanEnableLockoutManuallyAndLockout()
{
var mgr = CreateManager();
mgr.Options.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromHours(1);
mgr.Options.Lockout.AllowedForNewUsers = false;
mgr.Options.Lockout.MaxFailedAccessAttempts = 2;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Expand All @@ -902,7 +900,6 @@ public async Task CanEnableLockoutManuallyAndLockout()
public async Task UserNotLockedOutWithNullDateTimeAndIsSetToNullDate()
{
var mgr = CreateManager();
mgr.Options.Lockout.EnabledByDefault = true;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Assert.True(await mgr.GetLockoutEnabledAsync(user));
Expand All @@ -915,6 +912,7 @@ public async Task UserNotLockedOutWithNullDateTimeAndIsSetToNullDate()
public async Task LockoutFailsIfNotEnabled()
{
var mgr = CreateManager();
mgr.Options.Lockout.AllowedForNewUsers = false;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Assert.False(await mgr.GetLockoutEnabledAsync(user));
Expand All @@ -928,7 +926,6 @@ public async Task LockoutFailsIfNotEnabled()
public async Task LockoutEndToUtcNowMinus1SecInUserShouldNotBeLockedOut()
{
var mgr = CreateManager();
mgr.Options.Lockout.EnabledByDefault = true;
var user = CreateTestUser(lockoutEnd: DateTimeOffset.UtcNow.AddSeconds(-1));
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Assert.True(await mgr.GetLockoutEnabledAsync(user));
Expand All @@ -939,7 +936,6 @@ public async Task LockoutEndToUtcNowMinus1SecInUserShouldNotBeLockedOut()
public async Task LockoutEndToUtcNowSubOneSecondWithManagerShouldNotBeLockedOut()
{
var mgr = CreateManager();
mgr.Options.Lockout.EnabledByDefault = true;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Assert.True(await mgr.GetLockoutEnabledAsync(user));
Expand All @@ -951,7 +947,6 @@ public async Task LockoutEndToUtcNowSubOneSecondWithManagerShouldNotBeLockedOut(
public async Task LockoutEndToUtcNowPlus5ShouldBeLockedOut()
{
var mgr = CreateManager();
mgr.Options.Lockout.EnabledByDefault = true;
var lockoutEnd = DateTimeOffset.UtcNow.AddMinutes(5);
var user = CreateTestUser(lockoutEnd: lockoutEnd);
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Expand All @@ -963,7 +958,6 @@ public async Task LockoutEndToUtcNowPlus5ShouldBeLockedOut()
public async Task UserLockedOutWithDateTimeLocalKindNowPlus30()
{
var mgr = CreateManager();
mgr.Options.Lockout.EnabledByDefault = true;
var user = CreateTestUser();
IdentityResultAssert.IsSuccess(await mgr.CreateAsync(user));
Assert.True(await mgr.GetLockoutEnabledAsync(user));
Expand Down