Skip to content
This repository has been archived by the owner on Jan 8, 2022. It is now read-only.

Authenticate file downloads - requires server updates to deploy #104

Merged
merged 4 commits into from
Feb 6, 2020

Conversation

peetucket
Copy link
Contributor

@peetucket peetucket commented Jul 24, 2017

Fixes #392

Verified on hydrus-stage

Deployment notes

  • Coordinate deployment with Hannah - currently the week of Feb 3 looks good
  • After deploy:
    • confirm that the previous insecure /opt/app/hydrus/hydrus/current/public/uploads symlink is gone.
    • confirm that the new /opt/app/hydrus/hydrus/current/uploads symlink exists and goes to /opt/app/hydrus/hydrus/shared/uploads, which goes to /data/hydrus-files
    • test by uploading a file to a new test object and confirm you can't download that file from hydrus in a private browser (not logged in)
    • publish the test object and verify it accessioned correctly with a PURL
    • test a download of a file from a previously created object
  • After verified working, we should remove the now unused symlink that goes from /opt/app/hydrus/hydrus/shared/public/uploads --> /data/hydrus_files ... note that because we don't symlink in from the currently deployed public directory anymore, leaving this symlink is not a security risk, but it should be cleaned up anyway to avoid future confusion:
    • rm /opt/app/hydrus/hydrus/shared/public/uploads

Why the change?

Currently anyone can request download of any file that has been uploaded by other users (since they are all just dumped into the public/uploads folder), which is a potential security risk. This is an attempt to verify the person requesting to download the file (from within hydrus, not the stacks) is the one who owns it. To fix this, this PR:

  • Creates a new controller to manage file downloads so we can authenticate before streaming file.
  • Changes file upload location so files do not end up in public folder.
  • Changes tmp folder location to regular rails tmp folder.
  • Overrides default URLs to files generated by CarrierWave gem to our new route that confirms access restrictions.
  • Moves configuration information on upload file location to the new settings file.

@peetucket peetucket force-pushed the authenticate-file-downloads branch 2 times, most recently from 1c248a1 to 6983a2c Compare July 25, 2017 18:34
@peetucket peetucket changed the title [WIP] Authenticate file downloads Authenticate file downloads - requires server updates to deploy Jul 25, 2017
@peetucket peetucket force-pushed the authenticate-file-downloads branch 2 times, most recently from 619b4fe to d18c09f Compare July 27, 2017 18:14
@peetucket peetucket force-pushed the authenticate-file-downloads branch 3 times, most recently from 78f18f2 to b4dd4ed Compare August 8, 2017 21:40
@peetucket peetucket changed the title Authenticate file downloads - requires server updates to deploy [WIP] Authenticate file downloads - requires server updates to deploy Aug 9, 2017
@peetucket peetucket force-pushed the authenticate-file-downloads branch 2 times, most recently from bf33558 to 738b294 Compare August 9, 2017 23:34
@atz atz mentioned this pull request Apr 26, 2018
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.6%) to 91.896% when pulling cd5f736 on authenticate-file-downloads into 97ff319 on master.

@ndushay
Copy link
Contributor

ndushay commented Jan 8, 2020

can this be closed and the branch removed? it's over 2 years old.

@peetucket
Copy link
Contributor Author

I'll double-check with @hannahfrost about this and #149 ... if we don't feel concerned about the issues there were attempting to address, we can close.

@hannahfrost
Copy link
Contributor

After some discussion with @peetucket, I propose we pick this work back up rather than dispose of it. There is good reason to make sure access to these files on the Hydrus mount is authenticated.

@peetucket peetucket force-pushed the authenticate-file-downloads branch 3 times, most recently from ec0fe87 to 3c9ce2e Compare January 8, 2020 22:53
@peetucket peetucket changed the title [WIP] Authenticate file downloads - requires server updates to deploy Authenticate file downloads - requires server updates to deploy Jan 8, 2020
@peetucket peetucket changed the title Authenticate file downloads - requires server updates to deploy [HOLD FOR TESTING ON STAGE] Authenticate file downloads - requires server updates to deploy Jan 23, 2020
@peetucket peetucket changed the title [HOLD FOR TESTING ON STAGE] Authenticate file downloads - requires server updates to deploy [VERIFIED ON STAGE] Authenticate file downloads - requires server updates to deploy Jan 23, 2020
@peetucket peetucket changed the title [VERIFIED ON STAGE] Authenticate file downloads - requires server updates to deploy [HOLD] Authenticate file downloads - requires server updates to deploy Jan 24, 2020
@peetucket peetucket changed the title [HOLD] Authenticate file downloads - requires server updates to deploy Authenticate file downloads - requires server updates to deploy Feb 6, 2020
@peetucket peetucket merged commit 8e3f1a6 into master Feb 6, 2020
@peetucket peetucket deleted the authenticate-file-downloads branch February 6, 2020 19:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security implications for uploaded files
5 participants