From f66f9e95bf1e67ad13de9958d16103b858b58e72 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 15 Jan 2016 02:29:34 -0800 Subject: Give reporters the ability to download artifacts. Also fix a few places where page_404 should be render_404. --- app/controllers/projects/builds_controller.rb | 4 ++-- app/controllers/projects/commit_controller.rb | 2 +- app/models/ability.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index 0e965966ffa..92d9699fe84 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -42,7 +42,7 @@ class Projects::BuildsController < Projects::ApplicationController def retry unless @build.retryable? - return page_404 + return render_404 end build = Ci::Build.retry(@build) @@ -72,7 +72,7 @@ class Projects::BuildsController < Projects::ApplicationController def authorize_manage_builds! unless can?(current_user, :manage_builds, project) - return page_404 + return render_404 end end end diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 0aaba3792bf..870f6795219 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -79,7 +79,7 @@ class Projects::CommitController < Projects::ApplicationController def authorize_manage_builds! unless can?(current_user, :manage_builds, project) - return page_404 + return render_404 end end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 5375148a654..ab59a3506a2 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -160,6 +160,7 @@ class Ability @project_report_rules ||= project_guest_rules + [ :create_commit_status, :read_commit_statuses, + :read_build_artifacts, :download_code, :fork_project, :create_project_snippet, @@ -175,7 +176,6 @@ class Ability :create_merge_request, :create_wiki, :manage_builds, - :read_build_artifacts, :push_code ] end -- cgit v1.2.1 From b7bb56c7a3504e259887909a2e8515c5896a173e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 13:09:20 +0100 Subject: Expand builds namespace for feature tests in spinach --- features/project/builds.feature | 58 ------------------- features/project/builds/artifacts.feature | 53 ++++++++++++++++++ features/project/builds/summary.feature | 11 ++++ features/steps/project/builds.rb | 89 ------------------------------ features/steps/project/builds/artifacts.rb | 81 +++++++++++++++++++++++++++ features/steps/project/builds/summary.rb | 14 +++++ 6 files changed, 159 insertions(+), 147 deletions(-) delete mode 100644 features/project/builds.feature create mode 100644 features/project/builds/artifacts.feature create mode 100644 features/project/builds/summary.feature delete mode 100644 features/steps/project/builds.rb create mode 100644 features/steps/project/builds/artifacts.rb create mode 100644 features/steps/project/builds/summary.rb diff --git a/features/project/builds.feature b/features/project/builds.feature deleted file mode 100644 index c00b0a7ae07..00000000000 --- a/features/project/builds.feature +++ /dev/null @@ -1,58 +0,0 @@ -Feature: Project Builds - Background: - Given I sign in as a user - And I own a project - And CI is enabled - And I have recent build for my project - - Scenario: I browse build summary page - When I visit recent build summary page - Then I see summary for build - And I see build trace - - Scenario: I download build artifacts - Given recent build has artifacts available - When I visit recent build summary page - And I click artifacts download button - Then download of build artifacts archive starts - - Scenario: I browse build artifacts - Given recent build has artifacts available - And recent build has artifacts metadata available - When I visit recent build summary page - And I click artifacts browse button - Then I should see content of artifacts archive - - Scenario: I browse subdirectory of build artifacts - Given recent build has artifacts available - And recent build has artifacts metadata available - When I visit recent build summary page - And I click artifacts browse button - And I click link to subdirectory within build artifacts - Then I should see content of subdirectory within artifacts archive - - Scenario: I browse directory with UTF-8 characters in name - Given recent build has artifacts available - And recent build has artifacts metadata available - And recent build artifacts contain directory with UTF-8 characters - When I visit recent build summary page - And I click artifacts browse button - And I navigate to directory with UTF-8 characters in name - Then I should see content of directory with UTF-8 characters in name - - Scenario: I try to browse directory with invalid UTF-8 characters in name - Given recent build has artifacts available - And recent build has artifacts metadata available - And recent build artifacts contain directory with invalid UTF-8 characters - When I visit recent build summary page - And I click artifacts browse button - And I navigate to parent directory of directory with invalid name - Then I should not see directory with invalid name on the list - - Scenario: I download a single file from build artifacts - Given recent build has artifacts available - And recent build has artifacts metadata available - When I visit recent build summary page - And I click artifacts browse button - And I click download button for a file within build artifacts - Then download of a file extracted from build artifacts should start diff --git a/features/project/builds/artifacts.feature b/features/project/builds/artifacts.feature new file mode 100644 index 00000000000..b624a0bdb58 --- /dev/null +++ b/features/project/builds/artifacts.feature @@ -0,0 +1,53 @@ +Feature: Project Builds Artifacts + Background: + Given I sign in as a user + And I own a project + And CI is enabled + And I have recent build for my project + + Scenario: I download build artifacts + Given recent build has artifacts available + When I visit recent build summary page + And I click artifacts download button + Then download of build artifacts archive starts + + Scenario: I browse build artifacts + Given recent build has artifacts available + And recent build has artifacts metadata available + When I visit recent build summary page + And I click artifacts browse button + Then I should see content of artifacts archive + + Scenario: I browse subdirectory of build artifacts + Given recent build has artifacts available + And recent build has artifacts metadata available + When I visit recent build summary page + And I click artifacts browse button + And I click link to subdirectory within build artifacts + Then I should see content of subdirectory within artifacts archive + + Scenario: I browse directory with UTF-8 characters in name + Given recent build has artifacts available + And recent build has artifacts metadata available + And recent build artifacts contain directory with UTF-8 characters + When I visit recent build summary page + And I click artifacts browse button + And I navigate to directory with UTF-8 characters in name + Then I should see content of directory with UTF-8 characters in name + + Scenario: I try to browse directory with invalid UTF-8 characters in name + Given recent build has artifacts available + And recent build has artifacts metadata available + And recent build artifacts contain directory with invalid UTF-8 characters + When I visit recent build summary page + And I click artifacts browse button + And I navigate to parent directory of directory with invalid name + Then I should not see directory with invalid name on the list + + Scenario: I download a single file from build artifacts + Given recent build has artifacts available + And recent build has artifacts metadata available + When I visit recent build summary page + And I click artifacts browse button + And I click download button for a file within build artifacts + Then download of a file extracted from build artifacts should start diff --git a/features/project/builds/summary.feature b/features/project/builds/summary.feature new file mode 100644 index 00000000000..5e938ea090e --- /dev/null +++ b/features/project/builds/summary.feature @@ -0,0 +1,11 @@ +Feature: Project Builds Summary + Background: + Given I sign in as a user + And I own a project + And CI is enabled + And I have recent build for my project + + Scenario: I browse build summary page + When I visit recent build summary page + Then I see summary for build + And I see build trace diff --git a/features/steps/project/builds.rb b/features/steps/project/builds.rb deleted file mode 100644 index 28395281077..00000000000 --- a/features/steps/project/builds.rb +++ /dev/null @@ -1,89 +0,0 @@ -class Spinach::Features::ProjectBuilds < Spinach::FeatureSteps - include SharedAuthentication - include SharedProject - include SharedBuilds - include RepoHelpers - - step 'I see summary for build' do - expect(page).to have_content "Build ##{@build.id}" - end - - step 'I see build trace' do - expect(page).to have_css '#build-trace' - end - - step 'I click artifacts download button' do - page.within('.artifacts') { click_link 'Download' } - end - - step 'download of build artifacts archive starts' do - expect(page.response_headers['Content-Type']).to eq 'application/zip' - expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary' - end - - step 'I click artifacts browse button' do - page.within('.artifacts') { click_link 'Browse' } - end - - step 'I should see content of artifacts archive' do - page.within('.tree-table') do - expect(page).to have_no_content '..' - expect(page).to have_content 'other_artifacts_0.1.2' - expect(page).to have_content 'ci_artifacts.txt' - expect(page).to have_content 'rails_sample.jpg' - end - end - - step 'I click link to subdirectory within build artifacts' do - page.within('.tree-table') { click_link 'other_artifacts_0.1.2' } - end - - step 'I should see content of subdirectory within artifacts archive' do - page.within('.tree-table') do - expect(page).to have_content '..' - expect(page).to have_content 'another-subdirectory' - expect(page).to have_content 'doc_sample.txt' - end - end - - step 'recent build artifacts contain directory with UTF-8 characters' do - # metadata fixture contains relevant directory - end - - step 'I navigate to directory with UTF-8 characters in name' do - page.within('.tree-table') { click_link 'tests_encoding' } - page.within('.tree-table') { click_link 'utf8 test dir ✓' } - end - - step 'I should see content of directory with UTF-8 characters in name' do - page.within('.tree-table') do - expect(page).to have_content '..' - expect(page).to have_content 'regular_file_2' - end - end - - step 'recent build artifacts contain directory with invalid UTF-8 characters' do - # metadata fixture contains relevant directory - end - - step 'I navigate to parent directory of directory with invalid name' do - page.within('.tree-table') { click_link 'tests_encoding' } - end - - step 'I should not see directory with invalid name on the list' do - page.within('.tree-table') do - expect(page).to have_no_content('non-utf8-dir') - end - end - - step 'I click download button for a file within build artifacts' do - page.within('.tree-table') { first('.artifact-download').click } - end - - step 'download of a file extracted from build artifacts should start' do - # this will be accelerated by Workhorse - response_json = JSON.parse(page.body, symbolize_names: true) - expect(response_json[:archive]).to end_with('build_artifacts.zip') - expect(response_json[:entry]).to eq Base64.encode64('ci_artifacts.txt') - end -end diff --git a/features/steps/project/builds/artifacts.rb b/features/steps/project/builds/artifacts.rb new file mode 100644 index 00000000000..f4f91ad1d8c --- /dev/null +++ b/features/steps/project/builds/artifacts.rb @@ -0,0 +1,81 @@ +class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedBuilds + include RepoHelpers + + step 'I click artifacts download button' do + page.within('.artifacts') { click_link 'Download' } + end + + step 'download of build artifacts archive starts' do + expect(page.response_headers['Content-Type']).to eq 'application/zip' + expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary' + end + + step 'I click artifacts browse button' do + page.within('.artifacts') { click_link 'Browse' } + end + + step 'I should see content of artifacts archive' do + page.within('.tree-table') do + expect(page).to have_no_content '..' + expect(page).to have_content 'other_artifacts_0.1.2' + expect(page).to have_content 'ci_artifacts.txt' + expect(page).to have_content 'rails_sample.jpg' + end + end + + step 'I click link to subdirectory within build artifacts' do + page.within('.tree-table') { click_link 'other_artifacts_0.1.2' } + end + + step 'I should see content of subdirectory within artifacts archive' do + page.within('.tree-table') do + expect(page).to have_content '..' + expect(page).to have_content 'another-subdirectory' + expect(page).to have_content 'doc_sample.txt' + end + end + + step 'recent build artifacts contain directory with UTF-8 characters' do + # metadata fixture contains relevant directory + end + + step 'I navigate to directory with UTF-8 characters in name' do + page.within('.tree-table') { click_link 'tests_encoding' } + page.within('.tree-table') { click_link 'utf8 test dir ✓' } + end + + step 'I should see content of directory with UTF-8 characters in name' do + page.within('.tree-table') do + expect(page).to have_content '..' + expect(page).to have_content 'regular_file_2' + end + end + + step 'recent build artifacts contain directory with invalid UTF-8 characters' do + # metadata fixture contains relevant directory + end + + step 'I navigate to parent directory of directory with invalid name' do + page.within('.tree-table') { click_link 'tests_encoding' } + end + + step 'I should not see directory with invalid name on the list' do + page.within('.tree-table') do + expect(page).to have_no_content('non-utf8-dir') + end + end + + step 'I click download button for a file within build artifacts' do + page.within('.tree-table') { first('.artifact-download').click } + end + + step 'download of a file extracted from build artifacts should start' do + # this will be accelerated by Workhorse + response_json = JSON.parse(page.body, symbolize_names: true) + expect(response_json[:archive]).to end_with('build_artifacts.zip') + expect(response_json[:entry]).to eq Base64.encode64('ci_artifacts.txt') + end +end diff --git a/features/steps/project/builds/summary.rb b/features/steps/project/builds/summary.rb new file mode 100644 index 00000000000..2439d48fbef --- /dev/null +++ b/features/steps/project/builds/summary.rb @@ -0,0 +1,14 @@ +class Spinach::Features::ProjectBuildsSummary < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedBuilds + include RepoHelpers + + step 'I see summary for build' do + expect(page).to have_content "Build ##{@build.id}" + end + + step 'I see build trace' do + expect(page).to have_css '#build-trace' + end +end -- cgit v1.2.1 From 6b2f38f39a473e6791b39e61645d76638d4bd673 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 13:48:29 +0100 Subject: Fix nonexistent method in artifacts controller --- app/controllers/projects/artifacts_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index dff0732bdfe..f159a6d6dc6 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -8,7 +8,7 @@ class Projects::ArtifactsController < Projects::ApplicationController end unless artifacts_file.exists? - return not_found! + return render_404 end send_file artifacts_file.path, disposition: 'attachment' -- cgit v1.2.1 From 6a504c8256fe5281819cc0a6dc916230f4203c7c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 13:56:43 +0100 Subject: Add feature tests for permissions for build artifacts read ability --- features/project/builds/artifacts.feature | 4 ++-- features/project/builds/permissions.feature | 18 ++++++++++++++++++ features/project/builds/summary.feature | 4 ++-- features/steps/project/builds/artifacts.rb | 5 ----- features/steps/project/builds/permissions.rb | 7 +++++++ features/steps/shared/builds.rb | 13 +++++++++++-- features/steps/shared/project.rb | 18 +++++++++++++++++- 7 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 features/project/builds/permissions.feature create mode 100644 features/steps/project/builds/permissions.rb diff --git a/features/project/builds/artifacts.feature b/features/project/builds/artifacts.feature index b624a0bdb58..7a7dbb71b18 100644 --- a/features/project/builds/artifacts.feature +++ b/features/project/builds/artifacts.feature @@ -2,8 +2,8 @@ Feature: Project Builds Artifacts Background: Given I sign in as a user And I own a project - And CI is enabled - And I have recent build for my project + And project has CI enabled + And project has a recent build Scenario: I download build artifacts Given recent build has artifacts available diff --git a/features/project/builds/permissions.feature b/features/project/builds/permissions.feature new file mode 100644 index 00000000000..1193bcd74f6 --- /dev/null +++ b/features/project/builds/permissions.feature @@ -0,0 +1,18 @@ +Feature: Project Builds Permissions + Background: + Given I sign in as a user + And project exists in some group namespace + And project has CI enabled + And project has a recent build + + Scenario: I try to download build artifacts as guest + Given I am member of a project with a guest role + And recent build has artifacts available + When I access artifacts download page + Then page status code should be 404 + + Scenario: I try to download build artifacts as reporter + Given I am member of a project with a reporter role + And recent build has artifacts available + When I access artifacts download page + Then download of build artifacts archive starts diff --git a/features/project/builds/summary.feature b/features/project/builds/summary.feature index 5e938ea090e..e90ea592aab 100644 --- a/features/project/builds/summary.feature +++ b/features/project/builds/summary.feature @@ -2,8 +2,8 @@ Feature: Project Builds Summary Background: Given I sign in as a user And I own a project - And CI is enabled - And I have recent build for my project + And project has CI enabled + And project has a recent build Scenario: I browse build summary page When I visit recent build summary page diff --git a/features/steps/project/builds/artifacts.rb b/features/steps/project/builds/artifacts.rb index f4f91ad1d8c..f2c87da4717 100644 --- a/features/steps/project/builds/artifacts.rb +++ b/features/steps/project/builds/artifacts.rb @@ -8,11 +8,6 @@ class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps page.within('.artifacts') { click_link 'Download' } end - step 'download of build artifacts archive starts' do - expect(page.response_headers['Content-Type']).to eq 'application/zip' - expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary' - end - step 'I click artifacts browse button' do page.within('.artifacts') { click_link 'Browse' } end diff --git a/features/steps/project/builds/permissions.rb b/features/steps/project/builds/permissions.rb new file mode 100644 index 00000000000..6e9d6504fd5 --- /dev/null +++ b/features/steps/project/builds/permissions.rb @@ -0,0 +1,7 @@ +class Spinach::Features::ProjectBuildsPermissions < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedBuilds + include SharedPaths + include RepoHelpers +end diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb index a83d74e5946..f88b01af84e 100644 --- a/features/steps/shared/builds.rb +++ b/features/steps/shared/builds.rb @@ -1,11 +1,11 @@ module SharedBuilds include Spinach::DSL - step 'CI is enabled' do + step 'project has CI enabled' do @project.enable_ci end - step 'I have recent build for my project' do + step 'project has a recent build' do ci_commit = create :ci_commit, project: @project, sha: sample_commit.id @build = create :ci_build, commit: ci_commit end @@ -25,4 +25,13 @@ module SharedBuilds gzip = fixture_file_upload(metadata, 'application/x-gzip') @build.update_attributes(artifacts_metadata: gzip) end + + step 'download of build artifacts archive starts' do + expect(page.response_headers['Content-Type']).to eq 'application/zip' + expect(page.response_headers['Content-Transfer-Encoding']).to eq 'binary' + end + + step 'I access artifacts download page' do + visit download_namespace_project_build_artifacts_path(@project.namespace, @project, @build) + end end diff --git a/features/steps/shared/project.rb b/features/steps/shared/project.rb index d3501b5f5cb..d9c75d12238 100644 --- a/features/steps/shared/project.rb +++ b/features/steps/shared/project.rb @@ -7,6 +7,11 @@ module SharedProject @project.team << [@user, :master] end + step "project exists in some group namespace" do + @group = create(:group, name: 'some group') + @project = create(:project, namespace: @group) + end + # Create a specific project called "Shop" step 'I own project "Shop"' do @project = Project.find_by(name: "Shop") @@ -97,6 +102,18 @@ module SharedProject @project ||= Project.first end + # ---------------------------------------- + # Project permissions + # ---------------------------------------- + + step 'I am member of a project with a guest role' do + @project.team << [@user, Gitlab::Access::GUEST] + end + + step 'I am member of a project with a reporter role' do + @project.team << [@user, Gitlab::Access::REPORTER] + end + # ---------------------------------------- # Visibility of archived project # ---------------------------------------- @@ -229,5 +246,4 @@ module SharedProject project ||= create(:empty_project, visibility, name: project_name, namespace: user.namespace) project.team << [user, :master] end - end -- cgit v1.2.1 From 1558d8af6db7156a327bd7244c70fa30e892ae09 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 14:08:56 +0100 Subject: Add Changelog entry for new permissions for build artifacts [ci skip] --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index d01576be67a..539c7528574 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -54,6 +54,7 @@ v 8.4.0 (unreleased) - Allow broadcast messages to be edited - Autosize Markdown textareas - Import GitHub wiki into GitLab + - Add reporters ability to download and browse build artifacts (Andrew Johnson) v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) -- cgit v1.2.1