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] [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 e085d0453..3c48367d5 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 d9d12d1f9..10dc2a205 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 0116296d3..4ef0e43fa 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 6b4ed692d..4907efafc 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,