Merge branch 'main' into billing/PM-28100/2019-families-email

# Conflicts:
#	src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs
#	test/Billing.Test/Services/UpcomingInvoiceHandlerTests.cs

Author: kdenney

.github/workflows/build.yml

@@ -280,7 +280,7 @@ jobs:
           output-format: sarif
 
       - name: Upload Grype results to GitHub
-        uses: github/codeql-action/upload-sarif@0499de31b99561a6d14a36a5f662c2a54f91beee # v4.31.2
+        uses: github/codeql-action/upload-sarif@e12f0178983d466f2f6028f5cc7a6d786fd97f4b # v4.31.4
         with:
           sarif_file: ${{ steps.container-scan.outputs.sarif }}
           sha: ${{ contains(github.event_name, 'pull_request') && github.event.pull_request.head.sha || github.sha }}

src/Api/AdminConsole/Controllers/OrganizationsController.cs

@@ -12,7 +12,6 @@
 using Bit.Api.Models.Request.Organizations;
 using Bit.Api.Models.Response;
 using Bit.Core;
-using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Models.Business.Tokenables;
 using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
@@ -70,6 +69,7 @@ public class OrganizationsController : Controller
     private readonly IPolicyRequirementQuery _policyRequirementQuery;
     private readonly IPricingClient _pricingClient;
     private readonly IOrganizationUpdateKeysCommand _organizationUpdateKeysCommand;
+    private readonly IOrganizationUpdateCommand _organizationUpdateCommand;
 
     public OrganizationsController(
         IOrganizationRepository organizationRepository,
@@ -94,7 +94,8 @@ public OrganizationsController(
         IOrganizationDeleteCommand organizationDeleteCommand,
         IPolicyRequirementQuery policyRequirementQuery,
         IPricingClient pricingClient,
-        IOrganizationUpdateKeysCommand organizationUpdateKeysCommand)
+        IOrganizationUpdateKeysCommand organizationUpdateKeysCommand,
+        IOrganizationUpdateCommand organizationUpdateCommand)
     {
         _organizationRepository = organizationRepository;
         _organizationUserRepository = organizationUserRepository;
@@ -119,6 +120,7 @@ public OrganizationsController(
         _policyRequirementQuery = policyRequirementQuery;
         _pricingClient = pricingClient;
         _organizationUpdateKeysCommand = organizationUpdateKeysCommand;
+        _organizationUpdateCommand = organizationUpdateCommand;
     }
 
     [HttpGet("{id}")]
@@ -224,36 +226,31 @@ public async Task<OrganizationResponseModel> CreateWithoutPaymentAsync([FromBody
         return new OrganizationResponseModel(result.Organization, plan);
     }
 
-    [HttpPut("{id}")]
-    public async Task<OrganizationResponseModel> Put(string id, [FromBody] OrganizationUpdateRequestModel model)
+    [HttpPut("{organizationId:guid}")]
+    public async Task<IResult> Put(Guid organizationId, [FromBody] OrganizationUpdateRequestModel model)
     {
-        var orgIdGuid = new Guid(id);
+        // If billing email is being changed, require subscription editing permissions.
+        // Otherwise, organization owner permissions are sufficient.
+        var requiresBillingPermission = model.BillingEmail is not null;
+        var authorized = requiresBillingPermission
+            ? await _currentContext.EditSubscription(organizationId)
+            : await _currentContext.OrganizationOwner(organizationId);
 
-        var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
-        if (organization == null)
+        if (!authorized)
         {
-            throw new NotFoundException();
+            return TypedResults.Unauthorized();
         }
 
-        var updateBilling = ShouldUpdateBilling(model, organization);
-
-        var hasRequiredPermissions = updateBilling
-            ? await _currentContext.EditSubscription(orgIdGuid)
-            : await _currentContext.OrganizationOwner(orgIdGuid);
-
-        if (!hasRequiredPermissions)
-        {
-            throw new NotFoundException();
-        }
+        var commandRequest = model.ToCommandRequest(organizationId);
+        var updatedOrganization = await _organizationUpdateCommand.UpdateAsync(commandRequest);
 
-        await _organizationService.UpdateAsync(model.ToOrganization(organization, _globalSettings), updateBilling);
-        var plan = await _pricingClient.GetPlan(organization.PlanType);
-        return new OrganizationResponseModel(organization, plan);
+        var plan = await _pricingClient.GetPlan(updatedOrganization.PlanType);
+        return TypedResults.Ok(new OrganizationResponseModel(updatedOrganization, plan));
     }
 
     [HttpPost("{id}")]
     [Obsolete("This endpoint is deprecated. Use PUT method instead")]
-    public async Task<OrganizationResponseModel> PostPut(string id, [FromBody] OrganizationUpdateRequestModel model)
+    public async Task<IResult> PostPut(Guid id, [FromBody] OrganizationUpdateRequestModel model)
     {
         return await Put(id, model);
     }
@@ -588,11 +585,4 @@ public async Task<PlanType> GetPlanType(string id)
 
         return organization.PlanType;
     }
-
-    private bool ShouldUpdateBilling(OrganizationUpdateRequestModel model, Organization organization)
-    {
-        var organizationNameChanged = model.Name != organization.Name;
-        var billingEmailChanged = model.BillingEmail != organization.BillingEmail;
-        return !_globalSettings.SelfHosted && (organizationNameChanged || billingEmailChanged);
-    }
 }

