diff --git a/src/Core/AssemblyInfo.cs b/src/Core/AssemblyInfo.cs index a5edd1a27b..66f5b58ef8 100644 --- a/src/Core/AssemblyInfo.cs +++ b/src/Core/AssemblyInfo.cs @@ -1,3 +1,4 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Core.Test")] +[assembly: InternalsVisibleTo("Identity.IntegrationTest")] diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/Enums/SendGrantValidatorResultTypes.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/Enums/SendGrantValidatorResultTypes.cs deleted file mode 100644 index 343c15bd30..0000000000 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/Enums/SendGrantValidatorResultTypes.cs +++ /dev/null @@ -1,11 +0,0 @@ -namespace Bit.Identity.IdentityServer.RequestValidators.SendAccess.Enums; - -/// -/// These control the results of the SendGrantValidator. -/// -internal enum SendGrantValidatorResultTypes -{ - ValidSendGuid, - MissingSendId, - InvalidSendId -} diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/Enums/SendPasswordValidatorResultTypes.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/Enums/SendPasswordValidatorResultTypes.cs deleted file mode 100644 index 1950ca2978..0000000000 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/Enums/SendPasswordValidatorResultTypes.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace Bit.Identity.IdentityServer.RequestValidators.SendAccess.Enums; - -/// -/// These control the results of the SendPasswordValidator. -/// -internal enum SendPasswordValidatorResultTypes -{ - RequestPasswordDoesNotMatch -} diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs new file mode 100644 index 0000000000..952f4146ed --- /dev/null +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessConstants.cs @@ -0,0 +1,73 @@ +using Duende.IdentityServer.Validation; + +namespace Bit.Identity.IdentityServer.RequestValidators.SendAccess; + +/// +/// String constants for the Send Access user feature +/// +public static class SendAccessConstants +{ + /// + /// A catch all error type for send access related errors. Used mainly in the + /// + public const string SendAccessError = "send_access_error_type"; + public static class TokenRequest + { + /// + /// used to fetch Send from database. + /// + public const string SendId = "send_id"; + /// + /// used to validate Send protected passwords + /// + public const string ClientB64HashedPassword = "password_hash_b64"; + /// + /// email used to see if email is associated with the Send + /// + public const string Email = "email"; + /// + /// Otp code sent to email associated with the Send + /// + public const string Otp = "otp"; + } + + public static class GrantValidatorResults + { + /// + /// The sendId is valid and the request is well formed. + /// + public const string ValidSendGuid = "valid_send_guid"; + /// + /// The sendId is missing from the request. + /// + public const string MissingSendId = "send_id_required"; + /// + /// The sendId is invalid, does not match a known send. + /// + public const string InvalidSendId = "send_id_invalid"; + } + + public static class PasswordValidatorResults + { + /// + /// The passwordHashB64 does not match the send's password hash. + /// + public const string RequestPasswordDoesNotMatch = "password_hash_b64_invalid"; + /// + /// The passwordHashB64 is missing from the request. + /// + public const string RequestPasswordIsRequired = "password_hash_b64_required"; + } + + public static class EmailOtpValidatorResults + { + /// + /// Represents the error code indicating that an email address is required. + /// + public const string EmailRequired = "email_required"; + /// + /// Represents the status indicating that both email and OTP are required, and the OTP has been sent. + /// + public const string EmailOtpSent = "email_and_otp_required_otp_sent"; + } +} diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs index 020b3ec5d4..7cfa2acd2a 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendAccessGrantValidator.cs @@ -6,7 +6,6 @@ using Bit.Core.Tools.Models.Data; using Bit.Core.Tools.SendFeatures.Queries.Interfaces; using Bit.Core.Utilities; using Bit.Identity.IdentityServer.Enums; -using Bit.Identity.IdentityServer.RequestValidators.SendAccess.Enums; using Duende.IdentityServer.Models; using Duende.IdentityServer.Validation; @@ -20,11 +19,11 @@ public class SendAccessGrantValidator( { string IExtensionGrantValidator.GrantType => CustomGrantTypes.SendAccess; - private static readonly Dictionary - _sendGrantValidatorErrors = new() + private static readonly Dictionary + _sendGrantValidatorErrorDescriptions = new() { - { SendGrantValidatorResultTypes.MissingSendId, "send_id is required." }, - { SendGrantValidatorResultTypes.InvalidSendId, "send_id is invalid." } + { SendAccessConstants.GrantValidatorResults.MissingSendId, $"{SendAccessConstants.TokenRequest.SendId} is required." }, + { SendAccessConstants.GrantValidatorResults.InvalidSendId, $"{SendAccessConstants.TokenRequest.SendId} is invalid." } }; @@ -38,7 +37,7 @@ public class SendAccessGrantValidator( } var (sendIdGuid, result) = GetRequestSendId(context); - if (result != SendGrantValidatorResultTypes.ValidSendGuid) + if (result != SendAccessConstants.GrantValidatorResults.ValidSendGuid) { context.Result = BuildErrorResult(result); return; @@ -55,7 +54,7 @@ public class SendAccessGrantValidator( // We should only map to password or email + OTP protected. // If user submits password guess for a falsely protected send, then we will return invalid password. // If user submits email + OTP guess for a falsely protected send, then we will return email sent, do not actually send an email. - context.Result = BuildErrorResult(SendGrantValidatorResultTypes.InvalidSendId); + context.Result = BuildErrorResult(SendAccessConstants.GrantValidatorResults.InvalidSendId); return; case NotAuthenticated: @@ -64,7 +63,7 @@ public class SendAccessGrantValidator( return; case ResourcePassword rp: - // TODO PM-22675: Validate if the password is correct. + // Validate if the password is correct, or if we need to respond with a 400 stating a password has is required context.Result = _sendPasswordRequestValidator.ValidateSendPassword(context, rp, sendIdGuid); return; case EmailOtp eo: @@ -84,15 +83,15 @@ public class SendAccessGrantValidator( /// /// request context /// a parsed sendId Guid and success result or a Guid.Empty and error type otherwise - private static (Guid, SendGrantValidatorResultTypes) GetRequestSendId(ExtensionGrantValidationContext context) + private static (Guid, string) GetRequestSendId(ExtensionGrantValidationContext context) { var request = context.Request.Raw; - var sendId = request.Get("send_id"); + var sendId = request.Get(SendAccessConstants.TokenRequest.SendId); // if the sendId is null then the request is the wrong shape and the request is invalid if (sendId == null) { - return (Guid.Empty, SendGrantValidatorResultTypes.MissingSendId); + return (Guid.Empty, SendAccessConstants.GrantValidatorResults.MissingSendId); } // the send_id is not null so the request is the correct shape, so we will attempt to parse it try @@ -102,13 +101,13 @@ public class SendAccessGrantValidator( // Guid.Empty indicates an invalid send_id return invalid grant if (sendGuid == Guid.Empty) { - return (Guid.Empty, SendGrantValidatorResultTypes.InvalidSendId); + return (Guid.Empty, SendAccessConstants.GrantValidatorResults.InvalidSendId); } - return (sendGuid, SendGrantValidatorResultTypes.ValidSendGuid); + return (sendGuid, SendAccessConstants.GrantValidatorResults.ValidSendGuid); } catch { - return (Guid.Empty, SendGrantValidatorResultTypes.InvalidSendId); + return (Guid.Empty, SendAccessConstants.GrantValidatorResults.InvalidSendId); } } @@ -117,18 +116,26 @@ public class SendAccessGrantValidator( /// /// The error type. /// The error result. - private static GrantValidationResult BuildErrorResult(SendGrantValidatorResultTypes error) + private static GrantValidationResult BuildErrorResult(string error) { return error switch { // Request is the wrong shape - SendGrantValidatorResultTypes.MissingSendId => new GrantValidationResult( + SendAccessConstants.GrantValidatorResults.MissingSendId => new GrantValidationResult( TokenRequestErrors.InvalidRequest, - errorDescription: _sendGrantValidatorErrors[SendGrantValidatorResultTypes.MissingSendId]), + errorDescription: _sendGrantValidatorErrorDescriptions[SendAccessConstants.GrantValidatorResults.MissingSendId], + new Dictionary + { + { SendAccessConstants.SendAccessError, SendAccessConstants.GrantValidatorResults.MissingSendId} + }), // Request is correct shape but data is bad - SendGrantValidatorResultTypes.InvalidSendId => new GrantValidationResult( + SendAccessConstants.GrantValidatorResults.InvalidSendId => new GrantValidationResult( TokenRequestErrors.InvalidGrant, - errorDescription: _sendGrantValidatorErrors[SendGrantValidatorResultTypes.InvalidSendId]), + errorDescription: _sendGrantValidatorErrorDescriptions[SendAccessConstants.GrantValidatorResults.InvalidSendId], + new Dictionary + { + { SendAccessConstants.SendAccessError, SendAccessConstants.GrantValidatorResults.InvalidSendId } + }), // should never get here _ => new GrantValidationResult(TokenRequestErrors.InvalidRequest) }; diff --git a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendPasswordRequestValidator.cs b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendPasswordRequestValidator.cs index 194a0aaa5c..3449b4cb56 100644 --- a/src/Identity/IdentityServer/RequestValidators/SendAccess/SendPasswordRequestValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/SendAccess/SendPasswordRequestValidator.cs @@ -3,7 +3,6 @@ using Bit.Core.Identity; using Bit.Core.KeyManagement.Sends; using Bit.Core.Tools.Models.Data; using Bit.Identity.IdentityServer.Enums; -using Bit.Identity.IdentityServer.RequestValidators.SendAccess.Enums; using Duende.IdentityServer.Models; using Duende.IdentityServer.Validation; @@ -16,31 +15,44 @@ public class SendPasswordRequestValidator(ISendPasswordHasher sendPasswordHasher /// /// static object that contains the error messages for the SendPasswordRequestValidator. /// - private static Dictionary _sendPasswordValidatorErrors = new() + private static readonly Dictionary _sendPasswordValidatorErrorDescriptions = new() { - { SendPasswordValidatorResultTypes.RequestPasswordDoesNotMatch, "Request Password hash is invalid." } + { SendAccessConstants.PasswordValidatorResults.RequestPasswordDoesNotMatch, $"{SendAccessConstants.TokenRequest.ClientB64HashedPassword} is invalid." }, + { SendAccessConstants.PasswordValidatorResults.RequestPasswordIsRequired, $"{SendAccessConstants.TokenRequest.ClientB64HashedPassword} is required." } }; public GrantValidationResult ValidateSendPassword(ExtensionGrantValidationContext context, ResourcePassword resourcePassword, Guid sendId) { var request = context.Request.Raw; - var clientHashedPassword = request.Get("password_hash"); + var clientHashedPassword = request.Get(SendAccessConstants.TokenRequest.ClientB64HashedPassword); - if (string.IsNullOrEmpty(clientHashedPassword)) + // It is an invalid request _only_ if the passwordHashB64 is missing which indicated bad shape. + if (clientHashedPassword == null) { + // Request is the wrong shape and doesn't contain a passwordHashB64 field. return new GrantValidationResult( TokenRequestErrors.InvalidRequest, - errorDescription: _sendPasswordValidatorErrors[SendPasswordValidatorResultTypes.RequestPasswordDoesNotMatch]); + errorDescription: _sendPasswordValidatorErrorDescriptions[SendAccessConstants.PasswordValidatorResults.RequestPasswordIsRequired], + new Dictionary + { + { SendAccessConstants.SendAccessError, SendAccessConstants.PasswordValidatorResults.RequestPasswordIsRequired } + }); } + // _sendPasswordHasher.PasswordHashMatches checks for an empty string so no need to do it before we make the call. var hashMatches = _sendPasswordHasher.PasswordHashMatches( resourcePassword.Hash, clientHashedPassword); if (!hashMatches) { + // Request is the correct shape but the passwordHashB64 doesn't match, hash could be empty. return new GrantValidationResult( TokenRequestErrors.InvalidGrant, - errorDescription: _sendPasswordValidatorErrors[SendPasswordValidatorResultTypes.RequestPasswordDoesNotMatch]); + errorDescription: _sendPasswordValidatorErrorDescriptions[SendAccessConstants.PasswordValidatorResults.RequestPasswordDoesNotMatch], + new Dictionary + { + { SendAccessConstants.SendAccessError, SendAccessConstants.PasswordValidatorResults.RequestPasswordDoesNotMatch } + }); } return BuildSendPasswordSuccessResult(sendId); diff --git a/test/Identity.IntegrationTest/RequestValidation/SendAccessGrantValidatorIntegrationTests.cs b/test/Identity.IntegrationTest/RequestValidation/SendAccessGrantValidatorIntegrationTests.cs index f27da6e02e..4b8c267861 100644 --- a/test/Identity.IntegrationTest/RequestValidation/SendAccessGrantValidatorIntegrationTests.cs +++ b/test/Identity.IntegrationTest/RequestValidation/SendAccessGrantValidatorIntegrationTests.cs @@ -8,6 +8,7 @@ using Bit.Core.Utilities; using Bit.Identity.IdentityServer.Enums; using Bit.Identity.IdentityServer.RequestValidators.SendAccess; using Bit.IntegrationTestCommon.Factories; +using Duende.IdentityModel; using Duende.IdentityServer.Validation; using NSubstitute; using Xunit; @@ -96,8 +97,8 @@ public class SendAccessGrantValidatorIntegrationTests(IdentityApplicationFactory }).CreateClient(); var requestBody = new FormUrlEncodedContent([ - new KeyValuePair("grant_type", CustomGrantTypes.SendAccess), - new KeyValuePair("client_id", BitwardenClient.Send) + new KeyValuePair(OidcConstants.TokenRequest.GrantType, CustomGrantTypes.SendAccess), + new KeyValuePair(OidcConstants.TokenRequest.ClientId, BitwardenClient.Send) ]); // Act @@ -105,8 +106,8 @@ public class SendAccessGrantValidatorIntegrationTests(IdentityApplicationFactory // Assert var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("invalid_request", content); - Assert.Contains("send_id is required", content); + Assert.Contains(OidcConstants.TokenErrors.InvalidRequest, content); + Assert.Contains($"{SendAccessConstants.TokenRequest.SendId} is required", content); } [Fact] @@ -245,16 +246,16 @@ public class SendAccessGrantValidatorIntegrationTests(IdentityApplicationFactory var sendIdBase64 = CoreHelpers.Base64UrlEncode(sendId.ToByteArray()); var parameters = new List> { - new("grant_type", CustomGrantTypes.SendAccess), - new("client_id", BitwardenClient.Send ), - new("scope", ApiScopes.ApiSendAccess), + new(OidcConstants.TokenRequest.GrantType, CustomGrantTypes.SendAccess), + new(OidcConstants.TokenRequest.ClientId, BitwardenClient.Send ), + new(OidcConstants.TokenRequest.Scope, ApiScopes.ApiSendAccess), new("deviceType", ((int)DeviceType.FirefoxBrowser).ToString()), - new("send_id", sendIdBase64) + new(SendAccessConstants.TokenRequest.SendId, sendIdBase64) }; if (!string.IsNullOrEmpty(password)) { - parameters.Add(new("password_hash", password)); + parameters.Add(new(SendAccessConstants.TokenRequest.ClientB64HashedPassword, password)); } if (!string.IsNullOrEmpty(emailOtp) && !string.IsNullOrEmpty(sendEmail)) diff --git a/test/Identity.IntegrationTest/RequestValidation/SendPasswordRequestValidatorIntegrationTests.cs b/test/Identity.IntegrationTest/RequestValidation/SendPasswordRequestValidatorIntegrationTests.cs new file mode 100644 index 0000000000..232adb6884 --- /dev/null +++ b/test/Identity.IntegrationTest/RequestValidation/SendPasswordRequestValidatorIntegrationTests.cs @@ -0,0 +1,209 @@ +using Bit.Core.Enums; +using Bit.Core.IdentityServer; +using Bit.Core.KeyManagement.Sends; +using Bit.Core.Services; +using Bit.Core.Tools.Models.Data; +using Bit.Core.Tools.SendFeatures.Queries.Interfaces; +using Bit.Core.Utilities; +using Bit.Identity.IdentityServer.Enums; +using Bit.Identity.IdentityServer.RequestValidators.SendAccess; +using Bit.IntegrationTestCommon.Factories; +using Duende.IdentityModel; +using NSubstitute; +using Xunit; + +namespace Bit.Identity.IntegrationTest.RequestValidation; + +public class SendPasswordRequestValidatorIntegrationTests : IClassFixture +{ + private readonly IdentityApplicationFactory _factory; + + public SendPasswordRequestValidatorIntegrationTests(IdentityApplicationFactory factory) + { + _factory = factory; + } + + [Fact] + public async Task SendAccess_PasswordProtectedSend_ValidPassword_ReturnsAccessToken() + { + // Arrange + var sendId = Guid.NewGuid(); + var passwordHash = "stored-password-hash"; + var clientPasswordHash = "client-password-hash"; + + var client = _factory.WithWebHostBuilder(builder => + { + builder.ConfigureServices(services => + { + // Enable feature flag + var featureService = Substitute.For(); + featureService.IsEnabled(Arg.Any()).Returns(true); + services.AddSingleton(featureService); + + // Mock send authentication query + var sendAuthQuery = Substitute.For(); + sendAuthQuery.GetAuthenticationMethod(sendId) + .Returns(new ResourcePassword(passwordHash)); + services.AddSingleton(sendAuthQuery); + + // Mock password hasher to return true for matching passwords + var passwordHasher = Substitute.For(); + passwordHasher.PasswordHashMatches(passwordHash, clientPasswordHash) + .Returns(true); + services.AddSingleton(passwordHasher); + }); + }).CreateClient(); + + var requestBody = CreateTokenRequestBody(sendId, clientPasswordHash); + + // Act + var response = await client.PostAsync("/connect/token", requestBody); + + // Assert + Assert.True(response.IsSuccessStatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains(OidcConstants.TokenResponse.AccessToken, content); + Assert.Contains("bearer", content.ToLower()); + } + + [Fact] + public async Task SendAccess_PasswordProtectedSend_InvalidPassword_ReturnsInvalidGrant() + { + // Arrange + var sendId = Guid.NewGuid(); + var passwordHash = "stored-password-hash"; + var wrongClientPasswordHash = "wrong-client-password-hash"; + + var client = _factory.WithWebHostBuilder(builder => + { + builder.ConfigureServices(services => + { + var featureService = Substitute.For(); + featureService.IsEnabled(Arg.Any()).Returns(true); + services.AddSingleton(featureService); + + var sendAuthQuery = Substitute.For(); + sendAuthQuery.GetAuthenticationMethod(sendId) + .Returns(new ResourcePassword(passwordHash)); + services.AddSingleton(sendAuthQuery); + + // Mock password hasher to return false for wrong passwords + var passwordHasher = Substitute.For(); + passwordHasher.PasswordHashMatches(passwordHash, wrongClientPasswordHash) + .Returns(false); + services.AddSingleton(passwordHasher); + }); + }).CreateClient(); + + var requestBody = CreateTokenRequestBody(sendId, wrongClientPasswordHash); + + // Act + var response = await client.PostAsync("/connect/token", requestBody); + + // Assert + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains(OidcConstants.TokenErrors.InvalidGrant, content); + Assert.Contains($"{SendAccessConstants.TokenRequest.ClientB64HashedPassword} is invalid", content); + } + + [Fact] + public async Task SendAccess_PasswordProtectedSend_MissingPassword_ReturnsInvalidRequest() + { + // Arrange + var sendId = Guid.NewGuid(); + var passwordHash = "stored-password-hash"; + + var client = _factory.WithWebHostBuilder(builder => + { + builder.ConfigureServices(services => + { + var featureService = Substitute.For(); + featureService.IsEnabled(Arg.Any()).Returns(true); + services.AddSingleton(featureService); + + var sendAuthQuery = Substitute.For(); + sendAuthQuery.GetAuthenticationMethod(sendId) + .Returns(new ResourcePassword(passwordHash)); + services.AddSingleton(sendAuthQuery); + + var passwordHasher = Substitute.For(); + services.AddSingleton(passwordHasher); + }); + }).CreateClient(); + + var requestBody = CreateTokenRequestBody(sendId); // No password + + // Act + var response = await client.PostAsync("/connect/token", requestBody); + + // Assert + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains(OidcConstants.TokenErrors.InvalidRequest, content); + Assert.Contains($"{SendAccessConstants.TokenRequest.ClientB64HashedPassword} is required", content); + } + + /// + /// When the password has is empty or whitespace it doesn't get passed to the server when the request is made. + /// This leads to an invalid request error since the absence of the password hash is considered a malformed request. + /// In the case that the passwordB64Hash _is_ empty or whitespace it would be an invalid grant since the request + /// has the correct shape. + /// + [Fact] + public async Task SendAccess_PasswordProtectedSend_EmptyPassword_ReturnsInvalidRequest() + { + // Arrange + var sendId = Guid.NewGuid(); + var passwordHash = "stored-password-hash"; + + var client = _factory.WithWebHostBuilder(builder => + { + builder.ConfigureServices(services => + { + var featureService = Substitute.For(); + featureService.IsEnabled(Arg.Any()).Returns(true); + services.AddSingleton(featureService); + + var sendAuthQuery = Substitute.For(); + sendAuthQuery.GetAuthenticationMethod(sendId) + .Returns(new ResourcePassword(passwordHash)); + services.AddSingleton(sendAuthQuery); + + // Mock password hasher to return false for empty passwords + var passwordHasher = Substitute.For(); + passwordHasher.PasswordHashMatches(passwordHash, string.Empty) + .Returns(false); + services.AddSingleton(passwordHasher); + }); + }).CreateClient(); + + var requestBody = CreateTokenRequestBody(sendId, string.Empty); + + // Act + var response = await client.PostAsync("/connect/token", requestBody); + + // Assert + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains(OidcConstants.TokenErrors.InvalidRequest, content); + Assert.Contains($"{SendAccessConstants.TokenRequest.ClientB64HashedPassword} is required", content); + } + + private static FormUrlEncodedContent CreateTokenRequestBody(Guid sendId, string passwordHash = null) + { + var sendIdBase64 = CoreHelpers.Base64UrlEncode(sendId.ToByteArray()); + var parameters = new List> + { + new(OidcConstants.TokenRequest.GrantType, CustomGrantTypes.SendAccess), + new(OidcConstants.TokenRequest.ClientId, BitwardenClient.Send), + new(SendAccessConstants.TokenRequest.SendId, sendIdBase64), + new(OidcConstants.TokenRequest.Scope, ApiScopes.ApiSendAccess), + new("deviceType", "10") + }; + + if (passwordHash != null) + { + parameters.Add(new KeyValuePair(SendAccessConstants.TokenRequest.ClientB64HashedPassword, passwordHash)); + } + + return new FormUrlEncodedContent(parameters); + } +} diff --git a/test/Identity.Test/IdentityServer/SendAccessGrantValidatorTests.cs b/test/Identity.Test/IdentityServer/SendAccessGrantValidatorTests.cs index ca45558f19..c3d422c51a 100644 --- a/test/Identity.Test/IdentityServer/SendAccessGrantValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/SendAccessGrantValidatorTests.cs @@ -65,7 +65,7 @@ public class SendAccessGrantValidatorTests // Assert Assert.Equal(OidcConstants.TokenErrors.InvalidRequest, context.Result.Error); - Assert.Equal("send_id is required.", context.Result.ErrorDescription); + Assert.Equal($"{SendAccessConstants.TokenRequest.SendId} is required.", context.Result.ErrorDescription); } [Theory, BitAutoData] @@ -84,7 +84,7 @@ public class SendAccessGrantValidatorTests tokenRequest.Raw = CreateTokenRequestBody(Guid.Empty); // To preserve the CreateTokenRequestBody method for more general usage we over write the sendId - tokenRequest.Raw.Set("send_id", "invalid-guid-format"); + tokenRequest.Raw.Set(SendAccessConstants.TokenRequest.SendId, "invalid-guid-format"); context.Request = tokenRequest; // Act @@ -92,7 +92,7 @@ public class SendAccessGrantValidatorTests // Assert Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, context.Result.Error); - Assert.Equal("send_id is invalid.", context.Result.ErrorDescription); + Assert.Equal($"{SendAccessConstants.TokenRequest.SendId} is invalid.", context.Result.ErrorDescription); } [Theory, BitAutoData] @@ -111,7 +111,7 @@ public class SendAccessGrantValidatorTests // Assert Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, context.Result.Error); - Assert.Equal("send_id is invalid.", context.Result.ErrorDescription); + Assert.Equal($"{SendAccessConstants.TokenRequest.SendId} is invalid.", context.Result.ErrorDescription); } [Theory, BitAutoData] @@ -135,7 +135,7 @@ public class SendAccessGrantValidatorTests // Assert Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, context.Result.Error); - Assert.Equal("send_id is invalid.", context.Result.ErrorDescription); + Assert.Equal($"{SendAccessConstants.TokenRequest.SendId} is invalid.", context.Result.ErrorDescription); } [Theory, BitAutoData] @@ -297,37 +297,28 @@ public class SendAccessGrantValidatorTests var rawRequestParameters = new NameValueCollection { - { "grant_type", CustomGrantTypes.SendAccess }, - { "client_id", BitwardenClient.Send }, - { "scope", ApiScopes.ApiSendAccess }, + { OidcConstants.TokenRequest.GrantType, CustomGrantTypes.SendAccess }, + { OidcConstants.TokenRequest.ClientId, BitwardenClient.Send }, + { OidcConstants.TokenRequest.Scope, ApiScopes.ApiSendAccess }, { "deviceType", ((int)DeviceType.FirefoxBrowser).ToString() }, - { "send_id", sendIdBase64 } + { SendAccessConstants.TokenRequest.SendId, sendIdBase64 } }; if (passwordHash != null) { - rawRequestParameters.Add("password_hash", passwordHash); + rawRequestParameters.Add(SendAccessConstants.TokenRequest.ClientB64HashedPassword, passwordHash); } if (sendEmail != null) { - rawRequestParameters.Add("send_email", sendEmail); + rawRequestParameters.Add(SendAccessConstants.TokenRequest.Email, sendEmail); } if (otpCode != null && sendEmail != null) { - rawRequestParameters.Add("otp_code", otpCode); + rawRequestParameters.Add(SendAccessConstants.TokenRequest.Otp, otpCode); } return rawRequestParameters; } - - // we need a list of sendAuthentication methods to test against since we cannot create new objects in the BitAutoData - public static Dictionary SendAuthenticationMethods => new() - { - { "NeverAuthenticate", new NeverAuthenticate() }, // Send doesn't exist or is deleted - { "NotAuthenticated", new NotAuthenticated() }, // Public send, no auth needed - // TODO: PM-22675 - {"ResourcePassword", new ResourcePassword("clientHashedPassword")}; // Password protected send - // TODO: PM-22678 - {"EmailOtp", new EmailOtp(["emailOtp@test.dev"]}; // Email + OTP protected send - }; } diff --git a/test/Identity.Test/IdentityServer/SendPasswordRequestValidatorTests.cs b/test/Identity.Test/IdentityServer/SendPasswordRequestValidatorTests.cs new file mode 100644 index 0000000000..a776a70178 --- /dev/null +++ b/test/Identity.Test/IdentityServer/SendPasswordRequestValidatorTests.cs @@ -0,0 +1,297 @@ +using System.Collections.Specialized; +using Bit.Core.Auth.UserFeatures.SendAccess; +using Bit.Core.Enums; +using Bit.Core.Identity; +using Bit.Core.IdentityServer; +using Bit.Core.KeyManagement.Sends; +using Bit.Core.Tools.Models.Data; +using Bit.Core.Utilities; +using Bit.Identity.IdentityServer.Enums; +using Bit.Identity.IdentityServer.RequestValidators.SendAccess; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Duende.IdentityModel; +using Duende.IdentityServer.Validation; +using NSubstitute; +using Xunit; + +namespace Bit.Identity.Test.IdentityServer; + +[SutProviderCustomize] +public class SendPasswordRequestValidatorTests +{ + [Theory, BitAutoData] + public void ValidateSendPassword_MissingPasswordHash_ReturnsInvalidRequest( + SutProvider sutProvider, + [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + ResourcePassword resourcePassword, + Guid sendId) + { + // Arrange + tokenRequest.Raw = CreateValidatedTokenRequest(sendId); + + var context = new ExtensionGrantValidationContext + { + Request = tokenRequest + }; + + // Act + var result = sutProvider.Sut.ValidateSendPassword(context, resourcePassword, sendId); + + // Assert + Assert.True(result.IsError); + Assert.Equal(OidcConstants.TokenErrors.InvalidRequest, result.Error); + Assert.Equal($"{SendAccessConstants.TokenRequest.ClientB64HashedPassword} is required.", result.ErrorDescription); + + // Verify password hasher was not called + sutProvider.GetDependency() + .DidNotReceive() + .PasswordHashMatches(Arg.Any(), Arg.Any()); + } + + [Theory, BitAutoData] + public void ValidateSendPassword_PasswordHashMismatch_ReturnsInvalidGrant( + SutProvider sutProvider, + [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + ResourcePassword resourcePassword, + Guid sendId, + string clientPasswordHash) + { + // Arrange + tokenRequest.Raw = CreateValidatedTokenRequest(sendId, clientPasswordHash); + + var context = new ExtensionGrantValidationContext + { + Request = tokenRequest + }; + + sutProvider.GetDependency() + .PasswordHashMatches(resourcePassword.Hash, clientPasswordHash) + .Returns(false); + + // Act + var result = sutProvider.Sut.ValidateSendPassword(context, resourcePassword, sendId); + + // Assert + Assert.True(result.IsError); + Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, result.Error); + Assert.Equal($"{SendAccessConstants.TokenRequest.ClientB64HashedPassword} is invalid.", result.ErrorDescription); + + // Verify password hasher was called with correct parameters + sutProvider.GetDependency() + .Received(1) + .PasswordHashMatches(resourcePassword.Hash, clientPasswordHash); + } + + [Theory, BitAutoData] + public void ValidateSendPassword_PasswordHashMatches_ReturnsSuccess( + SutProvider sutProvider, + [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + ResourcePassword resourcePassword, + Guid sendId, + string clientPasswordHash) + { + // Arrange + tokenRequest.Raw = CreateValidatedTokenRequest(sendId, clientPasswordHash); + + var context = new ExtensionGrantValidationContext + { + Request = tokenRequest + }; + + sutProvider.GetDependency() + .PasswordHashMatches(resourcePassword.Hash, clientPasswordHash) + .Returns(true); + + // Act + var result = sutProvider.Sut.ValidateSendPassword(context, resourcePassword, sendId); + + // Assert + Assert.False(result.IsError); + + var sub = result.Subject; + Assert.Equal(sendId, sub.GetSendId()); + + // Verify claims + Assert.Contains(sub.Claims, c => c.Type == Claims.SendId && c.Value == sendId.ToString()); + Assert.Contains(sub.Claims, c => c.Type == Claims.Type && c.Value == IdentityClientType.Send.ToString()); + + // Verify password hasher was called + sutProvider.GetDependency() + .Received(1) + .PasswordHashMatches(resourcePassword.Hash, clientPasswordHash); + } + + [Theory, BitAutoData] + public void ValidateSendPassword_EmptyPasswordHash_CallsPasswordHasher( + SutProvider sutProvider, + [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + ResourcePassword resourcePassword, + Guid sendId) + { + // Arrange + tokenRequest.Raw = CreateValidatedTokenRequest(sendId, string.Empty); + + var context = new ExtensionGrantValidationContext + { + Request = tokenRequest + }; + + sutProvider.GetDependency() + .PasswordHashMatches(resourcePassword.Hash, string.Empty) + .Returns(false); + + // Act + var result = sutProvider.Sut.ValidateSendPassword(context, resourcePassword, sendId); + + // Assert + Assert.True(result.IsError); + Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, result.Error); + + // Verify password hasher was called with empty string + sutProvider.GetDependency() + .Received(1) + .PasswordHashMatches(resourcePassword.Hash, string.Empty); + } + + [Theory, BitAutoData] + public void ValidateSendPassword_WhitespacePasswordHash_CallsPasswordHasher( + SutProvider sutProvider, + [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + ResourcePassword resourcePassword, + Guid sendId) + { + // Arrange + var whitespacePassword = " "; + tokenRequest.Raw = CreateValidatedTokenRequest(sendId, whitespacePassword); + + var context = new ExtensionGrantValidationContext + { + Request = tokenRequest + }; + + sutProvider.GetDependency() + .PasswordHashMatches(resourcePassword.Hash, whitespacePassword) + .Returns(false); + + // Act + var result = sutProvider.Sut.ValidateSendPassword(context, resourcePassword, sendId); + + // Assert + Assert.True(result.IsError); + + // Verify password hasher was called with whitespace string + sutProvider.GetDependency() + .Received(1) + .PasswordHashMatches(resourcePassword.Hash, whitespacePassword); + } + + [Theory, BitAutoData] + public void ValidateSendPassword_MultiplePasswordHashParameters_ReturnsInvalidGrant( + SutProvider sutProvider, + [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + ResourcePassword resourcePassword, + Guid sendId) + { + // Arrange + var firstPassword = "first-password"; + var secondPassword = "second-password"; + tokenRequest.Raw = CreateValidatedTokenRequest(sendId, firstPassword, secondPassword); + + var context = new ExtensionGrantValidationContext + { + Request = tokenRequest + }; + + sutProvider.GetDependency() + .PasswordHashMatches(resourcePassword.Hash, firstPassword) + .Returns(true); + + // Act + var result = sutProvider.Sut.ValidateSendPassword(context, resourcePassword, sendId); + + // Assert + Assert.True(result.IsError); + Assert.Equal(OidcConstants.TokenErrors.InvalidGrant, result.Error); + + // Verify password hasher was called with first value + sutProvider.GetDependency() + .Received(1) + .PasswordHashMatches(resourcePassword.Hash, $"{firstPassword},{secondPassword}"); + } + + [Theory, BitAutoData] + public void ValidateSendPassword_SuccessResult_ContainsCorrectClaims( + SutProvider sutProvider, + [AutoFixture.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, + ResourcePassword resourcePassword, + Guid sendId, + string clientPasswordHash) + { + // Arrange + tokenRequest.Raw = CreateValidatedTokenRequest(sendId, clientPasswordHash); + + var context = new ExtensionGrantValidationContext + { + Request = tokenRequest + }; + + sutProvider.GetDependency() + .PasswordHashMatches(Arg.Any(), Arg.Any()) + .Returns(true); + + // Act + var result = sutProvider.Sut.ValidateSendPassword(context, resourcePassword, sendId); + + // Assert + Assert.False(result.IsError); + var sub = result.Subject; + + var sendIdClaim = sub.Claims.FirstOrDefault(c => c.Type == Claims.SendId); + Assert.NotNull(sendIdClaim); + Assert.Equal(sendId.ToString(), sendIdClaim.Value); + + var typeClaim = sub.Claims.FirstOrDefault(c => c.Type == Claims.Type); + Assert.NotNull(typeClaim); + Assert.Equal(IdentityClientType.Send.ToString(), typeClaim.Value); + } + + [Fact] + public void Constructor_WithValidParameters_CreatesInstance() + { + // Arrange + var sendPasswordHasher = Substitute.For(); + + // Act + var validator = new SendPasswordRequestValidator(sendPasswordHasher); + + // Assert + Assert.NotNull(validator); + } + + private static NameValueCollection CreateValidatedTokenRequest( + Guid sendId, + params string[] passwordHash) + { + var sendIdBase64 = CoreHelpers.Base64UrlEncode(sendId.ToByteArray()); + + var rawRequestParameters = new NameValueCollection + { + { OidcConstants.TokenRequest.GrantType, CustomGrantTypes.SendAccess }, + { OidcConstants.TokenRequest.ClientId, BitwardenClient.Send }, + { OidcConstants.TokenRequest.Scope, ApiScopes.ApiSendAccess }, + { "device_type", ((int)DeviceType.FirefoxBrowser).ToString() }, + { SendAccessConstants.TokenRequest.SendId, sendIdBase64 } + }; + + if (passwordHash != null && passwordHash.Length > 0) + { + foreach (var hash in passwordHash) + { + rawRequestParameters.Add(SendAccessConstants.TokenRequest.ClientB64HashedPassword, hash); + } + } + + return rawRequestParameters; + } +}