diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 241c28e6954b..fa7f69f0fc81 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -74,12 +74,12 @@ public async Task AcceptOrgUserByEmailTokenAsync(Guid organiza throw new BadRequestException("User invalid."); } - var tokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( - _orgUserInviteTokenDataFactory, emailToken, orgUser); + var tokenValidationError = OrgUserInviteTokenable.ValidateOrgUserInvite( + _orgUserInviteTokenDataFactory, emailToken, orgUser.Id, orgUser.Email); - if (!tokenValid) + if (tokenValidationError != null) { - throw new BadRequestException("Invalid token."); + throw new BadRequestException(tokenValidationError.ErrorMessage); } var existingOrgUserCount = await _organizationUserRepository.GetCountByOrganizationAsync( diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs index 5be7ed481f2a..5f0b7a15ab6a 100644 --- a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs @@ -37,18 +37,9 @@ public OrgUserInviteTokenable(OrganizationUser orgUser) : this() OrgUserEmail = orgUser?.Email; } - public bool TokenIsValid(OrganizationUser orgUser) - { - if (OrgUserId == default || OrgUserEmail == default || orgUser == null) - { - return false; - } - - return OrgUserId == orgUser.Id && - OrgUserEmail.Equals(orgUser.Email, StringComparison.InvariantCultureIgnoreCase); - } + public bool TokenIsValid(OrganizationUser? orgUser) => TokenIsValid(orgUser?.Id ?? default, orgUser?.Email); - public bool TokenIsValid(Guid orgUserId, string orgUserEmail) + public bool TokenIsValid(Guid orgUserId, string? orgUserEmail) { if (OrgUserId == default || OrgUserEmail == default || orgUserId == default || orgUserEmail == default) { @@ -63,22 +54,27 @@ public bool TokenIsValid(Guid orgUserId, string orgUserEmail) protected override bool TokenIsValid() => Identifier == TokenIdentifier && OrgUserId != default && !string.IsNullOrWhiteSpace(OrgUserEmail); + public static TokenableValidationError? ValidateOrgUserInvite( + IDataProtectorTokenFactory orgUserInviteTokenDataFactory, + string orgUserInviteToken, + Guid orgUserId, + string? orgUserEmail) => + orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) switch + { + true when decryptedToken.IsExpired => TokenableValidationError.ExpiringTokenables.Expired, + true when !(decryptedToken.Valid && decryptedToken.TokenIsValid(orgUserId, orgUserEmail)) => + TokenableValidationError.InvalidToken, + false => TokenableValidationError.InvalidToken, + _ => null + }; public static bool ValidateOrgUserInviteStringToken( IDataProtectorTokenFactory orgUserInviteTokenDataFactory, - string orgUserInviteToken, OrganizationUser orgUser) - { - return orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) - && decryptedToken.Valid - && decryptedToken.TokenIsValid(orgUser); - } + string orgUserInviteToken, OrganizationUser orgUser) => + ValidateOrgUserInvite(orgUserInviteTokenDataFactory, orgUserInviteToken, orgUser.Id, orgUser.Email) is null; public static bool ValidateOrgUserInviteStringToken( IDataProtectorTokenFactory orgUserInviteTokenDataFactory, - string orgUserInviteToken, Guid orgUserId, string orgUserEmail) - { - return orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) - && decryptedToken.Valid - && decryptedToken.TokenIsValid(orgUserId, orgUserEmail); - } + string orgUserInviteToken, Guid orgUserId, string orgUserEmail) => + ValidateOrgUserInvite(orgUserInviteTokenDataFactory, orgUserInviteToken, orgUserId, orgUserEmail) is null; } diff --git a/src/Core/Auth/Models/Business/Tokenables/TokenableValidationError.cs b/src/Core/Auth/Models/Business/Tokenables/TokenableValidationError.cs new file mode 100644 index 000000000000..961180dd4a6a --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/TokenableValidationError.cs @@ -0,0 +1,19 @@ +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public record TokenableValidationError +{ + public static readonly TokenableValidationError InvalidToken = new("Invalid token."); + + public string ErrorMessage { get; } + + private TokenableValidationError(string errorMessage) + { + ErrorMessage = errorMessage; + } + + public static class ExpiringTokenables + { + // Used by clients to show better error message on token expiration, adjust both strings as-needed + public static readonly TokenableValidationError Expired = new("Expired token."); + } +} diff --git a/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs b/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs index 823ec585ba0d..9170fb90e297 100644 --- a/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs +++ b/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs @@ -169,8 +169,10 @@ private void TryValidateOrgInviteToken(string orgInviteToken, Guid? orgUserId, U if (orgInviteTokenProvided && orgUserId.HasValue) { // We have token data so validate it - if (OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( - _orgUserInviteTokenDataFactory, orgInviteToken, orgUserId.Value, user.Email)) + var tokenValidationError = OrgUserInviteTokenable.ValidateOrgUserInvite( + _orgUserInviteTokenDataFactory, orgInviteToken, orgUserId.Value, user.Email); + + if (tokenValidationError == null) { return; } @@ -181,7 +183,7 @@ private void TryValidateOrgInviteToken(string orgInviteToken, Guid? orgUserId, U throw new BadRequestException(_disabledUserRegistrationExceptionMsg); } - throw new BadRequestException("Organization invite token is invalid."); + throw new BadRequestException(tokenValidationError.ErrorMessage); } // no token data or missing token data diff --git a/src/Core/Tokens/ExpiringTokenable.cs b/src/Core/Tokens/ExpiringTokenable.cs index 5e90a2406606..5aac7080fc41 100644 --- a/src/Core/Tokens/ExpiringTokenable.cs +++ b/src/Core/Tokens/ExpiringTokenable.cs @@ -12,7 +12,9 @@ public abstract class ExpiringTokenable : Tokenable /// Checks if the token is still within its valid duration and if its data is valid. /// For data validation, this property relies on the method. /// - public override bool Valid => ExpirationDate > DateTime.UtcNow && TokenIsValid(); + public override bool Valid => !IsExpired && TokenIsValid(); + + public bool IsExpired => ExpirationDate < DateTime.UtcNow; /// /// Validates that the token data properties are correct. diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 5cf660b90247..cd003b3ab07a 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -451,8 +451,39 @@ public async Task AcceptOrgUserByToken_ExpiredNewToken_ThrowsBadRequest( var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, newToken, _userService)); - Assert.Equal("Invalid token.", exception.Message); + Assert.Equal("Expired token.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByToken_InvalidNewToken_ThrowsBadRequest( + SutProvider sutProvider, + User user, OrganizationUser orgUser) + { + // Arrange + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order + // to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + + sutProvider.GetDependency() + .GetByIdAsync(orgUser.Id) + .Returns(Task.FromResult(orgUser)); + + // Must come after common mocks as they mutate the org user. + // Send a null org-user to force an invalid token result + _orgUserInviteTokenableFactory.CreateToken(orgUser).Returns(new OrgUserInviteTokenable(null!) + { + ExpirationDate = DateTime.UtcNow.AddDays(1), + }); + var newToken = CreateToken(orgUser); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, newToken, _userService)); + + Assert.Equal("Invalid token.", exception.Message); } [Theory] diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs index 866cdbfe2920..619bc8d8f136 100644 --- a/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs +++ b/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs @@ -2,6 +2,7 @@ using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Entities; using Bit.Core.Tokens; +using NSubstitute; using Xunit; @@ -257,6 +258,61 @@ public void FromToken_SerializedToken_PreservesOrgUserEmail(OrganizationUser org Assert.Equal(orgUser.Email, result.OrgUserEmail); } + // ValidateOrgUserInvite Parameters: + // bool tryUnprotectResult, bool useMatchingOrgUserId, bool useMatchingEmail, bool isExpired, string? expectedError + [Theory] + // TryUnprotect fails → "Invalid token." + [InlineData(false, true, true, false, "Invalid token.")] + // TryUnprotect succeeds, token is expired → "Expired token." + [InlineData(true, true, true, true, "Expired token.")] + // TryUnprotect succeeds, not expired, mismatched org user ID → "Invalid token." + [InlineData(true, false, true, false, "Invalid token.")] + // TryUnprotect succeeds, not expired, mismatched email → "Invalid token." + [InlineData(true, true, false, false, "Invalid token.")] + // TryUnprotect succeeds, not expired, both mismatched → "Invalid token." + [InlineData(true, false, false, false, "Invalid token.")] + // TryUnprotect succeeds, not expired, matching ID and email → null (valid) + [InlineData(true, true, true, false, null)] + public void ValidateOrgUserInvite_ReturnsExpectedErrors( + bool tryUnprotectResult, + bool useMatchingOrgUserId, + bool useMatchingEmail, + bool isExpired, + string? expectedError) + { + // Arrange + var orgUserId = Guid.NewGuid(); + var orgUserEmail = "test@example.com"; + + var tokenable = new OrgUserInviteTokenable + { + Identifier = OrgUserInviteTokenable.TokenIdentifier, + OrgUserId = orgUserId, + OrgUserEmail = orgUserEmail, + ExpirationDate = isExpired + ? DateTime.UtcNow.AddDays(-1) + : DateTime.UtcNow.AddDays(1) + }; + + var factory = Substitute.For>(); + factory.TryUnprotect(Arg.Any(), out Arg.Any()) + .Returns(callInfo => + { + callInfo[1] = tryUnprotectResult ? tokenable : null; + return tryUnprotectResult; + }); + + var inputOrgUserId = useMatchingOrgUserId ? orgUserId : Guid.NewGuid(); + var inputEmail = useMatchingEmail ? orgUserEmail : "wrong@example.com"; + + // Act + var result = OrgUserInviteTokenable.ValidateOrgUserInvite( + factory, "test-token", inputOrgUserId, inputEmail); + + // Assert + Assert.Equal(expectedError, result?.ErrorMessage); + } + private bool TimesAreCloseEnough(DateTime time1, DateTime time2, TimeSpan tolerance) { return (time1 - time2).Duration() < tolerance; diff --git a/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs b/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs index 5631fd7f5442..2881575769d1 100644 --- a/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/Registration/RegisterUserCommandTests.cs @@ -451,7 +451,7 @@ public async Task RegisterUserViaOrganizationInviteToken_MissingOrInvalidOrgInvi return true; }); - expectedErrorMessage = "Organization invite token is invalid."; + expectedErrorMessage = "Invalid token."; break; case "nullOrgInviteToken": orgInviteToken = null;