bitwarden/server
1
0
Fork 0
mirror of https://github.com/bitwarden/server synced 2025-12-30 23:23:37 +00:00
Code Issues Releases Wiki Activity

[PM-29556] Fix: changing organization plan nulls out public and private keys (#6738)

Browse Source 
Main fix: only assign new key value where old keys are not set
and new keys have been provided.

Refactors:
- use consistent DTO model for keypairs
- delete duplicate property assignment for new orgs
This commit is contained in:
Thomas Rittson
2025-12-26 10:13:12 +10:00
committed by GitHub
parent 96622d7928
commit 67534e2cda
18 changed files with 220 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/Api/AdminConsole/Controllers/ProviderClientsController.cs
View File

@@ -57,8 +57,7 @@ public class ProviderClientsController(
Owner = user,
BillingEmail = provider.BillingEmail,
OwnerKey = requestBody.Key,
PublicKey = requestBody.KeyPair.PublicKey,
PrivateKey = requestBody.KeyPair.EncryptedPrivateKey,
Keys = requestBody.KeyPair.ToPublicKeyEncryptionKeyPairData(),
CollectionName = requestBody.CollectionName,
IsFromProvider = true
};

5
src/Api/AdminConsole/Models/Request/Organizations/OrganizationCreateRequestModel.cs
View File

@@ -113,11 +113,10 @@ public class OrganizationCreateRequestModel : IValidatableObject
BillingAddressCountry = BillingAddressCountry,
},
InitiationPath = InitiationPath,
SkipTrial = SkipTrial
SkipTrial = SkipTrial,
Keys = Keys?.ToPublicKeyEncryptionKeyPairData()
};
Keys?.ToOrganizationSignup(orgSignup);
return orgSignup;
}

49
src/Api/AdminConsole/Models/Request/Organizations/OrganizationKeysRequestModel.cs
View File

@@ -2,8 +2,7 @@
#nullable disable
using System.ComponentModel.DataAnnotations;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.Models.Business;
using Bit.Core.KeyManagement.Models.Data;
namespace Bit.Api.AdminConsole.Models.Request.Organizations;
@@ -14,48 +13,10 @@ public class OrganizationKeysRequestModel
[Required]
public string EncryptedPrivateKey { get; set; }
public OrganizationSignup ToOrganizationSignup(OrganizationSignup existingSignup)
public PublicKeyEncryptionKeyPairData ToPublicKeyEncryptionKeyPairData()
{
if (string.IsNullOrWhiteSpace(existingSignup.PublicKey))
{
existingSignup.PublicKey = PublicKey;
}
if (string.IsNullOrWhiteSpace(existingSignup.PrivateKey))
{
existingSignup.PrivateKey = EncryptedPrivateKey;
}
return existingSignup;
}
public OrganizationUpgrade ToOrganizationUpgrade(OrganizationUpgrade existingUpgrade)
{
if (string.IsNullOrWhiteSpace(existingUpgrade.PublicKey))
{
existingUpgrade.PublicKey = PublicKey;
}
if (string.IsNullOrWhiteSpace(existingUpgrade.PrivateKey))
{
existingUpgrade.PrivateKey = EncryptedPrivateKey;
}
return existingUpgrade;
}
public Organization ToOrganization(Organization existingOrg)
{
if (string.IsNullOrWhiteSpace(existingOrg.PublicKey))
{
existingOrg.PublicKey = PublicKey;
}
if (string.IsNullOrWhiteSpace(existingOrg.PrivateKey))
{
existingOrg.PrivateKey = EncryptedPrivateKey;
}
return existingOrg;
return new PublicKeyEncryptionKeyPairData(
wrappedPrivateKey: EncryptedPrivateKey,
publicKey: PublicKey);
}
}

3
src/Api/AdminConsole/Models/Request/Organizations/OrganizationNoPaymentCreateRequest.cs
View File

