Greenfield: Users API fixes (#6425)

* Do not crash when creatuing users without password

* More precise error message for user approval toggling
This commit is contained in:
d11n
2024-11-25 01:38:44 +01:00
committed by GitHub
parent 76f87011a2
commit d6211327db
2 changed files with 34 additions and 13 deletions

View File

@@ -1052,6 +1052,13 @@ namespace BTCPayServer.Tests
IsAdministrator = true IsAdministrator = true
}); });
// Create user without password
await adminClient.CreateUser(new CreateApplicationUserRequest
{
Email = "nopassword@gmail.com"
});
// Regular user
var user1Acc = tester.NewAccount(); var user1Acc = tester.NewAccount();
user1Acc.UserId = user1.Id; user1Acc.UserId = user1.Id;
user1Acc.IsAdmin = false; user1Acc.IsAdmin = false;
@@ -4168,6 +4175,11 @@ namespace BTCPayServer.Tests
Assert.True((await adminClient.GetUserByIdOrEmail(unapprovedUser.UserId)).Approved); Assert.True((await adminClient.GetUserByIdOrEmail(unapprovedUser.UserId)).Approved);
Assert.True((await unapprovedUserApiKeyClient.GetCurrentUser()).Approved); Assert.True((await unapprovedUserApiKeyClient.GetCurrentUser()).Approved);
Assert.True((await unapprovedUserBasicAuthClient.GetCurrentUser()).Approved); Assert.True((await unapprovedUserBasicAuthClient.GetCurrentUser()).Approved);
var err = await AssertAPIError("invalid-state", async () =>
{
await adminClient.ApproveUser(unapprovedUser.UserId, true, CancellationToken.None);
});
Assert.Equal("User is already approved", err.APIError.Message);
// un-approve // un-approve
Assert.True(await adminClient.ApproveUser(unapprovedUser.UserId, false, CancellationToken.None)); Assert.True(await adminClient.ApproveUser(unapprovedUser.UserId, false, CancellationToken.None));
@@ -4180,6 +4192,11 @@ namespace BTCPayServer.Tests
{ {
await unapprovedUserBasicAuthClient.GetCurrentUser(); await unapprovedUserBasicAuthClient.GetCurrentUser();
}); });
err = await AssertAPIError("invalid-state", async () =>
{
await adminClient.ApproveUser(unapprovedUser.UserId, false, CancellationToken.None);
});
Assert.Equal("User is already unapproved", err.APIError.Message);
// reset policies to not require approval // reset policies to not require approval
await settings.UpdateSetting(new PoliciesSettings { LockSubscription = false, RequiresUserApproval = false }); await settings.UpdateSetting(new PoliciesSettings { LockSubscription = false, RequiresUserApproval = false });
@@ -4196,10 +4213,11 @@ namespace BTCPayServer.Tests
Assert.Single(await adminClient.GetNotifications(false)); Assert.Single(await adminClient.GetNotifications(false));
// try unapproving user which does not have the RequiresApproval flag // try unapproving user which does not have the RequiresApproval flag
await AssertAPIError("invalid-state", async () => err = await AssertAPIError("invalid-state", async () =>
{ {
await adminClient.ApproveUser(newUser.UserId, false, CancellationToken.None); await adminClient.ApproveUser(newUser.UserId, false, CancellationToken.None);
}); });
Assert.Equal("Unapproving user failed: No approval required", err.APIError.Message);
} }
[Fact(Timeout = 60 * 2 * 1000)] [Fact(Timeout = 60 * 2 * 1000)]

View File

@@ -112,14 +112,13 @@ namespace BTCPayServer.Controllers.Greenfield
return this.UserNotFound(); return this.UserNotFound();
} }
var success = false;
if (user.RequiresApproval) if (user.RequiresApproval)
{ {
success = await _userService.SetUserApproval(user.Id, request.Approved, Request.GetAbsoluteRootUri()); return await _userService.SetUserApproval(user.Id, request.Approved, Request.GetAbsoluteRootUri())
? Ok()
: this.CreateAPIError("invalid-state", $"User is already {(request.Approved ? "approved" : "unapproved")}");
} }
return this.CreateAPIError("invalid-state", $"{(request.Approved ? "Approving" : "Unapproving")} user failed: No approval required");
return success ? Ok() : this.CreateAPIError("invalid-state",
$"{(request.Approved ? "Approving" : "Unapproving")} user failed");
} }
[Authorize(Policy = Policies.CanViewUsers, AuthenticationSchemes = AuthenticationSchemes.Greenfield)] [Authorize(Policy = Policies.CanViewUsers, AuthenticationSchemes = AuthenticationSchemes.Greenfield)]
@@ -350,21 +349,25 @@ namespace BTCPayServer.Controllers.Greenfield
blob.Name = request.Name; blob.Name = request.Name;
blob.ImageUrl = request.ImageUrl; blob.ImageUrl = request.ImageUrl;
user.SetBlob(blob); user.SetBlob(blob);
var passwordValidation = await this._passwordValidator.ValidateAsync(_userManager, user, request.Password); var hasPassword = !string.IsNullOrEmpty(request.Password);
if (!passwordValidation.Succeeded) if (hasPassword)
{ {
foreach (var error in passwordValidation.Errors) var passwordValidation = await _passwordValidator.ValidateAsync(_userManager, user, request.Password);
if (!passwordValidation.Succeeded)
{ {
ModelState.AddModelError(nameof(request.Password), error.Description); foreach (var error in passwordValidation.Errors)
{
ModelState.AddModelError(nameof(request.Password), error.Description);
}
return this.CreateValidationError(ModelState);
} }
return this.CreateValidationError(ModelState);
} }
if (!isAdmin) if (!isAdmin)
{ {
if (!await _throttleService.Throttle(ZoneLimits.Register, this.HttpContext.Connection.RemoteIpAddress, cancellationToken)) if (!await _throttleService.Throttle(ZoneLimits.Register, this.HttpContext.Connection.RemoteIpAddress, cancellationToken))
return new TooManyRequestsResult(ZoneLimits.Register); return new TooManyRequestsResult(ZoneLimits.Register);
} }
var identityResult = await _userManager.CreateAsync(user, request.Password); var identityResult = hasPassword ? await _userManager.CreateAsync(user, request.Password) : await _userManager.CreateAsync(user);
if (!identityResult.Succeeded) if (!identityResult.Succeeded)
{ {
foreach (var error in identityResult.Errors) foreach (var error in identityResult.Errors)