From 1c3afcdffc99ae18601ec1c17f7967cdacf80949 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Thu, 17 Aug 2023 16:03:06 -0400 Subject: [PATCH] Trusted Device Encryption feature (#3151) * [PM-1203] feat: allow verification for all passwordless accounts (#3038) * [PM-1033] Org invite user creation flow 1 (#3028) * [PM-1033] feat: remove user verification from password enrollment * [PM-1033] feat: auto accept invitation when enrolling into password reset * [PM-1033] fix: controller tests * [PM-1033] refactor: `UpdateUserResetPasswordEnrollmentCommand` * [PM-1033] refactor(wip): make `AcceptUserCommand` * Revert "[PM-1033] refactor(wip): make `AcceptUserCommand`" This reverts commit dc1319e7fa70c4844bbc70e0b01089b682ac2843. * Revert "[PM-1033] refactor: `UpdateUserResetPasswordEnrollmentCommand`" This reverts commit 43df689c7f244af4f7ffec1f9768a72081a624c3. * [PM-1033] refactor: move invite accept to controller This avoids creating yet another method that depends on having `IUserService` passed in as a parameter * [PM-1033] fix: add missing changes * [PM-1381] Add Trusted Device Keys to Auth Response (#3066) * Return Keys for Trusted Device - Check whether the current logging in device is trusted - Return their keys on successful login * Formatting * Address PR Feedback * Add Remarks Comment * [PM-1338] `AuthRequest` Event Logs (#3046) * Update AuthRequestController - Only allow AdminApproval Requests to be created from authed endpoint - Add endpoint that has authentication to be able to create admin approval * Add PasswordlessAuthSettings - Add settings for customizing expiration times * Add new EventTypes * Add Logic for AdminApproval Type - Add logic for validating AdminApproval expiration - Add event logging for Approval/Disapproval of AdminApproval - Add logic for creating AdminApproval types * Add Test Helpers - Change BitAutoData to allow you to use string representations of common types. * Add/Update AuthRequestService Tests * Run Formatting * Switch to 7 Days * Add Test Covering ResponseDate Being Set * Address PR Feedback - Create helper for checking if date is expired - Move validation logic into smaller methods * Switch to User Event Type - Make RequestDeviceApproval user type - User types will log for each org user is in * [PM-2998] Move Approving Device Check (#3101) * Move Check for Approving Devices - Exclude currently logging in device - Remove old way of checking - Add tests asserting behavior * Update DeviceType list * Update Naming & Address PR Feedback * Fix Tests * Address PR Feedback * Formatting * Now Fully Update Naming? * Feature/auth/pm 2759/add can reset password to user decryption options (#3113) * PM-2759 - BaseRequestValidator.cs - CreateUserDecryptionOptionsAsync - Add new hasManageResetPasswordPermission for post SSO redirect logic required on client. * PM-2759 - Update IdentityServerSsoTests.cs to all pass based on the addition of HasManageResetPasswordPermission to TrustedDeviceUserDecryptionOption * IdentityServerSsoTests.cs - fix typo in test name: LoggingApproval --> LoginApproval * PM1259 - Add test case for verifying that TrustedDeviceOption.hasManageResetPasswordPermission is set properly based on user permission * dotnet format run * Feature/auth/pm 2759/add can reset password to user decryption options fix jit users (#3120) * PM-2759 - IdentityServer - CreateUserDecryptionOptionsAsync - hasManageResetPasswordPermission set logic was broken for JIT provisioned users as I assumed we would always have a list of at least 1 org during the SSO process. Added TODO for future test addition but getting this out there now as QA is blocked by being unable to create JIT provisioned users. * dotnet format * Tiny tweak * [PM-1339] Allow Rotating Device Keys (#3096) * Allow Rotation of Trusted Device Keys - Add endpoint for getting keys relating to rotation - Add endpoint for rotating your current device - In the same endpoint allow a list of other devices to rotate * Formatting * Use Extension Method * Add Tests from PR Co-authored-by: Jared Snider --------- Co-authored-by: Jared Snider * Check the user directly if they have the ResetPasswordKey (#3153) * PM-3327 - UpdateKeyAsync must exempt the currently calling device from the logout notification in order to prevent prematurely logging the user out before the client side key rotation process can complete. The calling device will log itself out once it is done. (#3170) * Allow OTP Requests When Users Are On TDE (#3184) * [PM-3356][PM-3292] Allow OTP For All (#3188) * Allow OTP For All - On a trusted device isn't a good check because a user might be using a trusted device locally but not trusted it long term - The logic wasn't working for KC users anyways * Remove Old Comment * [AC-1601] Added RequireSso policy as a dependency of TDE (#3209) * Added RequireSso policy as a dependency of TDE. * Added test for RequireSso for TDE. * Added save. * Fixed policy name. --------- Co-authored-by: Andreas Coroiu Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> Co-authored-by: Vincent Salucci Co-authored-by: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Co-authored-by: Jared Snider --- .../Controllers/AuthRequestsController.cs | 13 + .../Request/UpdateDevicesTrustRequestModel.cs | 14 + src/Api/Controllers/AccountsController.cs | 8 - src/Api/Controllers/DevicesController.cs | 71 +++- .../OrganizationUsersController.cs | 23 +- .../OrganizationUserRequestModels.cs | 3 +- .../Models/Response/DeviceResponseModel.cs | 11 +- .../Request/DeviceKeysUpdateRequestModel.cs | 21 + .../Response/ProtectedDeviceResponseModel.cs | 30 ++ .../Api/Response/UserDecryptionOptions.cs | 14 +- .../Implementations/AuthRequestService.cs | 151 ++++++- .../Implementations/SsoConfigService.cs | 9 +- src/Core/Auth/Utilities/DeviceExtensions.cs | 23 ++ src/Core/Entities/User.cs | 5 + src/Core/Enums/EventType.cs | 3 + src/Core/Services/IDeviceService.cs | 7 +- src/Core/Services/IOrganizationService.cs | 1 + .../Services/Implementations/DeviceService.cs | 62 ++- .../Implementations/OrganizationService.cs | 18 + .../Services/Implementations/PolicyService.cs | 1 + .../Services/Implementations/UserService.cs | 31 +- src/Core/Utilities/DeviceTypes.cs | 21 + .../IdentityServer/BaseRequestValidator.cs | 59 ++- .../Utilities/LoginApprovingDeviceTypes.cs | 19 + .../OrganizationUsersControllerTests.cs | 28 ++ .../Attributes/BitAutoDataAttribute.cs | 14 +- test/Common/Helpers/AssertHelper.cs | 1 + .../Helpers/BitAutoDataAttributeHelpers.cs | 85 +++- .../Auth/Services/AuthRequestServiceTests.cs | 369 +++++++++++++++-- test/Core.Test/Services/DeviceServiceTests.cs | 231 ++++++++++- test/Core.Test/Services/PolicyServiceTests.cs | 39 +- test/Core.Test/Services/UserServiceTests.cs | 155 ++++++- .../Endpoints/IdentityServerSsoTests.cs | 380 ++++++++++++------ 33 files changed, 1666 insertions(+), 254 deletions(-) create mode 100644 src/Api/Auth/Models/Request/UpdateDevicesTrustRequestModel.cs create mode 100644 src/Core/Auth/Models/Api/Request/DeviceKeysUpdateRequestModel.cs create mode 100644 src/Core/Auth/Models/Api/Response/ProtectedDeviceResponseModel.cs create mode 100644 src/Core/Auth/Utilities/DeviceExtensions.cs create mode 100644 src/Core/Utilities/DeviceTypes.cs create mode 100644 src/Identity/Utilities/LoginApprovingDeviceTypes.cs 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();