Enable azure direct upload by MGibson1 · Pull Request #1167 · bitwarden/server

MGibson1 · 2021-02-26T15:16:18Z

Overview

This PR creates endpoints to allow direct upload to Azure blob storage and local storage.

Files Changed

Api.csproj: Add event grid nuget dependency. This package allows for easier integration with Azure Event Grid, which is used to verify Send file sizes.
SendsController.cs: New endpoints:
- GetSendFileDownloadData: Grabs a download URL. Response model object is for future-proof returning more information. For now, it's just a download URL.
- PostFile: Creates a file type send and get a URL/SendResponse to upload the file to. FileUploadType is used to specify Azure vs Direct upload. This request makes and validates a send based on a promised file size. This is checked upon actual file upload.
- PostFileForExistingSend: The API endpoint for localStorage deployments. This maintains the 100MB upload limit, uploads the file to local storage. SendService also validates the length of the received file stream is within a grace size.
- AzureValidateFile: Azure event webhook endpoint. Accepts Azure Event Grid Event POSTs and handles blobCreate events, validating the final blob size. This method blocks the client's return from blob create writes.
ApiHelpers.cs: Azure requires a response verification code to prove we own the webhook endpoint we specify. This helper automatically handles Verification events and accepts a Dictionary of event handlers to call for other Azure events.
MultipartFormDataHelper.cs: GetSendFileAsync is a form containing only file data now since the URL has the send Id and file ID that have already been created.
FileUploadType.cs: Specifies the method of uploading a file. Direct is direct to Bitwarden. Azure is an Azure Upload. This is used by the client to upload in the appropriate manner.
SendRequestModel.cs: Optionally provide a hint of the File Size that will be uploaded for this Send. The FileLength is required for File Sends and ignored for Text sends.
SendFileUploadDataResponseModel: Specifies the Send URL, UploadType, and Generated Send. This is the response to a request to make a new File Type Send.
ISendService/SendService.cs:
- SaveFileSendAsync: Creates a File Send without file data and responds with a string to upload the file data to.
- UploadFileToExistingSend: The local storage endpoint. Should not be used for Azure deployments. Validates existence of Send, stores the stream, and validates its size prior to returning. Throws in the event of a size mismatch since the client will correctly interpret that throw as a toast error.
- ValidateSendFile.cs: Simply validates the specified file and deletes the send if it does not mesh within a reasonable leeway (set to 1MB).
AzureSendFileStorageService.cs:
- Blobs are stored under SendId. This is because Azure doesn't send metadata with Events. This way, we can grab Send objects in Controller, following the current pattern. It also allows us to eventually add multiple files to Sends if we want to.
- GetSendUploadUrl: Uses SAS string to authorize a 1 min create permission to the specified blob. This requires a few Azure settings
  1. CORS for blob storage needs to be set to allow any orgin for GET and PUT methods (upload is PUT only, but GET is needed for the download portion).
  2. The sendfiles container should have permissions set to off
- ValidateFile: Called from the Azure event webhook upon blob creation (which occurs after the file is finished uploading). Pulls the blob's size and sets some metadata/property attributes that could not be set prior to creation. We could offload this to the clients, but it makes sense to keep this file organization stuff in the server. If the size is not roughly equal to expected, returns false to indicate the Send should be removed due to unvalidated file size.
LocalSendFileStorageService: Validates that file on disk equals expected file size. We need to write to disk first since HttpStreams have no known length until they're read and we don't want to risk storing a huge file in memory to figure out the size.
NoopSendFileStorageService: Noop implementations.

Required Azure Changes

CORS for blob storage needs to be set to allow any orgin for GET and PUT methods (upload is PUT only, but GET is needed for the download portion).
The sendfiles container should have permissions set to off
Specify webhook event for blob creation with filters defined by @joseph-flinn. Endpoint is <<apiUrl>>/sends/file/validate/azure

Testing requirements

Need to test upload and download to both local and Azure hosted environments.
I've tested lying about file size, but a second set of eyes on that is a good idea.

The above require access to bitwarden/web#853

To validate file sizes in the event of a rogue client, Azure event webhooks will be hooked up to AzureValidateFile. Sends outside of a grace size will be deleted as non-compliant. TODO: LocalSendFileStorageService direct upload method/endpoint.

These shouldn't happen, but might if some errant get requests occur

It turns out that multipartHttpStreams do not have a length until read. this causes all long files to be "invalid". We need to write the entire stream, then validate length, just like Azure. the difference is, We can return an exception to local storage admonishing the client for lying

src/Api/Controllers/SendsController.cs

cscharf · 2021-02-26T16:54:44Z

src/Api/Controllers/SendsController.cs