@@ -110,10 +110,9 @@ public class OrganizationNoPaymentCreateRequest
BillingAddressCountry = BillingAddressCountry,
},
InitiationPath = InitiationPath,
Keys = Keys?.ToPublicKeyEncryptionKeyPairData()
};
Keys?.ToOrganizationSignup(orgSignup);
return orgSignup;
}
}

3
src/Api/AdminConsole/Models/Request/Organizations/OrganizationUpdateRequestModel.cs
View File

@@ -22,7 +22,6 @@ public class OrganizationUpdateRequestModel
OrganizationId = organizationId,
Name = Name,
BillingEmail = BillingEmail,
PublicKey = Keys?.PublicKey,
EncryptedPrivateKey = Keys?.EncryptedPrivateKey
Keys = Keys?.ToPublicKeyEncryptionKeyPairData()
};
}

5
src/Api/AdminConsole/Models/Request/Organizations/OrganizationUpgradeRequestModel.cs
View File

@@ -43,11 +43,10 @@ public class OrganizationUpgradeRequestModel
{
BillingAddressCountry = BillingAddressCountry,
BillingAddressPostalCode = BillingAddressPostalCode
}
},
Keys = Keys?.ToPublicKeyEncryptionKeyPairData()
};
Keys?.ToOrganizationUpgrade(orgUpgrade);
return orgUpgrade;
}
}

8
src/Api/Billing/Models/Requests/KeyPairRequestBody.cs
View File

@@ -2,6 +2,7 @@
#nullable disable
using System.ComponentModel.DataAnnotations;
using Bit.Core.KeyManagement.Models.Data;
namespace Bit.Api.Billing.Models.Requests;
@@ -12,4 +13,11 @@ public class KeyPairRequestBody
public string PublicKey { get; set; }
[Required(ErrorMessage = "'encryptedPrivateKey' must be provided")]
public string EncryptedPrivateKey { get; set; }
public PublicKeyEncryptionKeyPairData ToPublicKeyEncryptionKeyPairData()
{
return new PublicKeyEncryptionKeyPairData(
wrappedPrivateKey: EncryptedPrivateKey,
publicKey: PublicKey);
}
}

4
src/Core/AdminConsole/OrganizationFeatures/Organizations/CloudOrganizationSignUpCommand.cs
View File

