Skip to content

Commit

Permalink
Merge branch 'main' into km/userkey-rotation-v2
Browse files Browse the repository at this point in the history
  • Loading branch information
quexten authored Jan 31, 2025
2 parents 49a6a82 + 669c253 commit 0716858
Show file tree
Hide file tree
Showing 17 changed files with 328 additions and 80 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ jobs:
output-format: sarif

- name: Upload Grype results to GitHub
uses: github/codeql-action/upload-sarif@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0
uses: github/codeql-action/upload-sarif@dd746615b3b9d728a6a37ca2045b68ca76d4841a # v3.28.8
with:
sarif_file: ${{ steps.container-scan.outputs.sarif }}

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:
--output-path . ${{ env.INCREMENTAL }}
- name: Upload Checkmarx results to GitHub
uses: github/codeql-action/upload-sarif@48ab28a6f5dbc2a99bf1e0131198dd8f1df78169 # v3.28.0
uses: github/codeql-action/upload-sarif@dd746615b3b9d728a6a37ca2045b68ca76d4841a # v3.28.8
with:
sarif_file: cx_result.sarif

Expand Down
7 changes: 7 additions & 0 deletions bitwarden-server.sln
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Notifications.Test", "test\
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Infrastructure.Dapper.Test", "test\Infrastructure.Dapper.Test\Infrastructure.Dapper.Test.csproj", "{4A725DB3-BE4F-4C23-9087-82D0610D67AF}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Events.IntegrationTest", "test\Events.IntegrationTest\Events.IntegrationTest.csproj", "{4F4C63A9-AEE2-48C4-AB86-A5BCD665E401}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -313,6 +315,10 @@ Global
{4A725DB3-BE4F-4C23-9087-82D0610D67AF}.Debug|Any CPU.Build.0 = Debug|Any CPU
{4A725DB3-BE4F-4C23-9087-82D0610D67AF}.Release|Any CPU.ActiveCfg = Release|Any CPU
{4A725DB3-BE4F-4C23-9087-82D0610D67AF}.Release|Any CPU.Build.0 = Release|Any CPU
{4F4C63A9-AEE2-48C4-AB86-A5BCD665E401}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{4F4C63A9-AEE2-48C4-AB86-A5BCD665E401}.Debug|Any CPU.Build.0 = Debug|Any CPU
{4F4C63A9-AEE2-48C4-AB86-A5BCD665E401}.Release|Any CPU.ActiveCfg = Release|Any CPU
{4F4C63A9-AEE2-48C4-AB86-A5BCD665E401}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -363,6 +369,7 @@ Global
{81673EFB-7134-4B4B-A32F-1EA05F0EF3CE} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84F}
{90D85D8F-5577-4570-A96E-5A2E185F0F6F} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84F}
{4A725DB3-BE4F-4C23-9087-82D0610D67AF} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84F}
{4F4C63A9-AEE2-48C4-AB86-A5BCD665E401} = {DD5BD056-4AAE-43EF-BBD2-0B569B8DA84F}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {E01CBF68-2E20-425F-9EDB-E0A6510CA92F}
Expand Down
5 changes: 5 additions & 0 deletions src/Admin/AdminConsole/Controllers/OrganizationsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,11 @@ await _removeOrganizationFromProviderCommand.RemoveOrganizationFromProvider(

private void UpdateOrganization(Organization organization, OrganizationEditModel model)
{
if (_accessControlService.UserHasPermission(Permission.Org_Name_Edit))
{
organization.Name = WebUtility.HtmlEncode(model.Name);
}

if (_accessControlService.UserHasPermission(Permission.Org_CheckEnabledBox))
{
organization.Enabled = model.Enabled;
Expand Down
3 changes: 2 additions & 1 deletion src/Admin/AdminConsole/Views/Shared/_OrganizationForm.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
var canViewBilling = AccessControlService.UserHasPermission(Permission.Org_Billing_View);
var canViewPlan = AccessControlService.UserHasPermission(Permission.Org_Plan_View);
var canViewLicensing = AccessControlService.UserHasPermission(Permission.Org_Licensing_View);
var canEditName = AccessControlService.UserHasPermission(Permission.Org_Name_Edit);
var canCheckEnabled = AccessControlService.UserHasPermission(Permission.Org_CheckEnabledBox);
var canEditPlan = AccessControlService.UserHasPermission(Permission.Org_Plan_Edit);
var canEditLicensing = AccessControlService.UserHasPermission(Permission.Org_Licensing_Edit);
Expand All @@ -28,7 +29,7 @@
<div class="col-sm">
<div class="mb-3">
<label class="form-label" asp-for="Name"></label>
<input type="text" class="form-control" asp-for="Name" value="@Model.Name" required>
<input type="text" class="form-control" asp-for="Name" value="@Model.Name" required disabled="@(canEditName ? null : "disabled")">
</div>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions src/Admin/Enums/Permissions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public enum Permission
Org_List_View,
Org_OrgInformation_View,
Org_GeneralDetails_View,
Org_Name_Edit,
Org_CheckEnabledBox,
Org_BusinessInformation_View,
Org_InitiateTrial,
Expand Down
5 changes: 5 additions & 0 deletions src/Admin/Utilities/RolePermissionMapping.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public static class RolePermissionMapping
Permission.User_Billing_Edit,
Permission.User_Billing_LaunchGateway,
Permission.User_NewDeviceException_Edit,
Permission.Org_Name_Edit,
Permission.Org_CheckEnabledBox,
Permission.Org_List_View,
Permission.Org_OrgInformation_View,
Expand Down Expand Up @@ -71,6 +72,7 @@ public static class RolePermissionMapping
Permission.User_Billing_Edit,
Permission.User_Billing_LaunchGateway,
Permission.User_NewDeviceException_Edit,
Permission.Org_Name_Edit,
Permission.Org_CheckEnabledBox,
Permission.Org_List_View,
Permission.Org_OrgInformation_View,
Expand Down Expand Up @@ -116,6 +118,7 @@ public static class RolePermissionMapping
Permission.User_Billing_View,
Permission.User_Billing_LaunchGateway,
Permission.User_NewDeviceException_Edit,
Permission.Org_Name_Edit,
Permission.Org_CheckEnabledBox,
Permission.Org_List_View,
Permission.Org_OrgInformation_View,
Expand Down Expand Up @@ -148,6 +151,7 @@ public static class RolePermissionMapping
Permission.User_Billing_View,
Permission.User_Billing_Edit,
Permission.User_Billing_LaunchGateway,
Permission.Org_Name_Edit,
Permission.Org_CheckEnabledBox,
Permission.Org_List_View,
Permission.Org_OrgInformation_View,
Expand Down Expand Up @@ -185,6 +189,7 @@ public static class RolePermissionMapping
Permission.User_Premium_View,
Permission.User_Licensing_View,
Permission.User_Licensing_Edit,
Permission.Org_Name_Edit,
Permission.Org_CheckEnabledBox,
Permission.Org_List_View,
Permission.Org_OrgInformation_View,
Expand Down
1 change: 1 addition & 0 deletions src/Core/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ public static class FeatureFlagKeys
public const string VerifiedSsoDomainEndpoint = "pm-12337-refactor-sso-details-endpoint";
public const string IntegrationPage = "pm-14505-admin-console-integration-page";
public const string DeviceApprovalRequestAdminNotifications = "pm-15637-device-approval-request-admin-notifications";
public const string LimitItemDeletion = "pm-15493-restrict-item-deletion-to-can-manage-permission";

public const string ReturnErrorOnExistingKeypair = "return-error-on-existing-keypair";
public const string UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection";
Expand Down
34 changes: 18 additions & 16 deletions src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,28 +85,17 @@ public async Task<bool> ValidateRequestDeviceAsync(ValidatedTokenRequest request
}
}

// At this point we have established either new device verification is not required or the NewDeviceOtp is valid
// At this point we have established either new device verification is not required or the NewDeviceOtp is valid,
// so we save the device to the database and proceed with authentication
requestDevice.UserId = context.User.Id;
await _deviceService.SaveAsync(requestDevice);
context.Device = requestDevice;

// backwards compatibility -- If NewDeviceVerification not enabled send the new login emails
// PM-13340: removal Task; remove entire if block emails should no longer be sent
if (!_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification))
if (!_globalSettings.DisableEmailNewDevice)
{
// This ensures the user doesn't receive a "new device" email on the first login
var now = DateTime.UtcNow;
if (now - context.User.CreationDate > TimeSpan.FromMinutes(10))
{
var deviceType = requestDevice.Type.GetType().GetMember(requestDevice.Type.ToString())
.FirstOrDefault()?.GetCustomAttribute<DisplayAttribute>()?.GetName();
if (!_globalSettings.DisableEmailNewDevice)
{
await _mailService.SendNewDeviceLoggedInEmail(context.User.Email, deviceType, now,
_currentContext.IpAddress);
}
}
await SendNewDeviceLoginEmail(context.User, requestDevice);
}

return true;
}

