Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-14373] Introduce SecurityTask database table and repository #5025

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
20 changes: 20 additions & 0 deletions src/Core/Vault/Entities/SecurityTask.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
๏ปฟusing Bit.Core.Entities;
using Bit.Core.Utilities;

namespace Bit.Core.Vault.Entities;

public class SecurityTask : ITableObject<Guid>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โ“ Where'd the "security" come from? Did we discuss this and I'm crazy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not crazy haha, it was originally just Task.

But, that was going to cause conflicts with the System.Threading.Tasks Task and would require us to either use a fully qualified name Bit.Core.Vault.Entities.Task any time we want to reference our Task and/or use aliases everywhere. It seemed like a headache we could avoid by tweaking the name at the source.

We opted for SecurityTask originally as it was related to the initiative, but if you think there's a better name (maybe BitTask?) or if you think we should go ahead with the same Task name we're happy to discuss further!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming things is hard but you did the work already and I am not that opinionated about it so we can leave it.

{
public Guid Id { get; set; }
public Guid OrganizationId { get; set; }
public Guid? CipherId { get; set; }
public Enums.SecurityTaskType Type { get; set; }
public Enums.SecurityTaskStatus Status { get; set; }
public DateTime CreationDate { get; set; } = DateTime.UtcNow;
public DateTime RevisionDate { get; set; } = DateTime.UtcNow;

Check warning on line 14 in src/Core/Vault/Entities/SecurityTask.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/Vault/Entities/SecurityTask.cs#L8-L14

Added lines #L8 - L14 were not covered by tests

public void SetNewId()
{
Id = CoreHelpers.GenerateComb();
}

Check warning on line 19 in src/Core/Vault/Entities/SecurityTask.cs

View check run for this annotation

Codecov / codecov/patch

src/Core/Vault/Entities/SecurityTask.cs#L17-L19

Added lines #L17 - L19 were not covered by tests
}
18 changes: 18 additions & 0 deletions src/Core/Vault/Enums/SecurityTaskStatus.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
๏ปฟusing System.ComponentModel.DataAnnotations;

namespace Bit.Core.Vault.Enums;

public enum SecurityTaskStatus : byte
{
/// <summary>
/// Default status for newly created tasks that have not been completed.
/// </summary>
[Display(Name = "Pending")]
Pending = 0,

/// <summary>
/// Status when a task is considered complete and has no remaining actions
/// </summary>
[Display(Name = "Completed")]
Completed = 1,
}
12 changes: 12 additions & 0 deletions src/Core/Vault/Enums/SecurityTaskType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
๏ปฟusing System.ComponentModel.DataAnnotations;

namespace Bit.Core.Vault.Enums;

public enum SecurityTaskType : byte
{
/// <summary>
/// Task to update a cipher's password that was found to be at-risk by an administrator
/// </summary>
[Display(Name = "Update at-risk credential")]
UpdateAtRiskCredential = 0
}
9 changes: 9 additions & 0 deletions src/Core/Vault/Repositories/ISecurityTaskRepository.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
๏ปฟusing Bit.Core.Repositories;
using Bit.Core.Vault.Entities;

namespace Bit.Core.Vault.Repositories;

public interface ISecurityTaskRepository : IRepository<SecurityTask, Guid>
{

}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
services
.AddSingleton<IClientOrganizationMigrationRecordRepository, ClientOrganizationMigrationRecordRepository>();
services.AddSingleton<IPasswordHealthReportApplicationRepository, PasswordHealthReportApplicationRepository>();
services.AddSingleton<ISecurityTaskRepository, SecurityTaskRepository>();

Check warning on line 62 in src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs#L62

Added line #L62 was not covered by tests

if (selfHosted)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
๏ปฟusing Bit.Core.Settings;
using Bit.Core.Vault.Entities;
using Bit.Core.Vault.Repositories;
using Bit.Infrastructure.Dapper.Repositories;

namespace Bit.Infrastructure.Dapper.Vault.Repositories;

