diff --git a/src/Api/Tools/Controllers/ImportCiphersController.cs b/src/Api/Tools/Controllers/ImportCiphersController.cs index bebf7cbf296e..dd482a2bb8f0 100644 --- a/src/Api/Tools/Controllers/ImportCiphersController.cs +++ b/src/Api/Tools/Controllers/ImportCiphersController.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Api.Tools.Models.Request.Accounts; +using Bit.Api.Tools.Models.Request.Accounts; using Bit.Api.Tools.Models.Request.Organizations; using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Core.Context; @@ -56,7 +53,7 @@ public async Task PostImport([FromBody] ImportCiphersRequestModel model) throw new BadRequestException("You cannot import this much data at once."); } - var userId = _userService.GetProperUserId(User).Value; + var userId = _userService.GetProperUserId(User) ?? throw new InvalidOperationException("User ID not found"); var folders = model.Folders.Select(f => f.ToFolder(userId)).ToList(); var ciphers = model.Ciphers.Select(c => c.ToCipherDetails(userId, false)).ToList(); await _importCiphersCommand.ImportIntoIndividualVaultAsync(folders, ciphers, model.FolderRelationships, userId); @@ -84,7 +81,7 @@ public async Task PostImportOrganization([FromQuery] string organizationId, throw new BadRequestException("Not enough privileges to import into this organization."); } - var userId = _userService.GetProperUserId(User).Value; + var userId = _userService.GetProperUserId(User) ?? throw new InvalidOperationException("User ID not found"); var ciphers = model.Ciphers.Select(l => l.ToOrganizationCipherDetails(orgId)).ToList(); await _importCiphersCommand.ImportIntoOrganizationalVaultAsync(collections, ciphers, model.CollectionRelationships, userId); } diff --git a/src/Api/Tools/Controllers/SendsController.cs b/src/Api/Tools/Controllers/SendsController.cs index 37df338ccaff..9521233e4035 100644 --- a/src/Api/Tools/Controllers/SendsController.cs +++ b/src/Api/Tools/Controllers/SendsController.cs @@ -342,6 +342,8 @@ public async Task PostFile([FromBody] SendReque throw new BadRequestException($"Max file size is {SendFileSettingHelper.MAX_FILE_SIZE_READABLE}."); } + var file = model.File ?? throw new BadRequestException("File metadata is required for file sends."); + model.ValidateCreation(); var userId = _userService.GetProperUserId(User) ?? throw new InvalidOperationException("User ID not found"); var hasPremium = await _hasPremiumAccessQuery.HasPremiumAccessAsync(userId); @@ -351,7 +353,7 @@ public async Task PostFile([FromBody] SendReque throw new BadRequestException("Email verified Sends require a premium membership"); } - var (send, data) = model.ToSend(userId, model.File.FileName, _sendAuthorizationService); + var (send, data) = model.ToSend(userId, file.FileName!, _sendAuthorizationService); var uploadUrl = await _nonAnonymousSendCommand.SaveFileSendAsync(send, data, model.FileLength.Value); return new SendFileUploadDataResponseModel { diff --git a/src/Api/Tools/Models/Request/Accounts/ImportCiphersRequestModel.cs b/src/Api/Tools/Models/Request/Accounts/ImportCiphersRequestModel.cs index 8330e4fc5447..56c26728fed6 100644 --- a/src/Api/Tools/Models/Request/Accounts/ImportCiphersRequestModel.cs +++ b/src/Api/Tools/Models/Request/Accounts/ImportCiphersRequestModel.cs @@ -1,13 +1,10 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Api.Vault.Models.Request; +using Bit.Api.Vault.Models.Request; namespace Bit.Api.Tools.Models.Request.Accounts; public class ImportCiphersRequestModel { - public FolderWithIdRequestModel[] Folders { get; set; } - public CipherRequestModel[] Ciphers { get; set; } - public KeyValuePair[] FolderRelationships { get; set; } + public FolderWithIdRequestModel[] Folders { get; set; } = []; + public CipherRequestModel[] Ciphers { get; set; } = []; + public KeyValuePair[] FolderRelationships { get; set; } = []; } diff --git a/src/Api/Tools/Models/Request/Organizations/ImportOrganizationCiphersRequestModel.cs b/src/Api/Tools/Models/Request/Organizations/ImportOrganizationCiphersRequestModel.cs index 45f8dfdffdd4..f8e52c8e9a04 100644 --- a/src/Api/Tools/Models/Request/Organizations/ImportOrganizationCiphersRequestModel.cs +++ b/src/Api/Tools/Models/Request/Organizations/ImportOrganizationCiphersRequestModel.cs @@ -1,14 +1,11 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Api.Models.Request; +using Bit.Api.Models.Request; using Bit.Api.Vault.Models.Request; namespace Bit.Api.Tools.Models.Request.Organizations; public class ImportOrganizationCiphersRequestModel { - public CollectionWithIdRequestModel[] Collections { get; set; } - public CipherRequestModel[] Ciphers { get; set; } - public KeyValuePair[] CollectionRelationships { get; set; } + public CollectionWithIdRequestModel[] Collections { get; set; } = []; + public CipherRequestModel[] Ciphers { get; set; } = []; + public KeyValuePair[] CollectionRelationships { get; set; } = []; } diff --git a/src/Api/Tools/Models/Request/SendAccessRequestModel.cs b/src/Api/Tools/Models/Request/SendAccessRequestModel.cs index 15745ac85528..5b36a4355e86 100644 --- a/src/Api/Tools/Models/Request/SendAccessRequestModel.cs +++ b/src/Api/Tools/Models/Request/SendAccessRequestModel.cs @@ -1,12 +1,9 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; namespace Bit.Api.Tools.Models.Request; public class SendAccessRequestModel { [StringLength(300)] - public string Password { get; set; } + public string? Password { get; set; } } diff --git a/src/Api/Tools/Models/Request/SendRequestModel.cs b/src/Api/Tools/Models/Request/SendRequestModel.cs index 152d16e63548..2063c4d4ec87 100644 --- a/src/Api/Tools/Models/Request/SendRequestModel.cs +++ b/src/Api/Tools/Models/Request/SendRequestModel.cs @@ -1,7 +1,4 @@ -// 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; using Bit.Api.Tools.Utilities; using Bit.Core.Exceptions; @@ -10,7 +7,6 @@ using Bit.Core.Tools.Models.Data; using Bit.Core.Tools.Services; using Bit.Core.Utilities; - using static System.StringSplitOptions; namespace Bit.Api.Tools.Models.Request; @@ -41,14 +37,14 @@ public class SendRequestModel /// [EncryptedString] [EncryptedStringLength(1000)] - public string Name { get; set; } + public string? Name { get; set; } /// /// Notes for the send. This is only visible to the owner of the send. /// [EncryptedString] [EncryptedStringLength(1000)] - public string Notes { get; set; } + public string? Notes { get; set; } /// /// A base64-encoded byte array containing the Send's encryption key. This key is @@ -57,7 +53,7 @@ public class SendRequestModel [Required] [EncryptedString] [EncryptedStringLength(1000)] - public string Key { get; set; } + public required string Key { get; set; } /// /// The maximum number of times a send can be accessed before it expires. @@ -74,8 +70,8 @@ public class SendRequestModel /// /// The date after which a send may be automatically deleted from the server. - /// When this is , the send may be deleted after it has - /// exceeded the global send timeout limit. + /// The server enforces a maximum of 31 days from creation. A background job + /// deletes sends once this date has passed. /// [Required] public DateTime? DeletionDate { get; set; } @@ -84,26 +80,26 @@ public class SendRequestModel /// Contains file metadata uploaded with the send. /// The file content is uploaded separately. /// - public SendFileModel File { get; set; } + public SendFileModel? File { get; set; } /// /// Contains text data uploaded with the send. /// - public SendTextModel Text { get; set; } + public SendTextModel? Text { get; set; } /// /// Base64-encoded byte array of a password hash that grants access to the send. /// Mutually exclusive with . /// [StringLength(1000)] - public string Password { get; set; } + public string? Password { get; set; } /// /// Comma-separated list of emails that may access the send using OTP /// authentication. Mutually exclusive with . /// [StringLength(4000)] - public string Emails { get; set; } + public string? Emails { get; set; } /// /// When , send access is disabled. @@ -126,11 +122,7 @@ public class SendRequestModel /// The send object public Send ToSend(Guid userId, ISendAuthorizationService sendAuthorizationService) { - var send = new Send - { - Type = Type, - UserId = (Guid?)userId - }; + var send = new Send { Type = Type, UserId = (Guid?)userId }; send = UpdateSend(send, sendAuthorizationService); return send; } @@ -146,12 +138,8 @@ public Send ToSend(Guid userId, ISendAuthorizationService sendAuthorizationServi { // FIXME: This method does two things: creates a send and a send file data. // It should only do one thing. - var send = ToSendBase(new Send - { - Type = Type, - UserId = (Guid?)userId - }, sendAuthorizationService); - var data = new SendFileData(Name, Notes, fileName); + var send = ToSendBase(new Send { Type = Type, UserId = (Guid?)userId }, sendAuthorizationService); + var data = new SendFileData(Name ?? string.Empty, Notes, fileName); return (send, data); } @@ -167,8 +155,10 @@ public Send UpdateSend(Send existingSend, ISendAuthorizationService sendAuthoriz switch (existingSend.Type) { case SendType.File: - var fileData = JsonSerializer.Deserialize(existingSend.Data); - fileData.Name = Name; + // See Send.cs property definition for Data, it can safely be assumed non-null + var fileData = JsonSerializer.Deserialize(existingSend.Data!) ?? + throw new JsonException("Failed to deserialize send file data."); + fileData.Name = Name ?? string.Empty; fileData.Notes = Notes; existingSend.Data = JsonSerializer.Serialize(fileData, JsonHelpers.IgnoreWritingNull); break; @@ -178,6 +168,7 @@ public Send UpdateSend(Send existingSend, ISendAuthorizationService sendAuthoriz default: throw new ArgumentException("Unsupported type: " + nameof(Type) + "."); } + return existingSend; } @@ -195,8 +186,9 @@ public void ValidateCreation() if (ExpirationDate.HasValue && ExpirationDate.Value <= nowPlus1Minute) { throw new BadRequestException("You cannot create a Send that is already expired. " + - "Adjust the expiration date and try again."); + "Adjust the expiration date and try again."); } + ValidateEdit(); } @@ -217,25 +209,29 @@ public void ValidateEdit() if (DeletionDate.Value <= nowPlus1Minute) { throw new BadRequestException("You cannot have a Send with a deletion date in the past. " + - "Adjust the deletion date and try again."); + "Adjust the deletion date and try again."); } + if (DeletionDate.Value > now.AddDays(31)) { throw new BadRequestException("You cannot have a Send with a deletion date that far " + - "into the future. Adjust the Deletion Date to a value less than 31 days from now " + - "and try again."); + "into the future. Adjust the Deletion Date to a value less than 31 days from now " + + "and try again."); } } + if (ExpirationDate.HasValue) { if (ExpirationDate.Value <= nowPlus1Minute) { throw new BadRequestException("You cannot have a Send with an expiration date in the past. " + - "Adjust the expiration date and try again."); + "Adjust the expiration date and try again."); } - if (ExpirationDate.Value > DeletionDate.Value) + + if (DeletionDate.HasValue && ExpirationDate.Value > DeletionDate.Value) { - throw new BadRequestException("You cannot have a Send with an expiration date greater than the deletion date. " + + throw new BadRequestException( + "You cannot have a Send with an expiration date greater than the deletion date. " + "Adjust the expiration date and try again."); } } @@ -245,7 +241,7 @@ private Send ToSendBase(Send existingSend, ISendAuthorizationService authorizati { existingSend.Key = Key; existingSend.ExpirationDate = ExpirationDate; - existingSend.DeletionDate = DeletionDate.Value; + existingSend.DeletionDate = DeletionDate!.Value; existingSend.MaxAccessCount = MaxAccessCount; existingSend.Disabled = Disabled.GetValueOrDefault(); existingSend.HideEmail = HideEmail.GetValueOrDefault(); @@ -268,7 +264,7 @@ private Send ToSendBase(Send existingSend, ISendAuthorizationService authorizati existingSend.Password = null; break; case Core.Tools.Enums.AuthType.Password: - existingSend.Password = authorizationService.HashPassword(Password); + existingSend.Password = authorizationService.HashPassword(Password!); existingSend.Emails = null; break; case Core.Tools.Enums.AuthType.None: @@ -312,9 +308,11 @@ private Send ToSendBase(Send existingSend, ISendAuthorizationService authorizati return existingSend; } + // Only called from the SendType.Text branch of UpdateSend, Text is required by client and not null private SendTextData ToSendTextData() { - return new SendTextData(Name, Notes, Text.Text, Text.Hidden); + var text = Text ?? throw new ArgumentNullException(nameof(Text), "Text is required for text sends."); + return new SendTextData(Name ?? string.Empty, Notes, text.Text, text.Hidden); } } diff --git a/src/Api/Tools/Models/Response/SendAccessResponseModel.cs b/src/Api/Tools/Models/Response/SendAccessResponseModel.cs index b722dd5fff45..447a4ea51840 100644 --- a/src/Api/Tools/Models/Response/SendAccessResponseModel.cs +++ b/src/Api/Tools/Models/Response/SendAccessResponseModel.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.Text.Json; +using System.Text.Json; using Bit.Core.Models.Api; using Bit.Core.Tools.Entities; using Bit.Core.Tools.Enums; @@ -41,12 +38,20 @@ public SendAccessResponseModel(Send send) switch (send.Type) { case SendType.File: - var fileData = JsonSerializer.Deserialize(send.Data); + var fileData = + JsonSerializer.Deserialize(send.Data ?? + throw new NullReferenceException( + "Send Data is required")) ?? + throw new JsonException("Failed to deserialize send file data."); sendData = fileData; File = new SendFileModel(fileData); break; case SendType.Text: - var textData = JsonSerializer.Deserialize(send.Data); + var textData = + JsonSerializer.Deserialize(send.Data ?? + throw new NullReferenceException( + "Send Data is required")) ?? + throw new JsonException("Failed to deserialize send text data."); sendData = textData; Text = new SendTextModel(textData); break; @@ -89,12 +94,12 @@ public SendAccessResponseModel(Send send) /// File content is downloaded separately using /// /// - public SendFileModel File { get; set; } + public SendFileModel? File { get; set; } /// /// Contains text data uploaded with the send. /// - public SendTextModel Text { get; set; } + public SendTextModel? Text { get; set; } /// /// The date after which a send cannot be accessed. When this value is @@ -105,5 +110,5 @@ public SendAccessResponseModel(Send send) /// /// Indicates the person that created the send to the accessor. /// - public string CreatorIdentifier { get; set; } + public string? CreatorIdentifier { get; set; } } diff --git a/src/Api/Tools/Models/Response/SendFileDownloadDataResponseModel.cs b/src/Api/Tools/Models/Response/SendFileDownloadDataResponseModel.cs index 8e20062301d2..397a35f1a827 100644 --- a/src/Api/Tools/Models/Response/SendFileDownloadDataResponseModel.cs +++ b/src/Api/Tools/Models/Response/SendFileDownloadDataResponseModel.cs @@ -1,14 +1,11 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Models.Api; +using Bit.Core.Models.Api; namespace Bit.Api.Tools.Models.Response; public class SendFileDownloadDataResponseModel : ResponseModel { - public string Id { get; set; } - public string Url { get; set; } + public string? Id { get; set; } + public string? Url { get; set; } public SendFileDownloadDataResponseModel() : base("send-fileDownload") { } } diff --git a/src/Api/Tools/Models/Response/SendFileUploadDataResponseModel.cs b/src/Api/Tools/Models/Response/SendFileUploadDataResponseModel.cs index 4f263b7e9cfa..b87afdc4fca7 100644 --- a/src/Api/Tools/Models/Response/SendFileUploadDataResponseModel.cs +++ b/src/Api/Tools/Models/Response/SendFileUploadDataResponseModel.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Enums; +using Bit.Core.Enums; using Bit.Core.Models.Api; namespace Bit.Api.Tools.Models.Response; @@ -10,8 +7,8 @@ public class SendFileUploadDataResponseModel : ResponseModel { public SendFileUploadDataResponseModel() : base("send-fileUpload") { } - public string Url { get; set; } + public string? Url { get; set; } public FileUploadType FileUploadType { get; set; } - public SendResponseModel SendResponse { get; set; } + public SendResponseModel? SendResponse { get; set; } } diff --git a/src/Api/Tools/Models/Response/SendResponseModel.cs b/src/Api/Tools/Models/Response/SendResponseModel.cs index f7f6b683d6e1..31235e6290ec 100644 --- a/src/Api/Tools/Models/Response/SendResponseModel.cs +++ b/src/Api/Tools/Models/Response/SendResponseModel.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.Text.Json; +using System.Text.Json; using Bit.Api.Tools.Utilities; using Bit.Core.Models.Api; using Bit.Core.Tools.Entities; @@ -39,7 +36,7 @@ public SendResponseModel(Send send) AccessId = CoreHelpers.Base64UrlEncode(send.Id.ToByteArray()); Type = send.Type; AuthType = send.AuthType ?? SendUtilities.InferAuthType(send); - Key = send.Key; + Key = send.Key!; MaxAccessCount = send.MaxAccessCount; AccessCount = send.AccessCount; RevisionDate = send.RevisionDate; @@ -54,12 +51,20 @@ public SendResponseModel(Send send) switch (send.Type) { case SendType.File: - var fileData = JsonSerializer.Deserialize(send.Data); + var fileData = + JsonSerializer.Deserialize(send.Data ?? + throw new NullReferenceException( + "Send Data is required")) ?? + throw new JsonException("Failed to deserialize send file data."); sendData = fileData; File = new SendFileModel(fileData); break; case SendType.Text: - var textData = JsonSerializer.Deserialize(send.Data); + var textData = + JsonSerializer.Deserialize(send.Data ?? + throw new NullReferenceException( + "Send Data is required")) ?? + throw new JsonException("Failed to deserialize send text data."); sendData = textData; Text = new SendTextModel(textData); break; @@ -108,18 +113,18 @@ public SendResponseModel(Send send) /// This field contains a base64-encoded byte array. The array contains /// the E2E-encrypted encrypted content. /// - public string Notes { get; set; } + public string? Notes { get; set; } /// /// Contains file metadata uploaded with the send. /// The file content is uploaded separately. /// - public SendFileModel File { get; set; } + public SendFileModel? File { get; set; } /// /// Contains text data uploaded with the send. /// - public SendTextModel Text { get; set; } + public SendTextModel? Text { get; set; } /// /// A base64-encoded byte array containing the Send's encryption key. @@ -146,13 +151,13 @@ public SendResponseModel(Send send) /// Base64-encoded byte array of a password hash that grants access to the send. /// Mutually exclusive with . /// - public string Password { get; set; } + public string? Password { get; set; } /// /// Comma-separated list of emails that may access the send using OTP /// authentication. Mutually exclusive with . /// - public string Emails { get; set; } + public string? Emails { get; set; } /// /// When , send access is disabled. diff --git a/src/Api/Tools/Models/SendFileModel.cs b/src/Api/Tools/Models/SendFileModel.cs index 88deef4b13b5..704b1bf4d6a4 100644 --- a/src/Api/Tools/Models/SendFileModel.cs +++ b/src/Api/Tools/Models/SendFileModel.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.Text.Json.Serialization; +using System.Text.Json.Serialization; using Bit.Core.Tools.Models.Data; using Bit.Core.Utilities; @@ -19,11 +16,11 @@ public SendFileModel(SendFileData data) SizeName = CoreHelpers.ReadableBytesSize(data.Size); } - public string Id { get; set; } + public string? Id { get; set; } [EncryptedString] [EncryptedStringLength(1000)] - public string FileName { get; set; } + public string? FileName { get; set; } [JsonNumberHandling(JsonNumberHandling.AllowReadingFromString | JsonNumberHandling.WriteAsString)] public long? Size { get; set; } - public string SizeName { get; set; } + public string? SizeName { get; set; } } diff --git a/src/Api/Tools/Models/SendTextModel.cs b/src/Api/Tools/Models/SendTextModel.cs index fdc547c52243..10ece6968756 100644 --- a/src/Api/Tools/Models/SendTextModel.cs +++ b/src/Api/Tools/Models/SendTextModel.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Tools.Models.Data; +using Bit.Core.Tools.Models.Data; using Bit.Core.Utilities; namespace Bit.Api.Tools.Models; @@ -18,6 +15,6 @@ public SendTextModel(SendTextData data) [EncryptedString] [EncryptedStringLength(1000)] - public string Text { get; set; } + public string? Text { get; set; } public bool Hidden { get; set; } } diff --git a/src/Core/Tools/SendFeatures/Commands/AnonymousSendCommand.cs b/src/Core/Tools/SendFeatures/Commands/AnonymousSendCommand.cs index 82232cb757b4..d7c1da2d233e 100644 --- a/src/Core/Tools/SendFeatures/Commands/AnonymousSendCommand.cs +++ b/src/Core/Tools/SendFeatures/Commands/AnonymousSendCommand.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Exceptions; +using Bit.Core.Exceptions; using Bit.Core.Platform.Push; using Bit.Core.Tools.Entities; using Bit.Core.Tools.Enums; @@ -33,7 +30,7 @@ ISendAuthorizationService sendAuthorizationService } // Response: Send, password required, password invalid - public async Task<(string, SendAccessResult)> GetSendFileDownloadUrlAsync(Send send, string fileId, string password) + public async Task<(string?, SendAccessResult)> GetSendFileDownloadUrlAsync(Send send, string fileId, string? password) { if (send.Type != SendType.File) { diff --git a/src/Core/Tools/SendFeatures/Commands/Interfaces/IAnonymousSendCommand.cs b/src/Core/Tools/SendFeatures/Commands/Interfaces/IAnonymousSendCommand.cs index ad23d851705b..f1a6106749e4 100644 --- a/src/Core/Tools/SendFeatures/Commands/Interfaces/IAnonymousSendCommand.cs +++ b/src/Core/Tools/SendFeatures/Commands/Interfaces/IAnonymousSendCommand.cs @@ -17,5 +17,5 @@ public interface IAnonymousSendCommand /// Async Task object with Tuple containing the string of download url and /// to determine if the user can access send. /// - Task<(string, SendAccessResult)> GetSendFileDownloadUrlAsync(Send send, string fileId, string password); + Task<(string?, SendAccessResult)> GetSendFileDownloadUrlAsync(Send send, string fileId, string? password); } diff --git a/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs b/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs index 748a4e1d07ab..e839d57bec20 100644 --- a/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs +++ b/src/Core/Tools/SendFeatures/Services/AzureSendFileStorageService.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Azure.Storage.Blobs; +using Azure.Storage.Blobs; using Azure.Storage.Blobs.Models; using Azure.Storage.Sas; using Bit.Core.Enums; @@ -11,39 +8,40 @@ namespace Bit.Core.Tools.Services; -public class AzureSendFileStorageService : ISendFileStorageService +public class AzureSendFileStorageService( + GlobalSettings globalSettings, + ILogger logger) : ISendFileStorageService { public const string FilesContainerName = "sendfiles"; private static readonly TimeSpan _downloadLinkLiveTime = TimeSpan.FromMinutes(1); - private readonly BlobServiceClient _blobServiceClient; - private readonly ILogger _logger; - private BlobContainerClient _sendFilesContainerClient; + private readonly BlobServiceClient _blobServiceClient = new(globalSettings.Send.ConnectionString); + private readonly ILogger _logger = logger; + /* + * When this file was made nullable, multiple instances of ! were introduced asserting that + * _sendFilesContainerClient abd the blobClient it is used to construct are not null. + * + * See InitAsync() at end of file which is responsible for assigning value asynchronously ensuring + * _sendFilesContainerClient and blobClient are not null. + */ + private BlobContainerClient? _sendFilesContainerClient; public FileUploadType FileUploadType => FileUploadType.Azure; public static string SendIdFromBlobName(string blobName) => blobName.Split('/')[0]; public static string BlobName(Send send, string fileId) => $"{send.Id}/{fileId}"; - public AzureSendFileStorageService( - GlobalSettings globalSettings, - ILogger logger) - { - _blobServiceClient = new BlobServiceClient(globalSettings.Send.ConnectionString); - _logger = logger; - } - public async Task UploadNewFileAsync(Stream stream, Send send, string fileId) { await InitAsync(); - var blobClient = _sendFilesContainerClient.GetBlobClient(BlobName(send, fileId)); + var blobClient = _sendFilesContainerClient!.GetBlobClient(BlobName(send, fileId)); var metadata = new Dictionary(); if (send.UserId.HasValue) { metadata.Add("userId", send.UserId.Value.ToString()); } - else + else if (send.OrganizationId.HasValue) { metadata.Add("organizationId", send.OrganizationId.Value.ToString()); } @@ -61,7 +59,7 @@ public async Task UploadNewFileAsync(Stream stream, Send send, string fileId) public async Task DeleteBlobAsync(string blobName) { await InitAsync(); - var blobClient = _sendFilesContainerClient.GetBlobClient(blobName); + var blobClient = _sendFilesContainerClient!.GetBlobClient(blobName); await blobClient.DeleteIfExistsAsync(); } @@ -78,7 +76,7 @@ public async Task DeleteFilesForUserAsync(Guid userId) public async Task GetSendFileDownloadUrlAsync(Send send, string fileId) { await InitAsync(); - var blobClient = _sendFilesContainerClient.GetBlobClient(BlobName(send, fileId)); + var blobClient = _sendFilesContainerClient!.GetBlobClient(BlobName(send, fileId)); var sasUri = blobClient.GenerateSasUri(BlobSasPermissions.Read, DateTime.UtcNow.Add(_downloadLinkLiveTime)); return sasUri.ToString(); } @@ -86,7 +84,7 @@ public async Task GetSendFileDownloadUrlAsync(Send send, string fileId) public async Task GetSendFileUploadUrlAsync(Send send, string fileId) { await InitAsync(); - var blobClient = _sendFilesContainerClient.GetBlobClient(BlobName(send, fileId)); + var blobClient = _sendFilesContainerClient!.GetBlobClient(BlobName(send, fileId)); var sasUri = blobClient.GenerateSasUri(BlobSasPermissions.Create | BlobSasPermissions.Write, DateTime.UtcNow.Add(_downloadLinkLiveTime)); return sasUri.ToString(); } @@ -95,7 +93,7 @@ public async Task GetSendFileUploadUrlAsync(Send send, string fileId) { await InitAsync(); - var blobClient = _sendFilesContainerClient.GetBlobClient(BlobName(send, fileId)); + var blobClient = _sendFilesContainerClient!.GetBlobClient(BlobName(send, fileId)); try { @@ -106,7 +104,7 @@ public async Task GetSendFileUploadUrlAsync(Send send, string fileId) { metadata["userId"] = send.UserId.Value.ToString(); } - else + else if (send.OrganizationId.HasValue) { metadata["organizationId"] = send.OrganizationId.Value.ToString(); } diff --git a/src/Core/Tools/SendFeatures/Services/Interfaces/ISendAuthorizationService.cs b/src/Core/Tools/SendFeatures/Services/Interfaces/ISendAuthorizationService.cs index 9acf987ac548..477dfa8dce6f 100644 --- a/src/Core/Tools/SendFeatures/Services/Interfaces/ISendAuthorizationService.cs +++ b/src/Core/Tools/SendFeatures/Services/Interfaces/ISendAuthorizationService.cs @@ -15,9 +15,9 @@ public interface ISendAuthorizationService /// A hashed and base64-encoded password. This is compared with the send's password to authorize access. /// will be returned to determine if the user can access send. /// - Task AccessAsync(Send send, string password); + Task AccessAsync(Send send, string? password); SendAccessResult SendCanBeAccessed(Send send, - string password); + string? password); /// /// Hashes the password using the password hasher. diff --git a/src/Core/Tools/SendFeatures/Services/LocalSendStorageService.cs b/src/Core/Tools/SendFeatures/Services/LocalSendStorageService.cs index 6722e4f46b33..c120671f85ae 100644 --- a/src/Core/Tools/SendFeatures/Services/LocalSendStorageService.cs +++ b/src/Core/Tools/SendFeatures/Services/LocalSendStorageService.cs @@ -1,33 +1,25 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Enums; +using Bit.Core.Enums; using Bit.Core.Settings; using Bit.Core.Tools.Entities; namespace Bit.Core.Tools.Services; -public class LocalSendStorageService : ISendFileStorageService +public class LocalSendStorageService( + GlobalSettings globalSettings) : ISendFileStorageService { - private readonly string _baseDirPath; - private readonly string _baseSendUrl; - + private readonly string _baseDirPath = globalSettings.Send.BaseDirectory; + private readonly string _baseSendUrl = globalSettings.Send.BaseUrl; private string RelativeFilePath(Send send, string fileID) => $"{send.Id}/{fileID}"; private string FilePath(Send send, string fileID) => $"{_baseDirPath}/{RelativeFilePath(send, fileID)}"; public FileUploadType FileUploadType => FileUploadType.Direct; - public LocalSendStorageService( - GlobalSettings globalSettings) - { - _baseDirPath = globalSettings.Send.BaseDirectory; - _baseSendUrl = globalSettings.Send.BaseUrl; - } - public async Task UploadNewFileAsync(Stream stream, Send send, string fileId) { await InitAsync(); var path = FilePath(send, fileId); - Directory.CreateDirectory(Path.GetDirectoryName(path)); + // Path.GetDirectoryName will return null for a root path C:\\ or / + // This is not possible based upon path construction using send & fileId and so ! operator can be used + Directory.CreateDirectory(Path.GetDirectoryName(path)!); using (var fs = File.Create(path)) { stream.Seek(0, SeekOrigin.Begin); @@ -40,7 +32,9 @@ public async Task DeleteFileAsync(Send send, string fileId) await InitAsync(); var path = FilePath(send, fileId); DeleteFileIfExists(path); - DeleteDirectoryIfExistsAndEmpty(Path.GetDirectoryName(path)); + // Path.GetDirectoryName will return null for a root path C:\\ or / + // This is not possible based upon path construction using send & fileId and so ! operator can be used + DeleteDirectoryIfExistsAndEmpty(Path.GetDirectoryName(path)!); } public async Task DeleteFilesForOrganizationAsync(Guid organizationId) diff --git a/src/Core/Tools/SendFeatures/Services/SendAuthorizationService.cs b/src/Core/Tools/SendFeatures/Services/SendAuthorizationService.cs index 6252539166a6..b331297c8753 100644 --- a/src/Core/Tools/SendFeatures/Services/SendAuthorizationService.cs +++ b/src/Core/Tools/SendFeatures/Services/SendAuthorizationService.cs @@ -25,7 +25,7 @@ public SendAuthorizationService( } public SendAccessResult SendCanBeAccessed(Send send, - string password) + string? password) { var now = DateTime.UtcNow; if (send == null || send.MaxAccessCount.GetValueOrDefault(int.MaxValue) <= send.AccessCount || @@ -54,7 +54,7 @@ public SendAccessResult SendCanBeAccessed(Send send, return SendAccessResult.Granted; } - public async Task AccessAsync(Send sendToBeAccessed, string password) + public async Task AccessAsync(Send sendToBeAccessed, string? password) { var accessResult = SendCanBeAccessed(sendToBeAccessed, password); diff --git a/test/Api.Test/Tools/Controllers/SendsControllerTests.cs b/test/Api.Test/Tools/Controllers/SendsControllerTests.cs index b66844b10deb..442d5ad09954 100644 --- a/test/Api.Test/Tools/Controllers/SendsControllerTests.cs +++ b/test/Api.Test/Tools/Controllers/SendsControllerTests.cs @@ -110,7 +110,7 @@ public async Task Post_DeletionDateIsMoreThan31DaysFromNow_ThrowsBadRequest() var expected = "You cannot have a Send with a deletion date that far " + "into the future. Adjust the Deletion Date to a value less than 31 days from now " + "and try again."; - var request = new SendRequestModel() { DeletionDate = now.AddDays(32) }; + var request = new SendRequestModel() { Key = "test_key", DeletionDate = now.AddDays(32) }; var exception = await Assert.ThrowsAsync(() => _sut.Post(request)); Assert.Equal(expected, exception.Message); diff --git a/test/Api.Test/Tools/Models/Request/SendRequestModelTests.cs b/test/Api.Test/Tools/Models/Request/SendRequestModelTests.cs index b4f986d1193b..144729119ce9 100644 --- a/test/Api.Test/Tools/Models/Request/SendRequestModelTests.cs +++ b/test/Api.Test/Tools/Models/Request/SendRequestModelTests.cs @@ -64,6 +64,7 @@ public void ValidateEdit_DeletionDateInPast_ThrowsBadRequestException() { var send = new SendRequestModel { + Key = "test_key", DeletionDate = DateTime.UtcNow.AddMinutes(-5) }; @@ -75,6 +76,7 @@ public void ValidateEdit_DeletionDateTooFarInFuture_ThrowsBadRequestException() { var send = new SendRequestModel { + Key = "test_key", DeletionDate = DateTime.UtcNow.AddDays(32) }; @@ -86,6 +88,7 @@ public void ValidateEdit_ExpirationDateInPast_ThrowsBadRequestException() { var send = new SendRequestModel { + Key = "test_key", ExpirationDate = DateTime.UtcNow.AddMinutes(-5) }; @@ -97,6 +100,7 @@ public void ValidateEdit_ExpirationDateGreaterThanDeletionDate_ThrowsBadRequestE { var send = new SendRequestModel { + Key = "test_key", DeletionDate = DateTime.UtcNow.AddDays(1), ExpirationDate = DateTime.UtcNow.AddDays(2) }; @@ -109,6 +113,7 @@ public void ValidateEdit_ValidDates_Success() { var send = new SendRequestModel { + Key = "test_key", DeletionDate = DateTime.UtcNow.AddDays(10), ExpirationDate = DateTime.UtcNow.AddDays(5) };