From 798fca51e2f9250200f120ea3fde41d02981c886 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 16:53:30 -0400 Subject: [PATCH 01/10] Bumped version to 2023.8.3 (#3249) Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com> --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index 3a60d958ca..8aea71ad7b 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -2,7 +2,7 @@ net6.0 - 2023.8.2 + 2023.8.3 Bit.$(MSBuildProjectName) true enable From 952e77d3d74d0cb93538e88676fd5e3307ebdce3 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 14:57:29 -0600 Subject: [PATCH 02/10] Update kenchan0130/simplesamlphp Docker tag to v1.19.8 (#3060) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- dev/docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/docker-compose.yml b/dev/docker-compose.yml index 2e0e4113d7..4d27c0472a 100644 --- a/dev/docker-compose.yml +++ b/dev/docker-compose.yml @@ -68,7 +68,7 @@ services: - mysql idp: - image: kenchan0130/simplesamlphp:1.19.3 + image: kenchan0130/simplesamlphp:1.19.8 container_name: idp ports: - "8090:8080" From 8c75326439da7126b80b3d0846423f1c00f27b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Thu, 7 Sep 2023 10:42:04 +0100 Subject: [PATCH 03/10] [AC-1612] Updated CurrentContext.ViewAssignedCollections to check if the user has CreateNewCollections permission (#3233) * [AC-1612] Updated CurrentContext.ViewAssignedCollections to check if the user has CreateNewCollections permission * [AC-1612] Added comment to clarify the requirement of the added check in ViewAssignedCollections --- src/Core/Context/CurrentContext.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Core/Context/CurrentContext.cs b/src/Core/Context/CurrentContext.cs index 4877a9a8f2..4751beb797 100644 --- a/src/Core/Context/CurrentContext.cs +++ b/src/Core/Context/CurrentContext.cs @@ -358,7 +358,9 @@ public class CurrentContext : ICurrentContext public async Task ViewAssignedCollections(Guid orgId) { - return await EditAssignedCollections(orgId) || await DeleteAssignedCollections(orgId); + return await CreateNewCollections(orgId) // Required to display the existing collections under which the new collection can be nested + || await EditAssignedCollections(orgId) + || await DeleteAssignedCollections(orgId); } public async Task ManageGroups(Guid orgId) From 721c18e94a76ed3eccaaad9c42738e35c915a486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Thu, 7 Sep 2023 14:36:54 +0100 Subject: [PATCH 04/10] [AC-244] Consider a user's email as verified when they accept an organization invitation via the email link (#3199) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [AC-244] Saving User.EmailVerified = true when accepting organization invite * [AC-244] Added unit tests * [AC-244] Added the parameter 'verifyEmail' to OrganizationService.AcceptUserAsync * [AC-244] Refactored unit tests * [AC-244] Fixed failing unit tests * [AC-244] Marking email as verified only in the endpoint for accepting an invite with a token * Update src/Core/Services/IOrganizationService.cs Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> * [AC-244] Marking email as verified only if it was not * [AC-244] Updated unit test to have the user's email unverified at the start * [AC-244] dotnet format --------- Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Vincent Salucci --- src/Core/Services/IOrganizationService.cs | 6 +++ .../Implementations/OrganizationService.cs | 10 ++++- .../AutoFixture/OrganizationFixtures.cs | 29 +++++++++++++++ .../Services/OrganizationServiceTests.cs | 37 +++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index e085d04531..3c48367d53 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -39,6 +39,12 @@ public interface IOrganizationService OrganizationUserType type, bool accessAll, string externalId, IEnumerable collections, IEnumerable groups); Task>> ResendInvitesAsync(Guid organizationId, Guid? invitingUserId, IEnumerable organizationUsersId); Task ResendInviteAsync(Guid organizationId, Guid? invitingUserId, Guid organizationUserId, bool initOrganization = false); + /// + /// Moves an OrganizationUser into the Accepted status and marks their email as verified. + /// This method is used where the user has clicked the invitation link sent by email. + /// + /// The token embedded in the email invitation link + /// The accepted OrganizationUser. 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); diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index d9d12d1f9f..10dc2a2056 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1093,7 +1093,15 @@ public class OrganizationService : IOrganizationService throw new BadRequestException("User email does not match invite."); } - return await AcceptUserAsync(orgUser, user, userService); + var organizationUser = await AcceptUserAsync(orgUser, user, userService); + + if (user.EmailVerified == false) + { + user.EmailVerified = true; + await _userRepository.ReplaceAsync(user); + } + + return organizationUser; } public async Task AcceptUserAsync(string orgIdentifier, User user, IUserService userService) diff --git a/test/Core.Test/AutoFixture/OrganizationFixtures.cs b/test/Core.Test/AutoFixture/OrganizationFixtures.cs index 0116296d33..4ef0e43fa7 100644 --- a/test/Core.Test/AutoFixture/OrganizationFixtures.cs +++ b/test/Core.Test/AutoFixture/OrganizationFixtures.cs @@ -10,6 +10,7 @@ using Bit.Core.Models.Data; using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.DataProtection; namespace Bit.Core.Test.AutoFixture.OrganizationFixtures; @@ -187,3 +188,31 @@ internal class SecretsManagerOrganizationCustomizeAttribute : BitCustomizeAttrib public override ICustomization GetCustomization() => new SecretsManagerOrganizationCustomization(); } + +internal class EphemeralDataProtectionCustomization : ICustomization +{ + public void Customize(IFixture fixture) + { + fixture.Customizations.Add(new EphemeralDataProtectionProviderBuilder()); + } + + private class EphemeralDataProtectionProviderBuilder : ISpecimenBuilder + { + public object Create(object request, ISpecimenContext context) + { + var type = request as Type; + if (type == null || type != typeof(IDataProtectionProvider)) + { + return new NoSpecimen(); + } + + return new EphemeralDataProtectionProvider(); + } + } +} + +internal class EphemeralDataProtectionAutoDataAttribute : CustomAutoDataAttribute +{ + public EphemeralDataProtectionAutoDataAttribute() : base(new SutProviderCustomization(), new EphemeralDataProtectionCustomization()) + { } +} diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 6b4ed692d7..4907efafcb 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -28,6 +28,7 @@ using Bit.Core.Tools.Services; using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.DataProtection; using NSubstitute; using NSubstitute.ExceptionExtensions; using Xunit; @@ -1835,6 +1836,42 @@ public class OrganizationServiceTests sutProvider.Sut.ValidateSecretsManagerPlan(plan, signup); } + [Theory] + [EphemeralDataProtectionAutoData] + public async Task AcceptUserAsync_Success([OrganizationUser(OrganizationUserStatusType.Invited)] OrganizationUser organizationUser, + User user, SutProvider sutProvider) + { + var token = SetupAcceptUserAsyncTest(sutProvider, user, organizationUser); + var userService = Substitute.For(); + + await sutProvider.Sut.AcceptUserAsync(organizationUser.Id, user, token, userService); + + await sutProvider.GetDependency().Received(1).ReplaceAsync( + Arg.Is(ou => ou.Id == organizationUser.Id && ou.Status == OrganizationUserStatusType.Accepted)); + await sutProvider.GetDependency().Received(1).ReplaceAsync( + Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); + } + + private string SetupAcceptUserAsyncTest(SutProvider sutProvider, User user, + OrganizationUser organizationUser) + { + user.Email = organizationUser.Email; + user.EmailVerified = false; + + var dataProtector = sutProvider.GetDependency() + .CreateProtector("OrganizationServiceDataProtector"); + // Token matching the format used in OrganizationService.InviteUserAsync + var token = dataProtector.Protect( + $"OrganizationUserInvite {organizationUser.Id} {organizationUser.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); + + sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); + + sutProvider.GetDependency().GetByIdAsync(organizationUser.Id) + .Returns(organizationUser); + + return token; + } + [Theory] [OrganizationInviteCustomize( InviteeUserType = OrganizationUserType.Owner, From b8b2efa7672f29005c9158582c3d81c438748d63 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 7 Sep 2023 11:33:46 -0500 Subject: [PATCH 05/10] Remove indexing from data migration (#3247) --- .../2023-08-10_00_ClientSecretHashDataMigration.sql | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/util/Migrator/DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql b/util/Migrator/DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql index 66869d48b7..0462d64622 100644 --- a/util/Migrator/DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql +++ b/util/Migrator/DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql @@ -6,14 +6,6 @@ The final migration is in util/Migrator/DbScripts/2023-08-10_01_RemoveClientSecr IF COL_LENGTH('[dbo].[ApiKey]', 'ClientSecretHash') IS NOT NULL AND COL_LENGTH('[dbo].[ApiKey]', 'ClientSecret') IS NOT NULL BEGIN - -- Add index - IF NOT EXISTS(SELECT name FROM sys.indexes WHERE name = 'IX_ApiKey_ClientSecretHash') - BEGIN - CREATE NONCLUSTERED INDEX [IX_ApiKey_ClientSecretHash] - ON [dbo].[ApiKey]([ClientSecretHash] ASC) - WITH (ONLINE = ON) - END - -- Data Migration DECLARE @BatchSize INT = 10000 WHILE @BatchSize > 0 @@ -34,9 +26,5 @@ BEGIN COMMIT TRANSACTION Migrate_ClientSecretHash END - -- Drop index - DROP INDEX IF EXISTS [IX_ApiKey_ClientSecretHash] - ON [dbo].[ApiKey]; - END GO From 2aaef3cf64119440be897e937c0f68a40f27328a Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Thu, 7 Sep 2023 12:54:56 -0400 Subject: [PATCH 06/10] [PM-289] Add taxIdType for more countries in TaxInfo (#3186) * Add taxIdType for more countries in TaxInfo * Removed Afghanistan tax ID * Normalize country code in tax info model --- src/Core/Models/Business/TaxInfo.cs | 74 ++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/src/Core/Models/Business/TaxInfo.cs b/src/Core/Models/Business/TaxInfo.cs index e763b72235..4424576ec9 100644 --- a/src/Core/Models/Business/TaxInfo.cs +++ b/src/Core/Models/Business/TaxInfo.cs @@ -35,14 +35,23 @@ public class TaxInfo return _taxIdType; } - switch (BillingAddressCountry) + switch (BillingAddressCountry.ToUpper()) { + case "AD": + _taxIdType = "ad_nrt"; + break; case "AE": _taxIdType = "ae_trn"; break; + case "AR": + _taxIdType = "ar_cuit"; + break; case "AU": _taxIdType = "au_abn"; break; + case "BO": + _taxIdType = "bo_tin"; + break; case "BR": _taxIdType = "br_cnpj"; break; @@ -55,9 +64,45 @@ public class TaxInfo } _taxIdType = "ca_bn"; break; + case "CH": + _taxIdType = "ch_vat"; + break; case "CL": _taxIdType = "cl_tin"; break; + case "CN": + _taxIdType = "cn_tin"; + break; + case "CO": + _taxIdType = "co_nit"; + break; + case "CR": + _taxIdType = "cr_tin"; + break; + case "DO": + _taxIdType = "do_rcn"; + break; + case "EC": + _taxIdType = "ec_ruc"; + break; + case "EG": + _taxIdType = "eg_tin"; + break; + case "GE": + _taxIdType = "ge_vat"; + break; + case "ID": + _taxIdType = "id_npwp"; + break; + case "IL": + _taxIdType = "il_vat"; + break; + case "IS": + _taxIdType = "is_vat"; + break; + case "KE": + _taxIdType = "ke_pin"; + break; case "AT": case "BE": case "BG": @@ -115,6 +160,15 @@ public class TaxInfo case "NZ": _taxIdType = "nz_gst"; break; + case "PE": + _taxIdType = "pe_ruc"; + break; + case "PH": + _taxIdType = "ph_tin"; + break; + case "RS": + _taxIdType = "rs_pib"; + break; case "RU": _taxIdType = "ru_inn"; break; @@ -124,15 +178,33 @@ public class TaxInfo case "SG": _taxIdType = "sg_gst"; break; + case "SV": + _taxIdType = "sv_nit"; + break; case "TH": _taxIdType = "th_vat"; break; + case "TR": + _taxIdType = "tr_tin"; + break; case "TW": _taxIdType = "tw_vat"; break; + case "UA": + _taxIdType = "ua_vat"; + break; case "US": _taxIdType = "us_ein"; break; + case "UY": + _taxIdType = "uy_ruc"; + break; + case "VE": + _taxIdType = "ve_rif"; + break; + case "VN": + _taxIdType = "vn_tin"; + break; case "ZA": _taxIdType = "za_vat"; break; From 4b482f0a34b3811131fb4ab571267e11bd7b4de8 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Thu, 7 Sep 2023 17:51:35 -0500 Subject: [PATCH 07/10] [SM-918] Enforce project maximums on import (#3253) * Refactor MaxProjectsQuery for multiple adds * Update unit tests * Add max project enforcement to imports --- .../Queries/Projects/MaxProjectsQuery.cs | 4 +- .../Queries/Projects/MaxProjectsQueryTests.cs | 44 +++++++++++++------ .../Controllers/ProjectsController.cs | 4 +- .../SecretsManagerPortingController.cs | 17 ++++++- .../Projects/Interfaces/IMaxProjectsQuery.cs | 2 +- .../Controllers/ProjectsControllerTests.cs | 2 +- 6 files changed, 53 insertions(+), 20 deletions(-) diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs index 7cbb37f18c..fc30c2c6e8 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs @@ -20,7 +20,7 @@ public class MaxProjectsQuery : IMaxProjectsQuery _projectRepository = projectRepository; } - public async Task<(short? max, bool? atMax)> GetByOrgIdAsync(Guid organizationId) + public async Task<(short? max, bool? overMax)> GetByOrgIdAsync(Guid organizationId, int projectsToAdd) { var org = await _organizationRepository.GetByIdAsync(organizationId); if (org == null) @@ -37,7 +37,7 @@ public class MaxProjectsQuery : IMaxProjectsQuery if (plan.Type == PlanType.Free) { var projects = await _projectRepository.GetProjectCountByOrganizationIdAsync(organizationId); - return projects >= plan.MaxProjects ? (plan.MaxProjects, true) : (plan.MaxProjects, false); + return projects + projectsToAdd > plan.MaxProjects ? (plan.MaxProjects, true) : (plan.MaxProjects, false); } return (null, null); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs index 6706e01657..79ffb421e1 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs @@ -22,7 +22,7 @@ public class MaxProjectsQueryTests { sutProvider.GetDependency().GetByIdAsync(default).ReturnsNull(); - await Assert.ThrowsAsync(async () => await sutProvider.Sut.GetByOrgIdAsync(organizationId)); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.GetByOrgIdAsync(organizationId, 1)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .GetProjectCountByOrganizationIdAsync(organizationId); @@ -43,7 +43,7 @@ public class MaxProjectsQueryTests sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); await Assert.ThrowsAsync( - async () => await sutProvider.Sut.GetByOrgIdAsync(organization.Id)); + async () => await sutProvider.Sut.GetByOrgIdAsync(organization.Id, 1)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .GetProjectCountByOrganizationIdAsync(organization.Id); @@ -60,7 +60,7 @@ public class MaxProjectsQueryTests organization.PlanType = planType; sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - var (limit, overLimit) = await sutProvider.Sut.GetByOrgIdAsync(organization.Id); + var (limit, overLimit) = await sutProvider.Sut.GetByOrgIdAsync(organization.Id, 1); Assert.Null(limit); Assert.Null(overLimit); @@ -70,13 +70,31 @@ public class MaxProjectsQueryTests } [Theory] - [BitAutoData(PlanType.Free, 0, false)] - [BitAutoData(PlanType.Free, 1, false)] - [BitAutoData(PlanType.Free, 2, false)] - [BitAutoData(PlanType.Free, 3, true)] - [BitAutoData(PlanType.Free, 4, true)] - [BitAutoData(PlanType.Free, 40, true)] - public async Task GetByOrgIdAsync_SmFreePlan_Success(PlanType planType, int projects, bool shouldBeAtMax, + [BitAutoData(PlanType.Free, 0, 1, false)] + [BitAutoData(PlanType.Free, 1, 1, false)] + [BitAutoData(PlanType.Free, 2, 1, false)] + [BitAutoData(PlanType.Free, 3, 1, true)] + [BitAutoData(PlanType.Free, 4, 1, true)] + [BitAutoData(PlanType.Free, 40, 1, true)] + [BitAutoData(PlanType.Free, 0, 2, false)] + [BitAutoData(PlanType.Free, 1, 2, false)] + [BitAutoData(PlanType.Free, 2, 2, true)] + [BitAutoData(PlanType.Free, 3, 2, true)] + [BitAutoData(PlanType.Free, 4, 2, true)] + [BitAutoData(PlanType.Free, 40, 2, true)] + [BitAutoData(PlanType.Free, 0, 3, false)] + [BitAutoData(PlanType.Free, 1, 3, true)] + [BitAutoData(PlanType.Free, 2, 3, true)] + [BitAutoData(PlanType.Free, 3, 3, true)] + [BitAutoData(PlanType.Free, 4, 3, true)] + [BitAutoData(PlanType.Free, 40, 3, true)] + [BitAutoData(PlanType.Free, 0, 4, true)] + [BitAutoData(PlanType.Free, 1, 4, true)] + [BitAutoData(PlanType.Free, 2, 4, true)] + [BitAutoData(PlanType.Free, 3, 4, true)] + [BitAutoData(PlanType.Free, 4, 4, true)] + [BitAutoData(PlanType.Free, 40, 4, true)] + public async Task GetByOrgIdAsync_SmFreePlan__Success(PlanType planType, int projects, int projectsToAdd, bool expectedOverMax, SutProvider sutProvider, Organization organization) { organization.PlanType = planType; @@ -84,12 +102,12 @@ public class MaxProjectsQueryTests sutProvider.GetDependency().GetProjectCountByOrganizationIdAsync(organization.Id) .Returns(projects); - var (max, atMax) = await sutProvider.Sut.GetByOrgIdAsync(organization.Id); + var (max, overMax) = await sutProvider.Sut.GetByOrgIdAsync(organization.Id, projectsToAdd); Assert.NotNull(max); - Assert.NotNull(atMax); + Assert.NotNull(overMax); Assert.Equal(3, max.Value); - Assert.Equal(shouldBeAtMax, atMax); + Assert.Equal(expectedOverMax, overMax); await sutProvider.GetDependency().Received(1) .GetProjectCountByOrganizationIdAsync(organization.Id); diff --git a/src/Api/SecretsManager/Controllers/ProjectsController.cs b/src/Api/SecretsManager/Controllers/ProjectsController.cs index 4f3815ca78..e59918593a 100644 --- a/src/Api/SecretsManager/Controllers/ProjectsController.cs +++ b/src/Api/SecretsManager/Controllers/ProjectsController.cs @@ -79,8 +79,8 @@ public class ProjectsController : Controller throw new NotFoundException(); } - var (max, atMax) = await _maxProjectsQuery.GetByOrgIdAsync(organizationId); - if (atMax != null && atMax.Value) + var (max, overMax) = await _maxProjectsQuery.GetByOrgIdAsync(organizationId, 1); + if (overMax != null && overMax.Value) { throw new BadRequestException($"You have reached the maximum number of projects ({max}) for this plan."); } diff --git a/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs b/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs index ac2e3e5c2a..3d26c8f702 100644 --- a/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs +++ b/src/Api/SecretsManager/Controllers/SecretsManagerPortingController.cs @@ -4,6 +4,7 @@ using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Commands.Porting.Interfaces; +using Bit.Core.SecretsManager.Queries.Projects.Interfaces; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; using Bit.Core.Utilities; @@ -19,14 +20,18 @@ public class SecretsManagerPortingController : Controller private readonly ISecretRepository _secretRepository; private readonly IProjectRepository _projectRepository; private readonly IUserService _userService; + private readonly IMaxProjectsQuery _maxProjectsQuery; private readonly IImportCommand _importCommand; private readonly ICurrentContext _currentContext; - public SecretsManagerPortingController(ISecretRepository secretRepository, IProjectRepository projectRepository, IUserService userService, IImportCommand importCommand, ICurrentContext currentContext) + public SecretsManagerPortingController(ISecretRepository secretRepository, IProjectRepository projectRepository, + IUserService userService, IMaxProjectsQuery maxProjectsQuery, IImportCommand importCommand, + ICurrentContext currentContext) { _secretRepository = secretRepository; _projectRepository = projectRepository; _userService = userService; + _maxProjectsQuery = maxProjectsQuery; _importCommand = importCommand; _currentContext = currentContext; } @@ -69,6 +74,16 @@ public class SecretsManagerPortingController : Controller throw new BadRequestException("A secret can only be in one project at a time."); } + var projectsToAdd = importRequest.Projects?.Count(); + if (projectsToAdd is > 0) + { + var (max, overMax) = await _maxProjectsQuery.GetByOrgIdAsync(organizationId, projectsToAdd.Value); + if (overMax != null && overMax.Value) + { + throw new BadRequestException($"The maximum number of projects for this plan is ({max})."); + } + } + await _importCommand.ImportAsync(organizationId, importRequest.ToSMImport()); } } diff --git a/src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs b/src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs index e00f5ed674..6bb9a40c75 100644 --- a/src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs +++ b/src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs @@ -2,5 +2,5 @@ public interface IMaxProjectsQuery { - Task<(short? max, bool? atMax)> GetByOrgIdAsync(Guid organizationId); + Task<(short? max, bool? overMax)> GetByOrgIdAsync(Guid organizationId, int projectsToAdd); } diff --git a/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs index 32239159a6..27aa7ea713 100644 --- a/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs @@ -132,7 +132,7 @@ public class ProjectsControllerTests .AuthorizeAsync(Arg.Any(), data.ToProject(orgId), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); - sutProvider.GetDependency().GetByOrgIdAsync(orgId).Returns(((short)3, true)); + sutProvider.GetDependency().GetByOrgIdAsync(orgId, 1).Returns(((short)3, true)); await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(orgId, data)); From 917c657439ff5a58dbfc4c5ae1e683b20e952a93 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Sat, 9 Sep 2023 14:35:08 -0700 Subject: [PATCH 08/10] PM-2128 Enforce one time use of TOTP (#3152) * enforcing one time MFA token use * Updated cache TTL * renamed the cache * removed IP limit, added comment, updated cache Key * fixed build errors --- .../IdentityServer/BaseRequestValidator.cs | 24 ++++++++++++++++++- .../CustomTokenRequestValidator.cs | 7 ++++-- .../ResourceOwnerPasswordValidator.cs | 6 +++-- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/Identity/IdentityServer/BaseRequestValidator.cs b/src/Identity/IdentityServer/BaseRequestValidator.cs index e0cafbfa13..3e35cb5335 100644 --- a/src/Identity/IdentityServer/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/BaseRequestValidator.cs @@ -26,6 +26,7 @@ using Bit.Core.Utilities; using Bit.Identity.Utilities; using IdentityServer4.Validation; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Caching.Distributed; namespace Bit.Identity.IdentityServer; @@ -45,6 +46,8 @@ public abstract class BaseRequestValidator where T : class private readonly GlobalSettings _globalSettings; private readonly IUserRepository _userRepository; private readonly IDataProtectorTokenFactory _tokenDataFactory; + private readonly IDistributedCache _distributedCache; + private readonly DistributedCacheEntryOptions _cacheEntryOptions; protected ICurrentContext CurrentContext { get; } protected IPolicyService PolicyService { get; } @@ -69,7 +72,8 @@ public abstract class BaseRequestValidator where T : class IPolicyService policyService, IDataProtectorTokenFactory tokenDataFactory, IFeatureService featureService, - ISsoConfigRepository ssoConfigRepository) + ISsoConfigRepository ssoConfigRepository, + IDistributedCache distributedCache) { _userManager = userManager; _deviceRepository = deviceRepository; @@ -89,6 +93,14 @@ public abstract class BaseRequestValidator where T : class _tokenDataFactory = tokenDataFactory; FeatureService = featureService; SsoConfigRepository = ssoConfigRepository; + _distributedCache = distributedCache; + _cacheEntryOptions = new DistributedCacheEntryOptions + { + // This sets the time an item is cached to 15 minutes. This value is hard coded + // to 15 because to it covers all time-out windows for both Authenticators and + // Email TOTP. + AbsoluteExpirationRelativeToNow = new TimeSpan(0, 15, 0) + }; } protected async Task ValidateAsync(T context, ValidatedTokenRequest request, @@ -135,6 +147,15 @@ public abstract class BaseRequestValidator where T : class var verified = await VerifyTwoFactor(user, twoFactorOrganization, twoFactorProviderType, twoFactorToken); + var cacheKey = "TOTP_" + user.Email; + + var isOtpCached = Core.Utilities.DistributedCacheExtensions.TryGetValue(_distributedCache, cacheKey, out string _); + if (isOtpCached) + { + await BuildErrorResultAsync("Two-step token is invalid. Try again.", true, context, user); + return; + } + if ((!verified || isBot) && twoFactorProviderType != TwoFactorProviderType.Remember) { await UpdateFailedAuthDetailsAsync(user, true, !validatorContext.KnownDevice); @@ -148,6 +169,7 @@ public abstract class BaseRequestValidator where T : class await BuildTwoFactorResultAsync(user, twoFactorOrganization, context); return; } + await Core.Utilities.DistributedCacheExtensions.SetAsync(_distributedCache, cacheKey, twoFactorToken, _cacheEntryOptions); } else { diff --git a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/CustomTokenRequestValidator.cs index e6e39bade1..a15a738372 100644 --- a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/CustomTokenRequestValidator.cs @@ -14,6 +14,7 @@ using IdentityModel; using IdentityServer4.Extensions; using IdentityServer4.Validation; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Caching.Distributed; #nullable enable @@ -42,11 +43,13 @@ public class CustomTokenRequestValidator : BaseRequestValidator tokenDataFactory, - IFeatureService featureService) + IFeatureService featureService, + IDistributedCache distributedCache) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, - userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository) + userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, + distributedCache) { _userManager = userManager; } diff --git a/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs index df17a474a3..8b8b0be52d 100644 --- a/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs @@ -13,6 +13,7 @@ using Bit.Core.Utilities; using IdentityServer4.Models; using IdentityServer4.Validation; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Caching.Distributed; namespace Bit.Identity.IdentityServer; @@ -44,11 +45,12 @@ public class ResourceOwnerPasswordValidator : BaseRequestValidator tokenDataFactory, IFeatureService featureService, - ISsoConfigRepository ssoConfigRepository) + ISsoConfigRepository ssoConfigRepository, + IDistributedCache distributedCache) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, userRepository, policyService, - tokenDataFactory, featureService, ssoConfigRepository) + tokenDataFactory, featureService, ssoConfigRepository, distributedCache) { _userManager = userManager; _userService = userService; From f909563211b405d79ee6ea6a11792e52bc6951c0 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Mon, 11 Sep 2023 10:23:32 -0400 Subject: [PATCH 09/10] [PM-3487] prevent account enumeration on auth request endpoint (#3239) --- .../Implementations/AuthRequestService.cs | 29 +++++++++++-------- .../Auth/Services/AuthRequestServiceTests.cs | 14 +++++---- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/Core/Auth/Services/Implementations/AuthRequestService.cs b/src/Core/Auth/Services/Implementations/AuthRequestService.cs index b75a5c1303..e59177d9fd 100644 --- a/src/Core/Auth/Services/Implementations/AuthRequestService.cs +++ b/src/Core/Auth/Services/Implementations/AuthRequestService.cs @@ -82,34 +82,39 @@ public class AuthRequestService : IAuthRequestService /// public async Task CreateAuthRequestAsync(AuthRequestCreateRequestModel model) { - var user = await _userRepository.GetByEmailAsync(model.Email); - if (user == null) - { - throw new NotFoundException(); - } - if (!_currentContext.DeviceType.HasValue) { throw new BadRequestException("Device type not provided."); } - if (_globalSettings.PasswordlessAuth.KnownDevicesOnly) + var userNotFound = false; + var user = await _userRepository.GetByEmailAsync(model.Email); + if (user == null) + { + userNotFound = true; + } + else if (_globalSettings.PasswordlessAuth.KnownDevicesOnly) { var devices = await _deviceRepository.GetManyByUserIdAsync(user.Id); if (devices == null || !devices.Any(d => d.Identifier == model.DeviceIdentifier)) { - throw new BadRequestException( - "Login with device is only available on devices that have been previously logged in."); + userNotFound = true; } } + // Anonymous endpoints must not leak that a user exists or not + if (userNotFound) + { + throw new BadRequestException("User or known device not found."); + } + // 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 + // 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) @@ -173,7 +178,7 @@ public class AuthRequestService : IAuthRequestService switch (authRequest.Type) { case AuthRequestType.AdminApproval: - // AdminApproval has a different expiration time, by default is 7 days compared to + // 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)) { @@ -213,7 +218,7 @@ public class AuthRequestService : IAuthRequestService await _authRequestRepository.ReplaceAsync(authRequest); - // We only want to send an approval notification if the request is approved (or null), + // We only want to send an approval notification if the request is approved (or null), // to not leak that it was denied to the originating client if it was originated by a malicious actor. if (authRequest.Approved ?? true) { diff --git a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs index 3cac774052..cd7f85ae8b 100644 --- a/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs +++ b/test/Core.Test/Auth/Services/AuthRequestServiceTests.cs @@ -142,15 +142,19 @@ public class AuthRequestServiceTests } [Theory, BitAutoData] - public async Task CreateAuthRequestAsync_NoUser_ThrowsNotFound( + public async Task CreateAuthRequestAsync_NoUser_ThrowsBadRequest( SutProvider sutProvider, AuthRequestCreateRequestModel createModel) { + sutProvider.GetDependency() + .DeviceType + .Returns(DeviceType.Android); + sutProvider.GetDependency() .GetByEmailAsync(createModel.Email) .Returns((User?)null); - await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAuthRequestAsync(createModel)); + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAuthRequestAsync(createModel)); } [Theory, BitAutoData] @@ -253,7 +257,7 @@ public class AuthRequestServiceTests /// /// Story: If a user happens to exist to more than one organization, we will send the device approval request to - /// each of them. + /// each of them. /// [Theory, BitAutoData] public async Task CreateAuthRequestAsync_AdminApproval_CreatesForEachOrganization( @@ -627,8 +631,8 @@ public class AuthRequestServiceTests } /// - /// 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 + /// 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] From 0be766c98af1b8b127d85a9f6b4a4dd899bb9c74 Mon Sep 17 00:00:00 2001 From: Joseph Flinn <58369717+joseph-flinn@users.noreply.github.com> Date: Tue, 12 Sep 2023 07:34:50 -0700 Subject: [PATCH 10/10] Manually move data migration scripts (#3264) --- .../2023-08-03_00_PopulateResellerNames.sql | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename util/Migrator/{DbScripts_data_migration => DbScripts}/2023-08-03_00_PopulateResellerNames.sql (100%) diff --git a/util/Migrator/DbScripts_data_migration/2023-08-03_00_PopulateResellerNames.sql b/util/Migrator/DbScripts/2023-08-03_00_PopulateResellerNames.sql similarity index 100% rename from util/Migrator/DbScripts_data_migration/2023-08-03_00_PopulateResellerNames.sql rename to util/Migrator/DbScripts/2023-08-03_00_PopulateResellerNames.sql