Skip to content
This repository has been archived by the owner on May 22, 2021. It is now read-only.

Confusing behavior with invalid secret key #151

Closed
pdehaan opened this issue Jun 27, 2017 · 1 comment · Fixed by #412
Closed

Confusing behavior with invalid secret key #151

pdehaan opened this issue Jun 27, 2017 · 1 comment · Fixed by #412
Milestone

Comments

@pdehaan
Copy link
Collaborator

pdehaan commented Jun 27, 2017

Found in https://fileshare.stage.mozaws.net/__version__ (a7fcb1a):

{
  "commit": "a7fcb1a",
  "source": "https://github.com/mozilla/something-awesome/",
  "version": "0.1.2"
}

Steps to reproduce:

  1. Go to https://fileshare.stage.mozaws.net/
  2. Upload a file.
  3. Copy the download URL.
  4. Paste the URL into a new tab and shave a few characters off the delete_token in the URL. In my case, the random URL is https://fileshare.stage.mozaws.net/download/f9037eccaa5004e4c20db610044d0f95/#jcem2Ds6WLyL4HZ0yyagiA.
  5. After you've submitted the [now broken] URL, click the Download File button to download the file.

Actual results:

The Dev Tools console says:

The file has expired, or has already been deleted.

And

Uncaught (in promise) TypeError: Cannot read property 'Symbol(Symbol.iterator)' of undefined
at fileReceiver.download.catch.then (bundle.js:1)
at

download_your_file

But looking at my downloaded files, it doesn't look like the file actually ever downloaded.

Going to the URL again, it seems like the download has been deleted from the server, but never actually successfully downloaded to my computer:

https://fileshare.stage.mozaws.net/download/f9037eccaa5004e4c20db610044d0f95/#jcem now says:

download_your_file

Expected results:

  • The file shouldn't be deleted off the server if it wasn't successfully downloaded.
  • We should perhaps do better data validation and messaging to say things like "invalid delete_token" (although that probably doesn't mean anything to an end-user), or something that lets the user know that they maybe didn't copy the entire URL or something else happened.
@ghost ghost added this to the Elegant Eagle (July 28) milestone Jul 5, 2017
@abhinadduri
Copy link
Collaborator

abhinadduri commented Jul 6, 2017

If you're talking about the hash parameter in the URL: the parameter you're changing is the decryption token, not the delete token. The delete token is stored in local storage and is sent as part of a delete request only. The server never sees the decryption token, so there is no way to validate whether or not it's valid server-side. The file is completely client side at this point but only downloads to the file system from the browser if the decryption finishes successfully.

Maybe we can check the URL decryption token in the browser before ever issuing a download request to the server?

@dannycoates dannycoates marked this as a duplicate of #348 Jul 31, 2017
@dannycoates dannycoates changed the title Confusing behavior with invalid delete_token Confusing behavior with invalid secret key Jul 31, 2017
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 a pull request may close this issue.

2 participants