public class SecurityTaskRepository : Repository<SecurityTask, Guid>, ISecurityTaskRepository
{
public SecurityTaskRepository(GlobalSettings globalSettings)
: this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString)
{ }

Check warning on line 12 in src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs#L11-L12

Added lines #L11 - L12 were not covered by tests

public SecurityTaskRepository(string connectionString, string readOnlyConnectionString)
: base(connectionString, readOnlyConnectionString)
{ }

Check warning on line 16 in src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.Dapper/Vault/Repositories/SecurityTaskRepository.cs#L15-L16

Added lines #L15 - L16 were not covered by tests

}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public static void AddPasswordManagerEFRepositories(this IServiceCollection serv
services
.AddSingleton<IClientOrganizationMigrationRecordRepository, ClientOrganizationMigrationRecordRepository>();
services.AddSingleton<IPasswordHealthReportApplicationRepository, PasswordHealthReportApplicationRepository>();
services.AddSingleton<ISecurityTaskRepository, SecurityTaskRepository>();

if (selfHosted)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public DatabaseContext(DbContextOptions<DatabaseContext> options)
public DbSet<NotificationStatus> NotificationStatuses { get; set; }
public DbSet<ClientOrganizationMigrationRecord> ClientOrganizationMigrationRecords { get; set; }
public DbSet<PasswordHealthReportApplication> PasswordHealthReportApplications { get; set; }
public DbSet<SecurityTask> SecurityTasks { get; set; }

protected override void OnModelCreating(ModelBuilder builder)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
๏ปฟusing Bit.Infrastructure.EntityFramework.Vault.Models;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace Bit.Infrastructure.EntityFramework.Vault.Configurations;

public class SecurityTaskEntityTypeConfiguration : IEntityTypeConfiguration<SecurityTask>
{
public void Configure(EntityTypeBuilder<SecurityTask> builder)
{
builder
.Property(s => s.Id)
.ValueGeneratedNever();

builder
.HasKey(s => s.Id)
.IsClustered();

builder
.HasIndex(s => s.OrganizationId)
.IsClustered(false);

builder
.HasIndex(s => s.CipherId)
.IsClustered(false);

builder
.ToTable(nameof(SecurityTask));
}
}
18 changes: 18 additions & 0 deletions src/Infrastructure.EntityFramework/Vault/Models/SecurityTask.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
๏ปฟusing AutoMapper;
using Bit.Infrastructure.EntityFramework.AdminConsole.Models;

namespace Bit.Infrastructure.EntityFramework.Vault.Models;

public class SecurityTask : Core.Vault.Entities.SecurityTask
{
public virtual Organization Organization { get; set; }
public virtual Cipher Cipher { get; set; }

Check warning on line 9 in src/Infrastructure.EntityFramework/Vault/Models/SecurityTask.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.EntityFramework/Vault/Models/SecurityTask.cs#L8-L9

Added lines #L8 - L9 were not covered by tests
}

public class SecurityTaskMapperProfile : Profile
{
public SecurityTaskMapperProfile()
{
CreateMap<Core.Vault.Entities.SecurityTask, SecurityTask>().ReverseMap();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
๏ปฟusing AutoMapper;
using Bit.Core.Vault.Repositories;
using Bit.Infrastructure.EntityFramework.Repositories;
using Bit.Infrastructure.EntityFramework.Vault.Models;
using Microsoft.Extensions.DependencyInjection;

namespace Bit.Infrastructure.EntityFramework.Vault.Repositories;

public class SecurityTaskRepository : Repository<Core.Vault.Entities.SecurityTask, SecurityTask, Guid>, ISecurityTaskRepository
{
public SecurityTaskRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper)
: base(serviceScopeFactory, mapper, (context) => context.SecurityTasks)
{ }

Check warning on line 13 in src/Infrastructure.EntityFramework/Vault/Repositories/SecurityTaskRepository.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.EntityFramework/Vault/Repositories/SecurityTaskRepository.cs#L12-L13

Added lines #L12 - L13 were not covered by tests
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
CREATE PROCEDURE [dbo].[SecurityTask_Create]
@Id UNIQUEIDENTIFIER OUTPUT,
@OrganizationId UNIQUEIDENTIFIER,
@CipherId UNIQUEIDENTIFIER,
@Type TINYINT,
@Status TINYINT,
@CreationDate DATETIME2(7),
@RevisionDate DATETIME2(7)
AS
BEGIN
SET NOCOUNT ON

INSERT INTO [dbo].[SecurityTask] (
[Id],
[OrganizationId],
[CipherId],
[Type],
[Status],
[CreationDate],
[RevisionDate]
)
VALUES (
@Id,
@OrganizationId,
@CipherId,
@Type,
@Status,
@CreationDate,
@RevisionDate
)
END
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
CREATE PROCEDURE [dbo].[SecurityTask_DeleteById]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โ“ Is this even doable? I'd think we don't support it. I get that orgs or ciphers would cascade a delete, but we wouldn't directly delete a task.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great point, I had added the delete sproc to complete the base Repository implementation. But, you're right that we don't currently plan to support deleting a specific task at this time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โ“ how do we plan on maintaining the SecurityTask table?

@Id UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON

DELETE FROM
[dbo].[SecurityTask]
WHERE
[Id] = @Id
END
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
CREATE PROCEDURE [dbo].[SecurityTask_ReadById]
@Id UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON

SELECT *
FROM [dbo].[SecurityTaskView]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

โ›๏ธ I think there's some formatting weirdness from our norm here, as well as some lower files. I have noticed VS Code at least isn't doing what I'd expect. Running prettier on all changed files may get it.

WHERE [Id] = @Id
END
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
CREATE PROCEDURE [dbo].[SecurityTask_Update]
@Id UNIQUEIDENTIFIER,
@OrganizationId UNIQUEIDENTIFIER,
@CipherId UNIQUEIDENTIFIER,
@Type TINYINT,
@Status TINYINT,
@CreationDate DATETIME2(7),
@RevisionDate DATETIME2(7)
AS
BEGIN
SET NOCOUNT ON

UPDATE [dbo].[SecurityTask]
SET [OrganizationId] = @OrganizationId,
[CipherId] = @CipherId,
[Type] = @Type,
[Status] = @Status,
[CreationDate] = @CreationDate,
[RevisionDate] = @RevisionDate
WHERE [Id] = @Id
END
21 changes: 21 additions & 0 deletions src/Sql/Vault/dbo/Tables/SecurityTask.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
CREATE TABLE [dbo].[SecurityTask]
(
[Id] UNIQUEIDENTIFIER NOT NULL,
[OrganizationId] UNIQUEIDENTIFIER NOT NULL,
[CipherId] UNIQUEIDENTIFIER NULL,
[Type] TINYINT NOT NULL,
[Status] TINYINT NOT NULL,
[CreationDate] DATETIME2 (7) NOT NULL,
[RevisionDate] DATETIME2 (7) NOT NULL,
CONSTRAINT [PK_SecurityTask] PRIMARY KEY CLUSTERED ([Id] ASC),
CONSTRAINT [FK_SecurityTask_Organization] FOREIGN KEY ([OrganizationId]) REFERENCES [dbo].[Organization] ([Id]) ON DELETE CASCADE,
CONSTRAINT [FK_SecurityTask_Cipher] FOREIGN KEY ([CipherId]) REFERENCES [dbo].[Cipher] ([Id]) ON DELETE CASCADE,
);

GO
CREATE NONCLUSTERED INDEX [IX_SecurityTask_CipherId]
ON [dbo].[SecurityTask]([CipherId] ASC) WHERE CipherId IS NOT NULL;

GO
CREATE NONCLUSTERED INDEX [IX_SecurityTask_OrganizationId]
ON [dbo].[SecurityTask]([OrganizationId] ASC) WHERE OrganizationId IS NOT NULL;
withinfocus marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions src/Sql/Vault/dbo/Views/SecurityTaskView.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
CREATE VIEW [dbo].[SecurityTaskView]
AS
SELECT
*
FROM
[dbo].[SecurityTask]
Loading
Loading