From f66f9e95bf1e67ad13de9958d16103b858b58e72 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 15 Jan 2016 02:29:34 -0800 Subject: [PATCH 1/5] 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 0e965966ff..92d9699fe8 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 0aaba3792b..870f679521 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 5375148a65..ab59a3506a 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 From b7bb56c7a3504e259887909a2e8515c5896a173e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 13:09:20 +0100 Subject: [PATCH 2/5] Expand builds namespace for feature tests in spinach --- .../{builds.feature => builds/artifacts.feature} | 7 +------ features/project/builds/summary.feature | 11 +++++++++++ .../project/{builds.rb => builds/artifacts.rb} | 10 +--------- features/steps/project/builds/summary.rb | 14 ++++++++++++++ 4 files changed, 27 insertions(+), 15 deletions(-) rename features/project/{builds.feature => builds/artifacts.feature} (93%) create mode 100644 features/project/builds/summary.feature rename features/steps/project/{builds.rb => builds/artifacts.rb} (91%) create mode 100644 features/steps/project/builds/summary.rb diff --git a/features/project/builds.feature b/features/project/builds/artifacts.feature similarity index 93% rename from features/project/builds.feature rename to features/project/builds/artifacts.feature index c00b0a7ae0..b624a0bdb5 100644 --- a/features/project/builds.feature +++ b/features/project/builds/artifacts.feature @@ -1,15 +1,10 @@ -Feature: Project Builds +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 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 diff --git a/features/project/builds/summary.feature b/features/project/builds/summary.feature new file mode 100644 index 0000000000..5e938ea090 --- /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/artifacts.rb similarity index 91% rename from features/steps/project/builds.rb rename to features/steps/project/builds/artifacts.rb index 2839528107..f4f91ad1d8 100644 --- a/features/steps/project/builds.rb +++ b/features/steps/project/builds/artifacts.rb @@ -1,17 +1,9 @@ -class Spinach::Features::ProjectBuilds < Spinach::FeatureSteps +class Spinach::Features::ProjectBuildsArtifacts < 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 diff --git a/features/steps/project/builds/summary.rb b/features/steps/project/builds/summary.rb new file mode 100644 index 0000000000..2439d48fbe --- /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 From 6b2f38f39a473e6791b39e61645d76638d4bd673 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 13:48:29 +0100 Subject: [PATCH 3/5] 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 dff0732bdf..f159a6d6dc 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' From 6a504c8256fe5281819cc0a6dc916230f4203c7c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 13:56:43 +0100 Subject: [PATCH 4/5] 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 b624a0bdb5..7a7dbb71b1 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 0000000000..1193bcd74f --- /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 5e938ea090..e90ea592aa 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 f4f91ad1d8..f2c87da471 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 0000000000..6e9d6504fd --- /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 a83d74e594..f88b01af84 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 d3501b5f5c..d9c75d1223 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 From 1558d8af6db7156a327bd7244c70fa30e892ae09 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 14:08:56 +0100 Subject: [PATCH 5/5] 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 d01576be67..539c752857 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)