Expand Down Expand Up @@ -174,6 +163,19 @@ private async Task<DeviceValidationResultType> HandleNewDeviceVerificationAsync(
return DeviceValidationResultType.NewDeviceVerificationRequired;
}

private async Task SendNewDeviceLoginEmail(User user, Device requestDevice)
{
// Ensure that the user doesn't receive a "new device" email on the first login
var now = DateTime.UtcNow;
if (now - user.CreationDate > TimeSpan.FromMinutes(10))
{
var deviceType = requestDevice.Type.GetType().GetMember(requestDevice.Type.ToString())
.FirstOrDefault()?.GetCustomAttribute<DisplayAttribute>()?.GetName();
await _mailService.SendNewDeviceLoggedInEmail(user.Email, deviceType, now,
_currentContext.IpAddress);
}
}

public async Task<Device> GetKnownDeviceAsync(User user, Device device)
{
if (user == null || device == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,17 @@ public OrganizationDomainRepository(IServiceScopeFactory serviceScopeFactory, IM
using var scope = ServiceScopeFactory.CreateScope();
var dbContext = GetDatabaseContext(scope);

var domains = await dbContext.OrganizationDomains
.Where(x => x.VerifiedDate == null
&& x.JobRunCount != 3
&& x.NextRunDate.Year == date.Year
&& x.NextRunDate.Month == date.Month
&& x.NextRunDate.Day == date.Day
&& x.NextRunDate.Hour == date.Hour)
.AsNoTracking()
var start36HoursWindow = date.AddHours(-36);
var end36HoursWindow = date;

var pastDomains = await dbContext.OrganizationDomains
.Where(x => x.NextRunDate >= start36HoursWindow
&& x.NextRunDate <= end36HoursWindow
&& x.VerifiedDate == null
&& x.JobRunCount != 3)
.ToListAsync();

//Get records that have ignored/failed by the background service
var pastDomains = dbContext.OrganizationDomains
.AsEnumerable()
.Where(x => (date - x.NextRunDate).TotalHours > 36
&& x.VerifiedDate == null
&& x.JobRunCount != 3)
.ToList();

var results = domains.Union(pastDomains);

return Mapper.Map<List<Core.Entities.OrganizationDomain>>(results);
return Mapper.Map<List<Core.Entities.OrganizationDomain>>(pastDomains);
}

public async Task<OrganizationDomainSsoDetailsData?> GetOrganizationDomainSsoDetailsAsync(string email)
Expand Down
29 changes: 29 additions & 0 deletions test/Events.IntegrationTest/Controllers/CollectControllerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using System.Net.Http.Json;
using Bit.Core.Enums;
using Bit.Events.Models;

namespace Bit.Events.IntegrationTest.Controllers;

public class CollectControllerTests
{
// This is a very simple test, and should be updated to assert more things, but for now
// it ensures that the events startup doesn't throw any errors with fairly basic configuration.
[Fact]
public async Task Post_Works()
{
var eventsApplicationFactory = new EventsApplicationFactory();
var (accessToken, _) = await eventsApplicationFactory.LoginWithNewAccount();
var client = eventsApplicationFactory.CreateAuthedClient(accessToken);

var response = await client.PostAsJsonAsync<IEnumerable<EventModel>>("collect",
[
new EventModel
{
Type = EventType.User_ClientExportedVault,
Date = DateTime.UtcNow,
},
]);

response.EnsureSuccessStatusCode();
}
}
29 changes: 29 additions & 0 deletions test/Events.IntegrationTest/Events.IntegrationTest.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<IsPackable>false</IsPackable>
<IsTestProject>true</IsTestProject>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="$(MicrosoftNetTestSdkVersion)" />
<PackageReference Include="xunit" Version="$(XUnitVersion)" />
<PackageReference Include="xunit.runner.visualstudio" Version="$(XUnitRunnerVisualStudioVersion)">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="coverlet.collector" Version="$(CoverletCollectorVersion)">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\Events\Events.csproj" />
<ProjectReference Include="..\IntegrationTestCommon\IntegrationTestCommon.csproj" />
</ItemGroup>

</Project>
57 changes: 57 additions & 0 deletions test/Events.IntegrationTest/EventsApplicationFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
using Bit.Identity.Models.Request.Accounts;
using Bit.IntegrationTestCommon.Factories;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.TestHost;
using Microsoft.Data.Sqlite;
using Microsoft.Extensions.DependencyInjection;

namespace Bit.Events.IntegrationTest;

public class EventsApplicationFactory : WebApplicationFactoryBase<Startup>
{
private readonly IdentityApplicationFactory _identityApplicationFactory;
private const string _connectionString = "DataSource=:memory:";

public EventsApplicationFactory()
{
SqliteConnection = new SqliteConnection(_connectionString);
SqliteConnection.Open();

_identityApplicationFactory = new IdentityApplicationFactory();
_identityApplicationFactory.SqliteConnection = SqliteConnection;
}

protected override void ConfigureWebHost(IWebHostBuilder builder)
{
base.ConfigureWebHost(builder);

builder.ConfigureTestServices(services =>
{
services.Configure<JwtBearerOptions>(JwtBearerDefaults.AuthenticationScheme, options =>
{
options.BackchannelHttpHandler = _identityApplicationFactory.Server.CreateHandler();
});
});
}

/// <summary>
/// Helper for registering and logging in to a new account
/// </summary>
public async Task<(string Token, string RefreshToken)> LoginWithNewAccount(string email = "[email protected]", string masterPasswordHash = "master_password_hash")
{
await _identityApplicationFactory.RegisterAsync(new RegisterRequestModel
{
Email = email,
MasterPasswordHash = masterPasswordHash,
});

return await _identityApplicationFactory.TokenFromPasswordAsync(email, masterPasswordHash);
}

protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
SqliteConnection!.Dispose();
}
}
1 change: 1 addition & 0 deletions test/Events.IntegrationTest/GlobalUsings.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
global using Xunit;
12 changes: 3 additions & 9 deletions test/Identity.Test/IdentityServer/DeviceValidatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ public async void ValidateRequestDeviceAsync_ContextDeviceKnown_ContextDeviceMod
}