@@ -99,8 +99,8 @@ public class CloudOrganizationSignUpCommand(
ReferenceData = signup.Owner.ReferenceData,
Enabled = true,
LicenseKey = CoreHelpers.SecureRandomString(20),
PublicKey = signup.PublicKey,
PrivateKey = signup.PrivateKey,
PublicKey = signup.Keys?.PublicKey,
PrivateKey = signup.Keys?.WrappedPrivateKey,
CreationDate = DateTime.UtcNow,
RevisionDate = DateTime.UtcNow,
Status = OrganizationStatusType.Created,

28
src/Core/AdminConsole/OrganizationFeatures/Organizations/OrganizationExtensions.cs Normal file
View File

@@ -0,0 +1,28 @@
using Bit.Core.AdminConsole.Entities;
using Bit.Core.KeyManagement.Models.Data;
namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations;
public static class OrganizationExtensions
{
/// <summary>
/// Updates the organization public and private keys if provided and not already set.
/// This is legacy code for old organizations that were not created with a public/private keypair.
/// It is a soft migration that will silently migrate organizations when they perform certain actions,
/// e.g. change their details or upgrade their plan.
/// </summary>
public static void BackfillPublicPrivateKeys(this Organization organization, PublicKeyEncryptionKeyPairData? keyPair)
{
// Only backfill if both new keys are provided and both old keys are missing.
if (string.IsNullOrWhiteSpace(keyPair?.PublicKey) ||
string.IsNullOrWhiteSpace(keyPair.WrappedPrivateKey) ||
!string.IsNullOrWhiteSpace(organization.PublicKey) ||
!string.IsNullOrWhiteSpace(organization.PrivateKey))
{
return;
}
organization.PublicKey = keyPair.PublicKey;
organization.PrivateKey = keyPair.WrappedPrivateKey;
}
}

4
src/Core/AdminConsole/OrganizationFeatures/Organizations/ProviderClientOrganizationSignUpCommand.cs
View File

@@ -93,8 +93,8 @@ public class ProviderClientOrganizationSignUpCommand : IProviderClientOrganizati
ReferenceData = signup.Owner.ReferenceData,
Enabled = true,
LicenseKey = CoreHelpers.SecureRandomString(20),
PublicKey = signup.PublicKey,
PrivateKey = signup.PrivateKey,
PublicKey = signup.Keys?.PublicKey,
PrivateKey = signup.Keys?.WrappedPrivateKey,
CreationDate = DateTime.UtcNow,
RevisionDate = DateTime.UtcNow,
Status = OrganizationStatusType.Created,

18
src/Core/AdminConsole/OrganizationFeatures/Organizations/Update/OrganizationUpdateCommand.cs
View File

@@ -39,8 +39,20 @@ public class OrganizationUpdateCommand(
var originalBillingEmail = organization.BillingEmail;
// Apply updates to organization
organization.UpdateDetails(request);
organization.BackfillPublicPrivateKeys(request);
// These values may or may not be sent by the client depending on the operation being performed.
// Skip any values not provided.
if (request.Name is not null)
{
organization.Name = request.Name;
}
if (request.BillingEmail is not null)
{
organization.BillingEmail = request.BillingEmail.ToLowerInvariant().Trim();
}
organization.BackfillPublicPrivateKeys(request.Keys);
await organizationService.ReplaceAndUpdateCacheAsync(organization, EventType.Organization_Updated);
// Update billing information in Stripe if required
@@ -56,7 +68,7 @@ public class OrganizationUpdateCommand(
/// </summary>
private async Task<Organization> UpdateSelfHostedAsync(Organization organization, OrganizationUpdateRequest request)
{
organization.BackfillPublicPrivateKeys(request);
organization.BackfillPublicPrivateKeys(request.Keys);
await organizationService.ReplaceAndUpdateCacheAsync(organization, EventType.Organization_Updated);
return organization;
}

43
src/Core/AdminConsole/OrganizationFeatures/Organizations/Update/OrganizationUpdateExtensions.cs
View File

@@ -1,43 +0,0 @@
using Bit.Core.AdminConsole.Entities;
namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Update;
public static class OrganizationUpdateExtensions
{
/// <summary>
/// Updates the organization name and/or billing email.
/// Any null property on the request object will be skipped.
/// </summary>
public static void UpdateDetails(this Organization organization, OrganizationUpdateRequest request)
{
// These values may or may not be sent by the client depending on the operation being performed.
// Skip any values not provided.
if (request.Name is not null)
{
organization.Name = request.Name;
}
if (request.BillingEmail is not null)
{
organization.BillingEmail = request.BillingEmail.ToLowerInvariant().Trim();
}
}
/// <summary>
/// Updates the organization public and private keys if provided and not already set.
/// This is legacy code for old organizations that were not created with a public/private keypair. It is a soft
/// migration that will silently migrate organizations when they change their details.
/// </summary>
public static void BackfillPublicPrivateKeys(this Organization organization, OrganizationUpdateRequest request)
{
if (!string.IsNullOrWhiteSpace(request.PublicKey) && string.IsNullOrWhiteSpace(organization.PublicKey))
{
organization.PublicKey = request.PublicKey;
}
if (!string.IsNullOrWhiteSpace(request.EncryptedPrivateKey) && string.IsNullOrWhiteSpace(organization.PrivateKey))
{
organization.PrivateKey = request.EncryptedPrivateKey;
}
}
}

13
src/Core/AdminConsole/OrganizationFeatures/Organizations/Update/OrganizationUpdateRequest.cs
View File

@@ -1,4 +1,6 @@
namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Update;
using Bit.Core.KeyManagement.Models.Data;
namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Update;
/// <summary>
/// Request model for updating the name, billing email, and/or public-private keys for an organization (legacy migration code).
@@ -22,12 +24,7 @@ public record OrganizationUpdateRequest
public string? BillingEmail { get; init; }
/// <summary>
/// The organization's public key to set (optional, only set if not already present on the organization).
/// The organization's public/private key pair to set (optional, only set if not already present on the organization).
/// </summary>
public string? PublicKey { get; init; }
/// <summary>
/// The organization's encrypted private key to set (optional, only set if not already present on the organization).
/// </summary>
public string? EncryptedPrivateKey { get; init; }
public PublicKeyEncryptionKeyPairData? Keys { get; init; }
}

4
src/Core/Models/Business/OrganizationUpgrade.cs
View File

@@ -2,6 +2,7 @@
#nullable disable
using Bit.Core.Billing.Enums;
using Bit.Core.KeyManagement.Models.Data;
namespace Bit.Core.Models.Business;
@@ -13,8 +14,7 @@ public class OrganizationUpgrade
public short AdditionalStorageGb { get; set; }
public bool PremiumAccessAddon { get; set; }
public TaxInfo TaxInfo { get; set; }
public string PublicKey { get; set; }
public string PrivateKey { get; set; }
public PublicKeyEncryptionKeyPairData Keys { get; set; }
public int? AdditionalSmSeats { get; set; }
public int? AdditionalServiceAccounts { get; set; }
public bool UseSecretsManager { get; set; }

12
src/Core/OrganizationFeatures/OrganizationSubscriptions/UpgradeOrganizationPlanCommand.cs
View File

@@ -4,6 +4,7 @@
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Models.OrganizationConnectionConfigs;
using Bit.Core.AdminConsole.OrganizationFeatures.Organizations;
using Bit.Core.AdminConsole.Repositories;
using Bit.Core.Auth.Enums;
using Bit.Core.Auth.Repositories;
@@ -256,27 +257,20 @@ public class UpgradeOrganizationPlanCommand : IUpgradeOrganizationPlanCommand
organization.SelfHost = newPlan.HasSelfHost;
organization.UsePolicies = newPlan.HasPolicies;
organization.MaxStorageGb = (short)(newPlan.PasswordManager.BaseStorageGb + upgrade.AdditionalStorageGb);
organization.UseGroups = newPlan.HasGroups;
organization.UseDirectory = newPlan.HasDirectory;
organization.UseEvents = newPlan.HasEvents;
organization.UseTotp = newPlan.HasTotp;
organization.Use2fa = newPlan.Has2fa;
organization.UseApi = newPlan.HasApi;
organization.UseSso = newPlan.HasSso;
organization.UseOrganizationDomains = newPlan.HasOrganizationDomains;
organization.UseKeyConnector = newPlan.HasKeyConnector ? organization.UseKeyConnector : false;
organization.UseScim = newPlan.HasScim;
organization.UseResetPassword = newPlan.HasResetPassword;
organization.SelfHost = newPlan.HasSelfHost;
organization.UsersGetPremium = newPlan.UsersGetPremium || upgrade.PremiumAccessAddon;
organization.UseCustomPermissions = newPlan.HasCustomPermissions;
organization.Plan = newPlan.Name;
organization.Enabled = success;
organization.PublicKey = upgrade.PublicKey;
organization.PrivateKey = upgrade.PrivateKey;
organization.UsePasswordManager = true;
organization.UseSecretsManager = upgrade.UseSecretsManager;
organization.BackfillPublicPrivateKeys(upgrade.Keys);
if (upgrade.UseSecretsManager)
{
organization.SmSeats = newPlan.SecretsManager.BaseSeats + upgrade.AdditionalSmSeats.GetValueOrDefault();

4
test/Api.Test/AdminConsole/Controllers/ProviderClientsControllerTests.cs
View File

@@ -66,8 +66,8 @@ public class ProviderClientsControllerTests
signup.Plan == requestBody.PlanType &&
signup.AdditionalSeats == requestBody.Seats &&
signup.OwnerKey == requestBody.Key &&
signup.PublicKey == requestBody.KeyPair.PublicKey &&
signup.PrivateKey == requestBody.KeyPair.EncryptedPrivateKey &&
signup.Keys.PublicKey == requestBody.KeyPair.PublicKey &&
signup.Keys.WrappedPrivateKey == requestBody.KeyPair.EncryptedPrivateKey &&
signup.CollectionName == requestBody.CollectionName),
requestBody.OwnerEmail,
user)

16
test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationUpdateCommandTests.cs
View File

@@ -3,6 +3,7 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Update;
using Bit.Core.Billing.Organizations.Services;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.KeyManagement.Models.Data;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Core.Settings;
@@ -162,8 +163,9 @@ public class OrganizationUpdateCommandTests
OrganizationId = organizationId,
Name = organization.Name,
BillingEmail = organization.BillingEmail,
PublicKey = publicKey,
EncryptedPrivateKey = encryptedPrivateKey
Keys = new PublicKeyEncryptionKeyPairData(
wrappedPrivateKey: encryptedPrivateKey,
publicKey: publicKey)
};
// Act
@@ -207,8 +209,9 @@ public class OrganizationUpdateCommandTests
OrganizationId = organizationId,
Name = organization.Name,
BillingEmail = organization.BillingEmail,
PublicKey = newPublicKey,
EncryptedPrivateKey = newEncryptedPrivateKey
Keys = new PublicKeyEncryptionKeyPairData(
wrappedPrivateKey: newEncryptedPrivateKey,
publicKey: newPublicKey)
};
// Act
@@ -394,8 +397,9 @@ public class OrganizationUpdateCommandTests
OrganizationId = organizationId,
Name = newName, // Should be ignored
BillingEmail = newBillingEmail, // Should be ignored
PublicKey = publicKey,
EncryptedPrivateKey = encryptedPrivateKey
Keys = new PublicKeyEncryptionKeyPairData(
wrappedPrivateKey: encryptedPrivateKey,
publicKey: publicKey)
};
// Act

