diff --git a/src/Api/Auth/Controllers/AuthRequestsController.cs b/src/Api/Auth/Controllers/AuthRequestsController.cs index 8595ff4a4fbd..f7edc7dec45e 100644 --- a/src/Api/Auth/Controllers/AuthRequestsController.cs +++ b/src/Api/Auth/Controllers/AuthRequestsController.cs @@ -1,5 +1,6 @@ using Bit.Api.Auth.Models.Response; using Bit.Api.Models.Response; +using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.AuthRequest; using Bit.Core.Auth.Services; using Bit.Core.Exceptions; @@ -72,6 +73,18 @@ public async Task GetResponse(Guid id, [FromQuery] str [HttpPost("")] [AllowAnonymous] public async Task Post([FromBody] AuthRequestCreateRequestModel model) + { + if (model.Type == AuthRequestType.AdminApproval) + { + throw new BadRequestException("You must be authenticated to create a request of that type."); + } + var authRequest = await _authRequestService.CreateAuthRequestAsync(model); + var r = new AuthRequestResponseModel(authRequest, _globalSettings.BaseServiceUri.Vault); + return r; + } + + [HttpPost("admin-request")] + public async Task PostAdminRequest([FromBody] AuthRequestCreateRequestModel model) { var authRequest = await _authRequestService.CreateAuthRequestAsync(model); var r = new AuthRequestResponseModel(authRequest, _globalSettings.BaseServiceUri.Vault); diff --git a/src/Core/Auth/Services/Implementations/AuthRequestService.cs b/src/Core/Auth/Services/Implementations/AuthRequestService.cs index 9c0d8f11a8d8..b75a5c1303a8 100644 --- a/src/Core/Auth/Services/Implementations/AuthRequestService.cs +++ b/src/Core/Auth/Services/Implementations/AuthRequestService.cs @@ -1,8 +1,11 @@ -using Bit.Core.Auth.Entities; +using System.Diagnostics; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Exceptions; using Bit.Core.Auth.Models.Api.Request.AuthRequest; using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; @@ -21,6 +24,8 @@ public class AuthRequestService : IAuthRequestService private readonly IDeviceRepository _deviceRepository; private readonly ICurrentContext _currentContext; private readonly IPushNotificationService _pushNotificationService; + private readonly IEventService _eventService; + private readonly IOrganizationUserRepository _organizationUserRepository; public AuthRequestService( IAuthRequestRepository authRequestRepository, @@ -28,7 +33,9 @@ public AuthRequestService( IGlobalSettings globalSettings, IDeviceRepository deviceRepository, ICurrentContext currentContext, - IPushNotificationService pushNotificationService) + IPushNotificationService pushNotificationService, + IEventService eventService, + IOrganizationUserRepository organizationRepository) { _authRequestRepository = authRequestRepository; _userRepository = userRepository; @@ -36,6 +43,8 @@ public AuthRequestService( _deviceRepository = deviceRepository; _currentContext = currentContext; _pushNotificationService = pushNotificationService; + _eventService = eventService; + _organizationUserRepository = organizationRepository; } public async Task GetAuthRequestAsync(Guid id, Guid userId) @@ -52,9 +61,12 @@ public AuthRequestService( public async Task GetValidatedAuthRequestAsync(Guid id, string code) { var authRequest = await _authRequestRepository.GetByIdAsync(id); - if (authRequest == null || - !CoreHelpers.FixedTimeEquals(authRequest.AccessCode, code) || - authRequest.GetExpirationDate() < DateTime.UtcNow) + if (authRequest == null || !CoreHelpers.FixedTimeEquals(authRequest.AccessCode, code)) + { + return null; + } + + if (!IsAuthRequestValid(authRequest)) { return null; } @@ -91,6 +103,42 @@ public async Task CreateAuthRequestAsync(AuthRequestCreateRequestMo } } + // AdminApproval requests require correlating the user and their organization + if (model.Type == AuthRequestType.AdminApproval) + { + // TODO: When single org policy is turned on we should query for only a single organization from the current user + // and create only an AuthRequest for that organization and return only that one + + // This will send out the request to all organizations this user belongs to + var organizationUsers = await _organizationUserRepository.GetManyByUserAsync(_currentContext.UserId!.Value); + + if (organizationUsers.Count == 0) + { + throw new BadRequestException("User does not belong to any organizations."); + } + + // A user event will automatically create logs for each organization/provider this user belongs to. + await _eventService.LogUserEventAsync(user.Id, EventType.User_RequestedDeviceApproval); + + AuthRequest? firstAuthRequest = null; + foreach (var organizationUser in organizationUsers) + { + var createdAuthRequest = await CreateAuthRequestAsync(model, user, organizationUser.OrganizationId); + firstAuthRequest ??= createdAuthRequest; + } + + // I know this won't be null because I have already validated that at least one organization exists + return firstAuthRequest!; + } + + var authRequest = await CreateAuthRequestAsync(model, user, organizationId: null); + await _pushNotificationService.PushAuthRequestAsync(authRequest); + return authRequest; + } + + private async Task CreateAuthRequestAsync(AuthRequestCreateRequestModel model, User user, Guid? organizationId) + { + Debug.Assert(_currentContext.DeviceType.HasValue, "DeviceType should have already been validated to have a value."); var authRequest = new AuthRequest { RequestDeviceIdentifier = model.DeviceIdentifier, @@ -100,35 +148,58 @@ public async Task CreateAuthRequestAsync(AuthRequestCreateRequestMo PublicKey = model.PublicKey, UserId = user.Id, Type = model.Type.GetValueOrDefault(), + OrganizationId = organizationId, }; - authRequest = await _authRequestRepository.CreateAsync(authRequest); - await _pushNotificationService.PushAuthRequestAsync(authRequest); return authRequest; } - public async Task UpdateAuthRequestAsync(Guid authRequestId, Guid userId, AuthRequestUpdateRequestModel model) + public async Task UpdateAuthRequestAsync(Guid authRequestId, Guid currentUserId, AuthRequestUpdateRequestModel model) { var authRequest = await _authRequestRepository.GetByIdAsync(authRequestId); - if (authRequest == null || authRequest.UserId != userId || authRequest.GetExpirationDate() < DateTime.UtcNow) + + if (authRequest == null) { throw new NotFoundException(); } + // Once Approval/Disapproval has been set, this AuthRequest should not be updated again. if (authRequest.Approved is not null) { throw new DuplicateAuthRequestException(); } - // Admin approval responses are not tied to a specific device, so we don't need to validate it. - if (authRequest.Type != AuthRequestType.AdminApproval) + // Do type specific validation + switch (authRequest.Type) { - var device = await _deviceRepository.GetByIdentifierAsync(model.DeviceIdentifier, userId); - if (device == null) - { - throw new BadRequestException("Invalid device."); - } - authRequest.ResponseDeviceId = device.Id; + case AuthRequestType.AdminApproval: + // AdminApproval has a different expiration time, by default is 7 days compared to + // non-AdminApproval ones having a default of 15 minutes. + if (IsDateExpired(authRequest.CreationDate, _globalSettings.PasswordlessAuth.AdminRequestExpiration)) + { + throw new NotFoundException(); + } + break; + case AuthRequestType.AuthenticateAndUnlock: + case AuthRequestType.Unlock: + if (IsDateExpired(authRequest.CreationDate, _globalSettings.PasswordlessAuth.UserRequestExpiration)) + { + throw new NotFoundException(); + } + + if (authRequest.UserId != currentUserId) + { + throw new NotFoundException(); + } + + // Admin approval responses are not tied to a specific device, but these types are so we need to validate them + var device = await _deviceRepository.GetByIdentifierAsync(model.DeviceIdentifier, currentUserId); + if (device == null) + { + throw new BadRequestException("Invalid device."); + } + authRequest.ResponseDeviceId = device.Id; + break; } authRequest.ResponseDate = DateTime.UtcNow; @@ -146,9 +217,55 @@ public async Task UpdateAuthRequestAsync(Guid authRequestId, Guid u // to not leak that it was denied to the originating client if it was originated by a malicious actor. if (authRequest.Approved ?? true) { + if (authRequest.OrganizationId.HasValue) + { + var organizationUser = await _organizationUserRepository + .GetByOrganizationAsync(authRequest.OrganizationId.Value, authRequest.UserId); + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_ApprovedAuthRequest); + } + + // No matter what we want to push out the success notification await _pushNotificationService.PushAuthRequestResponseAsync(authRequest); } + // If the request is rejected by an organization admin then we want to log an event of that action + else if (authRequest.Approved.HasValue && !authRequest.Approved.Value && authRequest.OrganizationId.HasValue) + { + var organizationUser = await _organizationUserRepository + .GetByOrganizationAsync(authRequest.OrganizationId.Value, authRequest.UserId); + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_RejectedAuthRequest); + } return authRequest; } + + private bool IsAuthRequestValid(AuthRequest authRequest) + { + return authRequest.Type switch + { + AuthRequestType.AuthenticateAndUnlock or AuthRequestType.Unlock + => !IsDateExpired(authRequest.CreationDate, _globalSettings.PasswordlessAuth.UserRequestExpiration), + AuthRequestType.AdminApproval => IsAdminApprovalAuthRequestValid(authRequest), + _ => false, + }; + } + + private bool IsAdminApprovalAuthRequestValid(AuthRequest authRequest) + { + Debug.Assert(authRequest.Type == AuthRequestType.AdminApproval, "This method should only be called on AdminApproval type"); + // If an AdminApproval type has been approved it's expiration time is based on how long it's been since approved. + if (authRequest.Approved is true) + { + Debug.Assert(authRequest.ResponseDate.HasValue, "The response date should have been set when the request was updated."); + return !IsDateExpired(authRequest.ResponseDate.Value, _globalSettings.PasswordlessAuth.AfterAdminApprovalExpiration); + } + else + { + return !IsDateExpired(authRequest.CreationDate, _globalSettings.PasswordlessAuth.AdminRequestExpiration); + } + } + + private static bool IsDateExpired(DateTime savedDate, TimeSpan allowedLifetime) + { + return DateTime.UtcNow > savedDate.Add(allowedLifetime); + } } diff --git a/src/Core/Enums/EventType.cs b/src/Core/Enums/EventType.cs index 82339ac8a7ec..f03ce71a5200 100644 --- a/src/Core/Enums/EventType.cs +++ b/src/Core/Enums/EventType.cs @@ -13,6 +13,7 @@ public enum EventType : int User_ClientExportedVault = 1007, User_UpdatedTempPassword = 1008, User_MigratedKeyToKeyConnector = 1009, + User_RequestedDeviceApproval = 1010, Cipher_Created = 1100, Cipher_Updated = 1101, @@ -54,6 +55,8 @@ public enum EventType : int OrganizationUser_FirstSsoLogin = 1510, OrganizationUser_Revoked = 1511, OrganizationUser_Restored = 1512, + OrganizationUser_ApprovedAuthRequest = 1513, + OrganizationUser_RejectedAuthRequest = 1514, Organization_Updated = 1600, Organization_PurgedVault = 1601, diff --git a/test/Common/AutoFixture/Attributes/BitAutoDataAttribute.cs b/test/Common/AutoFixture/Attributes/BitAutoDataAttribute.cs index d859f81fc996..03a8cb466f16 100644 --- a/test/Common/AutoFixture/Attributes/BitAutoDataAttribute.cs +++ b/test/Common/AutoFixture/Attributes/BitAutoDataAttribute.cs @@ -1,4 +1,6 @@ -using System.Reflection; +#nullable enable + +using System.Reflection; using AutoFixture; using Bit.Test.Common.Helpers; using Xunit.Sdk; @@ -9,19 +11,21 @@ namespace Bit.Test.Common.AutoFixture.Attributes; public class BitAutoDataAttribute : DataAttribute { private readonly Func _createFixture; - private readonly object[] _fixedTestParameters; + private readonly object?[] _fixedTestParameters; + + public BitAutoDataAttribute() : this(Array.Empty()) { } - public BitAutoDataAttribute(params object[] fixedTestParameters) : + public BitAutoDataAttribute(params object?[] fixedTestParameters) : this(() => new Fixture(), fixedTestParameters) { } - public BitAutoDataAttribute(Func createFixture, params object[] fixedTestParameters) : + public BitAutoDataAttribute(Func createFixture, params object?[] fixedTestParameters) : base() { _createFixture = createFixture; _fixedTestParameters = fixedTestParameters; } - public override IEnumerable GetData(MethodInfo testMethod) + public override IEnumerable GetData(MethodInfo testMethod) => BitAutoDataAttributeHelpers.GetData(testMethod, _createFixture(), _fixedTestParameters); } diff --git a/test/Common/Helpers/BitAutoDataAttributeHelpers.cs b/test/Common/Helpers/BitAutoDataAttributeHelpers.cs index 32cacc49dcfe..b2ee4c4211ed 100644 --- a/test/Common/Helpers/BitAutoDataAttributeHelpers.cs +++ b/test/Common/Helpers/BitAutoDataAttributeHelpers.cs @@ -1,4 +1,7 @@ -using System.Reflection; +#nullable enable + +using System.ComponentModel; +using System.Reflection; using AutoFixture; using AutoFixture.Kernel; using AutoFixture.Xunit2; @@ -8,18 +11,23 @@ namespace Bit.Test.Common.Helpers; public static class BitAutoDataAttributeHelpers { - public static IEnumerable GetData(MethodInfo testMethod, IFixture fixture, object[] fixedTestParameters) + public static IEnumerable GetData(MethodInfo testMethod, IFixture fixture, object?[] fixedTestParameters) { var methodParameters = testMethod.GetParameters(); - var classCustomizations = testMethod.DeclaringType.GetCustomAttributes().Select(attr => attr.GetCustomization()); + // We aren't worried about a test method not having a class it belongs to. + var classCustomizations = testMethod.DeclaringType!.GetCustomAttributes().Select(attr => attr.GetCustomization()); var methodCustomizations = testMethod.GetCustomAttributes().Select(attr => attr.GetCustomization()); - fixedTestParameters = fixedTestParameters ?? Array.Empty(); + fixedTestParameters ??= Array.Empty(); fixture = ApplyCustomizations(ApplyCustomizations(fixture, classCustomizations), methodCustomizations); + + // The first n number of parameters should be match to the supplied parameters + var fixedTestInputParameters = methodParameters.Take(fixedTestParameters.Length).Zip(fixedTestParameters); + var missingParameters = methodParameters.Skip(fixedTestParameters.Length).Select(p => CustomizeAndCreate(p, fixture)); - return new object[1][] { fixedTestParameters.Concat(missingParameters).ToArray() }; + return new object?[1][] { ConvertFixedParameters(fixedTestInputParameters.ToArray()).Concat(missingParameters).ToArray() }; } public static object CustomizeAndCreate(ParameterInfo p, IFixture fixture) @@ -48,4 +56,71 @@ public static IFixture ApplyCustomizations(IFixture fixture, IEnumerable ConvertFixedParameters((ParameterInfo Parameter, object? Value)[] fixedParameters) + { + var output = new object?[fixedParameters.Length]; + for (var i = 0; i < fixedParameters.Length; i++) + { + var (parameter, value) = fixedParameters[i]; + // If the value is null, just return the value + if (value is null || value.GetType() == parameter.ParameterType) + { + output[i] = value; + continue; + } + + // If the value is a string and it's not a perfect match, try to convert it. + if (value is string stringValue) + { + // + if (parameter.ParameterType.IsGenericType && parameter.ParameterType.GetGenericTypeDefinition() == typeof(Nullable<>)) + { + if (TryConvertToType(stringValue, Nullable.GetUnderlyingType(parameter.ParameterType)!, out var nullableConvertedValue)) + { + output[i] = nullableConvertedValue; + continue; + } + + // We couldn't convert it, so set it as the input value and let XUnit throw + output[i] = value; + continue; + } + + if (TryConvertToType(stringValue, parameter.ParameterType, out var convertedValue)) + { + output[i] = convertedValue; + continue; + } + + // We couldn't convert it, so set it as the input value and let XUnit throw + output[i] = value; + } + + // No easy conversion, give them back the value + output[i] = value; + } + + return output; + } + + private static bool TryConvertToType(string value, Type destinationType, out object? convertedValue) + { + convertedValue = null; + + if (string.IsNullOrEmpty(value)) + { + return false; + } + + var converter = TypeDescriptor.GetConverter(destinationType); + + if (converter.CanConvertFrom(typeof(string))) + { + convertedValue = converter.ConvertFromInvariantString(value); + return true; + } + + return false; + } } diff --git a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs index a5955116630e..3cac77405288 100644 --- a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs +++ b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs @@ -1,4 +1,5 @@ using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Enums; using Bit.Core.Auth.Exceptions; using Bit.Core.Auth.Models.Api.Request.AuthRequest; using Bit.Core.Auth.Services.Implementations; @@ -11,6 +12,7 @@ using Bit.Core.Settings; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Helpers; using NSubstitute; using Xunit; @@ -69,12 +71,23 @@ public async Task GetValidatedAuthRequestAsync_IfCodeNotValid_ReturnsNull( Assert.Null(foundAuthRequest); } - [Theory, BitAutoData] + /// + /// Story: AdminApproval AuthRequests should have a longer expiration time by default and non-AdminApproval ones + /// should expire after 15 minutes by default. + /// + [Theory] + [BitAutoData(AuthRequestType.AdminApproval, "-10.00:00:00")] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock, "-00:16:00")] + [BitAutoData(AuthRequestType.Unlock, "-00:16:00")] public async Task GetValidatedAuthRequestAsync_IfExpired_ReturnsNull( + AuthRequestType authRequestType, + TimeSpan creationTimeBeforeNow, SutProvider sutProvider, AuthRequest authRequest) { - authRequest.CreationDate = DateTime.UtcNow.AddHours(-1); + authRequest.Type = authRequestType; + authRequest.CreationDate = DateTime.UtcNow.Add(creationTimeBeforeNow); + authRequest.Approved = false; sutProvider.GetDependency() .GetByIdAsync(authRequest.Id) @@ -85,6 +98,29 @@ public async Task GetValidatedAuthRequestAsync_IfExpired_ReturnsNull( Assert.Null(foundAuthRequest); } + /// + /// Story: Once a AdminApproval type has been approved it has a different expiration time based on time + /// after the response. + /// + [Theory] + [BitAutoData] + public async Task GetValidatedAuthRequestAsync_AdminApprovalApproved_HasLongerExpiration_ReturnsRequest( + SutProvider sutProvider, + AuthRequest authRequest) + { + authRequest.Type = AuthRequestType.AdminApproval; + authRequest.Approved = true; + authRequest.ResponseDate = DateTime.UtcNow.Add(TimeSpan.FromHours(-13)); + + sutProvider.GetDependency() + .GetByIdAsync(authRequest.Id) + .Returns(authRequest); + + var validatedAuthRequest = await sutProvider.Sut.GetValidatedAuthRequestAsync(authRequest.Id, authRequest.AccessCode); + + Assert.Null(validatedAuthRequest); + } + [Theory, BitAutoData] public async Task GetValidatedAuthRequestAsync_IfValid_ReturnsAuthRequest( SutProvider sutProvider, @@ -96,6 +132,10 @@ public async Task GetValidatedAuthRequestAsync_IfValid_ReturnsAuthRequest( .GetByIdAsync(authRequest.Id) .Returns(authRequest); + sutProvider.GetDependency() + .PasswordlessAuth + .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + var foundAuthRequest = await sutProvider.Sut.GetValidatedAuthRequestAsync(authRequest.Id, authRequest.AccessCode); Assert.NotNull(foundAuthRequest); @@ -136,13 +176,22 @@ public async Task CreateAuthRequestAsync_NoKnownDevice_ThrowsBadRequest( await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAuthRequestAsync(createModel)); } - [Theory, BitAutoData] + /// + /// Story: Non-AdminApproval requests should be created without a known device if the settings is set to false + /// Non-AdminApproval ones should also have a push notification sent about them. + /// + [Theory] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock)] + [BitAutoData(AuthRequestType.Unlock)] + [BitAutoData(new object?[1] { null })] public async Task CreateAuthRequestAsync_CreatesAuthRequest( + AuthRequestType? authRequestType, SutProvider sutProvider, AuthRequestCreateRequestModel createModel, User user) { user.Email = createModel.Email; + createModel.Type = authRequestType; sutProvider.GetDependency() .GetByEmailAsync(createModel.Email) @@ -152,28 +201,44 @@ public async Task CreateAuthRequestAsync_CreatesAuthRequest( .DeviceType .Returns(DeviceType.Android); + sutProvider.GetDependency() + .IpAddress + .Returns("1.1.1.1"); + sutProvider.GetDependency() .PasswordlessAuth.KnownDevicesOnly .Returns(false); - await sutProvider.Sut.CreateAuthRequestAsync(createModel); + sutProvider.GetDependency() + .CreateAsync(Arg.Any()) + .Returns(c => c.ArgAt(0)); + + var createdAuthRequest = await sutProvider.Sut.CreateAuthRequestAsync(createModel); await sutProvider.GetDependency() .Received() - .PushAuthRequestAsync(Arg.Any()); + .PushAuthRequestAsync(createdAuthRequest); await sutProvider.GetDependency() .Received() - .CreateAsync(Arg.Any()); + .CreateAsync(createdAuthRequest); } - [Theory, BitAutoData] + /// + /// Story: Since an AllowAnonymous endpoint calls this method we need + /// to verify that a device was able to be found via ICurrentContext + /// + [Theory] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock)] + [BitAutoData(AuthRequestType.Unlock)] public async Task CreateAuthRequestAsync_NoDeviceType_ThrowsBadRequest( + AuthRequestType authRequestType, SutProvider sutProvider, AuthRequestCreateRequestModel createModel, User user) { user.Email = createModel.Email; + createModel.Type = authRequestType; sutProvider.GetDependency() .GetByEmailAsync(createModel.Email) @@ -186,13 +251,92 @@ public async Task CreateAuthRequestAsync_NoDeviceType_ThrowsBadRequest( await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAuthRequestAsync(createModel)); } + /// + /// Story: If a user happens to exist to more than one organization, we will send the device approval request to + /// each of them. + /// [Theory, BitAutoData] + public async Task CreateAuthRequestAsync_AdminApproval_CreatesForEachOrganization( + SutProvider sutProvider, + AuthRequestCreateRequestModel createModel, + User user, + OrganizationUser organizationUser1, + OrganizationUser organizationUser2) + { + createModel.Type = AuthRequestType.AdminApproval; + user.Email = createModel.Email; + organizationUser1.UserId = user.Id; + organizationUser2.UserId = user.Id; + + sutProvider.GetDependency() + .GetByEmailAsync(user.Email) + .Returns(user); + + sutProvider.GetDependency() + .DeviceType + .Returns(DeviceType.ChromeExtension); + + sutProvider.GetDependency() + .UserId + .Returns(user.Id); + + sutProvider.GetDependency() + .PasswordlessAuth.KnownDevicesOnly + .Returns(false); + + + sutProvider.GetDependency() + .GetManyByUserAsync(user.Id) + .Returns(new List + { + organizationUser1, + organizationUser2, + }); + + sutProvider.GetDependency() + .CreateAsync(Arg.Any()) + .Returns(c => c.ArgAt(0)); + + var authRequest = await sutProvider.Sut.CreateAuthRequestAsync(createModel); + + Assert.Equal(organizationUser1.OrganizationId, authRequest.OrganizationId); + + await sutProvider.GetDependency() + .Received(1) + .CreateAsync(Arg.Is(o => o.OrganizationId == organizationUser1.OrganizationId)); + + await sutProvider.GetDependency() + .Received(1) + .CreateAsync(Arg.Is(o => o.OrganizationId == organizationUser2.OrganizationId)); + + await sutProvider.GetDependency() + .Received(2) + .CreateAsync(Arg.Any()); + + await sutProvider.GetDependency() + .Received(1) + .LogUserEventAsync(user.Id, EventType.User_RequestedDeviceApproval); + } + + /// + /// Story: When an is approved we want to update it in the database so it cannot have + /// it's status changed again and we want to push a notification to let the user know of the approval. + /// In the case of the AdminApproval we also want to log an event. + /// + [Theory] + [BitAutoData(AuthRequestType.AdminApproval, "7b055ea1-38be-42d0-b2e4-becb2340f8df")] + [BitAutoData(AuthRequestType.Unlock, null)] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock, null)] public async Task UpdateAuthRequestAsync_ValidResponse_SendsResponse( + AuthRequestType authRequestType, + Guid? organizationId, SutProvider sutProvider, AuthRequest authRequest) { authRequest.CreationDate = DateTime.UtcNow.AddMinutes(-10); authRequest.Approved = null; + authRequest.OrganizationId = organizationId; + authRequest.Type = authRequestType; sutProvider.GetDependency() .GetByIdAsync(authRequest.Id) @@ -208,6 +352,18 @@ public async Task UpdateAuthRequestAsync_ValidResponse_SendsResponse( .GetByIdentifierAsync(device.Identifier, authRequest.UserId) .Returns(device); + sutProvider.GetDependency() + .GetByOrganizationAsync(Arg.Any(), Arg.Any()) + .Returns(new OrganizationUser + { + UserId = authRequest.UserId, + OrganizationId = organizationId.GetValueOrDefault(), + }); + + sutProvider.GetDependency() + .PasswordlessAuth + .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + var updateModel = new AuthRequestUpdateRequestModel { Key = "test_key", @@ -220,37 +376,75 @@ public async Task UpdateAuthRequestAsync_ValidResponse_SendsResponse( Assert.Equal("my_hash", udpatedAuthRequest.MasterPasswordHash); + // On approval, the response date should be set to current date + Assert.NotNull(udpatedAuthRequest.ResponseDate); + AssertHelper.AssertRecent(udpatedAuthRequest.ResponseDate!.Value); + await sutProvider.GetDependency() - .Received() + .Received(1) .ReplaceAsync(udpatedAuthRequest); await sutProvider.GetDependency() - .Received() + .Received(1) .PushAuthRequestResponseAsync(udpatedAuthRequest); + + var expectedNumberOfCalls = organizationId.HasValue ? 1 : 0; + await sutProvider.GetDependency() + .Received(expectedNumberOfCalls) + .LogOrganizationUserEventAsync( + Arg.Is(ou => ou.UserId == authRequest.UserId && ou.OrganizationId == organizationId), + EventType.OrganizationUser_ApprovedAuthRequest); } - [Theory, BitAutoData] + /// + /// Story: When an is rejected we want to update it in the database so it cannot have + /// it's status changed again but we do not want to send a push notification to the original device + /// so as to not leak that it was rejected. In the case of an AdminApproval type we do want to log an event though + /// + [Theory] + [BitAutoData(AuthRequestType.AdminApproval, "7b055ea1-38be-42d0-b2e4-becb2340f8df")] + [BitAutoData(AuthRequestType.Unlock, null)] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock, null)] public async Task UpdateAuthRequestAsync_ResponseNotApproved_DoesNotLeakRejection( + AuthRequestType authRequestType, + Guid? organizationId, SutProvider sutProvider, AuthRequest authRequest) { + // Give it a recent creation time which is valid for all types of AuthRequests authRequest.CreationDate = DateTime.UtcNow.AddMinutes(-10); + authRequest.Type = authRequestType; + // Has not been decided already authRequest.Approved = null; + authRequest.OrganizationId = organizationId; sutProvider.GetDependency() .GetByIdAsync(authRequest.Id) .Returns(authRequest); + // Setup a device for all requests even though it will not be called for verification in a AdminApproval var device = new Device { Id = Guid.NewGuid(), Identifier = "test_identifier", }; + sutProvider.GetDependency() + .PasswordlessAuth + .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + sutProvider.GetDependency() .GetByIdentifierAsync(device.Identifier, authRequest.UserId) .Returns(device); + sutProvider.GetDependency() + .GetByOrganizationAsync(Arg.Any(), Arg.Any()) + .Returns(new OrganizationUser + { + UserId = authRequest.UserId, + OrganizationId = organizationId.GetValueOrDefault(), + }); + var updateModel = new AuthRequestUpdateRequestModel { Key = "test_key", @@ -262,6 +456,9 @@ public async Task UpdateAuthRequestAsync_ResponseNotApproved_DoesNotLeakRejectio var udpatedAuthRequest = await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, authRequest.UserId, updateModel); Assert.Equal(udpatedAuthRequest.MasterPasswordHash, authRequest.MasterPasswordHash); + Assert.False(udpatedAuthRequest.Approved); + Assert.NotNull(udpatedAuthRequest.ResponseDate); + AssertHelper.AssertRecent(udpatedAuthRequest.ResponseDate!.Value); await sutProvider.GetDependency() .Received() @@ -270,17 +467,37 @@ await sutProvider.GetDependency() await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() .PushAuthRequestResponseAsync(udpatedAuthRequest); + + var expectedNumberOfCalls = organizationId.HasValue ? 1 : 0; + + await sutProvider.GetDependency() + .Received(expectedNumberOfCalls) + .LogOrganizationUserEventAsync( + Arg.Is(ou => ou.UserId == authRequest.UserId && ou.OrganizationId == organizationId), + EventType.OrganizationUser_RejectedAuthRequest); } - [Theory, BitAutoData] + /// + /// Story: A bad actor is able to get ahold of the request id of a valid + /// and tries to approve it from their own Bitwarden account. We need to validate that the currently signed in user + /// is the same user that originally created the request and we want to pretend it does not exist at all by throwing + /// NotFoundException. + /// + [Theory] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock)] + [BitAutoData(AuthRequestType.Unlock)] public async Task UpdateAuthRequestAsync_InvalidUser_ThrowsNotFound( + AuthRequestType authRequestType, SutProvider sutProvider, AuthRequest authRequest, - Guid userId) + Guid authenticatedUserId) { // Give it a recent creation date so that it is valid authRequest.CreationDate = DateTime.UtcNow.AddMinutes(-10); - authRequest.Approved = false; + // The request hasn't been Approved/Disapproved already + authRequest.Approved = null; + // Has an type that needs the UserId property validated + authRequest.Type = authRequestType; // Auth request should not be null sutProvider.GetDependency() @@ -297,23 +514,39 @@ public async Task UpdateAuthRequestAsync_InvalidUser_ThrowsNotFound( // Give it a randomly generated userId such that it won't be valid for the AuthRequest await Assert.ThrowsAsync( - async () => await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, userId, updateModel)); + async () => await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, authenticatedUserId, updateModel)); } - [Theory, BitAutoData] + /// + /// Story: A user created this auth request and does not approve/reject the request + /// for 16 minutes, which is past the default expiration time. This auth request + /// will be purged from the database soon but might exist for some amount of time after it's expiration + /// this method should throw a NotFoundException since it theoretically should not exist, this + /// could be a user finally clicking Approve after the request sitting on their phone for a while. + /// + [Theory] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock, "-00:16:00")] + [BitAutoData(AuthRequestType.Unlock, "-00:16:00")] + [BitAutoData(AuthRequestType.AdminApproval, "-8.00:00:00")] public async Task UpdateAuthRequestAsync_OldAuthRequest_ThrowsNotFound( + AuthRequestType authRequestType, + TimeSpan timeBeforeCreation, SutProvider sutProvider, AuthRequest authRequest) { - // AuthRequest's have a valid lifetime of only 15 minutes, make it older than that - authRequest.CreationDate = DateTime.UtcNow.AddMinutes(-16); - authRequest.Approved = false; + // AuthRequest's have a default valid lifetime of only 15 minutes, make it older than that + authRequest.CreationDate = DateTime.UtcNow.Add(timeBeforeCreation); + // Make it so that the user has not made a decision on this request + authRequest.Approved = null; + // Make it one of the types that doesn't have longer expiration i.e AdminApproval + authRequest.Type = authRequestType; - // Auth request should not be null + // The item should still exist in the database sutProvider.GetDependency() .GetByIdAsync(authRequest.Id) .Returns(authRequest); + // Represents the user finally clicking approve. var updateModel = new AuthRequestUpdateRequestModel { Key = "test_key", @@ -322,27 +555,38 @@ public async Task UpdateAuthRequestAsync_OldAuthRequest_ThrowsNotFound( MasterPasswordHash = "my_hash", }; - // Give it a randomly generated userId such that it won't be valid for the AuthRequest await Assert.ThrowsAsync( async () => await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, authRequest.UserId, updateModel)); } - [Theory, BitAutoData] + /// + /// Story: non-AdminApproval types need to validate that the device used to respond to the + /// request is a known device to the authenticated user. + /// + [Theory] + [BitAutoData(AuthRequestType.AuthenticateAndUnlock)] + [BitAutoData(AuthRequestType.Unlock)] public async Task UpdateAuthRequestAsync_InvalidDeviceIdentifier_ThrowsBadRequest( + AuthRequestType authRequestType, SutProvider sutProvider, AuthRequest authRequest) { authRequest.CreationDate = DateTime.UtcNow.AddMinutes(-10); authRequest.Approved = null; + authRequest.Type = authRequestType; sutProvider.GetDependency() .GetByIdAsync(authRequest.Id) .Returns(authRequest); sutProvider.GetDependency() - .GetByIdentifierAsync(Arg.Any(), authRequest.UserId) + .GetByIdentifierAsync("invalid_identifier", authRequest.UserId) .Returns((Device?)null); + sutProvider.GetDependency() + .PasswordlessAuth + .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); + var updateModel = new AuthRequestUpdateRequestModel { Key = "test_key", @@ -355,39 +599,96 @@ await Assert.ThrowsAsync( async () => await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, authRequest.UserId, updateModel)); } + /// + /// Story: Once the destiny of an AuthRequest has been decided, it should be considered immutable + /// and new update request should be blocked. + /// [Theory, BitAutoData] public async Task UpdateAuthRequestAsync_AlreadyApprovedOrRejected_ThrowsDuplicateAuthRequestException( SutProvider sutProvider, AuthRequest authRequest) { - // Set CreationDate to a valid recent value and Approved to a non-null value - authRequest.CreationDate = DateTime.UtcNow.AddMinutes(-10); authRequest.Approved = true; sutProvider.GetDependency() .GetByIdAsync(authRequest.Id) .Returns(authRequest); - var device = new Device + var updateModel = new AuthRequestUpdateRequestModel { - Id = Guid.NewGuid(), - Identifier = "test_identifier", + Key = "test_key", + DeviceIdentifier = "test_identifier", + RequestApproved = true, + MasterPasswordHash = "my_hash", }; - sutProvider.GetDependency() - .GetByIdentifierAsync(device.Identifier, authRequest.UserId) - .Returns(device); + await Assert.ThrowsAsync( + async () => await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, authRequest.UserId, updateModel)); + } + + /// + /// Story: An admin approves a request for one of their org users. For auditing purposes we need to + /// log an event that correlates the action for who the request was approved for. On approval we also need to + /// push the notification to the user. + /// + [Theory, BitAutoData] + public async Task UpdateAuthRequestAsync_AdminApproved_LogsEvent( + SutProvider sutProvider, + AuthRequest authRequest, + OrganizationUser organizationUser) + { + authRequest.CreationDate = DateTime.UtcNow.AddMinutes(-10); + authRequest.Type = AuthRequestType.AdminApproval; + authRequest.OrganizationId = organizationUser.OrganizationId; + authRequest.Approved = null; + + sutProvider.GetDependency() + .GetByIdAsync(authRequest.Id) + .Returns(authRequest); + + sutProvider.GetDependency() + .GetByOrganizationAsync(authRequest.OrganizationId!.Value, authRequest.UserId) + .Returns(organizationUser); + + sutProvider.GetDependency() + .PasswordlessAuth + .Returns(new Settings.GlobalSettings.PasswordlessAuthSettings()); var updateModel = new AuthRequestUpdateRequestModel { Key = "test_key", - DeviceIdentifier = "test_identifier", RequestApproved = true, MasterPasswordHash = "my_hash", }; - await Assert.ThrowsAsync( - async () => await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, authRequest.UserId, updateModel)); + var updatedAuthRequest = await sutProvider.Sut.UpdateAuthRequestAsync(authRequest.Id, authRequest.UserId, updateModel); + + Assert.Equal("my_hash", updatedAuthRequest.MasterPasswordHash); + Assert.Equal("test_key", updatedAuthRequest.Key); + Assert.True(updatedAuthRequest.Approved); + Assert.NotNull(updatedAuthRequest.ResponseDate); + AssertHelper.AssertRecent(updatedAuthRequest.ResponseDate!.Value); + + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync( + Arg.Is(organizationUser), Arg.Is(EventType.OrganizationUser_ApprovedAuthRequest)); + + await sutProvider.GetDependency() + .Received(1) + .PushAuthRequestResponseAsync(authRequest); } + [Theory, BitAutoData] + public async Task UpdateAuthRequestAsync_BadId_ThrowsNotFound( + SutProvider sutProvider, + Guid authRequestId) + { + sutProvider.GetDependency() + .GetByIdAsync(authRequestId) + .Returns((AuthRequest?)null); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.UpdateAuthRequestAsync( + authRequestId, Guid.NewGuid(), new AuthRequestUpdateRequestModel())); + } }