diff --git a/.github/workflows/test-database.yml b/.github/workflows/test-database.yml index 134e96b339..6700438c2a 100644 --- a/.github/workflows/test-database.yml +++ b/.github/workflows/test-database.yml @@ -107,7 +107,7 @@ jobs: run: 'dotnet ef database update --connection "$CONN_STR" -- --GlobalSettings:MySql:ConnectionString="$CONN_STR"' env: CONN_STR: "server=localhost;uid=root;pwd=SET_A_PASSWORD_HERE_123;database=vault_dev;Allow User Variables=true" - + - name: Migrate MariaDB working-directory: "util/MySqlMigrations" run: 'dotnet ef database update --connection "$CONN_STR" -- --GlobalSettings:MySql:ConnectionString="$CONN_STR"' @@ -237,7 +237,7 @@ jobs: run: | if grep -q "" "report.xml"; then echo - echo "Migrations are out of sync with sqlproj!" + echo "Migration files are not in sync with the files in the Sql project. Review to make sure that any stored procedures / other db changes match with the stored procedures in the Sql project." exit 1 else echo "Report looks good" diff --git a/src/Api/Controllers/DevicesController.cs b/src/Api/Controllers/DevicesController.cs index f55b30eb27..aab898cd62 100644 --- a/src/Api/Controllers/DevicesController.cs +++ b/src/Api/Controllers/DevicesController.cs @@ -6,7 +6,6 @@ using Bit.Api.Models.Response; using Bit.Core.Auth.Models.Api.Request; using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; @@ -70,11 +69,17 @@ public class DevicesController : Controller } [HttpGet("")] - public async Task> Get() + public async Task> Get() { - ICollection devices = await _deviceRepository.GetManyByUserIdAsync(_userService.GetProperUserId(User).Value); - var responses = devices.Select(d => new DeviceResponseModel(d)); - return new ListResponseModel(responses); + var devicesWithPendingAuthData = await _deviceRepository.GetManyByUserIdWithDeviceAuth(_userService.GetProperUserId(User).Value); + + // Convert from DeviceAuthDetails to DeviceAuthRequestResponseModel + var deviceAuthRequestResponseList = devicesWithPendingAuthData + .Select(DeviceAuthRequestResponseModel.From) + .ToList(); + + var response = new ListResponseModel(deviceAuthRequestResponseList); + return response; } [HttpPost("")] diff --git a/src/Core/Auth/Enums/AuthRequestType.cs b/src/Core/Auth/Enums/AuthRequestType.cs index fff75e8d22..0a3bf4b3bc 100644 --- a/src/Core/Auth/Enums/AuthRequestType.cs +++ b/src/Core/Auth/Enums/AuthRequestType.cs @@ -1,5 +1,12 @@ namespace Bit.Core.Auth.Enums; +/** + * The type of auth request. + * + * Note: + * Used by the Device_ReadActiveWithPendingAuthRequestsByUserId.sql stored procedure. + * If the enum changes be aware of this reference. + */ public enum AuthRequestType : byte { AuthenticateAndUnlock = 0, diff --git a/src/Core/Auth/Models/Api/Response/DeviceAuthRequestResponseModel.cs b/src/Core/Auth/Models/Api/Response/DeviceAuthRequestResponseModel.cs new file mode 100644 index 0000000000..3cfea51ee3 --- /dev/null +++ b/src/Core/Auth/Models/Api/Response/DeviceAuthRequestResponseModel.cs @@ -0,0 +1,51 @@ +using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.Utilities; +using Bit.Core.Enums; +using Bit.Core.Models.Api; + +namespace Bit.Core.Auth.Models.Api.Response; + +public class DeviceAuthRequestResponseModel : ResponseModel +{ + public DeviceAuthRequestResponseModel() + : base("device") { } + + public static DeviceAuthRequestResponseModel From(DeviceAuthDetails deviceAuthDetails) + { + var converted = new DeviceAuthRequestResponseModel + { + Id = deviceAuthDetails.Id, + Name = deviceAuthDetails.Name, + Type = deviceAuthDetails.Type, + Identifier = deviceAuthDetails.Identifier, + CreationDate = deviceAuthDetails.CreationDate, + IsTrusted = deviceAuthDetails.IsTrusted() + }; + + if (deviceAuthDetails.AuthRequestId != null && deviceAuthDetails.AuthRequestCreatedAt != null) + { + converted.DevicePendingAuthRequest = new PendingAuthRequest + { + Id = (Guid)deviceAuthDetails.AuthRequestId, + CreationDate = (DateTime)deviceAuthDetails.AuthRequestCreatedAt + }; + } + + return converted; + } + + public Guid Id { get; set; } + public string Name { get; set; } + public DeviceType Type { get; set; } + public string Identifier { get; set; } + public DateTime CreationDate { get; set; } + public bool IsTrusted { get; set; } + + public PendingAuthRequest DevicePendingAuthRequest { get; set; } + + public class PendingAuthRequest + { + public Guid Id { get; set; } + public DateTime CreationDate { get; set; } + } +} diff --git a/src/Core/Auth/Models/Data/DeviceAuthDetails.cs b/src/Core/Auth/Models/Data/DeviceAuthDetails.cs new file mode 100644 index 0000000000..ef242705f4 --- /dev/null +++ b/src/Core/Auth/Models/Data/DeviceAuthDetails.cs @@ -0,0 +1,81 @@ +using Bit.Core.Auth.Utilities; +using Bit.Core.Entities; +using Bit.Core.Enums; + +namespace Bit.Core.Auth.Models.Data; + +public class DeviceAuthDetails : Device +{ + public bool IsTrusted { get; set; } + public Guid? AuthRequestId { get; set; } + public DateTime? AuthRequestCreatedAt { get; set; } + + /** + * Constructor for EF response. + */ + public DeviceAuthDetails( + Device device, + Guid? authRequestId, + DateTime? authRequestCreationDate) + { + if (device == null) + { + throw new ArgumentNullException(nameof(device)); + } + + Id = device.Id; + Name = device.Name; + Type = device.Type; + Identifier = device.Identifier; + CreationDate = device.CreationDate; + IsTrusted = device.IsTrusted(); + AuthRequestId = authRequestId; + AuthRequestCreatedAt = authRequestCreationDate; + } + + /** + * Constructor for dapper response. + * Note: if the authRequestId or authRequestCreationDate is null it comes back as + * an empty guid and a min value for datetime. That could change if the stored + * procedure runs on a different kind of db. + */ + public DeviceAuthDetails( + Guid id, + Guid userId, + string name, + short type, + string identifier, + string pushToken, + DateTime creationDate, + DateTime revisionDate, + string encryptedUserKey, + string encryptedPublicKey, + string encryptedPrivateKey, + bool active, + Guid authRequestId, + DateTime authRequestCreationDate) + { + Id = id; + Name = name; + Type = (DeviceType)type; + Identifier = identifier; + CreationDate = creationDate; + IsTrusted = new Device + { + Id = id, + UserId = userId, + Name = name, + Type = (DeviceType)type, + Identifier = identifier, + PushToken = pushToken, + RevisionDate = revisionDate, + EncryptedUserKey = encryptedUserKey, + EncryptedPublicKey = encryptedPublicKey, + EncryptedPrivateKey = encryptedPrivateKey, + Active = active + }.IsTrusted(); + AuthRequestId = authRequestId != Guid.Empty ? authRequestId : null; + AuthRequestCreatedAt = + authRequestCreationDate != DateTime.MinValue ? authRequestCreationDate : null; + } +} diff --git a/src/Core/Auth/Models/Data/EmergencyAccessDetails.cs b/src/Core/Auth/Models/Data/EmergencyAccessDetails.cs index 3c925d1a80..15ccad9cb1 100644 --- a/src/Core/Auth/Models/Data/EmergencyAccessDetails.cs +++ b/src/Core/Auth/Models/Data/EmergencyAccessDetails.cs @@ -1,5 +1,4 @@ - -using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Entities; namespace Bit.Core.Auth.Models.Data; diff --git a/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs b/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs index 89851fce23..834d2722cc 100644 --- a/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs +++ b/src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs @@ -23,7 +23,6 @@ namespace Bit.Core.Auth.UserFeatures.Registration.Implementations; public class RegisterUserCommand : IRegisterUserCommand { - private readonly IGlobalSettings _globalSettings; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IPolicyRepository _policyRepository; diff --git a/src/Core/Repositories/IDeviceRepository.cs b/src/Core/Repositories/IDeviceRepository.cs index c5d14a0945..c9809c1de6 100644 --- a/src/Core/Repositories/IDeviceRepository.cs +++ b/src/Core/Repositories/IDeviceRepository.cs @@ -1,4 +1,5 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; #nullable enable @@ -10,5 +11,9 @@ public interface IDeviceRepository : IRepository Task GetByIdentifierAsync(string identifier); Task GetByIdentifierAsync(string identifier, Guid userId); Task> GetManyByUserIdAsync(Guid userId); + // DeviceAuthDetails is passed back to decouple the response model from the + // repository in case more fields are ever added to the details response for + // other requests. + Task> GetManyByUserIdWithDeviceAuth(Guid userId); Task ClearPushTokenAsync(Guid id); } diff --git a/src/Core/Settings/IGlobalSettings.cs b/src/Core/Settings/IGlobalSettings.cs index 02d151ed95..afe35ed34b 100644 --- a/src/Core/Settings/IGlobalSettings.cs +++ b/src/Core/Settings/IGlobalSettings.cs @@ -24,5 +24,7 @@ public interface IGlobalSettings IPasswordlessAuthSettings PasswordlessAuth { get; set; } IDomainVerificationSettings DomainVerification { get; set; } ILaunchDarklySettings LaunchDarkly { get; set; } + string DatabaseProvider { get; set; } + GlobalSettings.SqlSettings SqlServer { get; set; } string DevelopmentDirectory { get; set; } } diff --git a/src/Infrastructure.Dapper/Repositories/DeviceRepository.cs b/src/Infrastructure.Dapper/Repositories/DeviceRepository.cs index 7216d87f57..4abf4a4649 100644 --- a/src/Infrastructure.Dapper/Repositories/DeviceRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/DeviceRepository.cs @@ -1,4 +1,5 @@ using System.Data; +using Bit.Core.Auth.Models.Data; using Bit.Core.Entities; using Bit.Core.Repositories; using Bit.Core.Settings; @@ -11,9 +12,13 @@ namespace Bit.Infrastructure.Dapper.Repositories; public class DeviceRepository : Repository, IDeviceRepository { + private readonly IGlobalSettings _globalSettings; + public DeviceRepository(GlobalSettings globalSettings) : this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString) - { } + { + _globalSettings = globalSettings; + } public DeviceRepository(string connectionString, string readOnlyConnectionString) : base(connectionString, readOnlyConnectionString) @@ -76,6 +81,24 @@ public class DeviceRepository : Repository, IDeviceRepository } } + public async Task> GetManyByUserIdWithDeviceAuth(Guid userId) + { + var expirationMinutes = _globalSettings.PasswordlessAuth.UserRequestExpiration.TotalMinutes; + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + $"[{Schema}].[{Table}_ReadActiveWithPendingAuthRequestsByUserId]", + new + { + UserId = userId, + ExpirationMinutes = expirationMinutes + }, + commandType: CommandType.StoredProcedure); + + return results.ToList(); + } + } + public async Task ClearPushTokenAsync(Guid id) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.Dapper/Repositories/Repository.cs b/src/Infrastructure.Dapper/Repositories/Repository.cs index fd37b611d0..43bffb3598 100644 --- a/src/Infrastructure.Dapper/Repositories/Repository.cs +++ b/src/Infrastructure.Dapper/Repositories/Repository.cs @@ -51,7 +51,7 @@ public abstract class Repository : BaseRepository, IRepository var parameters = new DynamicParameters(); parameters.AddDynamicParams(obj); parameters.Add("Id", obj.Id, direction: ParameterDirection.InputOutput); - var results = await connection.ExecuteAsync( + await connection.ExecuteAsync( $"[{Schema}].[{Table}_Create]", parameters, commandType: CommandType.StoredProcedure); @@ -64,7 +64,7 @@ public abstract class Repository : BaseRepository, IRepository { using (var connection = new SqlConnection(ConnectionString)) { - var results = await connection.ExecuteAsync( + await connection.ExecuteAsync( $"[{Schema}].[{Table}_Update]", obj, commandType: CommandType.StoredProcedure); diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/Queries/DeviceWithPendingAuthByUserIdQuery.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/Queries/DeviceWithPendingAuthByUserIdQuery.cs new file mode 100644 index 0000000000..5ab6d498e3 --- /dev/null +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/Queries/DeviceWithPendingAuthByUserIdQuery.cs @@ -0,0 +1,38 @@ +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Data; +using Bit.Infrastructure.EntityFramework.Repositories; + +namespace Bit.Infrastructure.EntityFramework.Auth.Repositories.Queries; + +public class DeviceWithPendingAuthByUserIdQuery +{ + public IQueryable GetQuery( + DatabaseContext dbContext, + Guid userId, + int expirationMinutes) + { + var devicesWithAuthQuery = ( + from device in dbContext.Devices + where device.UserId == userId && device.Active + select new + { + device, + authRequest = + ( + from authRequest in dbContext.AuthRequests + where authRequest.RequestDeviceIdentifier == device.Identifier + where authRequest.Type == AuthRequestType.AuthenticateAndUnlock || authRequest.Type == AuthRequestType.Unlock + where authRequest.Approved == null + where authRequest.UserId == userId + where authRequest.CreationDate.AddMinutes(expirationMinutes) > DateTime.UtcNow + orderby authRequest.CreationDate descending + select authRequest + ).First() + }).Select(deviceWithAuthRequest => new DeviceAuthDetails( + deviceWithAuthRequest.device, + deviceWithAuthRequest.authRequest.Id, + deviceWithAuthRequest.authRequest.CreationDate)); + + return devicesWithAuthQuery; + } +} diff --git a/src/Infrastructure.EntityFramework/Repositories/DeviceRepository.cs b/src/Infrastructure.EntityFramework/Repositories/DeviceRepository.cs index da82427cbb..ad31d0fb8b 100644 --- a/src/Infrastructure.EntityFramework/Repositories/DeviceRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/DeviceRepository.cs @@ -1,5 +1,8 @@ using AutoMapper; +using Bit.Core.Auth.Models.Data; using Bit.Core.Repositories; +using Bit.Core.Settings; +using Bit.Infrastructure.EntityFramework.Auth.Repositories.Queries; using Bit.Infrastructure.EntityFramework.Models; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; @@ -10,9 +13,17 @@ namespace Bit.Infrastructure.EntityFramework.Repositories; public class DeviceRepository : Repository, IDeviceRepository { - public DeviceRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) + private readonly IGlobalSettings _globalSettings; + + public DeviceRepository( + IServiceScopeFactory serviceScopeFactory, + IMapper mapper, + IGlobalSettings globalSettings + ) : base(serviceScopeFactory, mapper, (DatabaseContext context) => context.Devices) - { } + { + _globalSettings = globalSettings; + } public async Task ClearPushTokenAsync(Guid id) { @@ -69,4 +80,15 @@ public class DeviceRepository : Repository, return Mapper.Map>(devices); } } + + public async Task> GetManyByUserIdWithDeviceAuth(Guid userId) + { + var expirationMinutes = (int)_globalSettings.PasswordlessAuth.UserRequestExpiration.TotalMinutes; + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var query = new DeviceWithPendingAuthByUserIdQuery(); + return await query.GetQuery(dbContext, userId, expirationMinutes).ToListAsync(); + } + } } diff --git a/src/Sql/Auth/dbo/Stored Procedures/Device_ReadActiveWithPendingAuthRequestsByUserId.sql b/src/Sql/Auth/dbo/Stored Procedures/Device_ReadActiveWithPendingAuthRequestsByUserId.sql new file mode 100644 index 0000000000..015d0f7c1f --- /dev/null +++ b/src/Sql/Auth/dbo/Stored Procedures/Device_ReadActiveWithPendingAuthRequestsByUserId.sql @@ -0,0 +1,27 @@ +CREATE PROCEDURE [dbo].[Device_ReadActiveWithPendingAuthRequestsByUserId] + @UserId UNIQUEIDENTIFIER, + @ExpirationMinutes INT +AS +BEGIN + SET NOCOUNT ON; + + SELECT + D.*, + AR.Id as AuthRequestId, + AR.CreationDate as AuthRequestCreationDate + FROM dbo.DeviceView D + LEFT JOIN ( + SELECT TOP 1 -- Take only the top record sorted by auth request creation date + Id, + CreationDate, + RequestDeviceIdentifier + FROM dbo.AuthRequestView + WHERE Type IN (0, 1) -- Include only AuthenticateAndUnlock and Unlock types, excluding Admin Approval (type 2) + AND CreationDate >= DATEADD(MINUTE, -@ExpirationMinutes, GETUTCDATE()) -- Ensure the request hasn't expired + AND Approved IS NULL -- Include only requests that haven't been acknowledged or approved + ORDER BY CreationDate DESC + ) AR ON D.Identifier = AR.RequestDeviceIdentifier + WHERE + D.UserId = @UserId + AND D.Active = 1; -- Include only active devices +END; diff --git a/test/Api.Test/Auth/Controllers/DevicesControllerTests.cs b/test/Api.Test/Auth/Controllers/DevicesControllerTests.cs new file mode 100644 index 0000000000..3dcf2016c4 --- /dev/null +++ b/test/Api.Test/Auth/Controllers/DevicesControllerTests.cs @@ -0,0 +1,88 @@ +using Bit.Api.Controllers; +using Bit.Api.Models.Response; +using Bit.Core.Auth.Models.Api.Response; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Settings; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.Auth.Controllers; + +public class DevicesControllerTest +{ + private readonly IDeviceRepository _deviceRepositoryMock; + private readonly IDeviceService _deviceServiceMock; + private readonly IUserService _userServiceMock; + private readonly IUserRepository _userRepositoryMock; + private readonly ICurrentContext _currentContextMock; + private readonly IGlobalSettings _globalSettingsMock; + private readonly ILogger _loggerMock; + private readonly DevicesController _sut; + + public DevicesControllerTest() + { + _deviceRepositoryMock = Substitute.For(); + _deviceServiceMock = Substitute.For(); + _userServiceMock = Substitute.For(); + _userRepositoryMock = Substitute.For(); + _currentContextMock = Substitute.For(); + _loggerMock = Substitute.For>(); + + _sut = new DevicesController( + _deviceRepositoryMock, + _deviceServiceMock, + _userServiceMock, + _userRepositoryMock, + _currentContextMock, + _loggerMock); + } + + [Fact] + public async Task Get_ReturnsExpectedResult() + { + // Arrange + var userId = Guid.Parse("AD89E6F8-4E84-4CFE-A978-256CC0DBF974"); + + var authDateTimeResponse = new DateTime(2024, 12, 9, 12, 0, 0); + var devicesWithPendingAuthData = new List + { + new ( + new Device + { + Id = Guid.Parse("B3136B10-7818-444F-B05B-4D7A9B8C48BF"), + UserId = userId, + Name = "chrome", + Type = DeviceType.ChromeBrowser, + Identifier = Guid.Parse("811E9254-F77C-48C8-AF0A-A181943F5708").ToString() + }, + Guid.Parse("E09D6943-D574-49E5-AC85-C3F12B4E019E"), + authDateTimeResponse) + }; + + _userServiceMock.GetProperUserId(Arg.Any()).Returns(userId); + _deviceRepositoryMock.GetManyByUserIdWithDeviceAuth(userId).Returns(devicesWithPendingAuthData); + + // Act + var result = await _sut.Get(); + + // Assert + Assert.NotNull(result); + Assert.IsType>(result); + } + + [Fact] + public async Task Get_ThrowsException_WhenUserIdIsInvalid() + { + // Arrange + _userServiceMock.GetProperUserId(Arg.Any()).Returns((Guid?)null); + + // Act & Assert + await Assert.ThrowsAsync(() => _sut.Get()); + } +} diff --git a/test/Infrastructure.EFIntegration.Test/AutoFixture/DeviceFixtures.cs b/test/Infrastructure.EFIntegration.Test/AutoFixture/DeviceFixtures.cs index da5b5b7676..0ac3881511 100644 --- a/test/Infrastructure.EFIntegration.Test/AutoFixture/DeviceFixtures.cs +++ b/test/Infrastructure.EFIntegration.Test/AutoFixture/DeviceFixtures.cs @@ -2,7 +2,9 @@ using AutoFixture.Kernel; using Bit.Core.Entities; using Bit.Core.Test.AutoFixture.UserFixtures; +using Bit.Infrastructure.EFIntegration.Test.Auth.AutoFixture; using Bit.Infrastructure.EFIntegration.Test.AutoFixture.Relays; +using Bit.Infrastructure.EntityFramework.Auth.Repositories; using Bit.Infrastructure.EntityFramework.Repositories; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -39,8 +41,10 @@ internal class EfDevice : ICustomization fixture.Customizations.Add(new GlobalSettingsBuilder()); fixture.Customizations.Add(new DeviceBuilder()); fixture.Customizations.Add(new UserBuilder()); + fixture.Customizations.Add(new AuthRequestBuilder()); fixture.Customizations.Add(new EfRepositoryListBuilder()); fixture.Customizations.Add(new EfRepositoryListBuilder()); + fixture.Customizations.Add(new EfRepositoryListBuilder()); } } diff --git a/test/Infrastructure.EFIntegration.Test/Repositories/DeviceRepositoryTests.cs b/test/Infrastructure.EFIntegration.Test/Repositories/DeviceRepositoryTests.cs index 078fed0469..cc914d9aae 100644 --- a/test/Infrastructure.EFIntegration.Test/Repositories/DeviceRepositoryTests.cs +++ b/test/Infrastructure.EFIntegration.Test/Repositories/DeviceRepositoryTests.cs @@ -11,9 +11,13 @@ namespace Bit.Infrastructure.EFIntegration.Test.Repositories; public class DeviceRepositoryTests { [CiSkippedTheory, EfDeviceAutoData] - public async Task CreateAsync_Works_DataMatches(Device device, User user, - DeviceCompare equalityComparer, List suts, - List efUserRepos, SqlRepo.DeviceRepository sqlDeviceRepo, + public async Task CreateAsync_Works_DataMatches( + Device device, + User user, + DeviceCompare equalityComparer, + List suts, + List efUserRepos, + SqlRepo.DeviceRepository sqlDeviceRepo, SqlRepo.UserRepository sqlUserRepo) { var savedDevices = new List(); @@ -40,7 +44,6 @@ public class DeviceRepositoryTests savedDevices.Add(savedSqlDevice); var distinctItems = savedDevices.Distinct(equalityComparer); - Assert.True(!distinctItems.Skip(1).Any()); + Assert.False(distinctItems.Skip(1).Any()); } - } diff --git a/test/Infrastructure.IntegrationTest/Auth/Repositories/AuthRequestRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Auth/Repositories/AuthRequestRepositoryTests.cs index 9fddb571b9..8cd8cb607c 100644 --- a/test/Infrastructure.IntegrationTest/Auth/Repositories/AuthRequestRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/Auth/Repositories/AuthRequestRepositoryTests.cs @@ -8,9 +8,9 @@ namespace Bit.Infrastructure.IntegrationTest.Auth.Repositories; public class AuthRequestRepositoryTests { - private readonly static TimeSpan _userRequestExpiration = TimeSpan.FromMinutes(15); - private readonly static TimeSpan _adminRequestExpiration = TimeSpan.FromDays(6); - private readonly static TimeSpan _afterAdminApprovalExpiration = TimeSpan.FromHours(12); + private static readonly TimeSpan _userRequestExpiration = TimeSpan.FromMinutes(15); + private static readonly TimeSpan _adminRequestExpiration = TimeSpan.FromDays(6); + private static readonly TimeSpan _afterAdminApprovalExpiration = TimeSpan.FromHours(12); [DatabaseTheory, DatabaseData] public async Task DeleteExpiredAsync_Works( @@ -25,11 +25,11 @@ public class AuthRequestRepositoryTests SecurityStamp = "stamp", }); - // A user auth request type that has passed it's expiration time, should be deleted. + // A user auth request type that has passed its expiration time, should be deleted. var userExpiredAuthRequest = await authRequestRepository.CreateAsync( CreateAuthRequest(user.Id, AuthRequestType.AuthenticateAndUnlock, CreateExpiredDate(_userRequestExpiration))); - // An AdminApproval request that hasn't had any action taken on it and has passed it's expiration time, should be deleted. + // An AdminApproval request that hasn't had any action taken on it and has passed its expiration time, should be deleted. var adminApprovalExpiredAuthRequest = await authRequestRepository.CreateAsync( CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, CreateExpiredDate(_adminRequestExpiration))); @@ -37,7 +37,7 @@ public class AuthRequestRepositoryTests var adminApprovedExpiredAuthRequest = await authRequestRepository.CreateAsync( CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, DateTime.UtcNow.AddDays(-6), true, CreateExpiredDate(_afterAdminApprovalExpiration))); - // An AdminApproval request that was rejected within it's allowed lifetime but has no gone past it's expiration time, should be deleted. + // An AdminApproval request that was rejected within its allowed lifetime but has not gone past its expiration time, should be deleted. var adminRejectedExpiredAuthRequest = await authRequestRepository.CreateAsync( CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, CreateExpiredDate(_adminRequestExpiration), false, DateTime.UtcNow.AddHours(-1))); @@ -45,7 +45,7 @@ public class AuthRequestRepositoryTests var notExpiredUserAuthRequest = await authRequestRepository.CreateAsync( CreateAuthRequest(user.Id, AuthRequestType.Unlock, DateTime.UtcNow.AddMinutes(-1))); - // An AdminApproval AuthRequest that was create 6 days 23 hours 59 minutes 59 seconds ago which is right on the edge of still being valid + // An AdminApproval AuthRequest that was created 6 days 23 hours 59 minutes 59 seconds ago which is right on the edge of still being valid var notExpiredAdminApprovalRequest = await authRequestRepository.CreateAsync( CreateAuthRequest(user.Id, AuthRequestType.AdminApproval, DateTime.UtcNow.Add(new TimeSpan(days: 6, hours: 23, minutes: 59, seconds: 59)))); diff --git a/test/Infrastructure.IntegrationTest/Auth/Repositories/DeviceRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Auth/Repositories/DeviceRepositoryTests.cs new file mode 100644 index 0000000000..a9eec23194 --- /dev/null +++ b/test/Infrastructure.IntegrationTest/Auth/Repositories/DeviceRepositoryTests.cs @@ -0,0 +1,191 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Xunit; + +namespace Bit.Infrastructure.IntegrationTest.Auth.Repositories; + +public class DeviceRepositoryTests +{ + [DatabaseTheory] + [DatabaseData] + public async Task GetManyByUserIdWithDeviceAuth_Works_ReturnsExpectedResults( + IDeviceRepository sutRepository, + IUserRepository userRepository, + IAuthRequestRepository authRequestRepository) + { + // Arrange + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{Guid.NewGuid()}@email.com", + ApiKey = "TEST", + SecurityStamp = "stamp", + }); + + var device = await sutRepository.CreateAsync(new Device + { + Active = true, + Name = "chrome-test", + UserId = user.Id, + Type = DeviceType.ChromeBrowser, + Identifier = Guid.NewGuid().ToString(), + }); + + var staleAuthRequest = await authRequestRepository.CreateAsync(new AuthRequest + { + ResponseDeviceId = null, + Approved = null, + Type = AuthRequestType.AuthenticateAndUnlock, + OrganizationId = null, + UserId = user.Id, + RequestIpAddress = ":1", + RequestDeviceIdentifier = device.Identifier, + AccessCode = "AccessCode_1234", + PublicKey = "PublicKey_1234" + }); + staleAuthRequest.CreationDate = DateTime.UtcNow.AddMinutes(-10); + await authRequestRepository.ReplaceAsync(staleAuthRequest); + + var freshAuthRequest = await authRequestRepository.CreateAsync(new AuthRequest + { + ResponseDeviceId = null, + Approved = null, + Type = AuthRequestType.AuthenticateAndUnlock, + OrganizationId = null, + UserId = user.Id, + RequestIpAddress = ":1", + RequestDeviceIdentifier = device.Identifier, + AccessCode = "AccessCode_1234", + PublicKey = "PublicKey_1234", + Key = "Key_1234", + MasterPasswordHash = "MasterPasswordHash_1234" + }); + + // Act + var response = await sutRepository.GetManyByUserIdWithDeviceAuth(user.Id); + + // Assert + Assert.NotNull(response.First().AuthRequestId); + Assert.NotNull(response.First().AuthRequestCreatedAt); + Assert.Equal(response.First().AuthRequestId, freshAuthRequest.Id); + } + + [DatabaseTheory] + [DatabaseData] + public async Task GetManyByUserIdWithDeviceAuth_WorksWithNoAuthRequestAndMultipleDevices_ReturnsExpectedResults( + IDeviceRepository sutRepository, + IUserRepository userRepository) + { + // Arrange + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{Guid.NewGuid()}@email.com", + ApiKey = "TEST", + SecurityStamp = "stamp", + }); + + await sutRepository.CreateAsync(new Device + { + Active = true, + Name = "chrome-test", + UserId = user.Id, + Type = DeviceType.ChromeBrowser, + Identifier = Guid.NewGuid().ToString(), + }); + + await sutRepository.CreateAsync(new Device + { + Active = true, + Name = "macos-test", + UserId = user.Id, + Type = DeviceType.MacOsDesktop, + Identifier = Guid.NewGuid().ToString(), + }); + + // Act + var response = await sutRepository.GetManyByUserIdWithDeviceAuth(user.Id); + + // Assert + Assert.NotNull(response.First()); + Assert.Null(response.First().AuthRequestId); + Assert.True(response.Count == 2); + } + + [DatabaseTheory] + [DatabaseData] + public async Task GetManyByUserIdWithDeviceAuth_FailsToRespondWithAnyAuthData_ReturnsExpectedResults( + IDeviceRepository sutRepository, + IUserRepository userRepository, + IAuthRequestRepository authRequestRepository) + { + var casesThatCauseNoAuthDataInResponse = new[] + { + new + { + authRequestType = AuthRequestType.AdminApproval, // Device typing is wrong + authRequestApproved = (bool?)null, + expirey = DateTime.UtcNow.AddMinutes(0), + }, + new + { + authRequestType = AuthRequestType.AuthenticateAndUnlock, + authRequestApproved = (bool?)true, // Auth request is already approved + expirey = DateTime.UtcNow.AddMinutes(0), + }, + new + { + authRequestType = AuthRequestType.AuthenticateAndUnlock, + authRequestApproved = (bool?)null, + expirey = DateTime.UtcNow.AddMinutes(-30), // Past the point of expiring + } + }; + + foreach (var testCase in casesThatCauseNoAuthDataInResponse) + { + // Arrange + var user = await userRepository.CreateAsync(new User + { + Name = "Test User", + Email = $"test+{Guid.NewGuid()}@email.com", + ApiKey = "TEST", + SecurityStamp = "stamp", + }); + + var device = await sutRepository.CreateAsync(new Device + { + Active = true, + Name = "chrome-test", + UserId = user.Id, + Type = DeviceType.ChromeBrowser, + Identifier = Guid.NewGuid().ToString(), + }); + + var authRequest = await authRequestRepository.CreateAsync(new AuthRequest + { + ResponseDeviceId = null, + Approved = testCase.authRequestApproved, + Type = testCase.authRequestType, + OrganizationId = null, + UserId = user.Id, + RequestIpAddress = ":1", + RequestDeviceIdentifier = device.Identifier, + AccessCode = "AccessCode_1234", + PublicKey = "PublicKey_1234" + }); + + authRequest.CreationDate = testCase.expirey; + await authRequestRepository.ReplaceAsync(authRequest); + + // Act + var response = await sutRepository.GetManyByUserIdWithDeviceAuth(user.Id); + + // Assert + Assert.Null(response.First().AuthRequestId); + Assert.Null(response.First().AuthRequestCreatedAt); + } + } +} diff --git a/test/Infrastructure.IntegrationTest/DatabaseDataAttribute.cs b/test/Infrastructure.IntegrationTest/DatabaseDataAttribute.cs index 746ce988a4..498cc668c0 100644 --- a/test/Infrastructure.IntegrationTest/DatabaseDataAttribute.cs +++ b/test/Infrastructure.IntegrationTest/DatabaseDataAttribute.cs @@ -41,6 +41,9 @@ public class DatabaseDataAttribute : DataAttribute protected virtual IEnumerable GetDatabaseProviders(IConfiguration config) { + // This is for the device repository integration testing. + var userRequestExpiration = 15; + var configureLogging = (ILoggingBuilder builder) => { if (!config.GetValue("Quiet")) @@ -67,11 +70,15 @@ public class DatabaseDataAttribute : DataAttribute { ConnectionString = database.ConnectionString, }, + PasswordlessAuth = new GlobalSettings.PasswordlessAuthSettings + { + UserRequestExpiration = TimeSpan.FromMinutes(userRequestExpiration), + } }; dapperSqlServerCollection.AddSingleton(globalSettings); dapperSqlServerCollection.AddSingleton(globalSettings); dapperSqlServerCollection.AddSingleton(database); - dapperSqlServerCollection.AddDistributedSqlServerCache((o) => + dapperSqlServerCollection.AddDistributedSqlServerCache(o => { o.ConnectionString = database.ConnectionString; o.SchemaName = "dbo"; @@ -91,6 +98,17 @@ public class DatabaseDataAttribute : DataAttribute AddCommonServices(efCollection, configureLogging); efCollection.SetupEntityFramework(database.ConnectionString, database.Type); efCollection.AddPasswordManagerEFRepositories(SelfHosted); + + var globalSettings = new GlobalSettings + { + PasswordlessAuth = new GlobalSettings.PasswordlessAuthSettings + { + UserRequestExpiration = TimeSpan.FromMinutes(userRequestExpiration), + } + }; + efCollection.AddSingleton(globalSettings); + efCollection.AddSingleton(globalSettings); + efCollection.AddSingleton(database); efCollection.AddSingleton(); @@ -117,7 +135,7 @@ public class DatabaseDataAttribute : DataAttribute private void AddSqlMigrationTester(IServiceCollection services, string connectionString, string migrationName) { - services.AddSingleton(sp => new SqlMigrationTesterService(connectionString, migrationName)); + services.AddSingleton(_ => new SqlMigrationTesterService(connectionString, migrationName)); } private void AddEfMigrationTester(IServiceCollection services, SupportedDatabaseProviders databaseType, string migrationName) diff --git a/util/Migrator/DbScripts/2024-12-04_00_AddActiveDeviceWithPendingAuth.sql b/util/Migrator/DbScripts/2024-12-04_00_AddActiveDeviceWithPendingAuth.sql new file mode 100644 index 0000000000..1f358d53ab --- /dev/null +++ b/util/Migrator/DbScripts/2024-12-04_00_AddActiveDeviceWithPendingAuth.sql @@ -0,0 +1,27 @@ +CREATE OR ALTER PROCEDURE [dbo].[Device_ReadActiveWithPendingAuthRequestsByUserId] + @UserId UNIQUEIDENTIFIER, + @ExpirationMinutes INT +AS +BEGIN + SET NOCOUNT ON; + + SELECT + D.*, + AR.Id as AuthRequestId, + AR.CreationDate as AuthRequestCreationDate + FROM dbo.DeviceView D + LEFT JOIN ( + SELECT TOP 1 -- Take only the top record sorted by auth request creation date + Id, + CreationDate, + RequestDeviceIdentifier + FROM dbo.AuthRequestView + WHERE Type IN (0, 1) -- Include only AuthenticateAndUnlock and Unlock types, excluding Admin Approval (type 2) + AND CreationDate >= DATEADD(MINUTE, -@ExpirationMinutes, GETUTCDATE()) -- Ensure the request hasn't expired + AND Approved IS NULL -- Include only requests that haven't been acknowledged or approved + ORDER BY CreationDate DESC + ) AR ON D.Identifier = AR.RequestDeviceIdentifier + WHERE + D.UserId = @UserId + AND D.Active = 1; -- Include only active devices +END;