Skip to content

Prepare for send direct upload#1174

Merged
MGibson1 merged 4 commits intomasterfrom
prepare-for-send-direct-upload
Mar 1, 2021
Merged

Prepare for send direct upload#1174
MGibson1 merged 4 commits intomasterfrom
prepare-for-send-direct-upload

Conversation

@MGibson1
Copy link
Member

@MGibson1 MGibson1 commented Mar 1, 2021

Overview

Splits out Send download links from #1167. Keeps the changes to blob/file storage scheme to enable upload events in the future, but I'm running out of dev time and want to get downloads fixed.

This PR is a companion to bitwarden/jslib#288

Files Changed

  • SendsController: Add access validation to get file download url. This closes some race conditions on access count and solves an issue where storing the fileId of a Send would enable you to continually request download links
  • SendService:
  • SendFileStorageServices: Update send file/blob path to include the SendId. This is to allow for future event grid locating of Sends to validate file sizes
  • SendService: Re-validate access when requesting a download link.

MGibson1 added 2 commits March 1, 2021 11:56
Event Grid returns the blob path, which will be used to grab a Send and verify file size
Increment access count only when file is downloaded. File
name and size are leaked, but this is a good first step toward
solving the access-download race
Copy link
Contributor

@cscharf cscharf left a comment

Choose a reason for hiding this comment

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

Looks good, just some odd formatting made it in

@MGibson1 MGibson1 requested a review from cscharf March 1, 2021 19:30
@MGibson1 MGibson1 merged commit 8d5fc21 into master Mar 1, 2021
@MGibson1 MGibson1 deleted the prepare-for-send-direct-upload branch March 1, 2021 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants