From 9121d0e2a693f6441a5fb701ee144f5d7a0c4c4d Mon Sep 17 00:00:00 2001 From: Francis Pion Date: Tue, 30 Apr 2024 12:14:34 -0400 Subject: [PATCH 1/5] Change profile phone. --- .../Accounts/Commands/ChangePhoneCommand.cs | 6 ++ .../Commands/ChangePhoneCommandHandler.cs | 83 +++++++++++++++++++ .../Accounts/Commands/SignInCommandHandler.cs | 2 +- .../Accounts/IMessageService.cs | 1 + .../Accounts/IOneTimePasswordService.cs | 1 + .../Accounts/IUserService.cs | 1 + .../Accounts/OneTimePasswordExtensions.cs | 60 ++++++++++---- .../Accounts/UserExtensions.cs | 5 ++ .../Logitar.Master.Application/Activity.cs | 14 ++++ .../Accounts/ChangePhonePayload.cs | 28 +++++++ .../Accounts/ChangePhoneResult.cs | 21 +++++ .../IdentityServices/MessageService.cs | 10 +++ .../OneTimePasswordService.cs | 8 ++ .../IdentityServices/UserService.cs | 10 +++ .../Controllers/AccountController.cs | 8 ++ .../Accounts/UserExtensionsTests.cs | 11 --- 16 files changed, 239 insertions(+), 30 deletions(-) create mode 100644 backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommand.cs create mode 100644 backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs create mode 100644 backend/src/Logitar.Master.Contracts/Accounts/ChangePhonePayload.cs create mode 100644 backend/src/Logitar.Master.Contracts/Accounts/ChangePhoneResult.cs diff --git a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommand.cs b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommand.cs new file mode 100644 index 0000000..1eae9bb --- /dev/null +++ b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommand.cs @@ -0,0 +1,6 @@ +using Logitar.Master.Contracts.Accounts; +using MediatR; + +namespace Logitar.Master.Application.Accounts.Commands; + +public record ChangePhoneCommand(ChangePhonePayload Payload) : Activity, IRequest; diff --git a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs new file mode 100644 index 0000000..37bf52a --- /dev/null +++ b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs @@ -0,0 +1,83 @@ +using Logitar.Master.Contracts.Accounts; +using Logitar.Portal.Contracts.Messages; +using Logitar.Portal.Contracts.Passwords; +using Logitar.Portal.Contracts.Users; +using MediatR; + +namespace Logitar.Master.Application.Accounts.Commands; + +internal class ChangePhoneCommandHandler : IRequestHandler +{ + private const string ContactVerificationPurpose = "ContactVerification"; + private const string ContactVerificationTemplate = "ContactVerification{ContactType}"; + + private readonly IMessageService _messageService; + private readonly IOneTimePasswordService _oneTimePasswordService; + private readonly IUserService _userService; + + public ChangePhoneCommandHandler(IMessageService messageService, IOneTimePasswordService oneTimePasswordService, IUserService userService) + { + _messageService = messageService; + _oneTimePasswordService = oneTimePasswordService; + _userService = userService; + } + + public async Task Handle(ChangePhoneCommand command, CancellationToken cancellationToken) + { + if (command.User == null) + { + throw new ArgumentException($"An authenticated '{nameof(command.User)}' is required.", nameof(command)); + } + + ChangePhonePayload payload = command.Payload; // TODO(fpion): validate payload + + if (payload.NewPhone != null) + { + return await HandleNewPhoneAsync(payload.NewPhone, payload.Locale, command.User, cancellationToken); + } + else if (payload.OneTimePassword != null) + { + return await HandleOneTimePasswordAsync(payload.OneTimePassword, command.User, cancellationToken); + } + + throw new ArgumentException($"The '{nameof(command.Payload)}' is not valid.", nameof(command)); + } + + private async Task HandleNewPhoneAsync(AccountPhone newPhone, string locale, User user, CancellationToken cancellationToken) + { + Phone phone = newPhone.ToPhone(); + OneTimePassword oneTimePassword = await _oneTimePasswordService.CreateAsync(user, ContactVerificationPurpose, phone, cancellationToken); + if (oneTimePassword.Password == null) + { + throw new InvalidOperationException($"The One-Time Password (OTP) 'Id={oneTimePassword.Id}' has no password."); + } + Dictionary variables = new() + { + ["OneTimePassword"] = oneTimePassword.Password + }; + string template = ContactVerificationTemplate.Replace("{ContactType}", ContactType.Phone.ToString()); + SentMessages sentMessages = await _messageService.SendAsync(template, phone, locale, variables, cancellationToken); + SentMessage sentMessage = sentMessages.ToSentMessage(phone); + OneTimePasswordValidation oneTimePasswordValidation = new(oneTimePassword, sentMessage); + return new ChangePhoneResult(oneTimePasswordValidation); + } + + private async Task HandleOneTimePasswordAsync(OneTimePasswordPayload payload, User user, CancellationToken cancellationToken) + { + OneTimePassword oneTimePassword = await _oneTimePasswordService.ValidateAsync(payload, ContactVerificationPurpose, cancellationToken); + Guid userId = oneTimePassword.GetUserId(); + if (userId != user.Id) + { + StringBuilder message = new(); + message.AppendLine("The specified One-Time Password (OTP) cannot be used to change the specified user phone."); + message.Append("OneTimePasswordId: ").Append(oneTimePassword.Id).AppendLine(); + message.Append("ExpectedUserId: ").Append(userId).AppendLine(); + message.Append("ActualUserId: ").Append(user.Id).AppendLine(); + throw new InvalidOperationException(message.ToString()); // TODO(fpion): typed exception + } + Phone phone = oneTimePassword.GetPhone(); + phone.IsVerified = true; + user = await _userService.UpdatePhoneAsync(user.Id, phone, cancellationToken); + return new ChangePhoneResult(user.ToUserProfile()); + } +} diff --git a/backend/src/Logitar.Master.Application/Accounts/Commands/SignInCommandHandler.cs b/backend/src/Logitar.Master.Application/Accounts/Commands/SignInCommandHandler.cs index fb3e311..01b1c05 100644 --- a/backend/src/Logitar.Master.Application/Accounts/Commands/SignInCommandHandler.cs +++ b/backend/src/Logitar.Master.Application/Accounts/Commands/SignInCommandHandler.cs @@ -57,7 +57,7 @@ public async Task Handle(SignInCommand command, Cancellatio return await CompleteProfileAsync(payload.Profile, command.CustomAttributes, cancellationToken); } - throw new InvalidOperationException($"The {nameof(SignInPayload)} is not valid."); + throw new ArgumentException($"The '{nameof(command.Payload)}' is not valid.", nameof(command)); } private async Task HandleCredentialsAsync(Credentials credentials, string locale, IEnumerable customAttributes, CancellationToken cancellationToken) diff --git a/backend/src/Logitar.Master.Application/Accounts/IMessageService.cs b/backend/src/Logitar.Master.Application/Accounts/IMessageService.cs index 2813006..33353ae 100644 --- a/backend/src/Logitar.Master.Application/Accounts/IMessageService.cs +++ b/backend/src/Logitar.Master.Application/Accounts/IMessageService.cs @@ -6,5 +6,6 @@ namespace Logitar.Master.Application.Accounts; public interface IMessageService { Task SendAsync(string template, Email email, string? locale = null, Dictionary? variables = null, CancellationToken cancellationToken = default); + Task SendAsync(string template, Phone phone, string? locale = null, Dictionary? variables = null, CancellationToken cancellationToken = default); Task SendAsync(string template, User user, string? locale = null, Dictionary? variables = null, CancellationToken cancellationToken = default); } diff --git a/backend/src/Logitar.Master.Application/Accounts/IOneTimePasswordService.cs b/backend/src/Logitar.Master.Application/Accounts/IOneTimePasswordService.cs index caa51b4..3ac6792 100644 --- a/backend/src/Logitar.Master.Application/Accounts/IOneTimePasswordService.cs +++ b/backend/src/Logitar.Master.Application/Accounts/IOneTimePasswordService.cs @@ -7,5 +7,6 @@ namespace Logitar.Master.Application.Accounts; public interface IOneTimePasswordService { Task CreateAsync(User user, string purpose, CancellationToken cancellationToken = default); + Task CreateAsync(User user, string purpose, Phone? phone, CancellationToken cancellationToken = default); Task ValidateAsync(OneTimePasswordPayload payload, string purpose, CancellationToken cancellationToken = default); } diff --git a/backend/src/Logitar.Master.Application/Accounts/IUserService.cs b/backend/src/Logitar.Master.Application/Accounts/IUserService.cs index 31be19b..0b2f1c3 100644 --- a/backend/src/Logitar.Master.Application/Accounts/IUserService.cs +++ b/backend/src/Logitar.Master.Application/Accounts/IUserService.cs @@ -12,4 +12,5 @@ public interface IUserService Task FindAsync(Guid id, CancellationToken cancellationToken = default); Task SaveProfileAsync(Guid userId, SaveProfilePayload payload, CancellationToken cancellationToken = default); Task UpdateEmailAsync(Guid userId, Email email, CancellationToken cancellationToken = default); + Task UpdatePhoneAsync(Guid userId, Phone phone, CancellationToken cancellationToken = default); } diff --git a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs index 3c2d1ec..3c1899a 100644 --- a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs +++ b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs @@ -6,6 +6,9 @@ namespace Logitar.Master.Application.Accounts; public static class OneTimePasswordExtensions { + private const string PhoneCountryCodeKey = "PhoneCountryCode"; + private const string PhoneNumberKey = "PhoneNumber"; + private const string PhoneE164FormattedKey = "PhoneE164Formatted"; private const string PurposeKey = "Purpose"; private const string UserIdKey = "UserId"; @@ -17,17 +20,47 @@ public static Guid GetUserId(this OneTimePassword oneTimePassword) } public static void SetUserId(this CreateOneTimePasswordPayload payload, User user) { - CustomAttribute? customAttribute = payload.CustomAttributes.SingleOrDefault(x => x.Key == UserIdKey); - if (customAttribute == null) + payload.CustomAttributes.Add(new CustomAttribute(UserIdKey, user.Id.ToString())); + } + + public static Phone GetPhone(this OneTimePassword oneTimePassword) + { + return oneTimePassword.TryGetPhone() ?? throw new InvalidOperationException(); + } // TODO(fpion): unit tests + public static Phone? TryGetPhone(this OneTimePassword oneTimePassword) + { + Phone phone = new(); + foreach (CustomAttribute customAttribute in oneTimePassword.CustomAttributes) { - customAttribute = new(UserIdKey, user.Id.ToString()); - payload.CustomAttributes.Add(customAttribute); + switch (customAttribute.Key) + { + case PhoneCountryCodeKey: + phone.CountryCode = customAttribute.Value; + break; + case PhoneNumberKey: + phone.Number = customAttribute.Value; + break; + case PhoneE164FormattedKey: + phone.E164Formatted = customAttribute.Value; + break; + } } - else + + if (string.IsNullOrWhiteSpace(phone.Number) || string.IsNullOrWhiteSpace(phone.E164Formatted)) { - customAttribute.Value = user.Id.ToString(); + return null; } - } + return phone; + } // TODO(fpion): unit tests + public static void SetPhone(this CreateOneTimePasswordPayload payload, Phone phone) + { + if (phone.CountryCode != null) + { + payload.CustomAttributes.Add(new CustomAttribute(PhoneCountryCodeKey, phone.CountryCode)); + } + payload.CustomAttributes.Add(new CustomAttribute(PhoneNumberKey, phone.Number)); + payload.CustomAttributes.Add(new CustomAttribute(PhoneE164FormattedKey, phone.E164Formatted)); + } // TODO(fpion): unit tests public static void EnsurePurpose(this OneTimePassword oneTimePassword, string purpose) { @@ -43,7 +76,7 @@ public static bool HasPurpose(this OneTimePassword oneTimePassword, string purpo public static string GetPurpose(this OneTimePassword oneTimePassword) { string? purpose = oneTimePassword.TryGetPurpose(); - return purpose ?? throw new InvalidOperationException($"The One-Time Password (OTP) has no '{PurposeKey}' custom attribute."); + return purpose ?? throw new ArgumentException($"The One-Time Password (OTP) has no '{PurposeKey}' custom attribute.", nameof(oneTimePassword)); } public static string? TryGetPurpose(this OneTimePassword oneTimePassword) { @@ -52,15 +85,6 @@ public static string GetPurpose(this OneTimePassword oneTimePassword) } public static void SetPurpose(this CreateOneTimePasswordPayload payload, string purpose) { - CustomAttribute? customAttribute = payload.CustomAttributes.SingleOrDefault(x => x.Key == PurposeKey); - if (customAttribute == null) - { - customAttribute = new(PurposeKey, purpose); - payload.CustomAttributes.Add(customAttribute); - } - else - { - customAttribute.Value = purpose; - } + payload.CustomAttributes.Add(new CustomAttribute(PurposeKey, purpose)); } } diff --git a/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs b/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs index 0552d8f..349d4e5 100644 --- a/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs +++ b/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs @@ -36,6 +36,11 @@ public static bool IsProfileCompleted(this User user) return customAttribute == null ? null : DateTime.Parse(customAttribute.Value); } + public static Phone ToPhone(this AccountPhone phone) + { + return new Phone(phone.CountryCode, phone.Number, extension: null, e164Formatted: ""); // TODO(fpion): format to E.164 + } // TODO(fpion): unit tests + public static UserProfile ToUserProfile(this User user) => new() { CreatedOn = user.CreatedOn, diff --git a/backend/src/Logitar.Master.Application/Activity.cs b/backend/src/Logitar.Master.Application/Activity.cs index 72a3d7e..e6ce43c 100644 --- a/backend/src/Logitar.Master.Application/Activity.cs +++ b/backend/src/Logitar.Master.Application/Activity.cs @@ -1,5 +1,6 @@ using Logitar.EventSourcing; using Logitar.Portal.Contracts.Actors; +using Logitar.Portal.Contracts.Users; namespace Logitar.Master.Application; @@ -32,6 +33,19 @@ public Actor Actor } public ActorId ActorId => new(Actor.Id); + public User? User + { + get + { + if (_context == null) + { + throw new InvalidOperationException($"The activity has been been contextualized yet. You must call the '{nameof(Contextualize)}' method."); + } + + return _context.User; + } + } + public void Contextualize(ActivityContext context) { if (_context != null) diff --git a/backend/src/Logitar.Master.Contracts/Accounts/ChangePhonePayload.cs b/backend/src/Logitar.Master.Contracts/Accounts/ChangePhonePayload.cs new file mode 100644 index 0000000..e045f29 --- /dev/null +++ b/backend/src/Logitar.Master.Contracts/Accounts/ChangePhonePayload.cs @@ -0,0 +1,28 @@ +namespace Logitar.Master.Contracts.Accounts; + +public record ChangePhonePayload +{ + public string Locale { get; set; } + + public AccountPhone? NewPhone { get; set; } + public OneTimePasswordPayload? OneTimePassword { get; set; } + + public ChangePhonePayload() : this(string.Empty) + { + } + + public ChangePhonePayload(string locale) + { + Locale = locale; + } + + public ChangePhonePayload(string locale, AccountPhone newPhone) : this(locale) + { + NewPhone = newPhone; + } + + public ChangePhonePayload(string locale, OneTimePasswordPayload oneTimePassword) : this(locale) + { + OneTimePassword = oneTimePassword; + } +} diff --git a/backend/src/Logitar.Master.Contracts/Accounts/ChangePhoneResult.cs b/backend/src/Logitar.Master.Contracts/Accounts/ChangePhoneResult.cs new file mode 100644 index 0000000..2f23e3f --- /dev/null +++ b/backend/src/Logitar.Master.Contracts/Accounts/ChangePhoneResult.cs @@ -0,0 +1,21 @@ +namespace Logitar.Master.Contracts.Accounts; + +public record ChangePhoneResult +{ + public OneTimePasswordValidation? OneTimePasswordValidation { get; set; } + public UserProfile? UserProfile { get; set; } + + public ChangePhoneResult() + { + } + + public ChangePhoneResult(OneTimePasswordValidation oneTimePasswordValidation) + { + OneTimePasswordValidation = oneTimePasswordValidation; + } + + public ChangePhoneResult(UserProfile userProfile) + { + UserProfile = userProfile; + } +} diff --git a/backend/src/Logitar.Master.Infrastructure/IdentityServices/MessageService.cs b/backend/src/Logitar.Master.Infrastructure/IdentityServices/MessageService.cs index 7613bc5..4d2d1a1 100644 --- a/backend/src/Logitar.Master.Infrastructure/IdentityServices/MessageService.cs +++ b/backend/src/Logitar.Master.Infrastructure/IdentityServices/MessageService.cs @@ -24,6 +24,16 @@ public async Task SendAsync(string template, Email email, string? return await SendAsync(template, recipient, locale, variables, cancellationToken); } + public async Task SendAsync(string template, Phone phone, string? locale, Dictionary? variables, CancellationToken cancellationToken) + { + RecipientPayload recipient = new() + { + Type = RecipientType.To, + PhoneNumber = phone.E164Formatted + }; + return await SendAsync(template, recipient, locale, variables, cancellationToken); + } + public async Task SendAsync(string template, User user, string? locale, Dictionary? variables, CancellationToken cancellationToken) { RecipientPayload recipient = new() diff --git a/backend/src/Logitar.Master.Infrastructure/IdentityServices/OneTimePasswordService.cs b/backend/src/Logitar.Master.Infrastructure/IdentityServices/OneTimePasswordService.cs index 5d2e17c..eb62cfa 100644 --- a/backend/src/Logitar.Master.Infrastructure/IdentityServices/OneTimePasswordService.cs +++ b/backend/src/Logitar.Master.Infrastructure/IdentityServices/OneTimePasswordService.cs @@ -21,6 +21,10 @@ public OneTimePasswordService(IOneTimePasswordClient oneTimePasswordClient) } public async Task CreateAsync(User user, string purpose, CancellationToken cancellationToken) + { + return await CreateAsync(user, purpose, phone: null, cancellationToken); + } + public async Task CreateAsync(User user, string purpose, Phone? phone, CancellationToken cancellationToken) { CreateOneTimePasswordPayload payload = new(Characters, Length) { @@ -29,6 +33,10 @@ public async Task CreateAsync(User user, string purpose, Cancel }; payload.SetUserId(user); payload.SetPurpose(purpose); + if (phone != null) + { + payload.SetPhone(phone); + } RequestContext context = new(user.Id.ToString(), cancellationToken); return await _oneTimePasswordClient.CreateAsync(payload, context); } diff --git a/backend/src/Logitar.Master.Infrastructure/IdentityServices/UserService.cs b/backend/src/Logitar.Master.Infrastructure/IdentityServices/UserService.cs index 974af08..206d375 100644 --- a/backend/src/Logitar.Master.Infrastructure/IdentityServices/UserService.cs +++ b/backend/src/Logitar.Master.Infrastructure/IdentityServices/UserService.cs @@ -88,4 +88,14 @@ public async Task UpdateEmailAsync(Guid userId, Email email, CancellationT RequestContext context = new(userId.ToString(), cancellationToken); return await _userClient.UpdateAsync(userId, payload, context) ?? throw new InvalidOperationException($"The user 'Id={userId}' could not be found."); } + + public async Task UpdatePhoneAsync(Guid userId, Phone phone, CancellationToken cancellationToken) + { + UpdateUserPayload payload = new() + { + Phone = new Modification(new PhonePayload(phone.CountryCode, phone.Number, phone.Extension, phone.IsVerified)) + }; + RequestContext context = new(userId.ToString(), cancellationToken); + return await _userClient.UpdateAsync(userId, payload, context) ?? throw new InvalidOperationException($"The user 'Id={userId}' could not be found."); + } } diff --git a/backend/src/Logitar.Master/Controllers/AccountController.cs b/backend/src/Logitar.Master/Controllers/AccountController.cs index 970e50b..4c5ec91 100644 --- a/backend/src/Logitar.Master/Controllers/AccountController.cs +++ b/backend/src/Logitar.Master/Controllers/AccountController.cs @@ -63,6 +63,14 @@ public async Task> TokenAsync([FromBody] GetToken return Ok(response); } + [HttpPut("/phone/change")] + [Authorize] + public async Task> ChangePhoneAsync([FromBody] ChangePhonePayload payload, CancellationToken cancellationToken) + { + ChangePhoneResult result = await _requestPipeline.ExecuteAsync(new ChangePhoneCommand(payload), cancellationToken); + return Ok(result); + } + [HttpGet("/profile")] [Authorize] public ActionResult GetProfile() diff --git a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs index 734bfe1..e4d126f 100644 --- a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs +++ b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs @@ -93,17 +93,6 @@ public void SetMultiFactorAuthenticationMode_it_should_the_correct_custom_attrib Assert.Contains(payload.CustomAttributes, c => c.Key == nameof(MultiFactorAuthenticationMode) && c.Value == mfaMode.ToString()); } - //[Fact(DisplayName = "ToPhonePayload: it should return the correct phone payload.")] - //public void ToPhonePayload_it_should_return_the_correct_phone_payload() - //{ - // Contracts.Accounts.Phone phone = new("CA", "+15148454636"); - // PhonePayload payload = phone.ToPhonePayload(); - // Assert.Equal(phone.CountryCode, payload.CountryCode); - // Assert.Equal(phone.Number, payload.Number); - // Assert.Null(payload.Extension); - // Assert.True(payload.IsVerified); - //} - [Fact(DisplayName = "ToUserProfile: it should return the correct user profile.")] public void ToUserProfile_it_should_return_the_correct_user_profile() { From a97e783a4081c0c83bf90dcc6855412acf33ad1e Mon Sep 17 00:00:00 2001 From: Francis Pion Date: Tue, 30 Apr 2024 12:17:41 -0400 Subject: [PATCH 2/5] Fixed tests. --- .../Commands/ChangePhoneCommandHandler.cs | 2 +- .../Accounts/OneTimePasswordExtensions.cs | 4 ++-- .../src/Logitar.Master.Application/Activity.cs | 2 +- .../Accounts/OneTimePasswordExtensionsTests.cs | 18 ++++++++++-------- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs index 37bf52a..b4f6958 100644 --- a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs +++ b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs @@ -73,7 +73,7 @@ private async Task HandleOneTimePasswordAsync(OneTimePassword message.Append("OneTimePasswordId: ").Append(oneTimePassword.Id).AppendLine(); message.Append("ExpectedUserId: ").Append(userId).AppendLine(); message.Append("ActualUserId: ").Append(user.Id).AppendLine(); - throw new InvalidOperationException(message.ToString()); // TODO(fpion): typed exception + throw new NotImplementedException(message.ToString()); // TODO(fpion): typed exception } Phone phone = oneTimePassword.GetPhone(); phone.IsVerified = true; diff --git a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs index 3c1899a..7da0e11 100644 --- a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs +++ b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs @@ -15,7 +15,7 @@ public static class OneTimePasswordExtensions public static Guid GetUserId(this OneTimePassword oneTimePassword) { CustomAttribute customAttribute = oneTimePassword.CustomAttributes.SingleOrDefault(x => x.Key == UserIdKey) - ?? throw new InvalidOperationException($"The One-Time Password (OTP) has no '{UserIdKey}' custom attribute."); + ?? throw new ArgumentException($"The One-Time Password (OTP) has no '{UserIdKey}' custom attribute.", nameof(oneTimePassword)); return Guid.Parse(customAttribute.Value); } public static void SetUserId(this CreateOneTimePasswordPayload payload, User user) @@ -25,7 +25,7 @@ public static void SetUserId(this CreateOneTimePasswordPayload payload, User use public static Phone GetPhone(this OneTimePassword oneTimePassword) { - return oneTimePassword.TryGetPhone() ?? throw new InvalidOperationException(); + return oneTimePassword.TryGetPhone() ?? throw new NotImplementedException(); // TODO(fpion): typed exception } // TODO(fpion): unit tests public static Phone? TryGetPhone(this OneTimePassword oneTimePassword) { diff --git a/backend/src/Logitar.Master.Application/Activity.cs b/backend/src/Logitar.Master.Application/Activity.cs index e6ce43c..ed4b7c5 100644 --- a/backend/src/Logitar.Master.Application/Activity.cs +++ b/backend/src/Logitar.Master.Application/Activity.cs @@ -50,7 +50,7 @@ public void Contextualize(ActivityContext context) { if (_context != null) { - throw new InvalidOperationException($"The activity has already been populated. You may only call the '{nameof(Contextualize)}' method once."); + throw new InvalidOperationException($"The activity has already been contextualized. You may only call the '{nameof(Contextualize)}' method once."); } _context = context; diff --git a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs index d5a7706..f1aa962 100644 --- a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs +++ b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs @@ -58,13 +58,14 @@ public void GetPurpose_it_should_return_the_purpose_when_the_One_Time_Password_h Assert.Equal(Purpose, password.GetPurpose()); } - [Fact(DisplayName = "GetPurpose: it should throw InvalidOperationException when the One-Time Password has no purpose.")] - public void GetPurpose_it_should_throw_InvalidOperationException_when_the_One_Time_Password_has_no_purpose() + [Fact(DisplayName = "GetPurpose: it should throw ArgumentException when the One-Time Password has no purpose.")] + public void GetPurpose_it_should_throw_ArgumentException_when_the_One_Time_Password_has_no_purpose() { OneTimePassword password = new(); Assert.Empty(password.CustomAttributes); - var exception = Assert.Throws(password.GetPurpose); - Assert.Equal("The One-Time Password (OTP) has no 'Purpose' custom attribute.", exception.Message); + var exception = Assert.Throws(password.GetPurpose); + Assert.StartsWith("The One-Time Password (OTP) has no 'Purpose' custom attribute.", exception.Message); + Assert.Equal("oneTimePassword", exception.ParamName); } [Fact(DisplayName = "GetUserId: it should return the correct identifier.")] @@ -76,15 +77,16 @@ public void GetUserId_it_should_return_the_correct_identifier() Assert.Equal(userId, password.GetUserId()); } - [Fact(DisplayName = "GetUserId: it should throw InvalidOperationException when the One-Time Password does not have the custom attribute.")] - public void GetUserId_it_should_throw_InvalidOperationException_when_the_One_Time_Password_does_not_have_the_custom_attribute() + [Fact(DisplayName = "GetUserId: it should throw ArgumentException when the One-Time Password does not have the custom attribute.")] + public void GetUserId_it_should_throw_ArgumentException_when_the_One_Time_Password_does_not_have_the_custom_attribute() { Guid userId = Guid.NewGuid(); OneTimePassword password = new(); Assert.Empty(password.CustomAttributes); - var exception = Assert.Throws(() => password.GetUserId()); - Assert.Equal("The One-Time Password (OTP) has no 'UserId' custom attribute.", exception.Message); + var exception = Assert.Throws(() => password.GetUserId()); + Assert.StartsWith("The One-Time Password (OTP) has no 'UserId' custom attribute.", exception.Message); + Assert.Equal("oneTimePassword", exception.ParamName); } [Fact(DisplayName = "HasPurpose: it should return false when the One-Time Password does not have the expected purpose.")] From 08da420f0ae0362604e34c936ddd4035a00368b2 Mon Sep 17 00:00:00 2001 From: Francis Pion Date: Tue, 30 Apr 2024 16:58:08 -0400 Subject: [PATCH 3/5] Implemented unit tests. --- .../Commands/ChangePhoneCommandHandler.cs | 2 +- .../Accounts/OneTimePasswordExtensions.cs | 9 +- .../Accounts/UserExtensions.cs | 9 +- .../Logitar.Master.Domain.csproj | 1 + .../OneTimePasswordExtensionsTests.cs | 96 +++++++++++++++++++ .../Accounts/UserExtensionsTests.cs | 14 +++ 6 files changed, 123 insertions(+), 8 deletions(-) diff --git a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs index b4f6958..1b2b077 100644 --- a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs +++ b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs @@ -6,7 +6,7 @@ namespace Logitar.Master.Application.Accounts.Commands; -internal class ChangePhoneCommandHandler : IRequestHandler +internal class ChangePhoneCommandHandler : IRequestHandler // TODO(fpion): integration tests { private const string ContactVerificationPurpose = "ContactVerification"; private const string ContactVerificationTemplate = "ContactVerification{ContactType}"; diff --git a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs index 7da0e11..b800b71 100644 --- a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs +++ b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs @@ -25,8 +25,9 @@ public static void SetUserId(this CreateOneTimePasswordPayload payload, User use public static Phone GetPhone(this OneTimePassword oneTimePassword) { - return oneTimePassword.TryGetPhone() ?? throw new NotImplementedException(); // TODO(fpion): typed exception - } // TODO(fpion): unit tests + return oneTimePassword.TryGetPhone() + ?? throw new ArgumentException("The One-Time Password (OTP) does not have phone custom attributes.", nameof(oneTimePassword)); + } public static Phone? TryGetPhone(this OneTimePassword oneTimePassword) { Phone phone = new(); @@ -51,7 +52,7 @@ public static Phone GetPhone(this OneTimePassword oneTimePassword) return null; } return phone; - } // TODO(fpion): unit tests + } public static void SetPhone(this CreateOneTimePasswordPayload payload, Phone phone) { if (phone.CountryCode != null) @@ -60,7 +61,7 @@ public static void SetPhone(this CreateOneTimePasswordPayload payload, Phone pho } payload.CustomAttributes.Add(new CustomAttribute(PhoneNumberKey, phone.Number)); payload.CustomAttributes.Add(new CustomAttribute(PhoneE164FormattedKey, phone.E164Formatted)); - } // TODO(fpion): unit tests + } public static void EnsurePurpose(this OneTimePassword oneTimePassword, string purpose) { diff --git a/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs b/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs index 349d4e5..fb7740d 100644 --- a/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs +++ b/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs @@ -1,4 +1,5 @@ -using Logitar.Master.Contracts.Accounts; +using Logitar.Identity.Domain.Users; +using Logitar.Master.Contracts.Accounts; using Logitar.Portal.Contracts; using Logitar.Portal.Contracts.Users; @@ -38,8 +39,10 @@ public static bool IsProfileCompleted(this User user) public static Phone ToPhone(this AccountPhone phone) { - return new Phone(phone.CountryCode, phone.Number, extension: null, e164Formatted: ""); // TODO(fpion): format to E.164 - } // TODO(fpion): unit tests + Phone result = new(phone.CountryCode, phone.Number, extension: null, e164Formatted: string.Empty); + result.E164Formatted = result.FormatToE164(); + return result; + } public static UserProfile ToUserProfile(this User user) => new() { diff --git a/backend/src/Logitar.Master.Domain/Logitar.Master.Domain.csproj b/backend/src/Logitar.Master.Domain/Logitar.Master.Domain.csproj index 5f10db2..86757fa 100644 --- a/backend/src/Logitar.Master.Domain/Logitar.Master.Domain.csproj +++ b/backend/src/Logitar.Master.Domain/Logitar.Master.Domain.csproj @@ -17,6 +17,7 @@ + diff --git a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs index f1aa962..a116b7e 100644 --- a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs +++ b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs @@ -1,4 +1,5 @@ using Bogus; +using Logitar.Portal.Contracts; using Logitar.Portal.Contracts.Passwords; using Logitar.Portal.Contracts.Users; @@ -89,6 +90,33 @@ public void GetUserId_it_should_throw_ArgumentException_when_the_One_Time_Passwo Assert.Equal("oneTimePassword", exception.ParamName); } + [Fact(DisplayName = "GetPhone: it should return the correct phone.")] + public void GetPhone_it_should_return_the_correct_phone() + { + OneTimePassword oneTimePassword = new(); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneCountryCode", "CA")); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneNumber", "(514) 845-4636")); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneE164Formatted", "+15148454636")); + + Phone phone = oneTimePassword.GetPhone(); + Assert.Equal("CA", phone.CountryCode); + Assert.Equal("(514) 845-4636", phone.Number); + Assert.Null(phone.Extension); + Assert.Equal("+15148454636", phone.E164Formatted); + Assert.False(phone.IsVerified); + Assert.Null(phone.VerifiedBy); + Assert.Null(phone.VerifiedOn); + } + + [Fact(DisplayName = "GetPhone: it should throw ArgumentException when the One-Time Password does not have the custom attributes.")] + public void GetPhone_it_should_throw_ArgumentException_when_the_One_Time_Password_does_not_have_the_custom_attributes() + { + OneTimePassword oneTimePassword = new(); + var exception = Assert.Throws(() => oneTimePassword.GetPhone()); + Assert.StartsWith("The One-Time Password (OTP) does not have phone custom attributes.", exception.Message); + Assert.Equal("oneTimePassword", exception.ParamName); + } + [Fact(DisplayName = "HasPurpose: it should return false when the One-Time Password does not have the expected purpose.")] public void HasPurpose_it_should_return_false_when_the_One_Time_Password_does_not_have_the_expected_purpose() { @@ -108,6 +136,29 @@ public void HasPurpose_it_should_return_true_when_the_One_Time_Password_has_the_ Assert.True(password.HasPurpose(Purpose.ToLower())); } + [Theory(DisplayName = "SetPhone: it should set the correct custom attributes.")] + [InlineData(null, "(514) 845-4636", "+15148454636")] + [InlineData("CA", "(514) 845-4636", "+15148454636")] + public void SetPhone_it_should_set_the_correct_custom_attributes(string? countryCode, string number, string e164Formatted) + { + Phone phone = new(countryCode, number, extension: null, e164Formatted); + CreateOneTimePasswordPayload payload = new("0123456789", length: 6); + payload.SetPhone(phone); + + Assert.Contains(payload.CustomAttributes, c => c.Key == "PhoneNumber" && c.Value == number); + Assert.Contains(payload.CustomAttributes, c => c.Key == "PhoneE164Formatted" && c.Value == e164Formatted); + + if (countryCode == null) + { + Assert.Equal(2, payload.CustomAttributes.Count); + } + else + { + Assert.Equal(3, payload.CustomAttributes.Count); + Assert.Contains(payload.CustomAttributes, c => c.Key == "PhoneCountryCode" && c.Value == countryCode); + } + } + [Fact(DisplayName = "SetPurpose: it should add the correct custom attribute.")] public void SetPurpose_it_should_add_the_correct_custom_attribute() { @@ -156,6 +207,51 @@ public void SetUserId_it_should_replace_the_correct_custom_attribute() Assert.Single(payload.CustomAttributes, c => c.Key == "UserId" && c.Value == user.Id.ToString()); } + [Theory(DisplayName = "TryGetPhone: it should return null when the One Time Password does not have the custom attributes.")] + [InlineData(null, null)] + [InlineData("(514) 845-4636", null)] + [InlineData(null, "+15148454636")] + public void TryGetPhone_it_should_return_null_when_the_One_Time_Password_does_not_have_the_custom_attributes(string? number, string? e164Formatted) + { + Assert.True(number == null || e164Formatted == null); + + OneTimePassword oneTimePassword = new(); + if (number != null) + { + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneNumber", number)); + } + if (e164Formatted != null) + { + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneE164Formatted", e164Formatted)); + } + + Assert.Null(oneTimePassword.TryGetPhone()); + } + + [Theory(DisplayName = "TryGetPhone: it should return the correct phone.")] + [InlineData(null, "(514) 845-4636", "+15148454636")] + [InlineData("CA", "(514) 845-4636", "+15148454636")] + public void TryGetPhone_it_should_return_the_correct_phone(string? countryCode, string number, string e164Formatted) + { + OneTimePassword oneTimePassword = new(); + if (countryCode != null) + { + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneCountryCode", countryCode)); + } + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneNumber", number)); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneE164Formatted", e164Formatted)); + + Phone? phone = oneTimePassword.TryGetPhone(); + Assert.NotNull(phone); + Assert.Equal(countryCode, phone.CountryCode); + Assert.Equal(number, phone.Number); + Assert.Null(phone.Extension); + Assert.Equal(e164Formatted, phone.E164Formatted); + Assert.False(phone.IsVerified); + Assert.Null(phone.VerifiedBy); + Assert.Null(phone.VerifiedOn); + } + [Fact(DisplayName = "TryGetPurpose: it should return null when the One-Time Password has no purpose.")] public void TryGetPurpose_it_should_return_null_when_the_One_Time_Password_has_no_purpose() { diff --git a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs index e4d126f..efcb4a7 100644 --- a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs +++ b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs @@ -93,6 +93,20 @@ public void SetMultiFactorAuthenticationMode_it_should_the_correct_custom_attrib Assert.Contains(payload.CustomAttributes, c => c.Key == nameof(MultiFactorAuthenticationMode) && c.Value == mfaMode.ToString()); } + [Fact(DisplayName = "ToPhone: it should return the correct phone.")] + public void ToPhone_it_should_return_the_correct_phone() + { + AccountPhone phone = new("(514) 845-4636", "CA"); + Phone result = phone.ToPhone(); + Assert.Equal(phone.CountryCode, result.CountryCode); + Assert.Equal(phone.Number, result.Number); + Assert.Null(result.Extension); + Assert.Equal("+15148454636", result.E164Formatted); + Assert.False(result.IsVerified); + Assert.Null(result.VerifiedBy); + Assert.Null(result.VerifiedOn); + } + [Fact(DisplayName = "ToUserProfile: it should return the correct user profile.")] public void ToUserProfile_it_should_return_the_correct_user_profile() { From 303a4dc4a66d7cdd8319e60219e448885b1a7f65 Mon Sep 17 00:00:00 2001 From: Francis Pion Date: Tue, 30 Apr 2024 17:33:13 -0400 Subject: [PATCH 4/5] Fixes. --- .../Commands/ChangePhoneCommandHandler.cs | 14 +++---- .../InvalidOneTimePasswordPurposeException.cs | 9 ++--- .../InvalidOneTimePasswordUserException.cs | 39 +++++++++++++++++++ .../Accounts/OneTimePasswordExtensions.cs | 10 +++-- .../OneTimePasswordNotFoundException.cs | 9 ++--- .../Accounts/UserExtensions.cs | 2 +- .../Validators/AccountPhoneValidator.cs | 23 +++++++++++ .../Validators/ChangePhoneValidator.cs | 33 ++++++++++++++++ .../Validators/OneTimePasswordValidator.cs | 13 +++++++ .../Errors/InvalidCredentialsError.cs | 10 ----- .../OneTimePasswordExtensionsTests.cs | 18 +++++++++ .../Accounts/UserExtensionsTests.cs | 16 +++++--- 12 files changed, 156 insertions(+), 40 deletions(-) create mode 100644 backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordUserException.cs create mode 100644 backend/src/Logitar.Master.Application/Accounts/Validators/AccountPhoneValidator.cs create mode 100644 backend/src/Logitar.Master.Application/Accounts/Validators/ChangePhoneValidator.cs create mode 100644 backend/src/Logitar.Master.Application/Accounts/Validators/OneTimePasswordValidator.cs delete mode 100644 backend/src/Logitar.Master.Contracts/Errors/InvalidCredentialsError.cs diff --git a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs index 1b2b077..da5713a 100644 --- a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs +++ b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs @@ -1,4 +1,6 @@ -using Logitar.Master.Contracts.Accounts; +using FluentValidation; +using Logitar.Master.Application.Accounts.Validators; +using Logitar.Master.Contracts.Accounts; using Logitar.Portal.Contracts.Messages; using Logitar.Portal.Contracts.Passwords; using Logitar.Portal.Contracts.Users; @@ -29,7 +31,8 @@ public async Task Handle(ChangePhoneCommand command, Cancella throw new ArgumentException($"An authenticated '{nameof(command.User)}' is required.", nameof(command)); } - ChangePhonePayload payload = command.Payload; // TODO(fpion): validate payload + ChangePhonePayload payload = command.Payload; + new ChangePhoneValidator().ValidateAndThrow(payload); if (payload.NewPhone != null) { @@ -68,12 +71,7 @@ private async Task HandleOneTimePasswordAsync(OneTimePassword Guid userId = oneTimePassword.GetUserId(); if (userId != user.Id) { - StringBuilder message = new(); - message.AppendLine("The specified One-Time Password (OTP) cannot be used to change the specified user phone."); - message.Append("OneTimePasswordId: ").Append(oneTimePassword.Id).AppendLine(); - message.Append("ExpectedUserId: ").Append(userId).AppendLine(); - message.Append("ActualUserId: ").Append(user.Id).AppendLine(); - throw new NotImplementedException(message.ToString()); // TODO(fpion): typed exception + throw new InvalidOneTimePasswordUserException(oneTimePassword, user); } Phone phone = oneTimePassword.GetPhone(); phone.IsVerified = true; diff --git a/backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordPurposeException.cs b/backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordPurposeException.cs index 4ebb06f..eed54bf 100644 --- a/backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordPurposeException.cs +++ b/backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordPurposeException.cs @@ -1,12 +1,11 @@ -using Logitar.Master.Contracts.Errors; -using Logitar.Portal.Contracts.Errors; +using Logitar.Identity.Domain.Shared; using Logitar.Portal.Contracts.Passwords; namespace Logitar.Master.Application.Accounts; -public class InvalidOneTimePasswordPurposeException : BadRequestException +public class InvalidOneTimePasswordPurposeException : InvalidCredentialsException { - private const string ErrorMessage = "The specified purpose did not match the expected One-Time Passord (OTP) purpose."; + private new const string ErrorMessage = "The specified purpose did not match the expected One-Time Passord (OTP) purpose."; public Guid OneTimePasswordId { @@ -24,8 +23,6 @@ public string? ActualPurpose private set => Data[nameof(ActualPurpose)] = value; } - public override Error Error => new InvalidCredentialsError(); - public InvalidOneTimePasswordPurposeException(OneTimePassword oneTimePassword, string purpose) : base(BuildMessage(oneTimePassword, purpose)) { OneTimePasswordId = oneTimePassword.Id; diff --git a/backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordUserException.cs b/backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordUserException.cs new file mode 100644 index 0000000..153164d --- /dev/null +++ b/backend/src/Logitar.Master.Application/Accounts/InvalidOneTimePasswordUserException.cs @@ -0,0 +1,39 @@ +using Logitar.Identity.Domain.Shared; +using Logitar.Portal.Contracts.Passwords; +using Logitar.Portal.Contracts.Users; + +namespace Logitar.Master.Application.Accounts; + +public class InvalidOneTimePasswordUserException : InvalidCredentialsException +{ + private new const string ErrorMessage = "The specified user did not match the expected One-Time Passord (OTP) user."; + + public Guid OneTimePasswordId + { + get => (Guid)Data[nameof(OneTimePasswordId)]!; + private set => Data[nameof(OneTimePasswordId)] = value; + } + public Guid? ExpectedUserId + { + get => (Guid?)Data[nameof(ExpectedUserId)]; + private set => Data[nameof(ExpectedUserId)] = value; + } + public Guid? ActualUserId + { + get => (Guid?)Data[nameof(ActualUserId)]; + private set => Data[nameof(ActualUserId)] = value; + } + + public InvalidOneTimePasswordUserException(OneTimePassword oneTimePassword, User? user) : base(BuildMessage(oneTimePassword, user)) + { + OneTimePasswordId = oneTimePassword.Id; + ExpectedUserId = oneTimePassword.TryGetUserId(); + ActualUserId = user?.Id; + } + + private static string BuildMessage(OneTimePassword oneTimePassword, User? user) => new ErrorMessageBuilder(ErrorMessage) + .AddData(nameof(OneTimePasswordId), oneTimePassword.Id) + .AddData(nameof(ExpectedUserId), oneTimePassword.TryGetUserId(), "") + .AddData(nameof(ActualUserId), user?.Id, "") + .Build(); +} diff --git a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs index b800b71..51b0822 100644 --- a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs +++ b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordExtensions.cs @@ -14,9 +14,13 @@ public static class OneTimePasswordExtensions public static Guid GetUserId(this OneTimePassword oneTimePassword) { - CustomAttribute customAttribute = oneTimePassword.CustomAttributes.SingleOrDefault(x => x.Key == UserIdKey) - ?? throw new ArgumentException($"The One-Time Password (OTP) has no '{UserIdKey}' custom attribute.", nameof(oneTimePassword)); - return Guid.Parse(customAttribute.Value); + Guid? userId = oneTimePassword.TryGetUserId(); + return userId ?? throw new ArgumentException($"The One-Time Password (OTP) has no '{UserIdKey}' custom attribute.", nameof(oneTimePassword)); + } + public static Guid? TryGetUserId(this OneTimePassword oneTimePassword) + { + CustomAttribute? customAttribute = oneTimePassword.CustomAttributes.SingleOrDefault(x => x.Key == UserIdKey); + return customAttribute == null ? null : Guid.Parse(customAttribute.Value); } public static void SetUserId(this CreateOneTimePasswordPayload payload, User user) { diff --git a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordNotFoundException.cs b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordNotFoundException.cs index 4bd1275..4ef9798 100644 --- a/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordNotFoundException.cs +++ b/backend/src/Logitar.Master.Application/Accounts/OneTimePasswordNotFoundException.cs @@ -1,11 +1,10 @@ -using Logitar.Master.Contracts.Errors; -using Logitar.Portal.Contracts.Errors; +using Logitar.Identity.Domain.Shared; namespace Logitar.Master.Application.Accounts; -public class OneTimePasswordNotFoundException : BadRequestException +public class OneTimePasswordNotFoundException : InvalidCredentialsException { - private const string ErrorMessage = "The specified One-Time Password (OTP) could not be found."; + private new const string ErrorMessage = "The specified One-Time Password (OTP) could not be found."; public Guid OneTimePasswordId { @@ -13,8 +12,6 @@ public Guid OneTimePasswordId private set => Data[nameof(OneTimePasswordId)] = value; } - public override Error Error => new InvalidCredentialsError(); - public OneTimePasswordNotFoundException(Guid oneTimePasswordId) : base(BuildMessage(oneTimePasswordId)) { OneTimePasswordId = oneTimePasswordId; diff --git a/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs b/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs index fb7740d..0d4ba7f 100644 --- a/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs +++ b/backend/src/Logitar.Master.Application/Accounts/UserExtensions.cs @@ -39,7 +39,7 @@ public static bool IsProfileCompleted(this User user) public static Phone ToPhone(this AccountPhone phone) { - Phone result = new(phone.CountryCode, phone.Number, extension: null, e164Formatted: string.Empty); + Phone result = new(phone.CountryCode?.CleanTrim(), phone.Number.Trim(), extension: null, e164Formatted: string.Empty); result.E164Formatted = result.FormatToE164(); return result; } diff --git a/backend/src/Logitar.Master.Application/Accounts/Validators/AccountPhoneValidator.cs b/backend/src/Logitar.Master.Application/Accounts/Validators/AccountPhoneValidator.cs new file mode 100644 index 0000000..7f60579 --- /dev/null +++ b/backend/src/Logitar.Master.Application/Accounts/Validators/AccountPhoneValidator.cs @@ -0,0 +1,23 @@ +using FluentValidation; +using Logitar.Identity.Domain.Users; +using Logitar.Master.Contracts.Accounts; +using Logitar.Portal.Contracts.Users; + +namespace Logitar.Master.Application.Accounts.Validators; + +internal class AccountPhoneValidator : AbstractValidator +{ + public AccountPhoneValidator() + { + When(x => x.CountryCode != null, () => RuleFor(x => x.CountryCode).NotEmpty().Length(2)); + RuleFor(x => x.Number).NotEmpty().MaximumLength(20); + + RuleFor(x => x).Must(BeAValidPhone).WithErrorCode("PhoneValidator").WithMessage("'{PropertyName}' must be a valid phone."); + } + + private static bool BeAValidPhone(AccountPhone input) + { + Phone phone = new(input.CountryCode, input.Number, extension: null, e164Formatted: string.Empty); + return phone.IsValid(); + } +} diff --git a/backend/src/Logitar.Master.Application/Accounts/Validators/ChangePhoneValidator.cs b/backend/src/Logitar.Master.Application/Accounts/Validators/ChangePhoneValidator.cs new file mode 100644 index 0000000..8279910 --- /dev/null +++ b/backend/src/Logitar.Master.Application/Accounts/Validators/ChangePhoneValidator.cs @@ -0,0 +1,33 @@ +using FluentValidation; +using Logitar.Identity.Domain.Shared; +using Logitar.Master.Contracts.Accounts; + +namespace Logitar.Master.Application.Accounts.Validators; + +internal class ChangePhoneValidator : AbstractValidator +{ + public ChangePhoneValidator() + { + RuleFor(x => x.Locale).SetValidator(new LocaleValidator()); + + When(x => x.NewPhone != null, () => RuleFor(x => x.NewPhone!).SetValidator(new AccountPhoneValidator())); + When(x => x.OneTimePassword != null, () => RuleFor(x => x.OneTimePassword!).SetValidator(new OneTimePasswordValidator())); + + RuleFor(x => x).Must(BeAValidPayload).WithErrorCode(nameof(ChangePhoneValidator)) + .WithMessage(x => $"Exactly one of the following must be specified: {nameof(x.NewPhone)}, {nameof(x.OneTimePassword)}."); + } + + private static bool BeAValidPayload(ChangePhonePayload payload) + { + int count = 0; + if (payload.NewPhone != null) + { + count++; + } + if (payload.OneTimePassword != null) + { + count++; + } + return count == 1; + } +} diff --git a/backend/src/Logitar.Master.Application/Accounts/Validators/OneTimePasswordValidator.cs b/backend/src/Logitar.Master.Application/Accounts/Validators/OneTimePasswordValidator.cs new file mode 100644 index 0000000..0359602 --- /dev/null +++ b/backend/src/Logitar.Master.Application/Accounts/Validators/OneTimePasswordValidator.cs @@ -0,0 +1,13 @@ +using FluentValidation; +using Logitar.Master.Contracts.Accounts; + +namespace Logitar.Master.Application.Accounts.Validators; + +internal class OneTimePasswordValidator : AbstractValidator +{ + public OneTimePasswordValidator() + { + RuleFor(x => x.Id).NotEmpty(); + RuleFor(x => x.Code).NotEmpty(); + } +} diff --git a/backend/src/Logitar.Master.Contracts/Errors/InvalidCredentialsError.cs b/backend/src/Logitar.Master.Contracts/Errors/InvalidCredentialsError.cs deleted file mode 100644 index ea6cd7c..0000000 --- a/backend/src/Logitar.Master.Contracts/Errors/InvalidCredentialsError.cs +++ /dev/null @@ -1,10 +0,0 @@ -using Logitar.Portal.Contracts.Errors; - -namespace Logitar.Master.Contracts.Errors; - -public record InvalidCredentialsError : Error -{ - public InvalidCredentialsError() : base("InvalidCredentials", "The specified credentials did not match.") - { - } -} diff --git a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs index a116b7e..a23420d 100644 --- a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs +++ b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/OneTimePasswordExtensionsTests.cs @@ -267,4 +267,22 @@ public void TryGetPurpose_it_should_return_the_purpose_when_the_One_Time_Passwor password.CustomAttributes.Add(new("Purpose", Purpose)); Assert.Equal(Purpose, password.TryGetPurpose()); } + + [Fact(DisplayName = "TryGetUserId: it should return null when the One-Time Password does not have the custom attribute.")] + public void TryGetUserId_it_should_return_null_when_the_One_Time_Password_does_not_have_the_custom_attribute() + { + OneTimePassword oneTimePassword = new(); + Assert.Null(oneTimePassword.TryGetUserId()); + } + + [Fact(DisplayName = "TryGetUserId: it should return the user Id when the One Time Password has one.")] + public void TryGetUserId_it_should_return_the_user_Id_when_the_One_Time_Password_has_one() + { + Guid userId = Guid.NewGuid(); + + OneTimePassword oneTimePassword = new(); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("UserId", userId.ToString())); + + Assert.Equal(userId, oneTimePassword.TryGetUserId()); + } } diff --git a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs index efcb4a7..1aa02ab 100644 --- a/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs +++ b/backend/tests/Logitar.Master.Application.UnitTests/Accounts/UserExtensionsTests.cs @@ -93,15 +93,19 @@ public void SetMultiFactorAuthenticationMode_it_should_the_correct_custom_attrib Assert.Contains(payload.CustomAttributes, c => c.Key == nameof(MultiFactorAuthenticationMode) && c.Value == mfaMode.ToString()); } - [Fact(DisplayName = "ToPhone: it should return the correct phone.")] - public void ToPhone_it_should_return_the_correct_phone() + [Theory(DisplayName = "ToPhone: it should return the correct phone.")] + [InlineData("CA", "(514) 845-4636", "+15148454636")] + [InlineData("", "(514) 845-4636", "+15148454636")] + [InlineData(" ", " (514) 845-4636 ", "+15148454636")] + [InlineData(null, "(514) 845-4636", "+15148454636")] + public void ToPhone_it_should_return_the_correct_phone(string? countryCode, string number, string e164Formatted) { - AccountPhone phone = new("(514) 845-4636", "CA"); + AccountPhone phone = new(number, countryCode); Phone result = phone.ToPhone(); - Assert.Equal(phone.CountryCode, result.CountryCode); - Assert.Equal(phone.Number, result.Number); + Assert.Equal(phone.CountryCode?.CleanTrim(), result.CountryCode); + Assert.Equal(phone.Number.Trim(), result.Number); Assert.Null(result.Extension); - Assert.Equal("+15148454636", result.E164Formatted); + Assert.Equal(e164Formatted, result.E164Formatted); Assert.False(result.IsVerified); Assert.Null(result.VerifiedBy); Assert.Null(result.VerifiedOn); From 9772a8c350dc12086a346bf4860aef4366234411 Mon Sep 17 00:00:00 2001 From: Francis Pion Date: Tue, 30 Apr 2024 18:27:13 -0400 Subject: [PATCH 5/5] Implemented integration tests. --- .../Commands/ChangePhoneCommandHandler.cs | 2 +- .../Commands/ChangePhoneCommandTests.cs | 111 ++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 backend/tests/Logitar.Master.IntegrationTests/Application/Accounts/Commands/ChangePhoneCommandTests.cs diff --git a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs index da5713a..7c68ff7 100644 --- a/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs +++ b/backend/src/Logitar.Master.Application/Accounts/Commands/ChangePhoneCommandHandler.cs @@ -8,7 +8,7 @@ namespace Logitar.Master.Application.Accounts.Commands; -internal class ChangePhoneCommandHandler : IRequestHandler // TODO(fpion): integration tests +internal class ChangePhoneCommandHandler : IRequestHandler { private const string ContactVerificationPurpose = "ContactVerification"; private const string ContactVerificationTemplate = "ContactVerification{ContactType}"; diff --git a/backend/tests/Logitar.Master.IntegrationTests/Application/Accounts/Commands/ChangePhoneCommandTests.cs b/backend/tests/Logitar.Master.IntegrationTests/Application/Accounts/Commands/ChangePhoneCommandTests.cs new file mode 100644 index 0000000..36abb50 --- /dev/null +++ b/backend/tests/Logitar.Master.IntegrationTests/Application/Accounts/Commands/ChangePhoneCommandTests.cs @@ -0,0 +1,111 @@ +using FluentValidation.Results; +using Logitar.Master.Contracts.Accounts; +using Logitar.Portal.Contracts; +using Logitar.Portal.Contracts.Messages; +using Logitar.Portal.Contracts.Passwords; +using Logitar.Portal.Contracts.Users; +using Moq; + +namespace Logitar.Master.Application.Accounts.Commands; + +[Trait(Traits.Category, Categories.Integration)] +public class ChangePhoneCommandTests : IntegrationTests +{ + public ChangePhoneCommandTests() : base() + { + } + + [Fact(DisplayName = "It should send a One-Time Password text message.")] + public async Task It_should_send_a_One_Time_Password_text_message() + { + AccountPhone newPhone = new("(514) 845-4636", "CA"); + Phone phone = newPhone.ToPhone(); + + OneTimePassword oneTimePassword = new() + { + Id = Guid.NewGuid(), + Password = "123456" + }; + OneTimePasswordService.Setup(x => x.CreateAsync(It.Is(u => u.Id == Actor.Id), "ContactVerification", phone, CancellationToken)) + .ReturnsAsync(oneTimePassword); + + ChangePhonePayload payload = new(Faker.Locale, newPhone); + SentMessages sentMessages = new([Guid.NewGuid()]); + MessageService.Setup(x => x.SendAsync("ContactVerificationPhone", phone, + payload.Locale, It.Is>(v => v.Count == 1 && v["OneTimePassword"] == oneTimePassword.Password), CancellationToken) + ).ReturnsAsync(sentMessages); + + ChangePhoneCommand command = new(payload); + ChangePhoneResult result = await Pipeline.ExecuteAsync(command, CancellationToken); + + Assert.NotNull(result.OneTimePasswordValidation); + Assert.Equal(oneTimePassword.Id, result.OneTimePasswordValidation.OneTimePasswordId); + Assert.Equal(sentMessages.ToSentMessage(phone), result.OneTimePasswordValidation.SentMessage); + } + + [Fact(DisplayName = "It should throw InvalidOneTimePasswordUserException when the One Time Password was meant for another user.")] + public async Task It_should_throw_InvalidOneTimePasswordUserException_when_the_One_Time_Password_was_meant_for_another_user() + { + OneTimePassword oneTimePassword = new() + { + Id = Guid.NewGuid() + }; + oneTimePassword.CustomAttributes.Add(new CustomAttribute("UserId", Guid.Empty.ToString())); + OneTimePasswordPayload oneTimePasswordPayload = new(oneTimePassword.Id, "123456"); + OneTimePasswordService.Setup(x => x.ValidateAsync(oneTimePasswordPayload, "ContactVerification", CancellationToken)).ReturnsAsync(oneTimePassword); + + ChangePhonePayload payload = new(Faker.Locale, oneTimePasswordPayload); + ChangePhoneCommand command = new(payload); + var exception = await Assert.ThrowsAsync(async () => await Pipeline.ExecuteAsync(command, CancellationToken)); + Assert.Equal(oneTimePassword.Id, exception.OneTimePasswordId); + Assert.Equal(Guid.Empty, exception.ExpectedUserId); + Assert.Equal(Actor.Id, exception.ActualUserId); + } + + [Fact(DisplayName = "It should throw ValidationException when the payload is not valid.")] + public async Task It_should_throw_ValidationException_when_the_payload_is_not_valid() + { + ChangePhonePayload payload = new(Faker.Locale); + ChangePhoneCommand command = new(payload); + var exception = await Assert.ThrowsAsync(async () => await Pipeline.ExecuteAsync(command)); + + ValidationFailure error = Assert.Single(exception.Errors); + Assert.Equal("ChangePhoneValidator", error.ErrorCode); + Assert.Equal("Exactly one of the following must be specified: NewPhone, OneTimePassword.", error.ErrorMessage); + } + + [Fact(DisplayName = "It should update the phone of the user.")] + public async Task It_should_update_the_phone_of_the_user() + { + User user = new(Faker.Person.Email) + { + Id = Actor.Id, + Phone = new Phone("CA", "(514) 845-4636", extension: null, "+15148454636") + { + IsVerified = true + } + }; + UserService.Setup(x => x.UpdatePhoneAsync(Actor.Id, user.Phone, CancellationToken)).ReturnsAsync(user); + + OneTimePassword oneTimePassword = new() + { + Id = Guid.NewGuid() + }; + oneTimePassword.CustomAttributes.Add(new CustomAttribute("UserId", Actor.Id.ToString())); + Assert.NotNull(user.Phone.CountryCode); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneCountryCode", user.Phone.CountryCode)); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneNumber", user.Phone.Number)); + oneTimePassword.CustomAttributes.Add(new CustomAttribute("PhoneE164Formatted", user.Phone.E164Formatted)); + OneTimePasswordPayload oneTimePasswordPayload = new(oneTimePassword.Id, "123456"); + OneTimePasswordService.Setup(x => x.ValidateAsync(oneTimePasswordPayload, "ContactVerification", CancellationToken)).ReturnsAsync(oneTimePassword); + + ChangePhonePayload payload = new(Faker.Locale, oneTimePasswordPayload); + ChangePhoneCommand command = new(payload); + ChangePhoneResult result = await Pipeline.ExecuteAsync(command, CancellationToken); + + Assert.NotNull(result.UserProfile); + Assert.NotNull(result.UserProfile.Phone); + Assert.Equal(user.Phone.CountryCode, result.UserProfile.Phone.CountryCode); + Assert.Equal(user.Phone.Number, result.UserProfile.Phone.Number); + } +}