mirror of
https://github.com/bitwarden/server.git
synced 2025-06-30 15:42:48 -05:00
Auth/pm 2996/add auth request data to devices response model (#5152)
fix(auth): [PM-2996] Add Pending Auth Request Data to Devices Response - New stored procedure to fetch the appropriate data. - Updated devices controller to respond with the new data. - Tests written at the controller and repository level. Resolves PM-2996
This commit is contained in:

committed by
GitHub

parent
5ae232e336
commit
cc96e35072
88
test/Api.Test/Auth/Controllers/DevicesControllerTests.cs
Normal file
88
test/Api.Test/Auth/Controllers/DevicesControllerTests.cs
Normal file
@ -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<DevicesController> _loggerMock;
|
||||
private readonly DevicesController _sut;
|
||||
|
||||
public DevicesControllerTest()
|
||||
{
|
||||
_deviceRepositoryMock = Substitute.For<IDeviceRepository>();
|
||||
_deviceServiceMock = Substitute.For<IDeviceService>();
|
||||
_userServiceMock = Substitute.For<IUserService>();
|
||||
_userRepositoryMock = Substitute.For<IUserRepository>();
|
||||
_currentContextMock = Substitute.For<ICurrentContext>();
|
||||
_loggerMock = Substitute.For<ILogger<DevicesController>>();
|
||||
|
||||
_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<DeviceAuthDetails>
|
||||
{
|
||||
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<System.Security.Claims.ClaimsPrincipal>()).Returns(userId);
|
||||
_deviceRepositoryMock.GetManyByUserIdWithDeviceAuth(userId).Returns(devicesWithPendingAuthData);
|
||||
|
||||
// Act
|
||||
var result = await _sut.Get();
|
||||
|
||||
// Assert
|
||||
Assert.NotNull(result);
|
||||
Assert.IsType<ListResponseModel<DeviceAuthRequestResponseModel>>(result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Get_ThrowsException_WhenUserIdIsInvalid()
|
||||
{
|
||||
// Arrange
|
||||
_userServiceMock.GetProperUserId(Arg.Any<System.Security.Claims.ClaimsPrincipal>()).Returns((Guid?)null);
|
||||
|
||||
// Act & Assert
|
||||
await Assert.ThrowsAsync<InvalidOperationException>(() => _sut.Get());
|
||||
}
|
||||
}
|
@ -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<DeviceRepository>());
|
||||
fixture.Customizations.Add(new EfRepositoryListBuilder<UserRepository>());
|
||||
fixture.Customizations.Add(new EfRepositoryListBuilder<AuthRequestRepository>());
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -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<EfRepo.DeviceRepository> suts,
|
||||
List<EfRepo.UserRepository> efUserRepos, SqlRepo.DeviceRepository sqlDeviceRepo,
|
||||
public async Task CreateAsync_Works_DataMatches(
|
||||
Device device,
|
||||
User user,
|
||||
DeviceCompare equalityComparer,
|
||||
List<EfRepo.DeviceRepository> suts,
|
||||
List<EfRepo.UserRepository> efUserRepos,
|
||||
SqlRepo.DeviceRepository sqlDeviceRepo,
|
||||
SqlRepo.UserRepository sqlUserRepo)
|
||||
{
|
||||
var savedDevices = new List<Device>();
|
||||
@ -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());
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -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))));
|
||||
|
||||
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
@ -41,6 +41,9 @@ public class DatabaseDataAttribute : DataAttribute
|
||||
|
||||
protected virtual IEnumerable<IServiceProvider> GetDatabaseProviders(IConfiguration config)
|
||||
{
|
||||
// This is for the device repository integration testing.
|
||||
var userRequestExpiration = 15;
|
||||
|
||||
var configureLogging = (ILoggingBuilder builder) =>
|
||||
{
|
||||
if (!config.GetValue<bool>("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<IGlobalSettings>(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<IGlobalSettings>(globalSettings);
|
||||
|
||||
efCollection.AddSingleton(database);
|
||||
efCollection.AddSingleton<IDistributedCache, EntityFrameworkCache>();
|
||||
|
||||
@ -117,7 +135,7 @@ public class DatabaseDataAttribute : DataAttribute
|
||||
|
||||
private void AddSqlMigrationTester(IServiceCollection services, string connectionString, string migrationName)
|
||||
{
|
||||
services.AddSingleton<IMigrationTesterService, SqlMigrationTesterService>(sp => new SqlMigrationTesterService(connectionString, migrationName));
|
||||
services.AddSingleton<IMigrationTesterService, SqlMigrationTesterService>(_ => new SqlMigrationTesterService(connectionString, migrationName));
|
||||
}
|
||||
|
||||
private void AddEfMigrationTester(IServiceCollection services, SupportedDatabaseProviders databaseType, string migrationName)
|
||||
|
Reference in New Issue
Block a user