mirror of
https://github.com/bitwarden/server
synced 2025-12-06 00:03:34 +00:00
fix(invalid-auth-request-approvals): Auth/[PM-3387] Better Error Handling for Invalid Auth Request Approval (#6264)
If a user approves an invalid auth request, on the Requesting Device they currently they get stuck on the `LoginViaAuthRequestComponent` with a spinning wheel. This PR makes it so that when an Approving Device attempts to approve an invalid auth request, the Approving Device receives an error toast and the `UpdateAuthRequestAsync()` operation is blocked.
This commit is contained in:
@@ -102,7 +102,37 @@ public class AuthRequestsController(
|
||||
public async Task<AuthRequestResponseModel> Put(Guid id, [FromBody] AuthRequestUpdateRequestModel model)
|
||||
{
|
||||
var userId = _userService.GetProperUserId(User).Value;
|
||||
|
||||
// If the Approving Device is attempting to approve a request, validate the approval
|
||||
if (model.RequestApproved == true)
|
||||
{
|
||||
await ValidateApprovalOfMostRecentAuthRequest(id, userId);
|
||||
}
|
||||
|
||||
var authRequest = await _authRequestService.UpdateAuthRequestAsync(id, userId, model);
|
||||
return new AuthRequestResponseModel(authRequest, _globalSettings.BaseServiceUri.Vault);
|
||||
}
|
||||
|
||||
private async Task ValidateApprovalOfMostRecentAuthRequest(Guid id, Guid userId)
|
||||
{
|
||||
// Get the current auth request to find the device identifier
|
||||
var currentAuthRequest = await _authRequestService.GetAuthRequestAsync(id, userId);
|
||||
if (currentAuthRequest == null)
|
||||
{
|
||||
throw new NotFoundException();
|
||||
}
|
||||
|
||||
// Get all pending auth requests for this user (returns most recent per device)
|
||||
var pendingRequests = await _authRequestRepository.GetManyPendingAuthRequestByUserId(userId);
|
||||
|
||||
// Find the most recent request for the same device
|
||||
var mostRecentForDevice = pendingRequests
|
||||
.FirstOrDefault(pendingRequest => pendingRequest.RequestDeviceIdentifier == currentAuthRequest.RequestDeviceIdentifier);
|
||||
|
||||
var isMostRecentRequestForDevice = mostRecentForDevice?.Id == id;
|
||||
if (!isMostRecentRequestForDevice)
|
||||
{
|
||||
throw new BadRequestException("This request is no longer valid. Make sure to approve the most recent request.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -222,7 +222,7 @@ public class AuthRequestsControllerTests
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task Put_ReturnsAuthRequest(
|
||||
public async Task Put_WithRequestNotApproved_ReturnsAuthRequest(
|
||||
SutProvider<AuthRequestsController> sutProvider,
|
||||
User user,
|
||||
AuthRequestUpdateRequestModel requestModel,
|
||||
@@ -230,6 +230,7 @@ public class AuthRequestsControllerTests
|
||||
{
|
||||
// Arrange
|
||||
SetBaseServiceUri(sutProvider);
|
||||
requestModel.RequestApproved = false; // Not an approval, so validation should be skipped
|
||||
|
||||
sutProvider.GetDependency<IUserService>()
|
||||
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
|
||||
@@ -248,6 +249,117 @@ public class AuthRequestsControllerTests
|
||||
Assert.IsType<AuthRequestResponseModel>(result);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task Put_WithApprovedRequest_ValidatesAndReturnsAuthRequest(
|
||||
SutProvider<AuthRequestsController> sutProvider,
|
||||
User user,
|
||||
AuthRequestUpdateRequestModel requestModel,
|
||||
AuthRequest currentAuthRequest,
|
||||
AuthRequest updatedAuthRequest,
|
||||
List<PendingAuthRequestDetails> pendingRequests)
|
||||
{
|
||||
// Arrange
|
||||
SetBaseServiceUri(sutProvider);
|
||||
requestModel.RequestApproved = true; // Approval triggers validation
|
||||
currentAuthRequest.RequestDeviceIdentifier = "device-identifier-123";
|
||||
|
||||
// Setup pending requests - make the current request the most recent for its device
|
||||
var mostRecentForDevice = new PendingAuthRequestDetails(currentAuthRequest, Guid.NewGuid());
|
||||
pendingRequests.Add(mostRecentForDevice);
|
||||
|
||||
sutProvider.GetDependency<IUserService>()
|
||||
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
|
||||
.Returns(user.Id);
|
||||
|
||||
// Setup validation dependencies
|
||||
sutProvider.GetDependency<IAuthRequestService>()
|
||||
.GetAuthRequestAsync(currentAuthRequest.Id, user.Id)
|
||||
.Returns(currentAuthRequest);
|
||||
|
||||
sutProvider.GetDependency<IAuthRequestRepository>()
|
||||
.GetManyPendingAuthRequestByUserId(user.Id)
|
||||
.Returns(pendingRequests);
|
||||
|
||||
sutProvider.GetDependency<IAuthRequestService>()
|
||||
.UpdateAuthRequestAsync(currentAuthRequest.Id, user.Id, requestModel)
|
||||
.Returns(updatedAuthRequest);
|
||||
|
||||
// Act
|
||||
var result = await sutProvider.Sut
|
||||
.Put(currentAuthRequest.Id, requestModel);
|
||||
|
||||
// Assert
|
||||
Assert.NotNull(result);
|
||||
Assert.IsType<AuthRequestResponseModel>(result);
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task Put_WithApprovedRequest_CurrentAuthRequestNotFound_ThrowsNotFoundException(
|
||||
SutProvider<AuthRequestsController> sutProvider,
|
||||
User user,
|
||||
AuthRequestUpdateRequestModel requestModel,
|
||||
Guid authRequestId)
|
||||
{
|
||||
// Arrange
|
||||
requestModel.RequestApproved = true; // Approval triggers validation
|
||||
|
||||
sutProvider.GetDependency<IUserService>()
|
||||
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
|
||||
.Returns(user.Id);
|
||||
|
||||
// Current auth request not found
|
||||
sutProvider.GetDependency<IAuthRequestService>()
|
||||
.GetAuthRequestAsync(authRequestId, user.Id)
|
||||
.Returns((AuthRequest)null);
|
||||
|
||||
// Act & Assert
|
||||
var exception = await Assert.ThrowsAsync<NotFoundException>(
|
||||
() => sutProvider.Sut.Put(authRequestId, requestModel));
|
||||
}
|
||||
|
||||
[Theory, BitAutoData]
|
||||
public async Task Put_WithApprovedRequest_NotMostRecentForDevice_ThrowsBadRequestException(
|
||||
SutProvider<AuthRequestsController> sutProvider,
|
||||
User user,
|
||||
AuthRequestUpdateRequestModel requestModel,
|
||||
AuthRequest currentAuthRequest,
|
||||
List<PendingAuthRequestDetails> pendingRequests)
|
||||
{
|
||||
// Arrange
|
||||
requestModel.RequestApproved = true; // Approval triggers validation
|
||||
currentAuthRequest.RequestDeviceIdentifier = "device-identifier-123";
|
||||
|
||||
// Setup pending requests - make a different request the most recent for the same device
|
||||
var differentAuthRequest = new AuthRequest
|
||||
{
|
||||
Id = Guid.NewGuid(), // Different ID than current request
|
||||
RequestDeviceIdentifier = currentAuthRequest.RequestDeviceIdentifier,
|
||||
UserId = user.Id,
|
||||
Type = AuthRequestType.AuthenticateAndUnlock,
|
||||
CreationDate = DateTime.UtcNow
|
||||
};
|
||||
var mostRecentForDevice = new PendingAuthRequestDetails(differentAuthRequest, Guid.NewGuid());
|
||||
pendingRequests.Add(mostRecentForDevice);
|
||||
|
||||
sutProvider.GetDependency<IUserService>()
|
||||
.GetProperUserId(Arg.Any<ClaimsPrincipal>())
|
||||
.Returns(user.Id);
|
||||
|
||||
sutProvider.GetDependency<IAuthRequestService>()
|
||||
.GetAuthRequestAsync(currentAuthRequest.Id, user.Id)
|
||||
.Returns(currentAuthRequest);
|
||||
|
||||
sutProvider.GetDependency<IAuthRequestRepository>()
|
||||
.GetManyPendingAuthRequestByUserId(user.Id)
|
||||
.Returns(pendingRequests);
|
||||
|
||||
// Act & Assert
|
||||
var exception = await Assert.ThrowsAsync<BadRequestException>(
|
||||
() => sutProvider.Sut.Put(currentAuthRequest.Id, requestModel));
|
||||
|
||||
Assert.Equal("This request is no longer valid. Make sure to approve the most recent request.", exception.Message);
|
||||
}
|
||||
|
||||
private void SetBaseServiceUri(SutProvider<AuthRequestsController> sutProvider)
|
||||
{
|
||||
sutProvider.GetDependency<IGlobalSettings>()
|
||||
|
||||
Reference in New Issue
Block a user