131
test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpgradeOrganizationPlanCommandTests.cs
View File

@@ -2,6 +2,7 @@
using Bit.Core.Billing.Pricing;
using Bit.Core.Billing.Services;
using Bit.Core.Exceptions;
using Bit.Core.KeyManagement.Models.Data;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.OrganizationFeatures.OrganizationSubscriptions;
@@ -242,4 +243,134 @@ public class UpgradeOrganizationPlanCommandTests
await sutProvider.GetDependency<IOrganizationService>().DidNotReceiveWithAnyArgs().ReplaceAndUpdateCacheAsync(default);
}
[Theory]
[FreeOrganizationUpgradeCustomize, BitAutoData]
public async Task UpgradePlan_WhenOrganizationIsMissingPublicAndPrivateKeys_Backfills(
Organization organization,
OrganizationUpgrade upgrade,
string newPublicKey,
string newPrivateKey,
SutProvider<UpgradeOrganizationPlanCommand> sutProvider)
{
organization.PublicKey = null;
organization.PrivateKey = null;
upgrade.Plan = PlanType.TeamsAnnually;
upgrade.Keys = new PublicKeyEncryptionKeyPairData(
wrappedPrivateKey: newPrivateKey,
publicKey: newPublicKey);
upgrade.AdditionalSeats = 10;
sutProvider.GetDependency<IOrganizationRepository>()
.GetByIdAsync(organization.Id)
.Returns(organization);
sutProvider.GetDependency<IPricingClient>()
.GetPlanOrThrow(organization.PlanType)
.Returns(MockPlans.Get(organization.PlanType));
sutProvider.GetDependency<IPricingClient>()
.GetPlanOrThrow(upgrade.Plan)
.Returns(MockPlans.Get(upgrade.Plan));
sutProvider.GetDependency<IOrganizationRepository>()
.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id)
.Returns(new OrganizationSeatCounts { Sponsored = 0, Users = 1 });
// Act
await sutProvider.Sut.UpgradePlanAsync(organization.Id, upgrade);
// Assert
Assert.Equal(newPublicKey, organization.PublicKey);
Assert.Equal(newPrivateKey, organization.PrivateKey);
await sutProvider.GetDependency<IOrganizationService>()
.Received(1)
.ReplaceAndUpdateCacheAsync(organization);
}
[Theory]
[FreeOrganizationUpgradeCustomize, BitAutoData]
public async Task UpgradePlan_WhenOrganizationAlreadyHasPublicAndPrivateKeys_DoesNotOverwriteWithNull(
Organization organization,
OrganizationUpgrade upgrade,
SutProvider<UpgradeOrganizationPlanCommand> sutProvider)
{
// Arrange
const string existingPublicKey = "existing-public-key";
const string existingPrivateKey = "existing-private-key";
organization.PublicKey = existingPublicKey;
organization.PrivateKey = existingPrivateKey;
upgrade.Plan = PlanType.TeamsAnnually;
upgrade.Keys = null;
upgrade.AdditionalSeats = 10;
sutProvider.GetDependency<IOrganizationRepository>()
.GetByIdAsync(organization.Id)
.Returns(organization);
sutProvider.GetDependency<IPricingClient>()
.GetPlanOrThrow(organization.PlanType)
.Returns(MockPlans.Get(organization.PlanType));
sutProvider.GetDependency<IPricingClient>()
.GetPlanOrThrow(upgrade.Plan)
.Returns(MockPlans.Get(upgrade.Plan));
sutProvider.GetDependency<IOrganizationRepository>()
.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id)
.Returns(new OrganizationSeatCounts { Sponsored = 0, Users = 1 });
// Act
await sutProvider.Sut.UpgradePlanAsync(organization.Id, upgrade);
// Assert
Assert.Equal(existingPublicKey, organization.PublicKey);
Assert.Equal(existingPrivateKey, organization.PrivateKey);
await sutProvider.GetDependency<IOrganizationService>()
.Received(1)
.ReplaceAndUpdateCacheAsync(organization);
}
[Theory]
[FreeOrganizationUpgradeCustomize, BitAutoData]
public async Task UpgradePlan_WhenOrganizationAlreadyHasPublicAndPrivateKeys_DoesNotBackfillWithNewKeys(
Organization organization,
OrganizationUpgrade upgrade,
SutProvider<UpgradeOrganizationPlanCommand> sutProvider)
{
// Arrange
const string existingPublicKey = "existing-public-key";
const string existingPrivateKey = "existing-private-key";
const string newPublicKey = "new-public-key";
const string newPrivateKey = "new-private-key";
organization.PublicKey = existingPublicKey;
organization.PrivateKey = existingPrivateKey;
upgrade.Plan = PlanType.TeamsAnnually;
upgrade.Keys = new PublicKeyEncryptionKeyPairData(
wrappedPrivateKey: newPrivateKey,
publicKey: newPublicKey);
upgrade.AdditionalSeats = 10;
sutProvider.GetDependency<IOrganizationRepository>()
.GetByIdAsync(organization.Id)
.Returns(organization);
sutProvider.GetDependency<IPricingClient>()
.GetPlanOrThrow(organization.PlanType)
.Returns(MockPlans.Get(organization.PlanType));
sutProvider.GetDependency<IPricingClient>()
.GetPlanOrThrow(upgrade.Plan)
.Returns(MockPlans.Get(upgrade.Plan));
sutProvider.GetDependency<IOrganizationRepository>()
.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id)
.Returns(new OrganizationSeatCounts { Sponsored = 0, Users = 1 });
// Act
await sutProvider.Sut.UpgradePlanAsync(organization.Id, upgrade);
// Assert
Assert.Equal(existingPublicKey, organization.PublicKey);
Assert.Equal(existingPrivateKey, organization.PrivateKey);
await sutProvider.GetDependency<IOrganizationService>()
.Received(1)
.ReplaceAndUpdateCacheAsync(organization);
}
}