src/Api/AdminConsole/Models/Request/Organizations/OrganizationUpdateRequestModel.cs

@@ -1,41 +1,28 @@
-// FIXME: Update this file to be null safe and then delete the line below
-#nullable disable
-
-using System.ComponentModel.DataAnnotations;
+using System.ComponentModel.DataAnnotations;
 using System.Text.Json.Serialization;
-using Bit.Core.AdminConsole.Entities;
-using Bit.Core.Models.Data;
-using Bit.Core.Settings;
+using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Update;
 using Bit.Core.Utilities;
 
 namespace Bit.Api.AdminConsole.Models.Request.Organizations;
 
 public class OrganizationUpdateRequestModel
 {
-    [Required]
     [StringLength(50, ErrorMessage = "The field Name exceeds the maximum length.")]
     [JsonConverter(typeof(HtmlEncodingStringConverter))]
-    public string Name { get; set; }
-    [StringLength(50, ErrorMessage = "The field Business Name exceeds the maximum length.")]
-    [JsonConverter(typeof(HtmlEncodingStringConverter))]
-    public string BusinessName { get; set; }
+    public string? Name { get; set; }
+
     [EmailAddress]
-    [Required]
     [StringLength(256)]
-    public string BillingEmail { get; set; }
-    public Permissions Permissions { get; set; }
-    public OrganizationKeysRequestModel Keys { get; set; }
+    public string? BillingEmail { get; set; }
+
+    public OrganizationKeysRequestModel? Keys { get; set; }
 
-    public virtual Organization ToOrganization(Organization existingOrganization, GlobalSettings globalSettings)
+    public OrganizationUpdateRequest ToCommandRequest(Guid organizationId) => new()
     {
-        if (!globalSettings.SelfHosted)
-        {
-            // These items come from the license file
-            existingOrganization.Name = Name;
-            existingOrganization.BusinessName = BusinessName;
-            existingOrganization.BillingEmail = BillingEmail?.ToLowerInvariant()?.Trim();
-        }
-        Keys?.ToOrganization(existingOrganization);
-        return existingOrganization;
-    }
+        OrganizationId = organizationId,
+        Name = Name,
+        BillingEmail = BillingEmail,
+        PublicKey = Keys?.PublicKey,
+        EncryptedPrivateKey = Keys?.EncryptedPrivateKey
+    };
 }

src/Core/AdminConsole/OrganizationFeatures/Organizations/Interfaces/IOrganizationUpdateCommand.cs

@@ -0,0 +1,15 @@
+using Bit.Core.AdminConsole.Entities;
+using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Update;
+
+namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Interfaces;
+
+public interface IOrganizationUpdateCommand
+{
+    /// <summary>
+    /// Updates an organization's information in the Bitwarden database and Stripe (if required).
+    /// Also optionally updates an organization's public-private keypair if it was not created with one.
+    /// On self-host, only the public-private keys will be updated because all other properties are fixed by the license file.
+    /// </summary>
+    /// <param name="request">The update request containing the details to be updated.</param>
+    Task<Organization> UpdateAsync(OrganizationUpdateRequest request);
+}

src/Core/AdminConsole/OrganizationFeatures/Organizations/Update/OrganizationUpdateCommand.cs

