mirror of
https://github.com/Sea-Haven-Industries/shoc-backend.git
synced 2026-09-30 04:53:11 +00:00
Merge remote-tracking branch 'origin/feat/ab/sh-386-password-policy' into feat/ab/sh-385-invite-registration
This commit is contained in:
commit
e53415de39
6 changed files with 108 additions and 4 deletions
|
|
@ -166,6 +166,40 @@ public class AuthenticationControllerTests
|
|||
"Password must be at least 6 characters and include one uppercase letter, one number, and one special character.");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ChangePassword_ConfirmationMismatch_IsRejectedWithoutChangingThePassword()
|
||||
{
|
||||
var service = new Mock<IAuthenticationService>();
|
||||
var controller = NewController(service, "42");
|
||||
|
||||
var result = await controller.ChangePassword(
|
||||
new ChangePasswords { Currentpassword = "Current1!", Newpassword = "Next2@x", Confirmpassword = "Next2@y" },
|
||||
CancellationToken.None);
|
||||
|
||||
var bad = result.Should().BeOfType<BadRequestObjectResult>().Subject;
|
||||
bad.Value.Should().BeOfType<Response>().Subject.Message.Should().Be("Passwords don't match");
|
||||
service.Verify(
|
||||
s => s.ChangePasswordAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()),
|
||||
Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ChangePassword_NonPolicyFailure_ReturnsTheGenericMessage()
|
||||
{
|
||||
var service = new Mock<IAuthenticationService>();
|
||||
service.Setup(s => s.ChangePasswordAsync("42", "Current1!", "Next2@x", It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new ChangePasswordResultDTO { Status = ChangePasswordStatus.Failed });
|
||||
var controller = NewController(service, "42");
|
||||
|
||||
var result = await controller.ChangePassword(
|
||||
new ChangePasswords { Currentpassword = "Current1!", Newpassword = "Next2@x", Confirmpassword = "Next2@x" },
|
||||
CancellationToken.None);
|
||||
|
||||
var response = result.Should().BeOfType<BadRequestObjectResult>().Subject.Value.Should().BeOfType<Response>().Subject;
|
||||
response.Message.Should().Be("Your password could not be changed. Try again.");
|
||||
response.Message.Should().NotContain("at least 6 characters");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ChangePassword_RequiresAuthenticatedCaller()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -114,6 +114,44 @@ public sealed class PasswordPolicyTests : IAsyncDisposable
|
|||
(await UserManager.CheckPasswordAsync(reloaded!, "Next2@x")).Should().BeTrue();
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("ConcurrencyFailure")]
|
||||
[InlineData("PasswordMismatch")]
|
||||
[InlineData("DefaultError")]
|
||||
public async Task ChangePassword_NonPolicyIdentityFailure_IsNotReportedAsAWeakPassword(string code)
|
||||
{
|
||||
var user = new ApplicationUser { Id = "member-1", UserName = "member@example.com" };
|
||||
var userManager = new Mock<UserManager<ApplicationUser>>(
|
||||
Mock.Of<IUserStore<ApplicationUser>>(), null!, null!, null!, null!, null!, null!, null!, null!);
|
||||
userManager.Setup(m => m.FindByIdAsync(user.Id)).ReturnsAsync(user);
|
||||
userManager.Setup(m => m.CheckPasswordAsync(user, CurrentPassword)).ReturnsAsync(true);
|
||||
userManager.Setup(m => m.ChangePasswordAsync(user, CurrentPassword, "Next2@x"))
|
||||
.ReturnsAsync(IdentityResult.Failed(new IdentityError { Code = code, Description = "failed" }));
|
||||
var service = new AuthenticationService(
|
||||
userManager.Object,
|
||||
Microsoft.Extensions.Options.Options.Create(new JwtOptions { Secret = new string('x', 64) }),
|
||||
Mock.Of<IUserDataService>(),
|
||||
Mock.Of<IForgetPasswordDataService>(),
|
||||
Mock.Of<IEmailSender>());
|
||||
|
||||
var result = await service.ChangePasswordAsync(user.Id, CurrentPassword, "Next2@x", CancellationToken.None);
|
||||
|
||||
result.Status.Should().Be(ChangePasswordStatus.Failed);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("PasswordTooShort", true)]
|
||||
[InlineData("PasswordRequiresUpper", true)]
|
||||
[InlineData("PasswordRequiresDigit", true)]
|
||||
[InlineData("PasswordRequiresNonAlphanumeric", true)]
|
||||
[InlineData("ConcurrencyFailure", false)]
|
||||
[InlineData("PasswordMismatch", false)]
|
||||
public void IsPolicyRejection_MatchesOnlyThePasswordRuleCodes(string code, bool expected)
|
||||
{
|
||||
IdentityPasswordPolicy.IsPolicyRejection(IdentityResult.Failed(new IdentityError { Code = code }))
|
||||
.Should().Be(expected);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ChangePassword_CancelledToken_Throws()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -51,6 +51,11 @@ namespace Api.SeaHavenIndustries.Controllers
|
|||
[HttpPost]
|
||||
public async Task<IActionResult> ChangePassword(ChangePasswords usermodel, CancellationToken cancellationToken)
|
||||
{
|
||||
// [Compare] already rejects this during model validation; the check here keeps the
|
||||
// unconfirmed password from ever being set if that validation is bypassed.
|
||||
if (!string.Equals(usermodel.Newpassword, usermodel.Confirmpassword, StringComparison.Ordinal))
|
||||
return BadRequest(new Response { Status = "Password confirmation does not match", Message = "Passwords don't match" });
|
||||
|
||||
var userid = User.FindFirstValue(ClaimTypes.NameIdentifier) ?? "";
|
||||
var result = await _authenticationService.ChangePasswordAsync(userid, usermodel.Currentpassword, usermodel.Newpassword, cancellationToken);
|
||||
return result.Status switch
|
||||
|
|
@ -59,6 +64,8 @@ namespace Api.SeaHavenIndustries.Controllers
|
|||
Ok(new Response { Status = "Success ", Message = "Password successfully changed" }),
|
||||
ChangePasswordStatus.PasswordRejected =>
|
||||
BadRequest(new Response { Status = "Password does not meet requirements", Message = PasswordRequirementsMessage }),
|
||||
ChangePasswordStatus.Failed =>
|
||||
BadRequest(new Response { Status = "Password not changed", Message = "Your password could not be changed. Try again." }),
|
||||
_ => BadRequest(new Response { Status = "Old Password is incorrect", Message = "Current password is incorrect" })
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,5 +23,25 @@ namespace Data.SeaHavenIndustries
|
|||
options.RequireLowercase = false;
|
||||
options.RequiredUniqueChars = 1;
|
||||
}
|
||||
|
||||
private static readonly HashSet<string> PolicyErrorCodes = new(StringComparer.Ordinal)
|
||||
{
|
||||
nameof(IdentityErrorDescriber.PasswordTooShort),
|
||||
nameof(IdentityErrorDescriber.PasswordRequiresUpper),
|
||||
nameof(IdentityErrorDescriber.PasswordRequiresLower),
|
||||
nameof(IdentityErrorDescriber.PasswordRequiresDigit),
|
||||
nameof(IdentityErrorDescriber.PasswordRequiresNonAlphanumeric),
|
||||
nameof(IdentityErrorDescriber.PasswordRequiresUniqueChars)
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// True when Identity refused the password itself. Other failures, such as a
|
||||
/// concurrency conflict, must not be reported to the user as a weak password.
|
||||
/// </summary>
|
||||
public static bool IsPolicyRejection(IdentityResult result)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(result);
|
||||
return result.Errors.Any(error => PolicyErrorCodes.Contains(error.Code));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,7 +15,9 @@ namespace SeaHaven.Services.DTOs
|
|||
{
|
||||
Succeeded,
|
||||
CurrentPasswordIncorrect,
|
||||
PasswordRejected
|
||||
PasswordRejected,
|
||||
/// <summary>Identity failed for a reason other than the password policy.</summary>
|
||||
Failed
|
||||
}
|
||||
|
||||
public sealed class ChangePasswordResultDTO
|
||||
|
|
|
|||
|
|
@ -99,9 +99,12 @@ namespace SeaHaven.Services.Implementation
|
|||
return ChangePasswordResult(ChangePasswordStatus.CurrentPasswordIncorrect);
|
||||
|
||||
var result = await _userManager.ChangePasswordAsync(user, currentPassword ?? "", newPassword ?? "");
|
||||
return ChangePasswordResult(result.Succeeded
|
||||
? ChangePasswordStatus.Succeeded
|
||||
: ChangePasswordStatus.PasswordRejected);
|
||||
if (result.Succeeded)
|
||||
return ChangePasswordResult(ChangePasswordStatus.Succeeded);
|
||||
|
||||
return ChangePasswordResult(IdentityPasswordPolicy.IsPolicyRejection(result)
|
||||
? ChangePasswordStatus.PasswordRejected
|
||||
: ChangePasswordStatus.Failed);
|
||||
}
|
||||
|
||||
private static ChangePasswordResultDTO ChangePasswordResult(ChangePasswordStatus status) =>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue