Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access control does not prevent access to files in private resources from unauthenticated user #328

Closed
dtarb opened this issue Apr 23, 2015 · 8 comments
Assignees
Labels
Access Control bug Broad category when more specific Label can't be found

Comments

@dtarb
Copy link
Member

dtarb commented Apr 23, 2015

The URL pattern for downloading a resource is http://beta.hydroshare.org/django_irods/download/?path=4da5255ad5ec44c682f90cd7dc5932d9/Untitledresource-4_20_2015.csv. This specific resource is private but this URL results in the file being downloaded.

@dtarb dtarb added bug Broad category when more specific Label can't be found Access Control labels Apr 23, 2015
@ironfroggy
Copy link
Contributor

The view for download does not check permissions in anyway.

It also reads the entire file contents into memory before responding. This is extremely expensive and quite possibly dangerous.

@ironfroggy
Copy link
Contributor

I would recommend looking into a solution that utilizes the X-Sendfile header to pass the instruction along to the webserver directly to handle the response of the file contents more efficiently, upon verification of the requester being authorized.

@hyi
Copy link
Contributor

hyi commented Apr 28, 2015

Fix to this is pushed to github and a pull request is created to be merged into dev for testing.

@hyi hyi added this to the Release 1.5.0 milestone May 8, 2015
@hyi
Copy link
Contributor

hyi commented May 8, 2015

Tested on alpha and it is working now. Set it to ready for testing and assign milestone 1.5.0 since it is already included on alpha and working.

@dtarb
Copy link
Member Author

dtarb commented May 9, 2015

Tested on alpha. Closing.

@dtarb dtarb closed this as completed May 9, 2015
@hyi
Copy link
Contributor

hyi commented May 12, 2015

The fix to this issue introduced a regression error that prevents users from downloading bags. Reopen this issue and will make a further fix to fix this regression error.

@hyi hyi reopened this May 12, 2015
@dtarb
Copy link
Member Author

dtarb commented May 12, 2015

This underscores the need for more thorough automated testing. My testing of this tested that the file was not delivered to a user that did not have permissions and a direct link was entered. However it did not test everything else, and it is impossible for manual tests to always test everything else. I do not know if a unit test would have caught this, or whether this would have required some sort of end to end test.

hyi added a commit that referenced this issue May 12, 2015
hyi added a commit that referenced this issue May 12, 2015
…anges needed for irods browser which is not used in current HydroShare codebase without irods browser
@dtarb
Copy link
Member Author

dtarb commented May 17, 2015

Appears fixed in testing on dev so closing, lets hope for the last time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Access Control bug Broad category when more specific Label can't be found
Projects
None yet
Development

No branches or pull requests

3 participants