@@ -0,0 +1,77 @@
+using Bit.Core.AdminConsole.Entities;
+using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Interfaces;
+using Bit.Core.Billing.Organizations.Services;
+using Bit.Core.Enums;
+using Bit.Core.Exceptions;
+using Bit.Core.Repositories;
+using Bit.Core.Services;
+using Bit.Core.Settings;
+
+namespace Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Update;
+
+public class OrganizationUpdateCommand(
+    IOrganizationService organizationService,
+    IOrganizationRepository organizationRepository,
+    IGlobalSettings globalSettings,
+    IOrganizationBillingService organizationBillingService
+) : IOrganizationUpdateCommand
+{
+    public async Task<Organization> UpdateAsync(OrganizationUpdateRequest request)
+    {
+        var organization = await organizationRepository.GetByIdAsync(request.OrganizationId);
+        if (organization == null)
+        {
+            throw new NotFoundException();
+        }
+
+        if (globalSettings.SelfHosted)
+        {
+            return await UpdateSelfHostedAsync(organization, request);
+        }
+
+        return await UpdateCloudAsync(organization, request);
+    }
+
+    private async Task<Organization> UpdateCloudAsync(Organization organization, OrganizationUpdateRequest request)
+    {
+        // Store original values for comparison
+        var originalName = organization.Name;
+        var originalBillingEmail = organization.BillingEmail;
+
+        // Apply updates to organization
+        organization.UpdateDetails(request);
+        organization.BackfillPublicPrivateKeys(request);
+        await organizationService.ReplaceAndUpdateCacheAsync(organization, EventType.Organization_Updated);
+
+        // Update billing information in Stripe if required
+        await UpdateBillingAsync(organization, originalName, originalBillingEmail);
+
+        return organization;
+    }
+
+    /// <summary>
+    /// Self-host cannot update the organization details because they are set by the license file.
+    /// However, this command does offer a soft migration pathway for organizations without public and private keys.
+    /// If we remove this migration code in the future, this command and endpoint can become cloud only.
+    /// </summary>
+    private async Task<Organization> UpdateSelfHostedAsync(Organization organization, OrganizationUpdateRequest request)
+    {
+        organization.BackfillPublicPrivateKeys(request);
+        await organizationService.ReplaceAndUpdateCacheAsync(organization, EventType.Organization_Updated);
+        return organization;
+    }
+
+    private async Task UpdateBillingAsync(Organization organization, string originalName, string? originalBillingEmail)
+    {
+        // Update Stripe if name or billing email changed
+        var shouldUpdateBilling = originalName != organization.Name ||
+                                  originalBillingEmail != organization.BillingEmail;
+
+        if (!shouldUpdateBilling || string.IsNullOrWhiteSpace(organization.GatewayCustomerId))
+        {
+            return;
+        }
+
+        await organizationBillingService.UpdateOrganizationNameAndEmail(organization);
+    }
+}

src/Core/AdminConsole/OrganizationFeatures/Organizations/Update/OrganizationUpdateExtensions.cs

@@ -0,0 +1,43 @@
+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;
+        }
+    }
+}

src/Core/AdminConsole/OrganizationFeatures/Organizations/Update/OrganizationUpdateRequest.cs

@@ -0,0 +1,33 @@
+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).
+/// Any combination of these properties can be updated, so they are optional. If none are specified it will not update anything.
+/// </summary>
+public record OrganizationUpdateRequest
+{
+    /// <summary>
+    /// The ID of the organization to update.
+    /// </summary>
+    public required Guid OrganizationId { get; init; }
+
+    /// <summary>
+    /// The new organization name to apply (optional, this is skipped if not provided).
+    /// </summary>
+    public string? Name { get; init; }
+
+    /// <summary>
+    /// The new billing email address to apply (optional, this is skipped if not provided).
+    /// </summary>
+    public string? BillingEmail { get; init; }
+
+    /// <summary>
+    /// The organization's public key 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; }
+}

src/Core/Billing/Organizations/Services/IOrganizationBillingService.cs

@@ -56,4 +56,15 @@ Task UpdatePaymentMethod(
     /// <exception cref="ArgumentNullException">Thrown when the <paramref name="organization"/> is <see langword="null"/>.</exception>
     /// <exception cref="BillingException">Thrown when no payment method is found for the customer, no plan IDs are provided, or subscription update fails.</exception>
     Task UpdateSubscriptionPlanFrequency(Organization organization, PlanType newPlanType);
+
+    /// <summary>
+    /// Updates the organization name and email on the Stripe customer entry.
+    /// This only updates Stripe, not the Bitwarden database.
+    /// </summary>
+    /// <remarks>
+    /// The caller should ensure that the organization has a GatewayCustomerId before calling this method.
+    /// </remarks>
+    /// <param name="organization">The organization to update in Stripe.</param>
+    /// <exception cref="BillingException">Thrown when the organization does not have a GatewayCustomerId.</exception>
+    Task UpdateOrganizationNameAndEmail(Organization organization);
 }

src/Core/Billing/Organizations/Services/OrganizationBillingService.cs

@@ -176,6 +176,35 @@ public async Task UpdateSubscriptionPlanFrequency(
         }
     }
 
+    public async Task UpdateOrganizationNameAndEmail(Organization organization)
+    {
+        if (organization.GatewayCustomerId is null)
+        {
+            throw new BillingException("Cannot update an organization in Stripe without a GatewayCustomerId.");
+        }
+
+        var newDisplayName = organization.DisplayName();
+
+        await stripeAdapter.CustomerUpdateAsync(organization.GatewayCustomerId,
+            new CustomerUpdateOptions
+            {
+                Email = organization.BillingEmail,
+                Description = newDisplayName,
+                InvoiceSettings = new CustomerInvoiceSettingsOptions
+                {
+                    // This overwrites the existing custom fields for this organization
+                    CustomFields = [
+                        new CustomerInvoiceSettingsCustomFieldOptions
+                        {
+                            Name = organization.SubscriberType(),
+                            Value = newDisplayName.Length <= 30
+                                ? newDisplayName
+                                : newDisplayName[..30]
+                        }]
+                },
+            });
+    }
+
     #region Utilities
 
     private async Task<Customer> CreateCustomerAsync(