[Theory, BitAutoData]
public async void ValidateRequestDeviceAsync_NewDeviceVerificationFeatureFlagFalse_SendsEmail_ReturnsTrue(
public async void ValidateRequestDeviceAsync_ExistingUserNewDeviceLogin_SendNewDeviceLoginEmail_ReturnsTrue(
CustomValidatorRequestContext context,
[AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request)
{
Expand All @@ -237,8 +237,6 @@ public async void ValidateRequestDeviceAsync_NewDeviceVerificationFeatureFlagFal
_globalSettings.DisableEmailNewDevice = false;
_deviceRepository.GetByIdentifierAsync(context.Device.Identifier, context.User.Id)
.Returns(null as Device);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification)
.Returns(false);
// set user creation to more than 10 minutes ago
context.User.CreationDate = DateTime.UtcNow - TimeSpan.FromMinutes(11);

Expand All @@ -253,7 +251,7 @@ await _mailService.Received(1).SendNewDeviceLoggedInEmail(
}

[Theory, BitAutoData]
public async void ValidateRequestDeviceAsync_NewDeviceVerificationFeatureFlagFalse_NewUser_DoesNotSendEmail_ReturnsTrue(
public async void ValidateRequestDeviceAsync_NewUserNewDeviceLogin_DoesNotSendNewDeviceLoginEmail_ReturnsTrue(
CustomValidatorRequestContext context,
[AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request)
{
Expand All @@ -263,8 +261,6 @@ public async void ValidateRequestDeviceAsync_NewDeviceVerificationFeatureFlagFal
_globalSettings.DisableEmailNewDevice = false;
_deviceRepository.GetByIdentifierAsync(context.Device.Identifier, context.User.Id)
.Returns(null as Device);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification)
.Returns(false);
// set user creation to less than 10 minutes ago
context.User.CreationDate = DateTime.UtcNow - TimeSpan.FromMinutes(9);

Expand All @@ -279,7 +275,7 @@ await _mailService.Received(0).SendNewDeviceLoggedInEmail(
}

[Theory, BitAutoData]
public async void ValidateRequestDeviceAsync_NewDeviceVerificationFeatureFlagFalse_DisableEmailTrue_DoesNotSendEmail_ReturnsTrue(
public async void ValidateRequestDeviceAsynce_DisableNewDeviceLoginEmailTrue_DoesNotSendNewDeviceEmail_ReturnsTrue(
CustomValidatorRequestContext context,
[AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request)
{
Expand All @@ -289,8 +285,6 @@ public async void ValidateRequestDeviceAsync_NewDeviceVerificationFeatureFlagFal
_globalSettings.DisableEmailNewDevice = true;
_deviceRepository.GetByIdentifierAsync(context.Device.Identifier, context.User.Id)
.Returns(null as Device);
_featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification)
.Returns(false);

// Act
var result = await _sut.ValidateRequestDeviceAsync(request, context);
Expand Down
Loading

0 comments on commit 0716858

Please sign in to comment.