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

Tests for blocking path pastes and intranet URLs for secondary files... #4762

Merged
merged 2 commits into from Oct 9, 2017

Conversation

Projects
None yet
4 participants
@jmchilton
Member

jmchilton commented Oct 6, 2017

... during upload. Clears up a worry I had about some stuff added this release and ensures there no regressions in the security of this API.

xref #4289 (blocking intranet URLs by default)
xref #4404 (implementing path pastes for normal uploads with the file:// prefix).
xref #4417 (initial tests for blocking file:// prefixes for non-admins added in #4404)

extra_inputs={
"files_1|url_paste": "roadmaps content",
"files_1|type": "upload_dataset",
"files_2|url_paste": "file://%s/1.txt",

This comment has been minimized.

@nsoranzo

nsoranzo Oct 6, 2017

Member

The %s doesn't seem right here.

This comment has been minimized.

@jmchilton

jmchilton Oct 6, 2017

Member

Yeah - good point. One second...

This comment has been minimized.

@jmchilton

jmchilton Oct 6, 2017

Member

Rebased with a fix for that - it doesn't change the test outcome really but it definitely was support to be an actual valid file URI. Thanks for the catch.

Tests for blocking path pastes and intranet URLs for secondary files...
... during upload.

xref #4289 (blocking intranet URLs by default)
xref #4404 (implementing path pastes for normal uploads with the file:// prefix).
xref #4417 (initial tests for blocking file:// prefixes for non-admins added in #4404)

Rebased with a small fix thanks to the ever vigilant @nsoranzo.
@@ -64,6 +67,24 @@ def test(self):
# the newer API decorator that handles those details.
assert create_response.status_code >= 400
@skip_without_datatype("velvet")
def test_composite_datatype(self):

This comment has been minimized.

@nsoranzo

nsoranzo Oct 6, 2017

Member

Should this method be called test_secondary_file (or alternatively test_blocked_url_secondary_file renamed to test_blocked_url_composite_datatype)?

@nsoranzo nsoranzo merged commit 5a4f625 into galaxyproject:dev Oct 9, 2017

6 checks passed

api test Build finished. 303 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 161 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 57 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Oct 9, 2017

Thanks @jmchilton!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment