1
0
mirror of https://github.com/bitwarden/server.git synced 2024-11-25 12:45:18 +01:00

[SM-678] ClientSecret migration (#2943)

* Init ClientSecret migration

* Fix unit tests

* Move to src/Sql/dbo_future

* Formatting changes

* Update migration date for next release

* Swap to just executing sp_refreshview

* Fix formatting

* Add EF Migrations

* Rename to ClientSecretHash

* Fix unit test

* EF column rename

* Batch the migration

* Fix formatting

* Add deprecation notice to property

* Move data migration

* Swap to CREATE OR ALTER
This commit is contained in:
Thomas Avery 2023-06-21 13:16:06 -05:00 committed by GitHub
parent 7f8b6c0bce
commit bb3a9daf98
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 7011 additions and 41 deletions

View File

@ -1,6 +1,9 @@
using Bit.Core.Exceptions;
using System.Security.Cryptography;
using System.Text;
using Bit.Core.Exceptions;
using Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces;
using Bit.Core.SecretsManager.Entities;
using Bit.Core.SecretsManager.Models.Data;
using Bit.Core.SecretsManager.Repositories;
using Bit.Core.Utilities;
@ -16,14 +19,24 @@ public class CreateAccessTokenCommand : ICreateAccessTokenCommand
_apiKeyRepository = apiKeyRepository;
}
public async Task<ApiKey> CreateAsync(ApiKey apiKey)
public async Task<ApiKeyClientSecretDetails> CreateAsync(ApiKey apiKey)
{
if (apiKey.ServiceAccountId == null)
{
throw new BadRequestException();
}
apiKey.ClientSecret = CoreHelpers.SecureRandomString(_clientSecretMaxLength);
return await _apiKeyRepository.CreateAsync(apiKey);
var clientSecret = CoreHelpers.SecureRandomString(_clientSecretMaxLength);
apiKey.ClientSecretHash = GetHash(clientSecret);
var result = await _apiKeyRepository.CreateAsync(apiKey);
return new ApiKeyClientSecretDetails { ApiKey = result, ClientSecret = clientSecret };
}
private static string GetHash(string input)
{
using var sha = SHA256.Create();
var bytes = Encoding.UTF8.GetBytes(input);
var hash = sha.ComputeHash(bytes);
return Convert.ToBase64String(hash);
}
}

View File

@ -1,6 +1,6 @@
#nullable enable
using Bit.Core.Models.Api;
using Bit.Core.SecretsManager.Entities;
using Bit.Core.SecretsManager.Models.Data;
namespace Bit.Api.SecretsManager.Models.Response;
@ -8,14 +8,14 @@ public class AccessTokenCreationResponseModel : ResponseModel
{
private const string _objectName = "accessTokenCreation";
public AccessTokenCreationResponseModel(ApiKey apiKey) : base(_objectName)
public AccessTokenCreationResponseModel(ApiKeyClientSecretDetails details) : base(_objectName)
{
Id = apiKey.Id;
Name = apiKey.Name;
ClientSecret = apiKey.ClientSecret;
ExpireAt = apiKey.ExpireAt;
CreationDate = apiKey.CreationDate;
RevisionDate = apiKey.RevisionDate;
Id = details.ApiKey.Id;
Name = details.ApiKey.Name;
ExpireAt = details.ApiKey.ExpireAt;
CreationDate = details.ApiKey.CreationDate;
RevisionDate = details.ApiKey.RevisionDate;
ClientSecret = details.ClientSecret;
}
public AccessTokenCreationResponseModel() : base(_objectName)

View File

@ -1,8 +1,9 @@
using Bit.Core.SecretsManager.Entities;
using Bit.Core.SecretsManager.Models.Data;
namespace Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces;
public interface ICreateAccessTokenCommand
{
Task<ApiKey> CreateAsync(ApiKey apiKey);
Task<ApiKeyClientSecretDetails> CreateAsync(ApiKey apiKey);
}

View File

@ -10,8 +10,8 @@ public class ApiKey : ITableObject<Guid>
public Guid? ServiceAccountId { get; set; }
[MaxLength(200)]
public string Name { get; set; }
[MaxLength(30)]
public string ClientSecret { get; set; }
[MaxLength(128)]
public string ClientSecretHash { get; set; }
[MaxLength(4000)]
public string Scope { get; set; }
[MaxLength(4000)]

View File

@ -0,0 +1,9 @@
using Bit.Core.SecretsManager.Entities;
namespace Bit.Core.SecretsManager.Models.Data;
public class ApiKeyClientSecretDetails
{
public ApiKey ApiKey { get; set; }
public string ClientSecret { get; set; }
}

View File

@ -4,6 +4,8 @@ namespace Bit.Core.SecretsManager.Models.Data;
public class ApiKeyDetails : ApiKey
{
public string ClientSecret { get; set; } // Deprecated as of 2023-05-17
protected ApiKeyDetails() { }
protected ApiKeyDetails(ApiKey apiKey)
@ -11,7 +13,7 @@ public class ApiKeyDetails : ApiKey
Id = apiKey.Id;
ServiceAccountId = apiKey.ServiceAccountId;
Name = apiKey.Name;
ClientSecret = apiKey.ClientSecret;
ClientSecretHash = apiKey.ClientSecretHash;
Scope = apiKey.Scope;
EncryptedPayload = apiKey.EncryptedPayload;
Key = apiKey.Key;

View File

@ -107,11 +107,16 @@ public class ClientStore : IClientStore
break;
}
if (string.IsNullOrEmpty(apiKey.ClientSecretHash))
{
apiKey.ClientSecretHash = apiKey.ClientSecret.Sha256();
}
var client = new Client
{
ClientId = clientId,
RequireClientSecret = true,
ClientSecrets = { new Secret(apiKey.ClientSecret.Sha256()) },
ClientSecrets = { new Secret(apiKey.ClientSecretHash) },
AllowedScopes = apiKey.GetScopes(),
AllowedGrantTypes = GrantTypes.ClientCredentials,
AccessTokenLifetime = 3600 * 1,

View File

@ -2,7 +2,8 @@ CREATE PROCEDURE [dbo].[ApiKey_Create]
@Id UNIQUEIDENTIFIER OUTPUT,
@ServiceAccountId UNIQUEIDENTIFIER,
@Name VARCHAR(200),
@ClientSecret VARCHAR(30),
@ClientSecret VARCHAR(30) = 'migrated', -- Deprecated as of 2023-05-17
@ClientSecretHash VARCHAR(128) = NULL,
@Scope NVARCHAR(4000),
@EncryptedPayload NVARCHAR(4000),
@Key VARCHAR(MAX),
@ -13,12 +14,19 @@ AS
BEGIN
SET NOCOUNT ON
IF (@ClientSecretHash IS NULL)
BEGIN
DECLARE @hb VARBINARY(128) = HASHBYTES('SHA2_256', @ClientSecret);
SET @ClientSecretHash = CAST(N'' as xml).value('xs:base64Binary(sql:variable("@hb"))', 'VARCHAR(128)');
END
INSERT INTO [dbo].[ApiKey]
(
[Id],
[ServiceAccountId],
[Name],
[ClientSecret],
[ClientSecretHash],
[Scope],
[EncryptedPayload],
[Key],
@ -32,6 +40,7 @@ BEGIN
@ServiceAccountId,
@Name,
@ClientSecret,
@ClientSecretHash,
@Scope,
@EncryptedPayload,
@Key,

View File

@ -1,14 +1,15 @@
CREATE TABLE [dbo].[ApiKey] (
[Id] UNIQUEIDENTIFIER,
[ServiceAccountId] UNIQUEIDENTIFIER NULL,
[Name] VARCHAR(200) NOT NULL,
[ClientSecret] VARCHAR(30) NOT NULL,
[Scope] NVARCHAR (4000) NOT NULL,
[EncryptedPayload] NVARCHAR (4000) NOT NULL,
[Key] VARCHAR (MAX) NOT NULL,
[ExpireAt] DATETIME2(7) NULL,
[CreationDate] DATETIME2(7) NOT NULL,
[RevisionDate] DATETIME2(7) NOT NULL,
[Id] UNIQUEIDENTIFIER,
[ServiceAccountId] UNIQUEIDENTIFIER NULL,
[Name] VARCHAR(200) NOT NULL,
[ClientSecret] VARCHAR(30) NOT NULL,
[ClientSecretHash] VARCHAR(128) NULL,
[Scope] NVARCHAR (4000) NOT NULL,
[EncryptedPayload] NVARCHAR (4000) NOT NULL,
[Key] VARCHAR (MAX) NOT NULL,
[ExpireAt] DATETIME2(7) NULL,
[CreationDate] DATETIME2(7) NOT NULL,
[RevisionDate] DATETIME2(7) NOT NULL,
CONSTRAINT [PK_ApiKey] PRIMARY KEY CLUSTERED ([Id] ASC),
CONSTRAINT [FK_ApiKey_ServiceAccountId] FOREIGN KEY ([ServiceAccountId]) REFERENCES [dbo].[ServiceAccount] ([Id])
);

View File

@ -0,0 +1,42 @@
CREATE PROCEDURE [dbo].[ApiKey_Create]
@Id UNIQUEIDENTIFIER OUTPUT,
@ServiceAccountId UNIQUEIDENTIFIER,
@Name VARCHAR(200),
@ClientSecretHash VARCHAR(128),
@Scope NVARCHAR(4000),
@EncryptedPayload NVARCHAR(4000),
@Key VARCHAR(MAX),
@ExpireAt DATETIME2(7),
@CreationDate DATETIME2(7),
@RevisionDate DATETIME2(7)
AS
BEGIN
SET NOCOUNT ON
INSERT INTO [dbo].[ApiKey]
(
[Id],
[ServiceAccountId],
[Name],
[ClientSecretHash],
[Scope],
[EncryptedPayload],
[Key],
[ExpireAt],
[CreationDate],
[RevisionDate]
)
VALUES
(
@Id,
@ServiceAccountId,
@Name,
@ClientSecretHash,
@Scope,
@EncryptedPayload,
@Key,
@ExpireAt,
@CreationDate,
@RevisionDate
)
END

View File

@ -0,0 +1,18 @@
CREATE TABLE [dbo].[ApiKey] (
[Id] UNIQUEIDENTIFIER,
[ServiceAccountId] UNIQUEIDENTIFIER NULL,
[Name] VARCHAR(200) NOT NULL,
[ClientSecretHash] VARCHAR(128) NULL,
[Scope] NVARCHAR (4000) NOT NULL,
[EncryptedPayload] NVARCHAR (4000) NOT NULL,
[Key] VARCHAR (MAX) NOT NULL,
[ExpireAt] DATETIME2(7) NULL,
[CreationDate] DATETIME2(7) NOT NULL,
[RevisionDate] DATETIME2(7) NOT NULL,
CONSTRAINT [PK_ApiKey] PRIMARY KEY CLUSTERED ([Id] ASC),
CONSTRAINT [FK_ApiKey_ServiceAccountId] FOREIGN KEY ([ServiceAccountId]) REFERENCES [dbo].[ServiceAccount] ([Id])
);
GO
CREATE NONCLUSTERED INDEX [IX_ApiKey_ServiceAccountId]
ON [dbo].[ApiKey]([ServiceAccountId] ASC);

View File

@ -7,6 +7,7 @@ using Bit.Core.Exceptions;
using Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces;
using Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces;
using Bit.Core.SecretsManager.Entities;
using Bit.Core.SecretsManager.Models.Data;
using Bit.Core.SecretsManager.Repositories;
using Bit.Core.Services;
using Bit.Test.Common.AutoFixture;
@ -144,7 +145,7 @@ public class ServiceAccountsControllerTests
[Theory]
[BitAutoData]
public async void CreateAccessToken_NoAccess_Throws(SutProvider<ServiceAccountsController> sutProvider,
AccessTokenCreateRequestModel data, ServiceAccount serviceAccount)
AccessTokenCreateRequestModel data, ServiceAccount serviceAccount, string mockClientSecret)
{
sutProvider.GetDependency<IServiceAccountRepository>().GetByIdAsync(serviceAccount.Id).Returns(serviceAccount);
sutProvider.GetDependency<IAuthorizationService>()
@ -152,8 +153,9 @@ public class ServiceAccountsControllerTests
Arg.Any<IEnumerable<IAuthorizationRequirement>>()).ReturnsForAnyArgs(AuthorizationResult.Failed());
var resultAccessToken = data.ToApiKey(serviceAccount.Id);
sutProvider.GetDependency<ICreateAccessTokenCommand>().CreateAsync(default)
.ReturnsForAnyArgs(resultAccessToken);
sutProvider.GetDependency<ICreateAccessTokenCommand>()
.CreateAsync(default)
.ReturnsForAnyArgs(new ApiKeyClientSecretDetails { ApiKey = resultAccessToken, ClientSecret = mockClientSecret });
await Assert.ThrowsAsync<NotFoundException>(() =>
sutProvider.Sut.CreateAccessTokenAsync(serviceAccount.Id, data));
@ -164,7 +166,7 @@ public class ServiceAccountsControllerTests
[Theory]
[BitAutoData]
public async void CreateAccessToken_Success(SutProvider<ServiceAccountsController> sutProvider,
AccessTokenCreateRequestModel data, ServiceAccount serviceAccount)
AccessTokenCreateRequestModel data, ServiceAccount serviceAccount, string mockClientSecret)
{
sutProvider.GetDependency<IServiceAccountRepository>().GetByIdAsync(serviceAccount.Id).Returns(serviceAccount);
sutProvider.GetDependency<IAuthorizationService>()
@ -173,7 +175,7 @@ public class ServiceAccountsControllerTests
var resultAccessToken = data.ToApiKey(serviceAccount.Id);
sutProvider.GetDependency<ICreateAccessTokenCommand>().CreateAsync(default)
.ReturnsForAnyArgs(resultAccessToken);
.ReturnsForAnyArgs(new ApiKeyClientSecretDetails { ApiKey = resultAccessToken, ClientSecret = mockClientSecret });
await sutProvider.Sut.CreateAccessTokenAsync(serviceAccount.Id, data);
await sutProvider.GetDependency<ICreateAccessTokenCommand>().Received(1)

View File

@ -0,0 +1,72 @@
IF COL_LENGTH('[dbo].[ApiKey]', 'ClientSecretHash') IS NULL
BEGIN
ALTER TABLE [dbo].[ApiKey]
ADD [ClientSecretHash] VARCHAR(128);
END
GO
-- Refresh views
IF OBJECT_ID('[dbo].[ApiKeyDetailsView]') IS NOT NULL
BEGIN
EXECUTE sp_refreshview N'[dbo].[ApiKeyDetailsView]';
END
GO
IF OBJECT_ID('[dbo].[ApiKeyView]') IS NOT NULL
BEGIN
EXECUTE sp_refreshview N'[dbo].[ApiKeyView]';
END
GO
CREATE OR ALTER PROCEDURE [dbo].[ApiKey_Create]
@Id UNIQUEIDENTIFIER OUTPUT,
@ServiceAccountId UNIQUEIDENTIFIER,
@Name VARCHAR(200),
@ClientSecret VARCHAR(30) = 'migrated', -- Deprecated as of 2023-05-17
@ClientSecretHash VARCHAR(128) = NULL,
@Scope NVARCHAR(4000),
@EncryptedPayload NVARCHAR(4000),
@Key VARCHAR(MAX),
@ExpireAt DATETIME2(7),
@CreationDate DATETIME2(7),
@RevisionDate DATETIME2(7)
AS
BEGIN
SET NOCOUNT ON
IF (@ClientSecretHash IS NULL)
BEGIN
DECLARE @hb VARBINARY(128) = HASHBYTES('SHA2_256', @ClientSecret);
SET @ClientSecretHash = CAST(N'' as xml).value('xs:base64Binary(sql:variable("@hb"))', 'VARCHAR(128)');
END
INSERT INTO [dbo].[ApiKey]
(
[Id],
[ServiceAccountId],
[Name],
[ClientSecret],
[ClientSecretHash],
[Scope],
[EncryptedPayload],
[Key],
[ExpireAt],
[CreationDate],
[RevisionDate]
)
VALUES
(
@Id,
@ServiceAccountId,
@Name,
@ClientSecret,
@ClientSecretHash,
@Scope,
@EncryptedPayload,
@Key,
@ExpireAt,
@CreationDate,
@RevisionDate
)
END
GO

View File

@ -0,0 +1,42 @@
/*
This is the data migration script for the client secret hash updates.
The initial migration util/Migrator/DbScripts/2023-05-16_00_ClientSecretHash.sql should be run prior.
The final migration is in util/Migrator/DbScripts_future/2023-06-FutureMigration.sql.
*/
IF COL_LENGTH('[dbo].[ApiKey]', 'ClientSecretHash') IS NOT NULL AND COL_LENGTH('[dbo].[ApiKey]', 'ClientSecret') IS NOT NULL
BEGIN
-- Add index
IF NOT EXISTS(SELECT name FROM sys.indexes WHERE name = 'IX_ApiKey_ClientSecretHash')
BEGIN
CREATE NONCLUSTERED INDEX [IX_ApiKey_ClientSecretHash]
ON [dbo].[ApiKey]([ClientSecretHash] ASC)
WITH (ONLINE = ON)
END
-- Data Migration
DECLARE @BatchSize INT = 10000
WHILE @BatchSize > 0
BEGIN
BEGIN TRANSACTION Migrate_ClientSecretHash
UPDATE TOP(@BatchSize) [dbo].[ApiKey]
SET ClientSecretHash = (
SELECT CAST(N'' AS XML).value('xs:base64Binary(sql:column("HASH"))', 'VARCHAR(128)')
FROM (
SELECT HASHBYTES('SHA2_256', [ClientSecret]) AS HASH
) SRC
)
WHERE [ClientSecretHash] IS NULL
SET @BatchSize = @@ROWCOUNT
COMMIT TRANSACTION Migrate_ClientSecretHash
END
-- Drop index
DROP INDEX IF EXISTS [IX_ApiKey_ClientSecretHash]
ON [dbo].[ApiKey];
END
GO

View File

@ -0,0 +1,65 @@
-- Remove Column
IF COL_LENGTH('[dbo].[ApiKey]', 'ClientSecret') IS NOT NULL
BEGIN
ALTER TABLE
[dbo].[ApiKey]
DROP COLUMN
[ClientSecret]
END
GO
-- Refresh views
IF OBJECT_ID('[dbo].[ApiKeyDetailsView]') IS NOT NULL
BEGIN
EXECUTE sp_refreshview N'[dbo].[ApiKeyDetailsView]';
END
GO
IF OBJECT_ID('[dbo].[ApiKeyView]') IS NOT NULL
BEGIN
EXECUTE sp_refreshview N'[dbo].[ApiKeyView]';
END
GO
CREATE OR ALTER PROCEDURE [dbo].[ApiKey_Create]
@Id UNIQUEIDENTIFIER OUTPUT,
@ServiceAccountId UNIQUEIDENTIFIER,
@Name VARCHAR(200),
@ClientSecretHash VARCHAR(128),
@Scope NVARCHAR(4000),
@EncryptedPayload NVARCHAR(4000),
@Key VARCHAR(MAX),
@ExpireAt DATETIME2(7),
@CreationDate DATETIME2(7),
@RevisionDate DATETIME2(7)
AS
BEGIN
SET NOCOUNT ON
INSERT INTO [dbo].[ApiKey]
(
[Id],
[ServiceAccountId],
[Name],
[ClientSecretHash],
[Scope],
[EncryptedPayload],
[Key],
[ExpireAt],
[CreationDate],
[RevisionDate]
)
VALUES
(
@Id,
@ServiceAccountId,
@Name,
@ClientSecretHash,
@Scope,
@EncryptedPayload,
@Key,
@ExpireAt,
@CreationDate,
@RevisionDate
)
END

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,38 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.MySqlMigrations.Migrations;
public partial class ClientSecretHash : Migration
{
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "ClientSecret",
table: "ApiKey");
migrationBuilder.AddColumn<string>(
name: "ClientSecretHash",
table: "ApiKey",
type: "varchar(128)",
maxLength: 128,
nullable: true)
.Annotation("MySql:CharSet", "utf8mb4");
}
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "ClientSecretHash",
table: "ApiKey");
migrationBuilder.AddColumn<string>(
name: "ClientSecret",
table: "ApiKey",
type: "varchar(30)",
maxLength: 30,
nullable: true)
.Annotation("MySql:CharSet", "utf8mb4");
}
}

View File

@ -1335,9 +1335,9 @@ namespace Bit.MySqlMigrations.Migrations
b.Property<Guid>("Id")
.HasColumnType("char(36)");
b.Property<string>("ClientSecret")
.HasMaxLength(30)
.HasColumnType("varchar(30)");
b.Property<string>("ClientSecretHash")
.HasMaxLength(128)
.HasColumnType("varchar(128)");
b.Property<DateTime>("CreationDate")
.HasColumnType("datetime(6)");

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,36 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.PostgresMigrations.Migrations;
public partial class ClientSecretHash : Migration
{
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "ClientSecret",
table: "ApiKey");
migrationBuilder.AddColumn<string>(
name: "ClientSecretHash",
table: "ApiKey",
type: "character varying(128)",
maxLength: 128,
nullable: true);
}
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "ClientSecretHash",
table: "ApiKey");
migrationBuilder.AddColumn<string>(
name: "ClientSecret",
table: "ApiKey",
type: "character varying(30)",
maxLength: 30,
nullable: true);
}
}

View File

@ -1346,9 +1346,9 @@ namespace Bit.PostgresMigrations.Migrations
b.Property<Guid>("Id")
.HasColumnType("uuid");
b.Property<string>("ClientSecret")
.HasMaxLength(30)
.HasColumnType("character varying(30)");
b.Property<string>("ClientSecretHash")
.HasMaxLength(128)
.HasColumnType("character varying(128)");
b.Property<DateTime>("CreationDate")
.HasColumnType("timestamp with time zone");

File diff suppressed because it is too large Load Diff

View File

@ -0,0 +1,36 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace Bit.SqliteMigrations.Migrations;
public partial class ClientSecretHash : Migration
{
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "ClientSecret",
table: "ApiKey");
migrationBuilder.AddColumn<string>(
name: "ClientSecretHash",
table: "ApiKey",
type: "TEXT",
maxLength: 128,
nullable: true);
}
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "ClientSecretHash",
table: "ApiKey");
migrationBuilder.AddColumn<string>(
name: "ClientSecret",
table: "ApiKey",
type: "TEXT",
maxLength: 30,
nullable: true);
}
}

View File

@ -1333,8 +1333,8 @@ namespace Bit.SqliteMigrations.Migrations
b.Property<Guid>("Id")
.HasColumnType("TEXT");
b.Property<string>("ClientSecret")
.HasMaxLength(30)
b.Property<string>("ClientSecretHash")
.HasMaxLength(128)
.HasColumnType("TEXT");
b.Property<DateTime>("CreationDate")