From 98c12d3f417a8c623fcc92dea075fbd9e9df3a45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Wed, 22 Nov 2023 15:44:25 -0500 Subject: [PATCH] Tools - Make Entities and Repositories nullable (#3313) * support nullability in tools' entities and repositories * enables C# nullability checks in these files * includes documentation for affected files * refine documentation per code review * improve comments on SendFileData structure * fix ReferenceEvent.MaxAccessCount documentation * add value notation to SendFileData.FileName --- src/Core/Tools/Entities/IReferenceable.cs | 25 ++- src/Core/Tools/Entities/Send.cs | 104 +++++++++- .../Tools/Models/Business/ReferenceEvent.cs | 178 +++++++++++++++++- src/Core/Tools/Models/Data/SendData.cs | 26 ++- src/Core/Tools/Models/Data/SendFileData.cs | 54 +++++- src/Core/Tools/Models/Data/SendTextData.cs | 32 +++- .../Tools/Repositories/ISendRepository.cs | 28 ++- .../Tools/Repositories/SendRepository.cs | 7 +- .../Tools/Repositories/SendRepository.cs | 22 ++- 9 files changed, 447 insertions(+), 29 deletions(-) diff --git a/src/Core/Tools/Entities/IReferenceable.cs b/src/Core/Tools/Entities/IReferenceable.cs index 39d473c65..4b38ec6cc 100644 --- a/src/Core/Tools/Entities/IReferenceable.cs +++ b/src/Core/Tools/Entities/IReferenceable.cs @@ -1,8 +1,29 @@ -namespace Bit.Core.Tools.Entities; +#nullable enable +using Bit.Core.Tools.Models.Business; +namespace Bit.Core.Tools.Entities; + +/// +/// An entity that can be referenced by a . +/// public interface IReferenceable { + /// + /// Identifies the entity that generated the event. + /// Guid Id { get; set; } - string ReferenceData { get; set; } + + /// + /// Contextual information included in the event. + /// + /// + /// Do not store secrets in this field. + /// + string? ReferenceData { get; set; } + + /// + /// Returns when the entity is a user. + /// Otherwise returns . + /// bool IsUser(); } diff --git a/src/Core/Tools/Entities/Send.cs b/src/Core/Tools/Entities/Send.cs index ae30201e8..9f09ae6bd 100644 --- a/src/Core/Tools/Entities/Send.cs +++ b/src/Core/Tools/Entities/Send.cs @@ -1,29 +1,125 @@ -using System.ComponentModel.DataAnnotations; +#nullable enable + +using System.ComponentModel.DataAnnotations; using Bit.Core.Entities; using Bit.Core.Tools.Enums; using Bit.Core.Utilities; namespace Bit.Core.Tools.Entities; +/// +/// An end-to-end encrypted secret accessible to arbitrary +/// entities through a fixed URI. +/// public class Send : ITableObject { + /// + /// Uniquely identifies this send. + /// public Guid Id { get; set; } + + /// + /// Identifies the user that created this send. + /// public Guid? UserId { get; set; } + + /// + /// Identifies the organization that created this send. + /// + /// + /// Not presently in-use by client applications. + /// public Guid? OrganizationId { get; set; } + + /// + /// Describes the data being sent. This field determines how + /// the field is interpreted. + /// public SendType Type { get; set; } - public string Data { get; set; } - public string Key { get; set; } + + /// + /// Stores data containing or pointing to the transmitted secret. JSON. + /// + /// + /// Must be nullable due to several database column configuration. + /// The application and all other databases assume this is not nullable. + /// Tech debt ticket: PM-4128 + /// + public string? Data { get; set; } + + /// + /// Stores the data's encryption key. Encrypted. + /// + /// + /// Must be nullable due to MySql database column configuration. + /// The application and all other databases assume this is not nullable. + /// Tech debt ticket: PM-4128 + /// + public string? Key { get; set; } + + /// + /// Password provided by the user. Protected with pbkdf2. + /// [MaxLength(300)] - public string Password { get; set; } + public string? Password { get; set; } + + /// + /// The send becomes unavailable to API callers when + /// >= . + /// public int? MaxAccessCount { get; set; } + + /// + /// Number of times the content was accessed. + /// + /// + /// This value is owned by the server. Clients cannot alter it. + /// public int AccessCount { get; set; } + + /// + /// The date this send was created. + /// public DateTime CreationDate { get; internal set; } = DateTime.UtcNow; + + /// + /// The date this send was last modified. + /// public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow; + + /// + /// The date this send becomes unavailable to API callers. + /// public DateTime? ExpirationDate { get; set; } + + /// + /// The date this send will be unconditionally deleted. + /// + /// + /// This is set by server-side when the user doesn't specify a deletion date. + /// public DateTime DeletionDate { get; set; } + + /// + /// When this is true the send is not available to API callers, + /// unless they're the creator. + /// public bool Disabled { get; set; } + + /// + /// Whether the creator's email address should be shown to the recipient. + /// + /// + /// indicates the email may be shown. + /// indicates the email should be hidden. + /// indicates the client doesn't set the field and + /// the email should be hidden. + /// public bool? HideEmail { get; set; } + /// + /// Generates the send's + /// public void SetNewId() { Id = CoreHelpers.GenerateComb(); diff --git a/src/Core/Tools/Models/Business/ReferenceEvent.cs b/src/Core/Tools/Models/Business/ReferenceEvent.cs index 393708668..ac21c92e4 100644 --- a/src/Core/Tools/Models/Business/ReferenceEvent.cs +++ b/src/Core/Tools/Models/Business/ReferenceEvent.cs @@ -1,4 +1,6 @@ -using System.Text.Json.Serialization; +#nullable enable + +using System.Text.Json.Serialization; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Tools.Entities; @@ -6,10 +8,23 @@ using Bit.Core.Tools.Enums; namespace Bit.Core.Tools.Models.Business; +/// +/// Product support monitoring. +/// +/// +/// Do not store secrets in this type. +/// public class ReferenceEvent { + /// + /// Instantiates a . + /// public ReferenceEvent() { } + /// + /// Monitored event type. + /// Entity that created the event. + /// The conditions in which the event occurred. public ReferenceEvent(ReferenceEventType type, IReferenceable source, ICurrentContext currentContext) { Type = type; @@ -26,48 +41,197 @@ public class ReferenceEvent } } + /// + /// Monitored event type. + /// [JsonConverter(typeof(JsonStringEnumConverter))] public ReferenceEventType Type { get; set; } + /// + /// The kind of entity that created the event. + /// [JsonConverter(typeof(JsonStringEnumConverter))] public ReferenceEventSource Source { get; set; } + /// public Guid Id { get; set; } - public string ReferenceData { get; set; } + /// + public string? ReferenceData { get; set; } + /// + /// Moment the event occurred. + /// public DateTime EventDate { get; set; } = DateTime.UtcNow; + /// + /// Number of users sent invitations by an organization. + /// + /// + /// Should contain a value only on events. + /// Otherwise the value should be . + /// public int? Users { get; set; } + /// + /// Whether or not a subscription was canceled immediately or at the end of the billing period. + /// + /// + /// when a cancellation occurs immediately. + /// when a cancellation occurs at the end of a customer's billing period. + /// Should contain a value only on events. + /// Otherwise the value should be . + /// public bool? EndOfPeriod { get; set; } - public string PlanName { get; set; } + /// + /// Branded name of the subscription. + /// + /// + /// Should contain a value only for subscription management events. + /// Otherwise the value should be . + /// + public string? PlanName { get; set; } + /// + /// Identifies a subscription. + /// + /// + /// Should contain a value only for subscription management events. + /// Otherwise the value should be . + /// public PlanType? PlanType { get; set; } - public string OldPlanName { get; set; } + /// + /// The branded name of the prior plan. + /// + /// + /// Should contain a value only on events + /// initiated by organizations. + /// Otherwise the value should be . + /// + public string? OldPlanName { get; set; } + /// + /// Identifies the prior plan + /// + /// + /// Should contain a value only on events + /// initiated by organizations. + /// Otherwise the value should be . + /// public PlanType? OldPlanType { get; set; } + /// + /// Seat count when a billable action occurs. When adjusting seats, contains + /// the new seat count. + /// + /// + /// Should contain a value only on , + /// , , + /// and events initiated by organizations. + /// Otherwise the value should be . + /// public int? Seats { get; set; } + + /// + /// Seat count when a seat adjustment occurs. + /// + /// + /// Should contain a value only on + /// events initiated by organizations. + /// Otherwise the value should be . + /// public int? PreviousSeats { get; set; } + /// + /// Qty in GB of storage. When adjusting storage, contains the adjusted + /// storage qty. Otherwise contains the total storage quantity. + /// + /// + /// Should contain a value only on , + /// , , + /// and events. + /// Otherwise the value should be . + /// public short? Storage { get; set; } + /// + /// The type of send created or accessed. + /// + /// + /// Should contain a value only on + /// and events. + /// Otherwise the value should be . + /// [JsonConverter(typeof(JsonStringEnumConverter))] public SendType? SendType { get; set; } + /// + /// Whether the send has private notes. + /// + /// + /// when the send has private notes, otherwise . + /// Should contain a value only on + /// and events. + /// Otherwise the value should be . + /// public bool? SendHasNotes { get; set; } + /// + /// The send expires after its access count exceeds this value. + /// + /// + /// This field only contains a value when the send has a max access count + /// and is + /// or events. + /// Otherwise, the value should be . + /// public int? MaxAccessCount { get; set; } + /// + /// Whether the created send has a password. + /// + /// + /// Should contain a value only on + /// and events. + /// Otherwise the value should be . + /// public bool? HasPassword { get; set; } - public string EventRaisedByUser { get; set; } + /// + /// The administrator that performed the action. + /// + /// + /// Should contain a value only on + /// and events. + /// Otherwise the value should be . + /// + public string? EventRaisedByUser { get; set; } + /// + /// Whether or not an organization's trial period was started by a sales person. + /// + /// + /// Should contain a value only on + /// and events. + /// Otherwise the value should be . + /// public bool? SalesAssistedTrialStarted { get; set; } - public string ClientId { get; set; } - public Version ClientVersion { get; set; } + /// + /// The installation id of the application that originated the event. + /// + /// + /// when the event was not originated by an application. + /// + public string? ClientId { get; set; } + + /// + /// The version of the client application that originated the event. + /// + /// + /// when the event was not originated by an application. + /// + public Version? ClientVersion { get; set; } } diff --git a/src/Core/Tools/Models/Data/SendData.cs b/src/Core/Tools/Models/Data/SendData.cs index bdddb8482..c780cccb9 100644 --- a/src/Core/Tools/Models/Data/SendData.cs +++ b/src/Core/Tools/Models/Data/SendData.cs @@ -1,15 +1,33 @@ -namespace Bit.Core.Tools.Models.Data; +#nullable enable +namespace Bit.Core.Tools.Models.Data; + +/// +/// Shared data for a send +/// public abstract class SendData { + /// + /// Instantiates a . + /// public SendData() { } - public SendData(string name, string notes) + /// + /// User-provided name of the send. + /// User-provided private notes of the send. + public SendData(string name, string? notes) { Name = name; Notes = notes; } - public string Name { get; set; } - public string Notes { get; set; } + /// + /// User-provided name of the send. + /// + public string Name { get; set; } = string.Empty; + + /// + /// User-provided private notes of the send. + /// + public string? Notes { get; set; } = null; } diff --git a/src/Core/Tools/Models/Data/SendFileData.cs b/src/Core/Tools/Models/Data/SendFileData.cs index 6cf1fa3da..11379bef5 100644 --- a/src/Core/Tools/Models/Data/SendFileData.cs +++ b/src/Core/Tools/Models/Data/SendFileData.cs @@ -1,22 +1,64 @@ -using System.Text.Json.Serialization; +#nullable enable + +using System.Diagnostics.CodeAnalysis; +using System.Text.Json.Serialization; +using static System.Text.Json.Serialization.JsonNumberHandling; namespace Bit.Core.Tools.Models.Data; +/// +/// A file secret being sent. +/// public class SendFileData : SendData { + /// + /// Instantiates a . + /// public SendFileData() { } - public SendFileData(string name, string notes, string fileName) + /// + /// Attached file name. + /// User-provided private notes of the send. + /// Attached file name. + public SendFileData(string name, string? notes, string fileName) : base(name, notes) { FileName = fileName; } - // We serialize Size as a string since JSON (or Javascript) doesn't support full precision for long numbers - [JsonNumberHandling(JsonNumberHandling.WriteAsString | JsonNumberHandling.AllowReadingFromString)] + /// + /// Size of the attached file in bytes. + /// + /// + /// Serialized as a string since JSON (or Javascript) doesn't support + /// full precision for long numbers + /// + [JsonNumberHandling(WriteAsString | AllowReadingFromString)] public long Size { get; set; } - public string Id { get; set; } - public string FileName { get; set; } + /// + /// Uniquely identifies an uploaded file. + /// + /// + /// Should contain only when a file + /// upload is pending. Should never contain null once the + /// file upload completes. + /// + [DisallowNull] + public string? Id { get; set; } + + /// + /// Attached file name. + /// + /// + /// Should contain a non-empty string once the file upload completes. + /// + public string FileName { get; set; } = string.Empty; + + /// + /// When true the uploaded file's length was confirmed within + /// the expected tolerance and below the maximum supported + /// file size. + /// public bool Validated { get; set; } = true; } diff --git a/src/Core/Tools/Models/Data/SendTextData.cs b/src/Core/Tools/Models/Data/SendTextData.cs index 41c6b042a..5a1b8e9f3 100644 --- a/src/Core/Tools/Models/Data/SendTextData.cs +++ b/src/Core/Tools/Models/Data/SendTextData.cs @@ -1,16 +1,42 @@ -namespace Bit.Core.Tools.Models.Data; +#nullable enable +namespace Bit.Core.Tools.Models.Data; + +/// +/// A text secret being sent. +/// public class SendTextData : SendData { + /// + /// Instantiates a . + /// public SendTextData() { } - public SendTextData(string name, string notes, string text, bool hidden) + /// + /// Attached file name. + /// User-provided private notes of the send. + /// The secret being sent. + /// + /// Indicates whether the secret should be concealed when opening the send. + /// + public SendTextData(string name, string? notes, string? text, bool hidden) : base(name, notes) { Text = text; Hidden = hidden; } - public string Text { get; set; } + /// + /// The secret being sent. + /// + public string? Text { get; set; } + + /// + /// Indicates whether the secret should be concealed when opening the send. + /// + /// + /// when the secret should be concealed. + /// Otherwise . + /// public bool Hidden { get; set; } } diff --git a/src/Core/Tools/Repositories/ISendRepository.cs b/src/Core/Tools/Repositories/ISendRepository.cs index 3d67c990f..421a5f4aa 100644 --- a/src/Core/Tools/Repositories/ISendRepository.cs +++ b/src/Core/Tools/Repositories/ISendRepository.cs @@ -1,10 +1,36 @@ -using Bit.Core.Repositories; +#nullable enable + +using Bit.Core.Repositories; using Bit.Core.Tools.Entities; namespace Bit.Core.Tools.Repositories; +/// +/// Service for saving and loading s in persistent storage. +/// public interface ISendRepository : IRepository { + /// + /// Loads all s created by a user. + /// + /// + /// Identifies the user. + /// + /// + /// A task that completes once the s have been loaded. + /// The task's result contains the loaded s. + /// Task> GetManyByUserIdAsync(Guid userId); + + /// + /// Loads s scheduled for deletion. + /// + /// + /// Load sends whose is < this date. + /// + /// + /// A task that completes once the s have been loaded. + /// The task's result contains the loaded s. + /// Task> GetManyByDeletionDateAsync(DateTime deletionDateBefore); } diff --git a/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs b/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs index 24cdcbf77..0522cee67 100644 --- a/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs +++ b/src/Infrastructure.Dapper/Tools/Repositories/SendRepository.cs @@ -1,4 +1,6 @@ -using System.Data; +#nullable enable + +using System.Data; using Bit.Core.Settings; using Bit.Core.Tools.Entities; using Bit.Core.Tools.Repositories; @@ -8,6 +10,7 @@ using Microsoft.Data.SqlClient; namespace Bit.Infrastructure.Dapper.Tools.Repositories; +/// public class SendRepository : Repository, ISendRepository { public SendRepository(GlobalSettings globalSettings) @@ -18,6 +21,7 @@ public class SendRepository : Repository, ISendRepository : base(connectionString, readOnlyConnectionString) { } + /// public async Task> GetManyByUserIdAsync(Guid userId) { using (var connection = new SqlConnection(ConnectionString)) @@ -31,6 +35,7 @@ public class SendRepository : Repository, ISendRepository } } + /// public async Task> GetManyByDeletionDateAsync(DateTime deletionDateBefore) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs b/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs index 3207ec9e9..4c0c86660 100644 --- a/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs +++ b/src/Infrastructure.EntityFramework/Tools/Repositories/SendRepository.cs @@ -1,4 +1,6 @@ -using AutoMapper; +#nullable enable + +using AutoMapper; using Bit.Core.Tools.Repositories; using Bit.Infrastructure.EntityFramework.Models; using Bit.Infrastructure.EntityFramework.Repositories; @@ -7,12 +9,28 @@ using Microsoft.Extensions.DependencyInjection; namespace Bit.Infrastructure.EntityFramework.Tools.Repositories; +/// public class SendRepository : Repository, ISendRepository { + /// + /// Initializes the + /// + /// An IoC service locator. + /// An automapper service. public SendRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) : base(serviceScopeFactory, mapper, (DatabaseContext context) => context.Sends) { } + /// + /// Saves a in the database. + /// + /// + /// The send being saved. + /// + /// + /// A task that completes once the save is complete. + /// The task result contains the saved . + /// public override async Task CreateAsync(Core.Tools.Entities.Send send) { send = await base.CreateAsync(send); @@ -30,6 +48,7 @@ public class SendRepository : Repository, return send; } + /// public async Task> GetManyByDeletionDateAsync(DateTime deletionDateBefore) { using (var scope = ServiceScopeFactory.CreateScope()) @@ -40,6 +59,7 @@ public class SendRepository : Repository, } } + /// public async Task> GetManyByUserIdAsync(Guid userId) { using (var scope = ServiceScopeFactory.CreateScope())