+            var sendId = new Guid(CoreHelpers.Base64UrlDecode(encodedSendId));
+            var send = await _sendRepository.GetByIdAsync(sendId);
+
+            if (send == null)


Also missing a check here as to whether the Send is enabled or has exceeded it's access count. Although not sure if the access count parameter gets another check here since it would have already been incremented by 1 before downloading the file (assuming).

Pretty sizable changes to fix this, but I think I hit the points we discussed. there's still a bit of a data leak in that you learn the name and size of the file without incrementing AccessCount, but for now that seems a small issue

src/Api/Controllers/SendsController.cs

src/Api/Utilities/ApiHelpers.cs

src/Api/Controllers/SendsController.cs

src/Core/Services/Implementations/LocalSendStorageService.cs

cscharf · 2021-02-26T17:20:14Z

src/Core/Services/Implementations/LocalSendStorageService.cs

            }
        }

+        private void DeleteDirectoryIfExists(string path)


Not sure I like the idea of deleting directories and/or storing Send files in a directory per Send.

I can revert, not married to the idea. I just liked matching the file structure to Azure. There I have good reasons to want SendId in the blob path.

Co-authored-by: Chad Scharf <3904944+cscharf@users.noreply.github.com>

cscharf

Looks good, just 1 minor thing

src/Core/Services/Implementations/SendService.cs

Co-authored-by: Chad Scharf <3904944+cscharf@users.noreply.github.com>

MGibson1 · 2021-03-04T15:37:31Z

Split this PR into a download-side PR (#1174), with some simple preparations for this work (send blob locations).

This is held up by Azure/azure-sdk-for-js/issues/14025 blocking upload on Desktop.

I'm working through a REST-based upload scheme, but that's not yet ready. I will reopen this PR when this issue is solved

MGibson1 added 8 commits February 25, 2021 09:10

Direct upload to azure

566ed1c

To validate file sizes in the event of a rogue client, Azure event webhooks will be hooked up to AzureValidateFile. Sends outside of a grace size will be deleted as non-compliant. TODO: LocalSendFileStorageService direct upload method/endpoint.

Decode send Id from accessId

b87866f

Quick respond to no-body event calls

dc93065

These shouldn't happen, but might if some errant get requests occur

Event Grid only POSTS to webhook

8af6a46

Enable local storage direct file upload

41217c7

Increase file size difference leeway

93963f2

Upload through service

4861006

MGibson1 requested a review from a team February 26, 2021 15:16

MGibson1 mentioned this pull request Feb 26, 2021

Upload Directly to Azure or Local storage bitwarden/jslib#285

Closed

cscharf reviewed Feb 26, 2021

View reviewed changes

MGibson1 and others added 5 commits February 26, 2021 13:35

Update src/Api/Utilities/ApiHelpers.cs

e3aee91

Co-authored-by: Chad Scharf <3904944+cscharf@users.noreply.github.com>

Do not delete directory if it has files

4b186b3

Allow large uploads for self hosted instances

8aa423f

Fix formatting

135d1e2

Re-verfiy access and increment access count on download of Send File

aefe17c

MGibson1 requested a review from cscharf February 26, 2021 21:05

Merge branch 'master' into enable-azure-direct-upload

2fc6450

cscharf reviewed Feb 26, 2021

View reviewed changes

src/Core/Services/Implementations/SendService.cs Show resolved Hide resolved

Update src/Core/Services/Implementations/SendService.cs

995c5a4

Co-authored-by: Chad Scharf <3904944+cscharf@users.noreply.github.com>

MGibson1 requested a review from cscharf February 26, 2021 22:29

cscharf approved these changes Feb 26, 2021

View reviewed changes

MGibson1 mentioned this pull request Mar 1, 2021

Prepare for send direct upload #1174

Merged

MGibson1 closed this Mar 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable azure direct upload#1167

Enable azure direct upload#1167
MGibson1 wants to merge 15 commits intomasterfrom
enable-azure-direct-upload

MGibson1 commented Feb 26, 2021 •

edited

Loading

Uh oh!

Uh oh!

cscharf Feb 26, 2021

Uh oh!

MGibson1 Feb 26, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cscharf Feb 26, 2021

Uh oh!

MGibson1 Feb 26, 2021

Uh oh!

cscharf left a comment

Uh oh!

Uh oh!

MGibson1 commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

MGibson1 commented Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Files Changed

Required Azure Changes

Testing requirements

Uh oh!

Uh oh!

cscharf Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

MGibson1 Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cscharf Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

MGibson1 Feb 26, 2021

Choose a reason for hiding this comment

Uh oh!

cscharf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MGibson1 commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MGibson1 commented Feb 26, 2021 •

edited

Loading