diff --git a/src/Api/Auth/Controllers/AuthRequestsController.cs b/src/Api/Auth/Controllers/AuthRequestsController.cs index 8595ff4a4..f7edc7dec 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 class AuthRequestsController : Controller [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/Api/Auth/Models/Request/UpdateDevicesTrustRequestModel.cs b/src/Api/Auth/Models/Request/UpdateDevicesTrustRequestModel.cs new file mode 100644 index 000000000..779321103 --- /dev/null +++ b/src/Api/Auth/Models/Request/UpdateDevicesTrustRequestModel.cs @@ -0,0 +1,14 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Core.Auth.Models.Api.Request; + +#nullable enable + +namespace Bit.Api.Auth.Models.Request; + +public class UpdateDevicesTrustRequestModel : SecretVerificationRequestModel +{ + [Required] + public DeviceKeysUpdateRequestModel CurrentDevice { get; set; } = null!; + public IEnumerable? OtherDevices { get; set; } +} diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index c3b4d5bc1..d199c223d 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -879,10 +879,6 @@ public class AccountsController : Controller public async Task PostRequestOTP() { var user = await _userService.GetUserByPrincipalAsync(User); - if (user is not { UsesKeyConnector: true }) - { - throw new UnauthorizedAccessException(); - } await _userService.SendOTPAsync(user); } @@ -891,10 +887,6 @@ public class AccountsController : Controller public async Task VerifyOTP([FromBody] VerifyOTPRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); - if (user is not { UsesKeyConnector: true }) - { - throw new UnauthorizedAccessException(); - } if (!await _userService.VerifyOTPAsync(user, model.OTP)) { diff --git a/src/Api/Controllers/DevicesController.cs b/src/Api/Controllers/DevicesController.cs index 88d0e5c7a..b462e51df 100644 --- a/src/Api/Controllers/DevicesController.cs +++ b/src/Api/Controllers/DevicesController.cs @@ -1,7 +1,11 @@ -using Bit.Api.Models.Request; +using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Models.Request; using Bit.Api.Models.Response; +using Bit.Core.Auth.Models.Api.Request; +using Bit.Core.Auth.Models.Api.Response; +using Bit.Core.Context; using Bit.Core.Entities; -using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; @@ -19,17 +23,20 @@ public class DevicesController : Controller private readonly IDeviceService _deviceService; private readonly IUserService _userService; private readonly IUserRepository _userRepository; + private readonly ICurrentContext _currentContext; public DevicesController( IDeviceRepository deviceRepository, IDeviceService deviceService, IUserService userService, - IUserRepository userRepository) + IUserRepository userRepository, + ICurrentContext currentContext) { _deviceRepository = deviceRepository; _deviceService = deviceService; _userService = userService; _userRepository = userRepository; + _currentContext = currentContext; } [HttpGet("{id}")] @@ -66,15 +73,6 @@ public class DevicesController : Controller return new ListResponseModel(responses); } - [HttpPost("exist-by-types")] - public async Task> GetExistenceByTypes([FromBody] DeviceType[] deviceTypes) - { - var userId = _userService.GetProperUserId(User).Value; - var devices = await _deviceRepository.GetManyByUserIdAsync(userId); - var userHasDeviceOfTypes = devices.Any(d => deviceTypes.Contains(d.Type)); - return Ok(userHasDeviceOfTypes); - } - [HttpPost("")] public async Task Post([FromBody] DeviceRequestModel model) { @@ -117,6 +115,55 @@ public class DevicesController : Controller return response; } + [HttpPost("{identifier}/retrieve-keys")] + public async Task GetDeviceKeys(string identifier, [FromBody] SecretVerificationRequestModel model) + { + var user = await _userService.GetUserByPrincipalAsync(User); + + if (user == null) + { + throw new UnauthorizedAccessException(); + } + + if (!await _userService.VerifySecretAsync(user, model.Secret)) + { + await Task.Delay(2000); + throw new BadRequestException(string.Empty, "User verification failed."); + } + + var device = await _deviceRepository.GetByIdentifierAsync(identifier, user.Id); + + if (device == null) + { + throw new NotFoundException(); + } + + return new ProtectedDeviceResponseModel(device); + } + + [HttpPost("update-trust")] + public async Task PostUpdateTrust([FromBody] UpdateDevicesTrustRequestModel model) + { + var user = await _userService.GetUserByPrincipalAsync(User); + + if (user == null) + { + throw new UnauthorizedAccessException(); + } + + if (!await _userService.VerifySecretAsync(user, model.Secret)) + { + await Task.Delay(2000); + throw new BadRequestException(string.Empty, "User verification failed."); + } + + await _deviceService.UpdateDevicesTrustAsync( + _currentContext.DeviceIdentifier, + user.Id, + model.CurrentDevice, + model.OtherDevices ?? Enumerable.Empty()); + } + [HttpPut("identifier/{identifier}/token")] [HttpPost("identifier/{identifier}/token")] public async Task PutToken(string identifier, [FromBody] DeviceTokenRequestModel model) diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index 7e09cb8df..6d1424f01 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -2,7 +2,6 @@ using Bit.Api.Models.Response; using Bit.Api.Models.Response.Organizations; using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Business; @@ -313,7 +312,7 @@ public class OrganizationUsersController : Controller } [HttpPut("{userId}/reset-password-enrollment")] - public async Task PutResetPasswordEnrollment(string orgId, string userId, [FromBody] OrganizationUserResetPasswordEnrollmentRequestModel model) + public async Task PutResetPasswordEnrollment(Guid orgId, Guid userId, [FromBody] OrganizationUserResetPasswordEnrollmentRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -321,16 +320,14 @@ public class OrganizationUsersController : Controller throw new UnauthorizedAccessException(); } - if (model.ResetPasswordKey != null && !await _userService.VerifySecretAsync(user, model.Secret)) + var callingUserId = user.Id; + await _organizationService.UpdateUserResetPasswordEnrollmentAsync( + orgId, userId, model.ResetPasswordKey, callingUserId); + + var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, user.Id); + if (orgUser.Status == OrganizationUserStatusType.Invited) { - await Task.Delay(2000); - throw new BadRequestException("MasterPasswordHash", "Invalid password."); - } - else - { - var callingUserId = user.Id; - await _organizationService.UpdateUserResetPasswordEnrollmentAsync( - new Guid(orgId), new Guid(userId), model.ResetPasswordKey, callingUserId); + await _organizationService.AcceptUserAsync(orgId, user, _userService); } } @@ -466,7 +463,7 @@ public class OrganizationUsersController : Controller private async Task RestoreOrRevokeUserAsync( Guid orgId, Guid id, - Func statusAction) + Func statusAction) { if (!await _currentContext.ManageUsers(orgId)) { @@ -486,7 +483,7 @@ public class OrganizationUsersController : Controller private async Task> RestoreOrRevokeUsersAsync( Guid orgId, OrganizationUserBulkRequestModel model, - Func, Guid?, Task>>> statusAction) + Func, Guid?, Task>>> statusAction) { if (!await _currentContext.ManageUsers(orgId)) { diff --git a/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs b/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs index 92044c455..1206d3c97 100644 --- a/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs +++ b/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs @@ -1,5 +1,4 @@ using System.ComponentModel.DataAnnotations; -using Bit.Api.Auth.Models.Request.Accounts; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Models.Data; @@ -108,7 +107,7 @@ public class OrganizationUserUpdateGroupsRequestModel public IEnumerable GroupIds { get; set; } } -public class OrganizationUserResetPasswordEnrollmentRequestModel : SecretVerificationRequestModel +public class OrganizationUserResetPasswordEnrollmentRequestModel { public string ResetPasswordKey { get; set; } } diff --git a/src/Api/Models/Response/DeviceResponseModel.cs b/src/Api/Models/Response/DeviceResponseModel.cs index 939c22b02..7d36f0aa2 100644 --- a/src/Api/Models/Response/DeviceResponseModel.cs +++ b/src/Api/Models/Response/DeviceResponseModel.cs @@ -1,4 +1,5 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.Utilities; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Models.Api; @@ -19,9 +20,7 @@ public class DeviceResponseModel : ResponseModel Type = device.Type; Identifier = device.Identifier; CreationDate = device.CreationDate; - EncryptedUserKey = device.EncryptedUserKey; - EncryptedPublicKey = device.EncryptedPublicKey; - EncryptedPrivateKey = device.EncryptedPrivateKey; + IsTrusted = device.IsTrusted(); } public Guid Id { get; set; } @@ -29,7 +28,5 @@ public class DeviceResponseModel : ResponseModel public DeviceType Type { get; set; } public string Identifier { get; set; } public DateTime CreationDate { get; set; } - public string EncryptedUserKey { get; } - public string EncryptedPublicKey { get; } - public string EncryptedPrivateKey { get; } + public bool IsTrusted { get; set; } } diff --git a/src/Core/Auth/Models/Api/Request/DeviceKeysUpdateRequestModel.cs b/src/Core/Auth/Models/Api/Request/DeviceKeysUpdateRequestModel.cs new file mode 100644 index 000000000..2b815afd1 --- /dev/null +++ b/src/Core/Auth/Models/Api/Request/DeviceKeysUpdateRequestModel.cs @@ -0,0 +1,21 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.Utilities; + +namespace Bit.Core.Auth.Models.Api.Request; + +public class OtherDeviceKeysUpdateRequestModel : DeviceKeysUpdateRequestModel +{ + [Required] + public Guid DeviceId { get; set; } +} + +public class DeviceKeysUpdateRequestModel +{ + [Required] + [EncryptedString] + public string EncryptedPublicKey { get; set; } + + [Required] + [EncryptedString] + public string EncryptedUserKey { get; set; } +} diff --git a/src/Core/Auth/Models/Api/Response/ProtectedDeviceResponseModel.cs b/src/Core/Auth/Models/Api/Response/ProtectedDeviceResponseModel.cs new file mode 100644 index 000000000..c64c55297 --- /dev/null +++ b/src/Core/Auth/Models/Api/Response/ProtectedDeviceResponseModel.cs @@ -0,0 +1,30 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Api; + +namespace Bit.Core.Auth.Models.Api.Response; + +public class ProtectedDeviceResponseModel : ResponseModel +{ + public ProtectedDeviceResponseModel(Device device) + : base("protectedDevice") + { + ArgumentNullException.ThrowIfNull(device); + + Id = device.Id; + Name = device.Name; + Type = device.Type; + Identifier = device.Identifier; + CreationDate = device.CreationDate; + EncryptedUserKey = device.EncryptedUserKey; + EncryptedPublicKey = device.EncryptedPublicKey; + } + + public Guid Id { get; set; } + public string Name { get; set; } + public DeviceType Type { get; set; } + public string Identifier { get; set; } + public DateTime CreationDate { get; set; } + public string EncryptedUserKey { get; set; } + public string EncryptedPublicKey { get; set; } +} diff --git a/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs b/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs index 8cf2586ce..edfcce5a5 100644 --- a/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs +++ b/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs @@ -32,10 +32,22 @@ public class UserDecryptionOptions : ResponseModel public class TrustedDeviceUserDecryptionOption { public bool HasAdminApproval { get; } + public bool HasLoginApprovingDevice { get; } + public bool HasManageResetPasswordPermission { get; } + public string? EncryptedPrivateKey { get; } + public string? EncryptedUserKey { get; } - public TrustedDeviceUserDecryptionOption(bool hasAdminApproval) + public TrustedDeviceUserDecryptionOption(bool hasAdminApproval, + bool hasLoginApprovingDevice, + bool hasManageResetPasswordPermission, + string? encryptedPrivateKey, + string? encryptedUserKey) { HasAdminApproval = hasAdminApproval; + HasLoginApprovingDevice = hasLoginApprovingDevice; + HasManageResetPasswordPermission = hasManageResetPasswordPermission; + EncryptedPrivateKey = encryptedPrivateKey; + EncryptedUserKey = encryptedUserKey; } } diff --git a/src/Core/Auth/Services/Implementations/AuthRequestService.cs b/src/Core/Auth/Services/Implementations/AuthRequestService.cs index 9c0d8f11a..b75a5c130 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 class AuthRequestService : IAuthRequestService IGlobalSettings globalSettings, IDeviceRepository deviceRepository, ICurrentContext currentContext, - IPushNotificationService pushNotificationService) + IPushNotificationService pushNotificationService, + IEventService eventService, + IOrganizationUserRepository organizationRepository) { _authRequestRepository = authRequestRepository; _userRepository = userRepository; @@ -36,6 +43,8 @@ public class AuthRequestService : IAuthRequestService _deviceRepository = deviceRepository; _currentContext = currentContext; _pushNotificationService = pushNotificationService; + _eventService = eventService; + _organizationUserRepository = organizationRepository; } public async Task GetAuthRequestAsync(Guid id, Guid userId) @@ -52,9 +61,12 @@ public class AuthRequestService : IAuthRequestService 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 class AuthRequestService : IAuthRequestService } } + // 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 class AuthRequestService : IAuthRequestService 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 class AuthRequestService : IAuthRequestService // 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/Auth/Services/Implementations/SsoConfigService.cs b/src/Core/Auth/Services/Implementations/SsoConfigService.cs index a2cb906f6..a7e4784f4 100644 --- a/src/Core/Auth/Services/Implementations/SsoConfigService.cs +++ b/src/Core/Auth/Services/Implementations/SsoConfigService.cs @@ -63,7 +63,7 @@ public class SsoConfigService : ISsoConfigService throw new BadRequestException("Key Connector cannot be disabled at this moment."); } - // Automatically enable account recovery and single org policies if trusted device encryption is selected + // Automatically enable account recovery, SSO required, and single org policies if trusted device encryption is selected if (config.GetData().MemberDecryptionType == MemberDecryptionType.TrustedDeviceEncryption) { var singleOrgPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.SingleOrg) ?? @@ -78,8 +78,13 @@ public class SsoConfigService : ISsoConfigService resetPolicy.Enabled = true; resetPolicy.SetDataModel(new ResetPasswordDataModel { AutoEnrollEnabled = true }); - await _policyService.SaveAsync(resetPolicy, _userService, _organizationService, null); + + var ssoRequiredPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.RequireSso) ?? + new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.RequireSso, }; + + ssoRequiredPolicy.Enabled = true; + await _policyService.SaveAsync(ssoRequiredPolicy, _userService, _organizationService, null); } await LogEventsAsync(config, oldConfig); diff --git a/src/Core/Auth/Utilities/DeviceExtensions.cs b/src/Core/Auth/Utilities/DeviceExtensions.cs new file mode 100644 index 000000000..a224d7730 --- /dev/null +++ b/src/Core/Auth/Utilities/DeviceExtensions.cs @@ -0,0 +1,23 @@ +using Bit.Core.Entities; + +#nullable enable + +namespace Bit.Core.Auth.Utilities; + +public static class DeviceExtensions +{ + /// + /// Gets a boolean representing if the device has enough information on it to determine whether or not it is trusted. + /// + /// + /// It is possible for a device to be un-trusted client side and not notify the server side. This should not be + /// the source of truth for whether a device is fully trusted and should just be considered that, to the server, + /// a device has the necessary information to be "trusted". + /// + public static bool IsTrusted(this Device device) + { + return !string.IsNullOrEmpty(device.EncryptedUserKey) && + !string.IsNullOrEmpty(device.EncryptedPublicKey) && + !string.IsNullOrEmpty(device.EncryptedPrivateKey); + } +} diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index a749fe804..16eac4cb7 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -213,4 +213,9 @@ public class User : ITableObject, ISubscriber, IStorable, IStorableSubscri SecurityStamp = SecurityStamp }; } + + public bool HasMasterPassword() + { + return MasterPassword != null; + } } diff --git a/src/Core/Enums/EventType.cs b/src/Core/Enums/EventType.cs index 82339ac8a..f03ce71a5 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/src/Core/Services/IDeviceService.cs b/src/Core/Services/IDeviceService.cs index 3109cc107..cadc3e4be 100644 --- a/src/Core/Services/IDeviceService.cs +++ b/src/Core/Services/IDeviceService.cs @@ -1,4 +1,5 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.Models.Api.Request; +using Bit.Core.Entities; namespace Bit.Core.Services; @@ -7,4 +8,8 @@ public interface IDeviceService Task SaveAsync(Device device); Task ClearTokenAsync(Device device); Task DeleteAsync(Device device); + Task UpdateDevicesTrustAsync(string currentDeviceIdentifier, + Guid currentUserId, + DeviceKeysUpdateRequestModel currentDeviceUpdate, + IEnumerable alteredDevices); } diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index 05fa0775a..832aa9de4 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -41,6 +41,7 @@ public interface IOrganizationService Task ResendInviteAsync(Guid organizationId, Guid? invitingUserId, Guid organizationUserId, bool initOrganization = false); Task AcceptUserAsync(Guid organizationUserId, User user, string token, IUserService userService); Task AcceptUserAsync(string orgIdentifier, User user, IUserService userService); + Task AcceptUserAsync(Guid organizationId, User user, IUserService userService); Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId, IUserService userService); Task>> ConfirmUsersAsync(Guid organizationId, Dictionary keys, diff --git a/src/Core/Services/Implementations/DeviceService.cs b/src/Core/Services/Implementations/DeviceService.cs index 99f4648a3..5b1e4b0f0 100644 --- a/src/Core/Services/Implementations/DeviceService.cs +++ b/src/Core/Services/Implementations/DeviceService.cs @@ -1,4 +1,7 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.Models.Api.Request; +using Bit.Core.Auth.Utilities; +using Bit.Core.Entities; +using Bit.Core.Exceptions; using Bit.Core.Repositories; namespace Bit.Core.Services; @@ -43,4 +46,61 @@ public class DeviceService : IDeviceService await _deviceRepository.DeleteAsync(device); await _pushRegistrationService.DeleteRegistrationAsync(device.Id.ToString()); } + + public async Task UpdateDevicesTrustAsync(string currentDeviceIdentifier, + Guid currentUserId, + DeviceKeysUpdateRequestModel currentDeviceUpdate, + IEnumerable alteredDevices) + { + var existingDevices = await _deviceRepository.GetManyByUserIdAsync(currentUserId); + + var currentDevice = existingDevices.FirstOrDefault(d => d.Identifier == currentDeviceIdentifier); + + if (currentDevice == null) + { + throw new NotFoundException(); + } + + existingDevices.Remove(currentDevice); + + var alterDeviceKeysDict = alteredDevices.ToDictionary(d => d.DeviceId); + + if (alterDeviceKeysDict.ContainsKey(currentDevice.Id)) + { + throw new BadRequestException("Current device can not be an optional rotation."); + } + + currentDevice.EncryptedPublicKey = currentDeviceUpdate.EncryptedPublicKey; + currentDevice.EncryptedUserKey = currentDeviceUpdate.EncryptedUserKey; + + await _deviceRepository.UpsertAsync(currentDevice); + + foreach (var device in existingDevices) + { + if (!device.IsTrusted()) + { + // You can't update the trust of a device that isn't trusted to begin with + // should we throw and consider this a BadRequest? If we want to consider it a invalid request + // we need to check that information before we enter this foreach, we don't want to partially complete + // this process. + continue; + } + + if (alterDeviceKeysDict.TryGetValue(device.Id, out var updateRequest)) + { + // An update to this device was requested + device.EncryptedPublicKey = updateRequest.EncryptedPublicKey; + device.EncryptedUserKey = updateRequest.EncryptedUserKey; + } + else + { + // No update to this device requested, just untrust it + device.EncryptedUserKey = null; + device.EncryptedPublicKey = null; + device.EncryptedPrivateKey = null; + } + + await _deviceRepository.UpsertAsync(device); + } + } } diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index c0aa2fe10..b27e395b0 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1114,6 +1114,24 @@ public class OrganizationService : IOrganizationService return await AcceptUserAsync(orgUser, user, userService); } + public async Task AcceptUserAsync(Guid organizationId, User user, IUserService userService) + { + var org = await _organizationRepository.GetByIdAsync(organizationId); + if (org == null) + { + throw new BadRequestException("Organization invalid."); + } + + var usersOrgs = await _organizationUserRepository.GetManyByUserAsync(user.Id); + var orgUser = usersOrgs.FirstOrDefault(u => u.OrganizationId == org.Id); + if (orgUser == null) + { + throw new BadRequestException("User not found within organization."); + } + + return await AcceptUserAsync(orgUser, user, userService); + } + private async Task AcceptUserAsync(OrganizationUser orgUser, User user, IUserService userService) { diff --git a/src/Core/Services/Implementations/PolicyService.cs b/src/Core/Services/Implementations/PolicyService.cs index 6b1009093..64da29e4d 100644 --- a/src/Core/Services/Implementations/PolicyService.cs +++ b/src/Core/Services/Implementations/PolicyService.cs @@ -73,6 +73,7 @@ public class PolicyService : IPolicyService else { await RequiredByKeyConnectorAsync(org); + await RequiredBySsoTrustedDeviceEncryptionAsync(org); } break; diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index e80ea347a..643e1bf18 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -892,7 +892,7 @@ public class UserService : UserManager, IUserService, IDisposable await _userRepository.ReplaceAsync(user); } - await _pushService.PushLogOutAsync(user.Id); + await _pushService.PushLogOutAsync(user.Id, excludeCurrentContextFromPush: true); return IdentityResult.Success; } @@ -1455,26 +1455,35 @@ public class UserService : UserManager, IUserService, IDisposable throw new BadRequestException("No user email."); } - if (!user.UsesKeyConnector) - { - throw new BadRequestException("Not using Key Connector."); - } - var token = await base.GenerateUserTokenAsync(user, TokenOptions.DefaultEmailProvider, "otp:" + user.Email); await _mailService.SendOTPEmailAsync(user.Email, token); } - public Task VerifyOTPAsync(User user, string token) + public async Task VerifyOTPAsync(User user, string token) { - return base.VerifyUserTokenAsync(user, TokenOptions.DefaultEmailProvider, + return await base.VerifyUserTokenAsync(user, TokenOptions.DefaultEmailProvider, "otp:" + user.Email, token); } public async Task VerifySecretAsync(User user, string secret) { - return user.UsesKeyConnector - ? await VerifyOTPAsync(user, secret) - : await CheckPasswordAsync(user, secret); + bool isVerified; + if (user.HasMasterPassword()) + { + // If the user has a master password the secret is most likely going to be a hash + // of their password, but in certain scenarios, like when the user has logged into their + // device without a password (trusted device encryption) but the account + // does still have a password we will allow the use of OTP. + isVerified = await CheckPasswordAsync(user, secret) || + await VerifyOTPAsync(user, secret); + } + else + { + // If they don't have a password at all they can only do OTP + isVerified = await VerifyOTPAsync(user, secret); + } + + return isVerified; } } diff --git a/src/Core/Utilities/DeviceTypes.cs b/src/Core/Utilities/DeviceTypes.cs new file mode 100644 index 000000000..82616fd06 --- /dev/null +++ b/src/Core/Utilities/DeviceTypes.cs @@ -0,0 +1,21 @@ +using Bit.Core.Enums; + +namespace Bit.Core.Utilities; + +public static class DeviceTypes +{ + public static IReadOnlyCollection MobileTypes { get; } = new[] + { + DeviceType.Android, + DeviceType.iOS, + DeviceType.AndroidAmazon, + }; + + public static IReadOnlyCollection DesktopTypes { get; } = new[] + { + DeviceType.LinuxDesktop, + DeviceType.MacOsDesktop, + DeviceType.WindowsDesktop, + DeviceType.UWP, + }; +} diff --git a/src/Identity/IdentityServer/BaseRequestValidator.cs b/src/Identity/IdentityServer/BaseRequestValidator.cs index 9cfc19c13..e0cafbfa1 100644 --- a/src/Identity/IdentityServer/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/BaseRequestValidator.cs @@ -3,13 +3,14 @@ using System.Reflection; using System.Security.Claims; using System.Text.Json; using Bit.Core; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Identity; using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Models.Business.Tokenables; -using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; +using Bit.Core.Auth.Utilities; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -22,6 +23,7 @@ using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Tokens; using Bit.Core.Utilities; +using Bit.Identity.Utilities; using IdentityServer4.Validation; using Microsoft.AspNetCore.Identity; @@ -207,7 +209,7 @@ public abstract class BaseRequestValidator where T : class customResponse.Add("KdfIterations", user.KdfIterations); customResponse.Add("KdfMemory", user.KdfMemory); customResponse.Add("KdfParallelism", user.KdfParallelism); - customResponse.Add("UserDecryptionOptions", await CreateUserDecryptionOptionsAsync(user, GetSubject(context))); + customResponse.Add("UserDecryptionOptions", await CreateUserDecryptionOptionsAsync(user, device, GetSubject(context))); if (sendRememberToken) { @@ -350,7 +352,8 @@ public abstract class BaseRequestValidator where T : class return true; } - // Check if user belongs to any organization with an active SSO policy + + // Check if user belongs to any organization with an active SSO policy var anySsoPoliciesApplicableToUser = await PolicyService.AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.RequireSso, OrganizationUserStatusType.Confirmed); if (anySsoPoliciesApplicableToUser) { @@ -587,15 +590,17 @@ public abstract class BaseRequestValidator where T : class /// /// Used to create a list of all possible ways the newly authenticated user can decrypt their vault contents /// - private async Task CreateUserDecryptionOptionsAsync(User user, ClaimsPrincipal subject) + private async Task CreateUserDecryptionOptionsAsync(User user, Device device, ClaimsPrincipal subject) { - var ssoConfigurationData = await GetSsoConfigurationDataAsync(subject); + var ssoConfiguration = await GetSsoConfigurationDataAsync(subject); var userDecryptionOption = new UserDecryptionOptions { HasMasterPassword = !string.IsNullOrEmpty(user.MasterPassword) }; + var ssoConfigurationData = ssoConfiguration?.GetData(); + if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) { // KeyConnector makes it mutually exclusive @@ -606,15 +611,51 @@ public abstract class BaseRequestValidator where T : class // Only add the trusted device specific option when the flag is turned on if (FeatureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, CurrentContext) && ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }) { - var hasAdminApproval = await PolicyService.AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.ResetPassword); + string? encryptedPrivateKey = null; + string? encryptedUserKey = null; + if (device.IsTrusted()) + { + encryptedPrivateKey = device.EncryptedPrivateKey; + encryptedUserKey = device.EncryptedUserKey; + } + + var allDevices = await _deviceRepository.GetManyByUserIdAsync(user.Id); + // Checks if the current user has any devices that are capable of approving login with device requests except for + // their current device. + // NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. + var hasLoginApprovingDevice = allDevices + .Where(d => d.Identifier != device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) + .Any(); + + // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP + var hasManageResetPasswordPermission = false; + + // when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here + if (CurrentContext.Organizations.Any(o => o.Id == ssoConfiguration!.OrganizationId)) + { + // TDE requires single org so grabbing first org & id is fine. + hasManageResetPasswordPermission = await CurrentContext.ManageResetPassword(ssoConfiguration!.OrganizationId); + } + + // If sso configuration data is not null then I know for sure that ssoConfiguration isn't null + var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(ssoConfiguration!.OrganizationId, user.Id); + + // They are only able to be approved by an admin if they have enrolled is reset password + var hasAdminApproval = !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); + // TrustedDeviceEncryption only exists for SSO, but if that ever changes this value won't always be true - userDecryptionOption.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption(hasAdminApproval); + userDecryptionOption.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( + hasAdminApproval, + hasLoginApprovingDevice, + hasManageResetPasswordPermission, + encryptedPrivateKey, + encryptedUserKey); } return userDecryptionOption; } - private async Task GetSsoConfigurationDataAsync(ClaimsPrincipal subject) + private async Task GetSsoConfigurationDataAsync(ClaimsPrincipal subject) { var organizationClaim = subject?.FindFirstValue("organizationId"); @@ -629,6 +670,6 @@ public abstract class BaseRequestValidator where T : class return null; } - return ssoConfig.GetData(); + return ssoConfig; } } diff --git a/src/Identity/Utilities/LoginApprovingDeviceTypes.cs b/src/Identity/Utilities/LoginApprovingDeviceTypes.cs new file mode 100644 index 000000000..46b4606cc --- /dev/null +++ b/src/Identity/Utilities/LoginApprovingDeviceTypes.cs @@ -0,0 +1,19 @@ +using Bit.Core.Enums; +using Bit.Core.Utilities; + +namespace Bit.Identity.Utilities; + +public static class LoginApprovingDeviceTypes +{ + private static readonly IReadOnlyCollection _deviceTypes; + + static LoginApprovingDeviceTypes() + { + var deviceTypes = new List(); + deviceTypes.AddRange(DeviceTypes.DesktopTypes); + deviceTypes.AddRange(DeviceTypes.MobileTypes); + _deviceTypes = deviceTypes.AsReadOnly(); + } + + public static IReadOnlyCollection Types => _deviceTypes; +} diff --git a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs index c5c1019df..6053d17dd 100644 --- a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs @@ -16,6 +16,34 @@ namespace Bit.Api.Test.Controllers; [SutProviderCustomize] public class OrganizationUsersControllerTests { + [Theory] + [BitAutoData] + public async Task PutResetPasswordEnrollment_InivitedUser_AcceptsInvite(Guid orgId, Guid userId, OrganizationUserResetPasswordEnrollmentRequestModel model, + User user, OrganizationUser orgUser, SutProvider sutProvider) + { + orgUser.Status = Core.Enums.OrganizationUserStatusType.Invited; + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().GetByOrganizationAsync(default, default).ReturnsForAnyArgs(orgUser); + + await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); + + await sutProvider.GetDependency().Received(1).AcceptUserAsync(orgId, user, sutProvider.GetDependency()); + } + + [Theory] + [BitAutoData] + public async Task PutResetPasswordEnrollment_ConfirmedUser_AcceptsInvite(Guid orgId, Guid userId, OrganizationUserResetPasswordEnrollmentRequestModel model, + User user, OrganizationUser orgUser, SutProvider sutProvider) + { + orgUser.Status = Core.Enums.OrganizationUserStatusType.Confirmed; + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().GetByOrganizationAsync(default, default).ReturnsForAnyArgs(orgUser); + + await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); + + await sutProvider.GetDependency().Received(0).AcceptUserAsync(orgId, user, sutProvider.GetDependency()); + } + [Theory] [BitAutoData] public async Task Accept_RequiresKnownUser(Guid orgId, Guid orgUserId, OrganizationUserAcceptRequestModel model, diff --git a/test/Common/AutoFixture/Attributes/BitAutoDataAttribute.cs b/test/Common/AutoFixture/Attributes/BitAutoDataAttribute.cs index d859f81fc..03a8cb466 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(params object[] fixedTestParameters) : + public BitAutoDataAttribute() : this(Array.Empty()) { } + + 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/AssertHelper.cs b/test/Common/Helpers/AssertHelper.cs index a9f004bd3..0b6752a03 100644 --- a/test/Common/Helpers/AssertHelper.cs +++ b/test/Common/Helpers/AssertHelper.cs @@ -103,6 +103,7 @@ public static class AssertHelper public static Expression> AssertEqualExpected(T expected) => (T actual) => AssertEqualExpectedPredicate(expected)(actual); + [StackTraceHidden] public static JsonElement AssertJsonProperty(JsonElement element, string propertyName, JsonValueKind jsonValueKind) { if (!element.TryGetProperty(propertyName, out var subElement)) diff --git a/test/Common/Helpers/BitAutoDataAttributeHelpers.cs b/test/Common/Helpers/BitAutoDataAttributeHelpers.cs index 32cacc49d..b2ee4c421 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 class BitAutoDataAttributeHelpers return newFixture; } + + public static 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 a59551166..3cac77405 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.Services; 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 class AuthRequestServiceTests 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 class AuthRequestServiceTests 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 class AuthRequestServiceTests .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 class AuthRequestServiceTests 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 class AuthRequestServiceTests .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 class AuthRequestServiceTests 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 class AuthRequestServiceTests .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 class AuthRequestServiceTests 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 class AuthRequestServiceTests 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 @@ public class AuthRequestServiceTests 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 class AuthRequestServiceTests // 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 class AuthRequestServiceTests 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,29 +599,21 @@ public class AuthRequestServiceTests 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 - { - Id = Guid.NewGuid(), - Identifier = "test_identifier", - }; - - sutProvider.GetDependency() - .GetByIdentifierAsync(device.Identifier, authRequest.UserId) - .Returns(device); - var updateModel = new AuthRequestUpdateRequestModel { Key = "test_key", @@ -390,4 +626,69 @@ public class AuthRequestServiceTests 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", + RequestApproved = true, + MasterPasswordHash = "my_hash", + }; + + 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())); + } } diff --git a/test/Core.Test/Services/DeviceServiceTests.cs b/test/Core.Test/Services/DeviceServiceTests.cs index 8bc921283..bdd3fbdad 100644 --- a/test/Core.Test/Services/DeviceServiceTests.cs +++ b/test/Core.Test/Services/DeviceServiceTests.cs @@ -1,12 +1,18 @@ -using Bit.Core.Entities; +using System.Runtime.CompilerServices; +using Bit.Core.Auth.Models.Api.Request; +using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; namespace Bit.Core.Test.Services; +[SutProviderCustomize] public class DeviceServiceTests { [Fact] @@ -33,4 +39,227 @@ public class DeviceServiceTests await pushRepo.Received().CreateOrUpdateRegistrationAsync("testtoken", id.ToString(), userId.ToString(), "testid", DeviceType.Android); } + + /// + /// Story: A user choosed to keep trust in one of their current trusted devices, but not in another one of their + /// devices. We will rotate the trust of the currently signed in device as well as the device they chose but will + /// remove the trust of the device they didn't give new keys for. + /// + [Theory, BitAutoData] + public async Task UpdateDevicesTrustAsync_Works( + SutProvider sutProvider, + Guid currentUserId, + Device deviceOne, + Device deviceTwo, + Device deviceThree) + { + SetupOldTrust(deviceOne); + SetupOldTrust(deviceTwo); + SetupOldTrust(deviceThree); + + deviceOne.Identifier = "current_device"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(currentUserId) + .Returns(new List + { + deviceOne, + deviceTwo, + deviceThree, + }); + + var currentDeviceModel = new DeviceKeysUpdateRequestModel + { + EncryptedPublicKey = "current_encrypted_public_key", + EncryptedUserKey = "current_encrypted_user_key", + }; + + var alteredDeviceModels = new List + { + new OtherDeviceKeysUpdateRequestModel + { + DeviceId = deviceTwo.Id, + EncryptedPublicKey = "encrypted_public_key_two", + EncryptedUserKey = "encrypted_user_key_two", + }, + }; + + await sutProvider.Sut.UpdateDevicesTrustAsync("current_device", currentUserId, currentDeviceModel, alteredDeviceModels); + + // Updating trust, "current" or "other" only needs to change the EncryptedPublicKey & EncryptedUserKey + await sutProvider.GetDependency() + .Received(1) + .UpsertAsync(Arg.Is(d => + d.Id == deviceOne.Id && + d.EncryptedPublicKey == "current_encrypted_public_key" && + d.EncryptedUserKey == "current_encrypted_user_key" && + d.EncryptedPrivateKey == "old_private_deviceOne")); + + await sutProvider.GetDependency() + .Received(1) + .UpsertAsync(Arg.Is(d => + d.Id == deviceTwo.Id && + d.EncryptedPublicKey == "encrypted_public_key_two" && + d.EncryptedUserKey == "encrypted_user_key_two" && + d.EncryptedPrivateKey == "old_private_deviceTwo")); + + // Clearing trust should remove all key values + await sutProvider.GetDependency() + .Received(1) + .UpsertAsync(Arg.Is(d => + d.Id == deviceThree.Id && + d.EncryptedPublicKey == null && + d.EncryptedUserKey == null && + d.EncryptedPrivateKey == null)); + + // Should have recieved a total of 3 calls, the ones asserted above + await sutProvider.GetDependency() + .Received(3) + .UpsertAsync(Arg.Any()); + + // TODO: .NET 8: Use nameof for parameter name. + static void SetupOldTrust(Device device, [CallerArgumentExpression("device")] string expression = null) + { + device.EncryptedPublicKey = $"old_public_{expression}"; + device.EncryptedPrivateKey = $"old_private_{expression}"; + device.EncryptedUserKey = $"old_user_{expression}"; + } + } + + /// + /// Story: This could result from a poor implementation of this method, if they attempt add trust to a device + /// that doesn't already have trust. They would have to create brand new values and for that values to be accurate + /// they would technically have all the values needed to trust a device, that is why we don't consider this bad + /// enough to throw but do skip it because we'd rather keep number of ways for trust to be added to the endpoint we + /// already have. + /// + [Theory, BitAutoData] + public async Task UpdateDevicesTrustAsync_DoesNotUpdateUntrustedDevices( + SutProvider sutProvider, + Guid currentUserId, + Device deviceOne, + Device deviceTwo) + { + deviceOne.Identifier = "current_device"; + + // Make deviceTwo untrusted + deviceTwo.EncryptedUserKey = string.Empty; + deviceTwo.EncryptedPublicKey = string.Empty; + deviceTwo.EncryptedPrivateKey = string.Empty; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(currentUserId) + .Returns(new List + { + deviceOne, + deviceTwo, + }); + + var currentDeviceModel = new DeviceKeysUpdateRequestModel + { + EncryptedPublicKey = "current_encrypted_public_key", + EncryptedUserKey = "current_encrypted_user_key", + }; + + var alteredDeviceModels = new List + { + new OtherDeviceKeysUpdateRequestModel + { + DeviceId = deviceTwo.Id, + EncryptedPublicKey = "encrypted_public_key_two", + EncryptedUserKey = "encrypted_user_key_two", + }, + }; + + await sutProvider.Sut.UpdateDevicesTrustAsync("current_device", currentUserId, currentDeviceModel, alteredDeviceModels); + + // Check that UpsertAsync was called for the trusted device + await sutProvider.GetDependency() + .Received(1) + .UpsertAsync(Arg.Is(d => + d.Id == deviceOne.Id && + d.EncryptedPublicKey == "current_encrypted_public_key" && + d.EncryptedUserKey == "current_encrypted_user_key")); + + // Check that UpsertAsync was not called for the untrusted device + await sutProvider.GetDependency() + .DidNotReceive() + .UpsertAsync(Arg.Is(d => d.Id == deviceTwo.Id)); + } + + /// + /// Story: This should only happen if someone were to take the access token from a different device and try to rotate + /// a device that they don't actually have. + /// + [Theory, BitAutoData] + public async Task UpdateDevicesTrustAsync_ThrowsNotFoundException_WhenCurrentDeviceIdentifierDoesNotExist( + SutProvider sutProvider, + Guid currentUserId, + Device deviceOne, + Device deviceTwo) + { + deviceOne.Identifier = "some_other_device"; + deviceTwo.Identifier = "another_device"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(currentUserId) + .Returns(new List + { + deviceOne, + deviceTwo, + }); + + var currentDeviceModel = new DeviceKeysUpdateRequestModel + { + EncryptedPublicKey = "current_encrypted_public_key", + EncryptedUserKey = "current_encrypted_user_key", + }; + + await Assert.ThrowsAsync(() => + sutProvider.Sut.UpdateDevicesTrustAsync("current_device", currentUserId, currentDeviceModel, + Enumerable.Empty())); + } + + /// + /// Story: This should only happen from a poorly implemented user of this method but important to enforce someone + /// using the method correctly, a device should only be rotated intentionally and including it as both the current + /// device and one of the users other device would mean they could rotate it twice and we aren't sure + /// which one they would want to win out. + /// + [Theory, BitAutoData] + public async Task UpdateDevicesTrustAsync_ThrowsBadRequestException_WhenCurrentDeviceIsIncludedInAlteredDevices( + SutProvider sutProvider, + Guid currentUserId, + Device deviceOne, + Device deviceTwo) + { + deviceOne.Identifier = "current_device"; + + sutProvider.GetDependency() + .GetManyByUserIdAsync(currentUserId) + .Returns(new List + { + deviceOne, + deviceTwo, + }); + + var currentDeviceModel = new DeviceKeysUpdateRequestModel + { + EncryptedPublicKey = "current_encrypted_public_key", + EncryptedUserKey = "current_encrypted_user_key", + }; + + var alteredDeviceModels = new List + { + new OtherDeviceKeysUpdateRequestModel + { + DeviceId = deviceOne.Id, // current device is included in alteredDevices + EncryptedPublicKey = "encrypted_public_key_one", + EncryptedUserKey = "encrypted_user_key_one", + }, + }; + + await Assert.ThrowsAsync(() => + sutProvider.Sut.UpdateDevicesTrustAsync("current_device", currentUserId, currentDeviceModel, alteredDeviceModels)); + } } diff --git a/test/Core.Test/Services/PolicyServiceTests.cs b/test/Core.Test/Services/PolicyServiceTests.cs index 6c1218c84..aedc3cc58 100644 --- a/test/Core.Test/Services/PolicyServiceTests.cs +++ b/test/Core.Test/Services/PolicyServiceTests.cs @@ -406,7 +406,7 @@ public class PolicyServiceTests [BitAutoData(true, false)] [BitAutoData(false, true)] [BitAutoData(false, false)] - public async Task SaveAsync_PolicyRequiredByTrustedDeviceEncryption_DisablePolicyOrDisableAutomaticEnrollment_ThrowsBadRequest( + public async Task SaveAsync_ResetPasswordPolicyRequiredByTrustedDeviceEncryption_DisablePolicyOrDisableAutomaticEnrollment_ThrowsBadRequest( bool policyEnabled, bool autoEnrollEnabled, [PolicyFixtures.Policy(PolicyType.ResetPassword)] Policy policy, @@ -448,6 +448,43 @@ public class PolicyServiceTests .LogPolicyEventAsync(default, default, default); } + [Theory, BitAutoData] + public async Task SaveAsync_RequireSsoPolicyRequiredByTrustedDeviceEncryption_DisablePolicy_ThrowsBadRequest( + [PolicyFixtures.Policy(PolicyType.RequireSso)] Policy policy, + SutProvider sutProvider) + { + policy.Enabled = false; + + SetupOrg(sutProvider, policy.OrganizationId, new Organization + { + Id = policy.OrganizationId, + UsePolicies = true, + }); + + var ssoConfig = new SsoConfig { Enabled = true }; + ssoConfig.SetData(new SsoConfigurationData { MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption }); + + sutProvider.GetDependency() + .GetByOrganizationIdAsync(policy.OrganizationId) + .Returns(ssoConfig); + + var badRequestException = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(policy, + Substitute.For(), + Substitute.For(), + Guid.NewGuid())); + + Assert.Contains("Trusted device encryption is on and requires this policy.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpsertAsync(default); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogPolicyEventAsync(default, default, default); + } + [Theory, BitAutoData] public async Task SaveAsync_PolicyRequiredForAccountRecovery_NotEnabled_ThrowsBadRequestAsync( [PolicyFixtures.Policy(Enums.PolicyType.ResetPassword)] Policy policy, SutProvider sutProvider) diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 9cdda95de..92de67a4e 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -1,15 +1,23 @@ using System.Text.Json; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; +using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Models.Business; using Bit.Core.Models.Data.Organizations; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Settings; +using Bit.Core.Tools.Services; +using Bit.Core.Vault.Repositories; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; +using Fido2NetLib; +using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NSubstitute; using NSubstitute.ReceivedExtensions; using Xunit; @@ -134,7 +142,7 @@ public class UserServiceTests } [Theory, BitAutoData] - public async void HasPremiumFromOrganization_Returns_False_If_No_Orgs(SutProvider sutProvider, User user) + public async Task HasPremiumFromOrganization_Returns_False_If_No_Orgs(SutProvider sutProvider, User user) { sutProvider.GetDependency().GetManyByUserAsync(user.Id).Returns(new List()); Assert.False(await sutProvider.Sut.HasPremiumFromOrganization(user)); @@ -144,7 +152,7 @@ public class UserServiceTests [Theory] [BitAutoData(false, true)] [BitAutoData(true, false)] - public async void HasPremiumFromOrganization_Returns_False_If_Org_Not_Eligible(bool orgEnabled, bool orgUsersGetPremium, SutProvider sutProvider, User user, OrganizationUser orgUser, Organization organization) + public async Task HasPremiumFromOrganization_Returns_False_If_Org_Not_Eligible(bool orgEnabled, bool orgUsersGetPremium, SutProvider sutProvider, User user, OrganizationUser orgUser, Organization organization) { orgUser.OrganizationId = organization.Id; organization.Enabled = orgEnabled; @@ -158,7 +166,7 @@ public class UserServiceTests } [Theory, BitAutoData] - public async void HasPremiumFromOrganization_Returns_True_If_Org_Eligible(SutProvider sutProvider, User user, OrganizationUser orgUser, Organization organization) + public async Task HasPremiumFromOrganization_Returns_True_If_Org_Eligible(SutProvider sutProvider, User user, OrganizationUser orgUser, Organization organization) { orgUser.OrganizationId = organization.Id; organization.Enabled = true; @@ -170,4 +178,145 @@ public class UserServiceTests Assert.True(await sutProvider.Sut.HasPremiumFromOrganization(user)); } + + [Flags] + public enum ShouldCheck + { + Password = 0x1, + OTP = 0x2, + } + + [Theory] + // A user who has a password, and the password is valid should only check for that password + [BitAutoData(true, "test_password", true, ShouldCheck.Password)] + // A user who does not have a password, should only check if the OTP is valid + [BitAutoData(false, "otp_token", true, ShouldCheck.OTP)] + // A user who has a password but supplied a OTP, it will check password first and then try OTP + [BitAutoData(true, "otp_token", true, ShouldCheck.Password | ShouldCheck.OTP)] + // A user who does not have a password and supplied an invalid OTP token, should only check OTP and return invalid + [BitAutoData(false, "bad_otp_token", false, ShouldCheck.OTP)] + // A user who does have a password but they supply a bad one, we will check both but it will still be invalid + [BitAutoData(true, "bad_test_password", false, ShouldCheck.Password | ShouldCheck.OTP)] + public async Task VerifySecretAsync_Works( + bool shouldHavePassword, string secret, bool expectedIsVerified, ShouldCheck shouldCheck, // inline theory data + SutProvider sutProvider, User user) // AutoFixture injected data + { + // Arrange + var tokenProvider = SetupFakeTokenProvider(sutProvider, user); + SetupUserAndDevice(user, shouldHavePassword); + + // Setup the fake password verification + var substitutedUserPasswordStore = Substitute.For>(); + substitutedUserPasswordStore + .GetPasswordHashAsync(user, Arg.Any()) + .Returns((ci) => + { + return Task.FromResult("hashed_test_password"); + }); + + sutProvider.SetDependency>(substitutedUserPasswordStore, "store"); + + sutProvider.GetDependency>("passwordHasher") + .VerifyHashedPassword(user, "hashed_test_password", "test_password") + .Returns((ci) => + { + return PasswordVerificationResult.Success; + }); + + // HACK: SutProvider is being weird about not injecting the IPasswordHasher that I configured + var sut = new UserService( + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency>(), + sutProvider.GetDependency>(), + sutProvider.GetDependency>(), + sutProvider.GetDependency>>(), + sutProvider.GetDependency>>(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency>>(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency()); + + var actualIsVerified = await sut.VerifySecretAsync(user, secret); + + Assert.Equal(expectedIsVerified, actualIsVerified); + + await tokenProvider + .Received(shouldCheck.HasFlag(ShouldCheck.OTP) ? 1 : 0) + .ValidateAsync(Arg.Any(), secret, Arg.Any>(), user); + + sutProvider.GetDependency>() + .Received(shouldCheck.HasFlag(ShouldCheck.Password) ? 1 : 0) + .VerifyHashedPassword(user, "hashed_test_password", secret); + } + + private static void SetupUserAndDevice(User user, + bool shouldHavePassword) + { + if (shouldHavePassword) + { + user.MasterPassword = "test_password"; + } + else + { + user.MasterPassword = null; + } + } + + private static IUserTwoFactorTokenProvider SetupFakeTokenProvider(SutProvider sutProvider, User user) + { + var fakeUserTwoFactorProvider = Substitute.For>(); + + fakeUserTwoFactorProvider + .GenerateAsync(Arg.Any(), Arg.Any>(), user) + .Returns("OTP_TOKEN"); + + fakeUserTwoFactorProvider + .ValidateAsync(Arg.Any(), Arg.Is(s => s != "otp_token"), Arg.Any>(), user) + .Returns(false); + + fakeUserTwoFactorProvider + .ValidateAsync(Arg.Any(), "otp_token", Arg.Any>(), user) + .Returns(true); + + sutProvider.GetDependency>() + .Value.Returns(new IdentityOptions + { + Tokens = new TokenOptions + { + ProviderMap = new Dictionary() + { + ["Email"] = new TokenProviderDescriptor(typeof(IUserTwoFactorTokenProvider)) + { + ProviderInstance = fakeUserTwoFactorProvider, + } + } + } + }); + + // The above arranging of dependencies is used in the constructor of UserManager + // ref: https://github.com/dotnet/aspnetcore/blob/bfeb3bf9005c36b081d1e48725531ee0e15a9dfb/src/Identity/Extensions.Core/src/UserManager.cs#L103-L120 + // since the constructor of the Sut has ran already (when injected) I need to recreate it to get it to run again + sutProvider.Create(); + + return fakeUserTwoFactorProvider; + } } diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs index da23a2acd..e67217d08 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerSsoTests.cs @@ -9,6 +9,7 @@ using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Utilities; @@ -33,33 +34,10 @@ public class IdentityServerSsoTests public async Task Test_MasterPassword_DecryptionType() { // Arrange - var challenge = new string('c', 50); - var factory = await CreateFactoryAsync(new SsoConfigurationData - { - MemberDecryptionType = MemberDecryptionType.MasterPassword, - }, challenge); - - // Act - var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary - { - { "scope", "api offline_access" }, - { "client_id", "web" }, - { "deviceType", "10" }, - { "deviceIdentifier", "test_id" }, - { "deviceName", "firefox" }, - { "twoFactorToken", "TEST"}, - { "twoFactorProvider", "5" }, // RememberMe Provider - { "twoFactorRemember", "0" }, - { "grant_type", "authorization_code" }, - { "code", "test_code" }, - { "code_verifier", challenge }, - { "redirect_uri", "https://localhost:8080/sso-connector.html" } - })); + using var responseBody = await RunSuccessTestAsync(MemberDecryptionType.MasterPassword); // Assert // If the organization has a member decryption type of MasterPassword that should be the only option in the reply - Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); - using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); var userDecryptionOptions = AssertHelper.AssertJsonProperty(root, "UserDecryptionOptions", JsonValueKind.Object); @@ -80,34 +58,11 @@ public class IdentityServerSsoTests public async Task SsoLogin_TrustedDeviceEncryption_ReturnsOptions() { // Arrange - var challenge = new string('c', 50); - var factory = await CreateFactoryAsync(new SsoConfigurationData - { - MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption, - }, challenge); - - // Act - var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary - { - { "scope", "api offline_access" }, - { "client_id", "web" }, - { "deviceType", "10" }, - { "deviceIdentifier", "test_id" }, - { "deviceName", "firefox" }, - { "twoFactorToken", "TEST"}, - { "twoFactorProvider", "5" }, // RememberMe Provider - { "twoFactorRemember", "0" }, - { "grant_type", "authorization_code" }, - { "code", "test_code" }, - { "code_verifier", challenge }, - { "redirect_uri", "https://localhost:8080/sso-connector.html" } - })); + using var responseBody = await RunSuccessTestAsync(MemberDecryptionType.TrustedDeviceEncryption); // Assert // If the organization has selected TrustedDeviceEncryption but the user still has their master password // they can decrypt with either option - Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); - using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); var userDecryptionOptions = AssertHelper.AssertJsonProperty(root, "UserDecryptionOptions", JsonValueKind.Object); @@ -132,47 +87,25 @@ public class IdentityServerSsoTests public async Task SsoLogin_TrustedDeviceEncryption_WithAdminResetPolicy_ReturnsOptions() { // Arrange - var challenge = new string('c', 50); - var factory = await CreateFactoryAsync(new SsoConfigurationData + using var responseBody = await RunSuccessTestAsync(async factory => { - MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption, - }, challenge); + var database = factory.GetDatabaseContext(); - var database = factory.GetDatabaseContext(); + var organization = await database.Organizations.SingleAsync(); - var organization = await database.Organizations.SingleAsync(); + var user = await database.Users.SingleAsync(u => u.Email == TestEmail); - var policyRepository = factory.Services.GetRequiredService(); - await policyRepository.CreateAsync(new Policy - { - Type = PolicyType.ResetPassword, - Enabled = true, - Data = "{\"autoEnrollEnabled\": false }", - OrganizationId = organization.Id, - }); + var organizationUser = await database.OrganizationUsers.SingleAsync( + ou => ou.OrganizationId == organization.Id && ou.UserId == user.Id); - // Act - var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary - { - { "scope", "api offline_access" }, - { "client_id", "web" }, - { "deviceType", "10" }, - { "deviceIdentifier", "test_id" }, - { "deviceName", "firefox" }, - { "twoFactorToken", "TEST"}, - { "twoFactorProvider", "5" }, // RememberMe Provider - { "twoFactorRemember", "0" }, - { "grant_type", "authorization_code" }, - { "code", "test_code" }, - { "code_verifier", challenge }, - { "redirect_uri", "https://localhost:8080/sso-connector.html" } - })); + organizationUser.ResetPasswordKey = "something"; + + await database.SaveChangesAsync(); + }, MemberDecryptionType.TrustedDeviceEncryption); // Assert // If the organization has selected TrustedDeviceEncryption but the user still has their master password // they can decrypt with either option - Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); - using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); @@ -183,7 +116,8 @@ public class IdentityServerSsoTests // "Object": "userDecryptionOptions" // "HasMasterPassword": true, // "TrustedDeviceOption": { - // "HasAdminApproval": true + // "HasAdminApproval": true, + // "HasManageResetPasswordPermission": false // } // } @@ -196,6 +130,126 @@ public class IdentityServerSsoTests [Fact] public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_ReturnsOneOption() + { + using var responseBody = await RunSuccessTestAsync(async factory => + { + await UpdateUserAsync(factory, user => user.MasterPassword = null); + + }, MemberDecryptionType.TrustedDeviceEncryption); + + // Assert + // If the organization has selected TrustedDeviceEncryption but the user still has their master password + // they can decrypt with either option + var root = responseBody.RootElement; + AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); + var userDecryptionOptions = AssertHelper.AssertJsonProperty(root, "UserDecryptionOptions", JsonValueKind.Object); + + // Expected to look like: + // "UserDecryptionOptions": { + // "Object": "userDecryptionOptions" + // "HasMasterPassword": false, + // "TrustedDeviceOption": { + // "HasAdminApproval": true, + // "HasLoginApprovingDevice": false, + // "HasManageResetPasswordPermission": false + // } + // } + + var trustedDeviceOption = AssertHelper.AssertJsonProperty(userDecryptionOptions, "TrustedDeviceOption", JsonValueKind.Object); + AssertHelper.AssertJsonProperty(trustedDeviceOption, "HasAdminApproval", JsonValueKind.False); + AssertHelper.AssertJsonProperty(trustedDeviceOption, "HasManageResetPasswordPermission", JsonValueKind.False); + + // This asserts that device keys are not coming back in the response because this should be a new device. + // if we ever add new properties that come back from here it is fine to change the expected number of properties + // but it should still be asserted in some way that keys are not amongst them. + Assert.Collection(trustedDeviceOption.EnumerateObject(), + p => + { + Assert.Equal("HasAdminApproval", p.Name); + Assert.Equal(JsonValueKind.False, p.Value.ValueKind); + }, + p => + { + Assert.Equal("HasLoginApprovingDevice", p.Name); + Assert.Equal(JsonValueKind.False, p.Value.ValueKind); + }, + p => + { + Assert.Equal("HasManageResetPasswordPermission", p.Name); + Assert.Equal(JsonValueKind.False, p.Value.ValueKind); + }); + } + + /// + /// If a user has a device that is able to accept login with device requests, we should return that state + /// with the user decryption options. + /// + [Fact] + public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_HasLoginApprovingDevice_ReturnsTrue() + { + using var responseBody = await RunSuccessTestAsync(async factory => + { + await UpdateUserAsync(factory, user => user.MasterPassword = null); + var userRepository = factory.Services.GetRequiredService(); + var user = await userRepository.GetByEmailAsync(TestEmail); + + var deviceRepository = factory.Services.GetRequiredService(); + await deviceRepository.CreateAsync(new Device + { + Identifier = "my_other_device", + Type = DeviceType.Android, + Name = "Android", + UserId = user.Id, + }); + }, MemberDecryptionType.TrustedDeviceEncryption); + + // Assert + // If the organization has selected TrustedDeviceEncryption but the user still has their master password + // they can decrypt with either option + var root = responseBody.RootElement; + AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); + var userDecryptionOptions = AssertHelper.AssertJsonProperty(root, "UserDecryptionOptions", JsonValueKind.Object); + + // Expected to look like: + // "UserDecryptionOptions": { + // "Object": "userDecryptionOptions" + // "HasMasterPassword": false, + // "TrustedDeviceOption": { + // "HasAdminApproval": true, + // "HasLoginApprovingDevice": true, + // "HasManageResetPasswordPermission": false + // } + // } + + var trustedDeviceOption = AssertHelper.AssertJsonProperty(userDecryptionOptions, "TrustedDeviceOption", JsonValueKind.Object); + + // This asserts that device keys are not coming back in the response because this should be a new device. + // if we ever add new properties that come back from here it is fine to change the expected number of properties + // but it should still be asserted in some way that keys are not amongst them. + Assert.Collection(trustedDeviceOption.EnumerateObject(), + p => + { + Assert.Equal("HasAdminApproval", p.Name); + Assert.Equal(JsonValueKind.False, p.Value.ValueKind); + }, + p => + { + Assert.Equal("HasLoginApprovingDevice", p.Name); + Assert.Equal(JsonValueKind.True, p.Value.ValueKind); + }, + p => + { + Assert.Equal("HasManageResetPasswordPermission", p.Name); + Assert.Equal(JsonValueKind.False, p.Value.ValueKind); + }); + } + + /// + /// Story: When a user signs in with SSO on a device they have already signed in with we need to return the keys + /// back to them for the current device if it has been trusted before. + /// + [Fact] + public async Task SsoLogin_TrustedDeviceEncryptionAndNoMasterPassword_DeviceAlreadyTrusted_ReturnsOneOption() { // Arrange var challenge = new string('c', 50); @@ -206,13 +260,33 @@ public class IdentityServerSsoTests await UpdateUserAsync(factory, user => user.MasterPassword = null); + var deviceRepository = factory.Services.GetRequiredService(); + + var deviceIdentifier = $"test_id_{Guid.NewGuid()}"; + + var user = await factory.Services.GetRequiredService().GetByEmailAsync(TestEmail); + + const string expectedPrivateKey = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; + const string expectedUserKey = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA=="; + + var device = await deviceRepository.CreateAsync(new Device + { + Type = DeviceType.FirefoxBrowser, + Identifier = deviceIdentifier, + Name = "Thing", + UserId = user.Id, + EncryptedPrivateKey = expectedPrivateKey, + EncryptedPublicKey = "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", + EncryptedUserKey = expectedUserKey, + }); + // Act var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary { { "scope", "api offline_access" }, { "client_id", "web" }, { "deviceType", "10" }, - { "deviceIdentifier", "test_id" }, + { "deviceIdentifier", deviceIdentifier }, { "deviceName", "firefox" }, { "twoFactorToken", "TEST"}, { "twoFactorProvider", "5" }, // RememberMe Provider @@ -237,30 +311,43 @@ public class IdentityServerSsoTests // "Object": "userDecryptionOptions" // "HasMasterPassword": false, // "TrustedDeviceOption": { - // "HasAdminApproval": true + // "HasAdminApproval": true, + // "HasManageResetPasswordPermission": false, + // "EncryptedPrivateKey": "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", + // "EncryptedUserKey": "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==" // } // } var trustedDeviceOption = AssertHelper.AssertJsonProperty(userDecryptionOptions, "TrustedDeviceOption", JsonValueKind.Object); AssertHelper.AssertJsonProperty(trustedDeviceOption, "HasAdminApproval", JsonValueKind.False); + AssertHelper.AssertJsonProperty(trustedDeviceOption, "HasManageResetPasswordPermission", JsonValueKind.False); + + var actualPrivateKey = AssertHelper.AssertJsonProperty(trustedDeviceOption, "EncryptedPrivateKey", JsonValueKind.String).GetString(); + Assert.Equal(expectedPrivateKey, actualPrivateKey); + var actualUserKey = AssertHelper.AssertJsonProperty(trustedDeviceOption, "EncryptedUserKey", JsonValueKind.String).GetString(); + Assert.Equal(expectedUserKey, actualUserKey); } + // we should add a test case for JIT provisioned users. They don't have any orgs which caused + // an error in the UserHasManageResetPasswordPermission set logic. + + /// + /// Story: When a user with TDE and the manage reset password permission signs in with SSO, we should return + /// TrustedDeviceEncryption.HasManageResetPasswordPermission as true + /// [Fact] - public async Task SsoLogin_TrustedDeviceEncryption_FlagTurnedOff_DoesNotReturnOption() + public async Task SsoLogin_TrustedDeviceEncryption_UserHasManageResetPasswordPermission_ReturnsTrue() { // Arrange var challenge = new string('c', 50); - // This creates SsoConfig that HAS enabled trusted device encryption which should have only been - // done with the feature flag turned on but we are testing that even if they have done that, this will turn off - // if returning as an option if the flag has later been turned off. We should be very careful turning the flag - // back off. + // create user permissions with the ManageResetPassword permission + var permissionsWithManageResetPassword = new Permissions() { ManageResetPassword = true }; + var factory = await CreateFactoryAsync(new SsoConfigurationData { MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption, - }, challenge, trustedDeviceEnabled: false); - - await UpdateUserAsync(factory, user => user.MasterPassword = null); + }, challenge, permissions: permissionsWithManageResetPassword); // Act var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary @@ -286,6 +373,33 @@ public class IdentityServerSsoTests using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); + + var userDecryptionOptions = AssertHelper.AssertJsonProperty(root, "UserDecryptionOptions", JsonValueKind.Object); + + var trustedDeviceOption = AssertHelper.AssertJsonProperty(userDecryptionOptions, "TrustedDeviceOption", JsonValueKind.Object); + AssertHelper.AssertJsonProperty(trustedDeviceOption, "HasAdminApproval", JsonValueKind.False); + AssertHelper.AssertJsonProperty(trustedDeviceOption, "HasManageResetPasswordPermission", JsonValueKind.True); + + } + + + [Fact] + public async Task SsoLogin_TrustedDeviceEncryption_FlagTurnedOff_DoesNotReturnOption() + { + // This creates SsoConfig that HAS enabled trusted device encryption which should have only been + // done with the feature flag turned on but we are testing that even if they have done that, this will turn off + // if returning as an option if the flag has later been turned off. We should be very careful turning the flag + // back off. + using var responseBody = await RunSuccessTestAsync(async factory => + { + await UpdateUserAsync(factory, user => user.MasterPassword = null); + }, MemberDecryptionType.TrustedDeviceEncryption, trustedDeviceEnabled: false); + + // Assert + // If the organization has selected TrustedDeviceEncryption but the user still has their master password + // they can decrypt with either option + var root = responseBody.RootElement; + AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); var userDecryptionOptions = AssertHelper.AssertJsonProperty(root, "UserDecryptionOptions", JsonValueKind.Object); // Expected to look like: @@ -301,36 +415,11 @@ public class IdentityServerSsoTests [Fact] public async Task SsoLogin_KeyConnector_ReturnsOptions() { - // Arrange - var challenge = new string('c', 50); - var factory = await CreateFactoryAsync(new SsoConfigurationData + using var responseBody = await RunSuccessTestAsync(async factory => { - MemberDecryptionType = MemberDecryptionType.KeyConnector, - KeyConnectorUrl = "https://key_connector.com" - }, challenge); + await UpdateUserAsync(factory, user => user.MasterPassword = null); + }, MemberDecryptionType.KeyConnector, "https://key_connector.com"); - await UpdateUserAsync(factory, user => user.MasterPassword = null); - - // Act - var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary - { - { "scope", "api offline_access" }, - { "client_id", "web" }, - { "deviceType", "10" }, - { "deviceIdentifier", "test_id" }, - { "deviceName", "firefox" }, - { "twoFactorToken", "TEST"}, - { "twoFactorProvider", "5" }, // RememberMe Provider - { "twoFactorRemember", "0" }, - { "grant_type", "authorization_code" }, - { "code", "test_code" }, - { "code_verifier", challenge }, - { "redirect_uri", "https://localhost:8080/sso-connector.html" } - })); - - // Assert - Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); - using var responseBody = await AssertHelper.AssertResponseTypeIs(context); var root = responseBody.RootElement; AssertHelper.AssertJsonProperty(root, "access_token", JsonValueKind.String); @@ -354,7 +443,51 @@ public class IdentityServerSsoTests Assert.Equal("https://key_connector.com", keyConnectorUrl); } - private static async Task CreateFactoryAsync(SsoConfigurationData ssoConfigurationData, string challenge, bool trustedDeviceEnabled = true) + private static async Task RunSuccessTestAsync(MemberDecryptionType memberDecryptionType) + { + return await RunSuccessTestAsync(factory => Task.CompletedTask, memberDecryptionType); + } + + private static async Task RunSuccessTestAsync(Func configureFactory, + MemberDecryptionType memberDecryptionType, + string? keyConnectorUrl = null, + bool trustedDeviceEnabled = true) + { + var challenge = new string('c', 50); + var factory = await CreateFactoryAsync(new SsoConfigurationData + { + MemberDecryptionType = memberDecryptionType, + KeyConnectorUrl = keyConnectorUrl, + }, challenge, trustedDeviceEnabled); + + await configureFactory(factory); + var context = await factory.Server.PostAsync("/connect/token", new FormUrlEncodedContent(new Dictionary + { + { "scope", "api offline_access" }, + { "client_id", "web" }, + { "deviceType", "10" }, + { "deviceIdentifier", "test_id" }, + { "deviceName", "firefox" }, + { "twoFactorToken", "TEST"}, + { "twoFactorProvider", "5" }, // RememberMe Provider + { "twoFactorRemember", "0" }, + { "grant_type", "authorization_code" }, + { "code", "test_code" }, + { "code_verifier", challenge }, + { "redirect_uri", "https://localhost:8080/sso-connector.html" } + })); + + // Only calls that result in a 200 OK should call this helper + Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); + + return await AssertHelper.AssertResponseTypeIs(context); + } + + private static async Task CreateFactoryAsync( + SsoConfigurationData ssoConfigurationData, + string challenge, + bool trustedDeviceEnabled = true, + Permissions? permissions = null) { var factory = new IdentityApplicationFactory(); @@ -367,7 +500,7 @@ public class IdentityServerSsoTests RedirectUri = "https://localhost:8080/sso-connector.html", RequestedScopes = new[] { "api", "offline_access" }, CodeChallenge = challenge.Sha256(), - CodeChallengeMethod = "plain", // + CodeChallengeMethod = "plain", // Subject = null, // Temporarily set it to null }; @@ -400,12 +533,17 @@ public class IdentityServerSsoTests }); var organizationUserRepository = factory.Services.GetRequiredService(); + + var orgUserPermissions = + (permissions == null) ? null : JsonSerializer.Serialize(permissions, JsonHelpers.CamelCase); + var organizationUser = await organizationUserRepository.CreateAsync(new OrganizationUser { UserId = user.Id, OrganizationId = organization.Id, Status = OrganizationUserStatusType.Confirmed, Type = OrganizationUserType.User, + Permissions = orgUserPermissions }); var ssoConfigRepository = factory.Services.GetRequiredService();