Skip to content

Commit

Permalink
Auth/pm 2996/add auth request data to devices response model (#5152)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Patrick-Pimentel-Bitwarden authored Jan 7, 2025
1 parent 5ae232e commit cc96e35
Show file tree
Hide file tree
Showing 21 changed files with 620 additions and 30 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/test-database.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"'
Expand Down Expand Up @@ -237,7 +237,7 @@ jobs:
run: |
if grep -q "<Operations>" "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"
Expand Down
15 changes: 10 additions & 5 deletions src/Api/Controllers/DevicesController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
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;
Expand Down Expand Up @@ -70,11 +69,17 @@ public async Task<DeviceResponseModel> GetByIdentifier(string identifier)
}

[HttpGet("")]
public async Task<ListResponseModel<DeviceResponseModel>> Get()
public async Task<ListResponseModel<DeviceAuthRequestResponseModel>> Get()
{
ICollection<Device> devices = await _deviceRepository.GetManyByUserIdAsync(_userService.GetProperUserId(User).Value);
var responses = devices.Select(d => new DeviceResponseModel(d));
return new ListResponseModel<DeviceResponseModel>(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<DeviceAuthRequestResponseModel>(deviceAuthRequestResponseList);
return response;
}

[HttpPost("")]
Expand Down
7 changes: 7 additions & 0 deletions src/Core/Auth/Enums/AuthRequestType.cs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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; }
}
}
81 changes: 81 additions & 0 deletions src/Core/Auth/Models/Data/DeviceAuthDetails.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
3 changes: 1 addition & 2 deletions src/Core/Auth/Models/Data/EmergencyAccessDetails.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@

using Bit.Core.Auth.Entities;
using Bit.Core.Auth.Entities;

namespace Bit.Core.Auth.Models.Data;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion src/Core/Repositories/IDeviceRepository.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Bit.Core.Entities;
using Bit.Core.Auth.Models.Data;
using Bit.Core.Entities;

#nullable enable

Expand All @@ -10,5 +11,9 @@ public interface IDeviceRepository : IRepository<Device, Guid>
Task<Device?> GetByIdentifierAsync(string identifier);
Task<Device?> GetByIdentifierAsync(string identifier, Guid userId);
Task<ICollection<Device>> 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<ICollection<DeviceAuthDetails>> GetManyByUserIdWithDeviceAuth(Guid userId);
Task ClearPushTokenAsync(Guid id);
}
2 changes: 2 additions & 0 deletions src/Core/Settings/IGlobalSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
25 changes: 24 additions & 1 deletion src/Infrastructure.Dapper/Repositories/DeviceRepository.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -11,9 +12,13 @@ namespace Bit.Infrastructure.Dapper.Repositories;

public class DeviceRepository : Repository<Device, Guid>, IDeviceRepository
{
private readonly IGlobalSettings _globalSettings;

public DeviceRepository(GlobalSettings globalSettings)
: this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString)
{ }
{
_globalSettings = globalSettings;
}

public DeviceRepository(string connectionString, string readOnlyConnectionString)

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Icons, ./src)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (EventsProcessor, ./src)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Api, ./src)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Identity, ./src)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Billing, ./src)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Events, ./src)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Run tests

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Scim, ./bitwarden_license/src, true)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Notifications, ./src)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Sso, ./bitwarden_license/src, true)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Build artifacts (Admin, ./src, true)

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Run tests

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.

Check warning on line 23 in src/Infrastructure.Dapper/Repositories/DeviceRepository.cs

View workflow job for this annotation

GitHub Actions / Upload

Non-nullable field '_globalSettings' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.
: base(connectionString, readOnlyConnectionString)
Expand Down Expand Up @@ -76,6 +81,24 @@ public async Task<ICollection<Device>> GetManyByUserIdAsync(Guid userId)
}
}

public async Task<ICollection<DeviceAuthDetails>> GetManyByUserIdWithDeviceAuth(Guid userId)
{
var expirationMinutes = _globalSettings.PasswordlessAuth.UserRequestExpiration.TotalMinutes;
using (var connection = new SqlConnection(ConnectionString))
{
var results = await connection.QueryAsync<DeviceAuthDetails>(
$"[{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))
Expand Down
4 changes: 2 additions & 2 deletions src/Infrastructure.Dapper/Repositories/Repository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public virtual async Task<T> CreateAsync(T obj)
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);
Expand All @@ -64,7 +64,7 @@ public virtual async Task ReplaceAsync(T obj)
{
using (var connection = new SqlConnection(ConnectionString))
{
var results = await connection.ExecuteAsync(
await connection.ExecuteAsync(
$"[{Schema}].[{Table}_Update]",
obj,
commandType: CommandType.StoredProcedure);
Expand Down
Original file line number Diff line number Diff line change
@@ -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<DeviceAuthDetails> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -10,9 +13,17 @@ namespace Bit.Infrastructure.EntityFramework.Repositories;

public class DeviceRepository : Repository<Core.Entities.Device, Device, Guid>, 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)
{
Expand Down Expand Up @@ -69,4 +80,15 @@ public async Task ClearPushTokenAsync(Guid id)
return Mapper.Map<List<Core.Entities.Device>>(devices);
}
}

public async Task<ICollection<DeviceAuthDetails>> 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();
}
}
}
Loading

0 comments on commit cc96e35

Please sign in to comment.