Commit 447f1e30 authored by Kamil Trzcinski's avatar Kamil Trzcinski

Limit guest access builds

This solves https://dev.gitlab.org/gitlab/gitlabhq/issues/2646

1. This MR simplifies CI permission model:
    - read_build: allows to read a list of builds, artifacts and trace
    - update_build: allows to cancel and retry builds
    - admin_build: allows to manage triggers, runners and variables
    - read_commit_status: allows to read a list of commit statuses (including the status of a build, but doesn't allow to see a build details)
    - create_commit_status: allows to create a new commit status using API

2. I do make sure that the proper permissions are used in all places where the CI can be shown.

3. Add the `read_build` ability if user is anonymous or guest and allow_guest_to_access_builds is enabled.

4. Add CI setting: public_builds.

5. The artifacts specific permission are removed, since they are covered by `*_build`.
parent 07556b56
...@@ -3,52 +3,5 @@ module Ci ...@@ -3,52 +3,5 @@ module Ci
def self.railtie_helpers_paths def self.railtie_helpers_paths
"app/helpers/ci" "app/helpers/ci"
end end
private
def authorize_access_project!
unless can?(current_user, :read_project, project)
return page_404
end
end
def authorize_manage_builds!
unless can?(current_user, :manage_builds, project)
return page_404
end
end
def authenticate_admin!
return render_404 unless current_user.is_admin?
end
def authorize_manage_project!
unless can?(current_user, :admin_project, project)
return page_404
end
end
def page_404
render file: "#{Rails.root}/public/404.html", status: 404, layout: false
end
def default_headers
headers['X-Frame-Options'] = 'DENY'
headers['X-XSS-Protection'] = '1; mode=block'
end
# JSON for infinite scroll via Pager object
def pager_json(partial, count)
html = render_to_string(
partial,
layout: false,
formats: [:html]
)
render json: {
html: html,
count: count
}
end
end end
end end
module Ci module Ci
class ProjectsController < Ci::ApplicationController class ProjectsController < Ci::ApplicationController
before_action :project, except: [:index] before_action :project
before_action :authenticate_user!, except: [:index, :build, :badge] before_action :authorize_read_project!, except: [:badge]
before_action :authorize_access_project!, except: [:index, :badge]
before_action :no_cache, only: [:badge] before_action :no_cache, only: [:badge]
protect_from_forgery protect_from_forgery
......
class Projects::ArtifactsController < Projects::ApplicationController class Projects::ArtifactsController < Projects::ApplicationController
layout 'project' layout 'project'
before_action :authorize_read_build_artifacts! before_action :authorize_read_build!
def download def download
unless artifacts_file.file_storage? unless artifacts_file.file_storage?
...@@ -43,14 +43,4 @@ class Projects::ArtifactsController < Projects::ApplicationController ...@@ -43,14 +43,4 @@ class Projects::ArtifactsController < Projects::ApplicationController
def artifacts_file def artifacts_file
@artifacts_file ||= build.artifacts_file @artifacts_file ||= build.artifacts_file
end end
def authorize_read_build_artifacts!
unless can?(current_user, :read_build_artifacts, @project)
if current_user.nil?
return authenticate_user!
else
return render_404
end
end
end
end end
class Projects::BuildsController < Projects::ApplicationController class Projects::BuildsController < Projects::ApplicationController
before_action :build, except: [:index, :cancel_all] before_action :build, except: [:index, :cancel_all]
before_action :authorize_manage_builds!, except: [:index, :show, :status] before_action :authorize_read_build!, except: [:cancel, :cancel_all, :retry]
before_action :authorize_update_build!, except: [:index, :show, :status]
layout "project" layout "project"
...@@ -69,10 +70,4 @@ class Projects::BuildsController < Projects::ApplicationController ...@@ -69,10 +70,4 @@ class Projects::BuildsController < Projects::ApplicationController
def build_path(build) def build_path(build)
namespace_project_build_path(build.project.namespace, build.project, build) namespace_project_build_path(build.project.namespace, build.project, build)
end end
def authorize_manage_builds!
unless can?(current_user, :manage_builds, project)
return render_404
end
end
end end
...@@ -4,10 +4,10 @@ ...@@ -4,10 +4,10 @@
class Projects::CommitController < Projects::ApplicationController class Projects::CommitController < Projects::ApplicationController
# Authorize # Authorize
before_action :require_non_empty_project before_action :require_non_empty_project
before_action :authorize_download_code!, except: [:cancel_builds] before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds]
before_action :authorize_manage_builds!, only: [:cancel_builds] before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds]
before_action :authorize_read_commit_status!, only: [:builds]
before_action :commit before_action :commit
before_action :authorize_manage_builds!, only: [:cancel_builds, :retry_builds]
before_action :define_show_vars, only: [:show, :builds] before_action :define_show_vars, only: [:show, :builds]
def show def show
...@@ -77,10 +77,4 @@ class Projects::CommitController < Projects::ApplicationController ...@@ -77,10 +77,4 @@ class Projects::CommitController < Projects::ApplicationController
@statuses = ci_commit.statuses if ci_commit @statuses = ci_commit.statuses if ci_commit
end end
def authorize_manage_builds!
unless can?(current_user, :manage_builds, project)
return render_404
end
end
end end
class Projects::RunnerProjectsController < Projects::ApplicationController class Projects::RunnerProjectsController < Projects::ApplicationController
before_action :authorize_admin_project! before_action :authorize_admin_build!
layout 'project_settings' layout 'project_settings'
......
class Projects::RunnersController < Projects::ApplicationController class Projects::RunnersController < Projects::ApplicationController
before_action :authorize_admin_build!
before_action :set_runner, only: [:edit, :update, :destroy, :pause, :resume, :show] before_action :set_runner, only: [:edit, :update, :destroy, :pause, :resume, :show]
before_action :authorize_admin_project!
layout 'project_settings' layout 'project_settings'
......
class Projects::TriggersController < Projects::ApplicationController class Projects::TriggersController < Projects::ApplicationController
before_action :authorize_admin_project! before_action :authorize_admin_build!
layout 'project_settings' layout 'project_settings'
......
class Projects::VariablesController < Projects::ApplicationController class Projects::VariablesController < Projects::ApplicationController
before_action :authorize_admin_project! before_action :authorize_admin_build!
layout 'project_settings' layout 'project_settings'
......
...@@ -223,6 +223,7 @@ class ProjectsController < ApplicationController ...@@ -223,6 +223,7 @@ class ProjectsController < ApplicationController
:issues_enabled, :merge_requests_enabled, :snippets_enabled, :issues_tracker_id, :default_branch, :issues_enabled, :merge_requests_enabled, :snippets_enabled, :issues_tracker_id, :default_branch,
:wiki_enabled, :visibility_level, :import_url, :last_activity_at, :namespace_id, :avatar, :wiki_enabled, :visibility_level, :import_url, :last_activity_at, :namespace_id, :avatar,
:builds_enabled, :build_allow_git_fetch, :build_timeout_in_minutes, :build_coverage_regex, :builds_enabled, :build_allow_git_fetch, :build_timeout_in_minutes, :build_coverage_regex,
:public_builds,
) )
end end
......
...@@ -126,7 +126,7 @@ module ProjectsHelper ...@@ -126,7 +126,7 @@ module ProjectsHelper
nav_tabs << :merge_requests nav_tabs << :merge_requests
end end
if project.builds_enabled? && can?(current_user, :read_build, project) if can?(current_user, :read_build, project)
nav_tabs << :builds nav_tabs << :builds
end end
......
...@@ -5,17 +5,18 @@ class Ability ...@@ -5,17 +5,18 @@ class Ability
return [] unless user.is_a?(User) return [] unless user.is_a?(User)
return [] if user.blocked? return [] if user.blocked?
case subject.class.name case subject
when "Project" then project_abilities(user, subject) when CommitStatus then commit_status_abilities(user, subject)
when "Issue" then issue_abilities(user, subject) when Project then project_abilities(user, subject)
when "Note" then note_abilities(user, subject) when Issue then issue_abilities(user, subject)
when "ProjectSnippet" then project_snippet_abilities(user, subject) when Note then note_abilities(user, subject)
when "PersonalSnippet" then personal_snippet_abilities(user, subject) when ProjectSnippet then project_snippet_abilities(user, subject)
when "MergeRequest" then merge_request_abilities(user, subject) when PersonalSnippet then personal_snippet_abilities(user, subject)
when "Group" then group_abilities(user, subject) when MergeRequest then merge_request_abilities(user, subject)
when "Namespace" then namespace_abilities(user, subject) when Group then group_abilities(user, subject)
when "GroupMember" then group_member_abilities(user, subject) when Namespace then namespace_abilities(user, subject)
when "ProjectMember" then project_member_abilities(user, subject) when GroupMember then group_member_abilities(user, subject)
when ProjectMember then project_member_abilities(user, subject)
else [] else []
end.concat(global_abilities(user)) end.concat(global_abilities(user))
end end
...@@ -25,6 +26,8 @@ class Ability ...@@ -25,6 +26,8 @@ class Ability
case true case true
when subject.is_a?(PersonalSnippet) when subject.is_a?(PersonalSnippet)
anonymous_personal_snippet_abilities(subject) anonymous_personal_snippet_abilities(subject)
when subject.is_a?(CommitStatus)
anonymous_commit_status_abilities(subject)
when subject.is_a?(Project) || subject.respond_to?(:project) when subject.is_a?(Project) || subject.respond_to?(:project)
anonymous_project_abilities(subject) anonymous_project_abilities(subject)
when subject.is_a?(Group) || subject.respond_to?(:group) when subject.is_a?(Group) || subject.respond_to?(:group)
...@@ -52,16 +55,26 @@ class Ability ...@@ -52,16 +55,26 @@ class Ability
:read_project_member, :read_project_member,
:read_merge_request, :read_merge_request,
:read_note, :read_note,
:read_build, :read_commit_status,
:download_code :download_code
] ]
# Allow to read builds by anonymous user if guests are allowed
rules << :read_build if project.public_builds?
rules - project_disabled_features_rules(project) rules - project_disabled_features_rules(project)
else else
[] []
end end
end end
def anonymous_commit_status_abilities(subject)
rules = anonymous_project_abilities(subject.project)
# If subject is Ci::Build which inherits from CommitStatus filter the abilities
rules = filter_build_abilities(rules) if subject.is_a?(Ci::Build)
rules
end
def anonymous_group_abilities(subject) def anonymous_group_abilities(subject)
group = if subject.is_a?(Group) group = if subject.is_a?(Group)
subject subject
...@@ -113,6 +126,9 @@ class Ability ...@@ -113,6 +126,9 @@ class Ability
if project.public? || project.internal? if project.public? || project.internal?
rules.push(*public_project_rules) rules.push(*public_project_rules)
# Allow to read builds for internal projects
rules << :read_build if project.public_builds?
end end
if project.owner == user || user.admin? if project.owner == user || user.admin?
...@@ -134,7 +150,8 @@ class Ability ...@@ -134,7 +150,8 @@ class Ability
def public_project_rules def public_project_rules
@public_project_rules ||= project_guest_rules + [ @public_project_rules ||= project_guest_rules + [
:download_code, :download_code,
:fork_project :fork_project,
:read_commit_status,
] ]
end end
...@@ -149,7 +166,6 @@ class Ability ...@@ -149,7 +166,6 @@ class Ability
:read_project_member, :read_project_member,
:read_merge_request, :read_merge_request,
:read_note, :read_note,
:read_build,
:create_project, :create_project,
:create_issue, :create_issue,
:create_note :create_note
...@@ -158,24 +174,26 @@ class Ability ...@@ -158,24 +174,26 @@ class Ability
def project_report_rules def project_report_rules
@project_report_rules ||= project_guest_rules + [ @project_report_rules ||= project_guest_rules + [
:create_commit_status,
:read_commit_statuses,
:read_build_artifacts,
:download_code, :download_code,
:fork_project, :fork_project,
:create_project_snippet, :create_project_snippet,
:update_issue, :update_issue,
:admin_issue, :admin_issue,
:admin_label :admin_label,
:read_commit_status,
:read_build,
] ]
end end
def project_dev_rules def project_dev_rules
@project_dev_rules ||= project_report_rules + [ @project_dev_rules ||= project_report_rules + [
:admin_merge_request, :admin_merge_request,
:create_commit_status,
:update_commit_status,
:create_build,
:update_build,
:create_merge_request, :create_merge_request,
:create_wiki, :create_wiki,
:manage_builds,
:push_code :push_code
] ]
end end
...@@ -201,7 +219,9 @@ class Ability ...@@ -201,7 +219,9 @@ class Ability
:admin_merge_request, :admin_merge_request,
:admin_note, :admin_note,
:admin_wiki, :admin_wiki,
:admin_project :admin_project,
:admin_commit_status,
:admin_build
] ]
end end
...@@ -240,6 +260,10 @@ class Ability ...@@ -240,6 +260,10 @@ class Ability
rules += named_abilities('wiki') rules += named_abilities('wiki')
end end
unless project.builds_enabled
rules += named_abilities('build')
end
rules rules
end end
...@@ -376,6 +400,22 @@ class Ability ...@@ -376,6 +400,22 @@ class Ability
rules rules
end end
def commit_status_abilities(user, subject)
rules = project_abilities(user, subject.project)
# If subject is Ci::Build which inherits from CommitStatus filter the abilities
rules = filter_build_abilities(rules) if subject.is_a?(Ci::Build)
rules
end
def filter_build_abilities(rules)
# If we can't read build we should also not have that
# ability when looking at this in context of commit_status
%w(read create update admin).each do |rule|
rules.delete(:"#{rule}_commit_status") unless rules.include?(:"#{rule}_build")
end
rules
end
def abilities def abilities
@abilities ||= begin @abilities ||= begin
abilities = Six.new abilities = Six.new
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
= ci_status_with_icon(build.status) = ci_status_with_icon(build.status)
%td.build-link %td.build-link
- if build.target_url - if can?(current_user, :read_build, project) && build.target_url
= link_to build.target_url do = link_to build.target_url do
%strong Build ##{build.id} %strong Build ##{build.id}
- else - else
...@@ -60,10 +60,10 @@ ...@@ -60,10 +60,10 @@
%td %td
.pull-right .pull-right
- if current_user && can?(current_user, :read_build_artifacts, project) && build.artifacts? - if can?(current_user, :read_build, project) && build.artifacts?
= link_to build.artifacts_download_url, title: 'Download artifacts' do = link_to build.artifacts_download_url, title: 'Download artifacts' do
%i.fa.fa-download %i.fa.fa-download
- if current_user && can?(current_user, :manage_builds, build.project) - if can?(current_user, :update_build, build.project)
- if build.active? - if build.active?
- if build.cancel_url - if build.cancel_url
= link_to build.cancel_url, method: :post, title: 'Cancel' do = link_to build.cancel_url, method: :post, title: 'Cancel' do
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
.project-issuable-filter .project-issuable-filter
.controls .controls
- if can?(current_user, :manage_builds, @project) - if can?(current_user, :update_build, @project)
.pull-left.hidden-xs .pull-left.hidden-xs
- if @all_builds.running_or_pending.any? - if @all_builds.running_or_pending.any?
= link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post = link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post
......
...@@ -89,8 +89,7 @@ ...@@ -89,8 +89,7 @@
Test coverage Test coverage
%h1 #{@build.coverage}% %h1 #{@build.coverage}%
- if current_user && can?(current_user, :read_build_artifacts, @project) && @build.artifacts? - if can?(current_user, :read_build, @project) && @build.artifacts?
.build-widget.artifacts .build-widget.artifacts
%h4.title Build artifacts %h4.title Build artifacts
.center .center
...@@ -102,7 +101,7 @@ ...@@ -102,7 +101,7 @@
.build-widget .build-widget
%h4.title %h4.title
Build ##{@build.id} Build ##{@build.id}
- if current_user && can?(current_user, :manage_builds, @project) - if can?(current_user, :update_build, @project)
.pull-right .pull-right
- if @build.cancel_url - if @build.cancel_url
= link_to "Cancel", @build.cancel_url, class: 'btn btn-sm btn-danger', method: :post = link_to "Cancel", @build.cancel_url, class: 'btn btn-sm btn-danger', method: :post
......
.gray-content-block.middle-block .gray-content-block.middle-block
.pull-right .pull-right
- if can?(current_user, :manage_builds, @ci_commit.project) - if can?(current_user, :update_build, @ci_commit.project)
- if @ci_commit.builds.latest.failed.any?(&:retryable?) - if @ci_commit.builds.latest.failed.any?(&:retryable?)
= link_to "Retry failed", retry_builds_namespace_project_commit_path(@ci_commit.project.namespace, @ci_commit.project, @ci_commit.sha), class: 'btn btn-grouped btn-primary', method: :post = link_to "Retry failed", retry_builds_namespace_project_commit_path(@ci_commit.project.namespace, @ci_commit.project, @ci_commit.sha), class: 'btn btn-grouped btn-primary', method: :post
......
%tr.commit_status %tr.commit_status
%td.status %td.status
- if commit_status.target_url - if can?(current_user, :read_commit_status, commit_status) && commit_status.target_url
= link_to commit_status.target_url, class: "ci-status ci-#{commit_status.status}" do = link_to commit_status.target_url, class: "ci-status ci-#{commit_status.status}" do
= ci_icon_for_status(commit_status.status) = ci_icon_for_status(commit_status.status)
= commit_status.status = commit_status.status
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
= ci_status_with_icon(commit_status.status) = ci_status_with_icon(commit_status.status)
%td.commit_status-link %td.commit_status-link
- if commit_status.target_url - if can?(current_user, :read_commit_status, commit_status) && commit_status.target_url
= link_to commit_status.target_url do = link_to commit_status.target_url do
%strong ##{commit_status.id} %strong ##{commit_status.id}
- else - else
...@@ -66,10 +66,10 @@ ...@@ -66,10 +66,10 @@
%td %td
.pull-right .pull-right
- if current_user && can?(current_user, :read_build_artifacts, commit_status.project) && commit_status.artifacts_download_url - if can?(current_user, :read_commit_status, commit_status) && commit_status.artifacts_download_url
= link_to commit_status.artifacts_download_url, title: 'Download artifacts' do = link_to commit_status.artifacts_download_url, title: 'Download artifacts' do
%i.fa.fa-download %i.fa.fa-download
- if current_user && can?(current_user, :manage_builds, commit_status.project) - if can?(current_user, :update_commit_status, commit_status)
- if commit_status.active? - if commit_status.active?
- if commit_status.cancel_url - if commit_status.cancel_url
= link_to commit_status.cancel_url, method: :post, title: 'Cancel' do = link_to commit_status.cancel_url, method: :post, title: 'Cancel' do
......
...@@ -130,6 +130,7 @@ ...@@ -130,6 +130,7 @@
%strong git fetch %strong git fetch
%br %br
%span.descr Faster %span.descr Faster
.form-group .form-group
= f.label :build_timeout_in_minutes, 'Timeout', class: 'control-label' = f.label :build_timeout_in_minutes, 'Timeout', class: 'control-label'
.col-sm-10 .col-sm-10
...@@ -158,6 +159,13 @@ ...@@ -158,6 +159,13 @@
phpunit --coverage-text --colors=never (PHP) - phpunit --coverage-text --colors=never (PHP) -
%code ^\s*Lines:\s*\d+.\d+\% %code ^\s*Lines:\s*\d+.\d+\%
.form-group
.col-sm-offset-2.col-sm-10
.checkbox
= f.label :public_builds do
= f.check_box :public_builds
%strong Public builds
.help-block Allow everyone to access builds for Public and Internal projects
%fieldset.features %fieldset.features
%legend %legend
......
class AddAllowGuestToAccessBuildsProject < ActiveRecord::Migration
def change
add_column :projects, :public_builds, :boolean, default: true, null: false
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20160128233227) do ActiveRecord::Schema.define(version: 20160202164642) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -679,6 +679,7 @@ ActiveRecord::Schema.define(version: 20160128233227) do ...@@ -679,6 +679,7 @@ ActiveRecord::Schema.define(version: 20160128233227) do
t.string "build_coverage_regex" t.string "build_coverage_regex"
t.boolean "build_allow_git_fetch", default: true, null: false t.boolean "build_allow_git_fetch", default: true, null: false
t.integer "build_timeout", default: 3600, null: false t.integer "build_timeout", default: 3600, null: false
t.boolean "public_builds", default: true, null: false
end end
add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree
......
...@@ -79,7 +79,9 @@ Parameters: ...@@ -79,7 +79,9 @@ Parameters:
"avatar_url": "http://example.com/uploads/project/avatar/4/uploads/avatar.png", "avatar_url": "http://example.com/uploads/project/avatar/4/uploads/avatar.png",
"shared_runners_enabled": true, "shared_runners_enabled": true,
"forks_count": 0, "forks_count": 0,
"star_count": 0 "star_count": 0,
"runners_token": "b8547b1dc37721d05889db52fa2f02",
"public_builds": true
}, },
{ {
"id": 6, "id": 6,
...@@ -136,7 +138,8 @@ Parameters: ...@@ -136,7 +138,8 @@ Parameters:
"shared_runners_enabled": true, "shared_runners_enabled": true,
"forks_count": 0, "forks_count": 0,
"star_count": 0, "star_count": 0,
"runners_token": "b8547b1dc37721d05889db52fa2f02" "runners_token": "b8547b1dc37721d05889db52fa2f02",
"public_builds": true
} }
] ]
``` ```
...@@ -420,6 +423,7 @@ Parameters: ...@@ -420,6 +423,7 @@ Parameters:
- `public` (optional) - if `true` same as setting visibility_level = 20 - `public` (optional) - if `true` same as setting visibility_level = 20
- `visibility_level` (optional) - `visibility_level` (optional)
- `import_url` (optional) - `import_url` (optional)
- `public_builds` (optional)
### Create project for user ### Create project for user
...@@ -442,6 +446,7 @@ Parameters: ...@@ -442,6 +446,7 @@ Parameters:
- `public` (optional) - if `true` same as setting visibility_level = 20 - `public` (optional) - if `true` same as setting visibility_level = 20
- `visibility_level` (optional) - `visibility_level` (optional)
- `import_url` (optional) - `import_url` (optional)
- `public_builds` (optional)
### Edit project ### Edit project
...@@ -465,6 +470,7 @@ Parameters: ...@@ -465,6 +470,7 @@ Parameters:
- `snippets_enabled` (optional) - `snippets_enabled` (optional)
- `public` (optional) - if `true` same as setting visibility_level = 20 - `public` (optional) - if `true` same as setting visibility_level = 20
- `visibility_level` (optional) - `visibility_level` (optional)
- `public_builds` (optional)
On success, method returns 200 with the updated project. If parameters are On success, method returns 200 with the updated project. If parameters are
invalid, 400 is returned. invalid, 400 is returned.
......
...@@ -18,6 +18,9 @@ documentation](../workflow/add-user/add-user.md). ...@@ -18,6 +18,9 @@ documentation](../workflow/add-user/add-user.md).
|---------------------------------------|---------|------------|-------------|----------|--------| |---------------------------------------|---------|------------|-------------|----------|--------|
| Create new issue | ✓ | ✓ | ✓ | ✓ | ✓ | | Create new issue | ✓ | ✓ | ✓ | ✓ | ✓ |
| Leave comments | ✓ | ✓ | ✓ | ✓ | ✓ | | Leave comments | ✓ | ✓ | ✓ | ✓ | ✓ |
| See a list of builds | ✓ [^1] | ✓ | ✓ | ✓ | ✓ |
| See a build log | ✓ [^1] | ✓ | ✓ | ✓ | ✓ |
| Download and browse build artifacts | ✓ [^1] | ✓ | ✓ | ✓ | ✓ |
| Pull project code | | ✓ | ✓ | ✓ | ✓ | | Pull project code | | ✓ | ✓ | ✓ | ✓ |
| Download project | | ✓ | ✓ | ✓ | ✓ | | Download project | | ✓ | ✓ | ✓ | ✓ |
| Create code snippets | | ✓ | ✓ | ✓ | ✓ | | Create code snippets | | ✓ | ✓ | ✓ | ✓ |
...@@ -31,6 +34,7 @@ documentation](../workflow/add-user/add-user.md). ...@@ -31,6 +34,7 @@ documentation](../workflow/add-user/add-user.md).
| Remove non-protected branches | | | ✓ | ✓ | ✓ | | Remove non-protected branches | | | ✓ | ✓ | ✓ |
| Add tags | | | ✓ | ✓ | ✓ | | Add tags | | | ✓ | ✓ | ✓ |
| Write a wiki | | | ✓ | ✓ | ✓ | | Write a wiki | | | ✓ | ✓ | ✓ |
| Cancel and retry builds | | | ✓ | ✓ | ✓ |
| Create new milestones | | | | ✓ | ✓ | | Create new milestones | | | | ✓ | ✓ |
| Add new team members | | | | ✓ | ✓ | | Add new team members | | | | ✓ | ✓ |
| Push to protected branches | | | | ✓ | ✓ | | Push to protected branches | | | | ✓ | ✓ |
...@@ -40,12 +44,17 @@ documentation](../workflow/add-user/add-user.md). ...@@ -40,12 +44,17 @@ documentation](../workflow/add-user/add-user.md).
| Edit project | | | | ✓ | ✓ | | Edit project | | | | ✓ | ✓ |
| Add deploy keys to project | | | | ✓ | ✓ | | Add deploy keys to project | | | | ✓ | ✓ |
| Configure project hooks | | | | ✓ | ✓ | | Configure project hooks | | | | ✓ | ✓ |
| Manage runners | | | | ✓ | ✓ |
| Manage build triggers | | | | ✓ | ✓ |
| Manage variables | | | | ✓ | ✓ |
| Switch visibility level | | | | | ✓ | | Switch visibility level | | | | | ✓ |
| Transfer project to another namespace | | | | | ✓ | | Transfer project to another namespace | | | | | ✓ |
| Remove project | | | | | ✓ | | Remove project | | | | | ✓ |
| Force push to protected branches | | | | | | | Force push to protected branches | | | | | |
| Remove protected branches | | | | | | | Remove protected branches | | | | | |
[^1]: If **Allow guest to access builds** is enabled in CI settings
## Group ## Group
In order for a group to appear as public and be browsable, it must contain at In order for a group to appear as public and be browsable, it must contain at
......
...@@ -5,6 +5,41 @@ Feature: Project Builds Permissions ...@@ -5,6 +5,41 @@ Feature: Project Builds Permissions
And project has CI enabled And project has CI enabled
And project has a recent build And project has a recent build
Scenario: I try to visit build details as guest
Given I am member of a project with a guest role
When I visit recent build details page
Then page status code should be 404
Scenario: I try to visit project builds page as guest
Given I am member of a project with a guest role
When I visit project builds page
Then page status code should be 404
Scenario: I try to visit build details of internal project without access to builds
Given The project is internal
And public access for builds is disabled
When I visit recent build details page
Then page status code should be 404
Scenario: I try to visit internal project builds page without access to builds
Given The project is internal
And public access for builds is disabled
When I visit project builds page
Then page status code should be 404
Scenario: I try to visit build details of internal project with access to builds
Given The project is internal
And public access for builds is enabled
When I visit recent build details page
Then I see details of a build
And I see build trace
Scenario: I try to visit internal project builds page with access to builds
Given The project is internal
And public access for builds is enabled
When I visit project builds page
Then I see the build
Scenario: I try to download build artifacts as guest Scenario: I try to download build artifacts as guest
Given I am member of a project with a guest role Given I am member of a project with a guest role
And recent build has artifacts available And recent build has artifacts available
......
...@@ -34,4 +34,21 @@ module SharedBuilds ...@@ -34,4 +34,21 @@ module SharedBuilds
step 'I access artifacts download page' do step 'I access artifacts download page' do
visit download_namespace_project_build_artifacts_path(@project.namespace, @project, @build) visit download_namespace_project_build_artifacts_path(@project.namespace, @project, @build)
end end
step 'I see details of a 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 see the build' do
page.within('.commit_status') do
expect(page).to have_content "##{@build.id}"
expect(page).to have_content @build.sha[0..7]
expect(page).to have_content @build.ref
expect(page).to have_content @build.name
end
end
end end
...@@ -240,6 +240,18 @@ module SharedProject ...@@ -240,6 +240,18 @@ module SharedProject
end end
end end
step 'The project is internal' do
@project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
end
step 'public access for builds is enabled' do
@project.update(public_builds: true)
end
step 'public access for builds is disabled' do
@project.update(public_builds: false)
end
def user_owns_project(user_name:, project_name:, visibility: :private) def user_owns_project(user_name:, project_name:, visibility: :private)
user = user_exists(user_name, username: user_name.gsub(/\s/, '').underscore) user = user_exists(user_name, username: user_name.gsub(/\s/, '').underscore)
project = Project.find_by(name: project_name) project = Project.find_by(name: project_name)
......
...@@ -13,11 +13,12 @@ module API ...@@ -13,11 +13,12 @@ module API
# Example Request: # Example Request:
# GET /projects/:id/builds # GET /projects/:id/builds
get ':id/builds' do get ':id/builds' do
builds = user_project.builds.order('id DESC') builds = user_project.builds.order('id DESC')
builds = filter_builds(builds, params[:scope]) builds = filter_builds(builds, params[:scope])
present paginate(builds), with: Entities::Build, present paginate(builds), with: Entities::Build,
user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) user_can_download_artifacts: can?(current_user, :read_build, user_project)
end end
# Get builds for a specific commit of a project # Get builds for a specific commit of a project
...@@ -30,6 +31,8 @@ module API ...@@ -30,6 +31,8 @@ module API
# Example Request: # Example Request:
# GET /projects/:id/repository/commits/:sha/builds # GET /projects/:id/repository/commits/:sha/builds
get ':id/repository/commits/:sha/builds' do get ':id/repository/commits/:sha/builds' do
authorize_read_builds!
commit = user_project.ci_commits.find_by_sha(params[:sha]) commit = user_project.ci_commits.find_by_sha(params[:sha])
return not_found! unless commit return not_found! unless commit
...@@ -37,7 +40,7 @@ module API ...@@ -37,7 +40,7 @@ module API
builds = filter_builds(builds, params[:scope]) builds = filter_builds(builds, params[:scope])
present paginate(builds), with: Entities::Build, present paginate(builds), with: Entities::Build,
user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) user_can_download_artifacts: can?(current_user, :read_build, user_project)
end end
# Get a specific build of a project # Get a specific build of a project
...@@ -48,11 +51,13 @@ module API ...@@ -48,11 +51,13 @@ module API
# Example Request: # Example Request:
# GET /projects/:id/builds/:build_id # GET /projects/:id/builds/:build_id
get ':id/builds/:build_id' do get ':id/builds/:build_id' do
authorize_read_builds!
build = get_build(params[:build_id]) build = get_build(params[:build_id])
return not_found!(build) unless build return not_found!(build) unless build
present build, with: Entities::Build, present build, with: Entities::Build,
user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) user_can_download_artifacts: can?(current_user, :read_build, user_project)
end end
# Get a trace of a specific build of a project # Get a trace of a specific build of a project
...@@ -67,6 +72,8 @@ module API ...@@ -67,6 +72,8 @@ module API
# is saved in the DB instead of file). But before that, we need to consider how to replace the value of # is saved in the DB instead of file). But before that, we need to consider how to replace the value of
# `runners_token` with some mask (like `xxxxxx`) when sending trace file directly by workhorse. # `runners_token` with some mask (like `xxxxxx`) when sending trace file directly by workhorse.
get ':id/builds/:build_id/trace' do get ':id/builds/:build_id/trace' do
authorize_read_builds!
build = get_build(params[:build_id]) build = get_build(params[:build_id])
return not_found!(build) unless build return not_found!(build) unless build
...@@ -86,7 +93,7 @@ module API ...@@ -86,7 +93,7 @@ module API
# example request: # example request:
# post /projects/:id/build/:build_id/cancel # post /projects/:id/build/:build_id/cancel
post ':id/builds/:build_id/cancel' do post ':id/builds/:build_id/cancel' do
authorize_manage_builds! authorize_update_builds!
build = get_build(params[:build_id]) build = get_build(params[:build_id])
return not_found!(build) unless build return not_found!(build) unless build
...@@ -94,7 +101,7 @@ module API ...@@ -94,7 +101,7 @@ module API
build.cancel build.cancel
present build, with: Entities::Build, present build, with: Entities::Build,
user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) user_can_download_artifacts: can?(current_user, :read_build, user_project)
end end
# Retry a specific build of a project # Retry a specific build of a project
...@@ -105,7 +112,7 @@ module API ...@@ -105,7 +112,7 @@ module API
# example request: # example request:
# post /projects/:id/build/:build_id/retry # post /projects/:id/build/:build_id/retry
post ':id/builds/:build_id/retry' do post ':id/builds/:build_id/retry' do
authorize_manage_builds! authorize_update_builds!
build = get_build(params[:build_id]) build = get_build(params[:build_id])
return forbidden!('Build is not retryable') unless build && build.retryable? return forbidden!('Build is not retryable') unless build && build.retryable?
...@@ -113,7 +120,7 @@ module API ...@@ -113,7 +120,7 @@ module API
build = Ci::Build.retry(build) build = Ci::Build.retry(build)
present build, with: Entities::Build, present build, with: Entities::Build,
user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) user_can_download_artifacts: can?(current_user, :read_build, user_project)
end end
end end
...@@ -141,8 +148,12 @@ module API ...@@ -141,8 +148,12 @@ module API
builds.where(status: available_statuses && scope) builds.where(status: available_statuses && scope)
end end
def authorize_manage_builds! def authorize_read_builds!
authorize! :manage_builds, user_project authorize! :read_build, user_project
end
def authorize_update_builds!
authorize! :update_build, user_project
end end
end end
end end
......
...@@ -18,7 +18,7 @@ module API ...@@ -18,7 +18,7 @@ module API
# Examples: # Examples:
# GET /projects/:id/repository/commits/:sha/statuses # GET /projects/:id/repository/commits/:sha/statuses
get ':id/repository/commits/:sha/statuses' do get ':id/repository/commits/:sha/statuses' do
authorize! :read_commit_statuses, user_project authorize! :read_commit_status, user_project
sha = params[:sha] sha = params[:sha]
ci_commit = user_project.ci_commit(sha) ci_commit = user_project.ci_commit(sha)
not_found! 'Commit' unless ci_commit not_found! 'Commit' unless ci_commit
......
...@@ -72,6 +72,7 @@ module API ...@@ -72,6 +72,7 @@ module API
expose :star_count, :forks_count expose :star_count, :forks_count
expose :open_issues_count, if: lambda { |project, options| project.issues_enabled? && project.default_issues_tracker? } expose :open_issues_count, if: lambda { |project, options| project.issues_enabled? && project.default_issues_tracker? }
expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] } expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] }
expose :public_builds
end end
class ProjectMember < UserBasic class ProjectMember < UserBasic
...@@ -383,7 +384,7 @@ module API ...@@ -383,7 +384,7 @@ module API
# for downloading of artifacts (see: https://gitlab.com/gitlab-org/gitlab-ce/issues/4255) # for downloading of artifacts (see: https://gitlab.com/gitlab-org/gitlab-ce/issues/4255)
expose :download_url do |repo_obj, options| expose :download_url do |repo_obj, options|
if options[:user_can_download_artifacts] if options[:user_can_download_artifacts]
repo_obj.download_url repo_obj.artifacts_download_url
end end
end end
expose :commit, with: RepoCommit do |repo_obj, _options| expose :commit, with: RepoCommit do |repo_obj, _options|
......
...@@ -99,6 +99,7 @@ module API ...@@ -99,6 +99,7 @@ module API
# public (optional) - if true same as setting visibility_level = 20 # public (optional) - if true same as setting visibility_level = 20
# visibility_level (optional) - 0 by default # visibility_level (optional) - 0 by default
# import_url (optional) # import_url (optional)
# public_builds (optional)
# Example Request # Example Request
# POST /projects # POST /projects
post do post do
...@@ -115,7 +116,8 @@ module API ...@@ -115,7 +116,8 @@ module API
:namespace_id, :namespace_id,
:public, :public,
:visibility_level, :visibility_level,
:import_url] :import_url,
:public_builds]
attrs = map_public_to_visibility_level(attrs) attrs = map_public_to_visibility_level(attrs)
@project = ::Projects::CreateService.new(current_user, attrs).execute @project = ::Projects::CreateService.new(current_user, attrs).execute
if @project.saved? if @project.saved?
...@@ -145,6 +147,7 @@ module API ...@@ -145,6 +147,7 @@ module API
# public (optional) - if true same as setting visibility_level = 20 # public (optional) - if true same as setting visibility_level = 20
# visibility_level (optional) # visibility_level (optional)
# import_url (optional) # import_url (optional)
# public_builds (optional)
# Example Request # Example Request
# POST /projects/user/:user_id # POST /projects/user/:user_id
post "user/:user_id" do post "user/:user_id" do
...@@ -161,7 +164,8 @@ module API ...@@ -161,7 +164,8 @@ module API
:shared_runners_enabled, :shared_runners_enabled,
:public, :public,
:visibility_level, :visibility_level,
:import_url] :import_url,
:public_builds]
attrs = map_public_to_visibility_level(attrs) attrs = map_public_to_visibility_level(attrs)
@project = ::Projects::CreateService.new(user, attrs).execute @project = ::Projects::CreateService.new(user, attrs).execute
if @project.saved? if @project.saved?
...@@ -205,6 +209,7 @@ module API ...@@ -205,6 +209,7 @@ module API
# shared_runners_enabled (optional) # shared_runners_enabled (optional)
# public (optional) - if true same as setting visibility_level = 20 # public (optional) - if true same as setting visibility_level = 20
# visibility_level (optional) - visibility level of a project # visibility_level (optional) - visibility level of a project
# public_builds (optional)
# Example Request # Example Request
# PUT /projects/:id # PUT /projects/:id
put ':id' do put ':id' do
...@@ -219,7 +224,8 @@ module API ...@@ -219,7 +224,8 @@ module API
:snippets_enabled, :snippets_enabled,
:shared_runners_enabled, :shared_runners_enabled,
:public, :public,
:visibility_level] :visibility_level,
:public_builds]
attrs = map_public_to_visibility_level(attrs) attrs = map_public_to_visibility_level(attrs)
authorize_admin_project authorize_admin_project
authorize! :rename_project, user_project if attrs[:name].present? authorize! :rename_project, user_project if attrs[:name].present?
......
...@@ -54,7 +54,7 @@ module API ...@@ -54,7 +54,7 @@ module API
# GET /projects/:id/triggers # GET /projects/:id/triggers
get ':id/triggers' do get ':id/triggers' do
authenticate! authenticate!
authorize_admin_project authorize! :admin_build, user_project
triggers = user_project.triggers.includes(:trigger_requests) triggers = user_project.triggers.includes(:trigger_requests)
triggers = paginate(triggers) triggers = paginate(triggers)
...@@ -71,7 +71,7 @@ module API ...@@ -71,7 +71,7 @@ module API
# GET /projects/:id/triggers/:token # GET /projects/:id/triggers/:token
get ':id/triggers/:token' do get ':id/triggers/:token' do
authenticate! authenticate!
authorize_admin_project authorize! :admin_build, user_project
trigger = user_project.triggers.find_by(token: params[:token].to_s) trigger = user_project.triggers.find_by(token: params[:token].to_s)
return not_found!('Trigger') unless trigger return not_found!('Trigger') unless trigger
...@@ -87,7 +87,7 @@ module API ...@@ -87,7 +87,7 @@ module API
# POST /projects/:id/triggers # POST /projects/:id/triggers
post ':id/triggers' do post ':id/triggers' do
authenticate! authenticate!
authorize_admin_project authorize! :admin_build, user_project
trigger = user_project.triggers.create trigger = user_project.triggers.create
...@@ -103,7 +103,7 @@ module API ...@@ -103,7 +103,7 @@ module API
# DELETE /projects/:id/triggers/:token # DELETE /projects/:id/triggers/:token
delete ':id/triggers/:token' do delete ':id/triggers/:token' do
authenticate! authenticate!
authorize_admin_project authorize! :admin_build, user_project
trigger = user_project.triggers.find_by(token: params[:token].to_s) trigger = user_project.triggers.find_by(token: params[:token].to_s)
return not_found!('Trigger') unless trigger return not_found!('Trigger') unless trigger
......
...@@ -2,7 +2,7 @@ module API ...@@ -2,7 +2,7 @@ module API
# Projects variables API # Projects variables API
class Variables < Grape::API class Variables < Grape::API
before { authenticate! } before { authenticate! }
before { authorize_admin_project } before { authorize! :admin_build, user_project }
resource :projects do resource :projects do
# Get project variables # Get project variables
......
...@@ -8,7 +8,7 @@ describe "Builds" do ...@@ -8,7 +8,7 @@ describe "Builds" do
@commit = FactoryGirl.create :ci_commit @commit = FactoryGirl.create :ci_commit
@build = FactoryGirl.create :ci_build, commit: @commit @build = FactoryGirl.create :ci_build, commit: @commit
@project = @commit.project @project = @commit.project
@project.team << [@user, :master] @project.team << [@user, :developer]
end end
describe "GET /:project/builds" do describe "GET /:project/builds" do
......
...@@ -8,7 +8,6 @@ describe 'Commits' do ...@@ -8,7 +8,6 @@ describe 'Commits' do
describe 'CI' do describe 'CI' do
before do before do
login_as :user login_as :user
project.team << [@user, :master]
stub_ci_commit_to_return_yaml_file stub_ci_commit_to_return_yaml_file
end end
...@@ -19,6 +18,10 @@ describe 'Commits' do ...@@ -19,6 +18,10 @@ describe 'Commits' do
context 'commit status is Generic Commit Status' do context 'commit status is Generic Commit Status' do
let!(:status) { FactoryGirl.create :generic_commit_status, commit: commit } let!(:status) { FactoryGirl.create :generic_commit_status, commit: commit }
before do
project.team << [@user, :reporter]
end
describe 'Commit builds' do describe 'Commit builds' do
before do before do
visit ci_status_path(commit) visit ci_status_path(commit)
...@@ -37,6 +40,12 @@ describe 'Commits' do ...@@ -37,6 +40,12 @@ describe 'Commits' do
context 'commit status is Ci Build' do context 'commit status is Ci Build' do
let!(:build) { FactoryGirl.create :ci_build, commit: commit } let!(:build) { FactoryGirl.create :ci_build, commit: commit }
let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
context 'when logged as developer' do
before do
project.team << [@user, :developer]
end
describe 'Project commits' do describe 'Project commits' do
before do before do
...@@ -61,8 +70,6 @@ describe 'Commits' do ...@@ -61,8 +70,6 @@ describe 'Commits' do
end end
context 'Download artifacts' do context 'Download artifacts' do
let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
before do before do
build.update_attributes(artifacts_file: artifacts_file) build.update_attributes(artifacts_file: artifacts_file)
end end
...@@ -117,5 +124,42 @@ describe 'Commits' do ...@@ -117,5 +124,42 @@ describe 'Commits' do
end end
end end
end end
context "when logged as reporter" do
before do
project.team << [@user, :reporter]
build.update_attributes(artifacts_file: artifacts_file)
visit ci_status_path(commit)
end
it do
expect(page).to have_content commit.sha[0..7]
expect(page).to have_content commit.git_commit_message
expect(page).to have_content commit.git_author_name
expect(page).to have_link('Download artifacts')
expect(page).to_not have_link('Cancel running')
expect(page).to_not have_link('Retry failed')
end
end
context 'when accessing internal project with disallowed access' do
before do
project.update(
visibility_level: Gitlab::VisibilityLevel::INTERNAL,
public_builds: false)
build.update_attributes(artifacts_file: artifacts_file)
visit ci_status_path(commit)
end
it do
expect(page).to have_content commit.sha[0..7]
expect(page).to have_content commit.git_commit_message
expect(page).to have_content commit.git_author_name
expect(page).to_not have_link('Download artifacts')
expect(page).to_not have_link('Cancel running')
expect(page).to_not have_link('Retry failed')
end
end
end
end end
end end
...@@ -96,6 +96,60 @@ describe "Public Project Access", feature: true do ...@@ -96,6 +96,60 @@ describe "Public Project Access", feature: true do
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for :visitor }
end end
describe "GET /:project_path/builds" do
subject { namespace_project_builds_path(project.namespace, project) }
context "when allowed for public" do
before { project.update(public_builds: true) }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :visitor }
end
context "when disallowed for public" do
before { project.update(public_builds: false) }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
end
end
describe "GET /:project_path/builds/:id" do
let(:commit) { create(:ci_commit, project: project) }
let(:build) { create(:ci_build, commit: commit) }
subject { namespace_project_build_path(project.namespace, project, build.id) }
context "when allowed for public" do
before { project.update(public_builds: true) }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :visitor }
end
context "when disallowed for public" do
before { project.update(public_builds: false) }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
end
end
describe "GET /:project_path/blob" do describe "GET /:project_path/blob" do
before do before do
commit = project.repository.commit commit = project.repository.commit
......
...@@ -113,7 +113,7 @@ describe API::API, api: true do ...@@ -113,7 +113,7 @@ describe API::API, api: true do
describe 'POST /projects/:id/builds/:build_id/cancel' do describe 'POST /projects/:id/builds/:build_id/cancel' do
context 'authorized user' do context 'authorized user' do
context 'user with :manage_builds persmission' do context 'user with :update_build persmission' do
it 'should cancel running or pending build' do it 'should cancel running or pending build' do
post api("/projects/#{project.id}/builds/#{build.id}/cancel", user) post api("/projects/#{project.id}/builds/#{build.id}/cancel", user)
...@@ -122,7 +122,7 @@ describe API::API, api: true do ...@@ -122,7 +122,7 @@ describe API::API, api: true do
end end
end end
context 'user without :manage_builds permission' do context 'user without :update_build permission' do
it 'should not cancel build' do it 'should not cancel build' do
post api("/projects/#{project.id}/builds/#{build.id}/cancel", user2) post api("/projects/#{project.id}/builds/#{build.id}/cancel", user2)
...@@ -142,7 +142,7 @@ describe API::API, api: true do ...@@ -142,7 +142,7 @@ describe API::API, api: true do
describe 'POST /projects/:id/builds/:build_id/retry' do describe 'POST /projects/:id/builds/:build_id/retry' do
context 'authorized user' do context 'authorized user' do
context 'user with :manage_builds persmission' do context 'user with :update_build persmission' do
it 'should retry non-running build' do it 'should retry non-running build' do
post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user) post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user)
...@@ -152,7 +152,7 @@ describe API::API, api: true do ...@@ -152,7 +152,7 @@ describe API::API, api: true do
end end
end end
context 'user without :manage_builds permission' do context 'user without :update_build permission' do
it 'should not retry build' do it 'should not retry build' do
post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user2) post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user2)
......
...@@ -2,18 +2,17 @@ require 'spec_helper' ...@@ -2,18 +2,17 @@ require 'spec_helper'
describe API::CommitStatus, api: true do describe API::CommitStatus, api: true do
include ApiHelpers include ApiHelpers
let(:user) { create(:user) } let!(:project) { create(:project) }
let(:user2) { create(:user) }
let!(:project) { create(:project, creator_id: user.id) }
let!(:reporter) { create(:project_member, user: user, project: project, access_level: ProjectMember::REPORTER) }
let!(:guest) { create(:project_member, user: user2, project: project, access_level: ProjectMember::GUEST) }
let(:commit) { project.repository.commit } let(:commit) { project.repository.commit }
let!(:ci_commit) { project.ensure_ci_commit(commit.id) } let!(:ci_commit) { project.ensure_ci_commit(commit.id) }
let(:commit_status) { create(:commit_status, commit: ci_commit) } let(:commit_status) { create(:commit_status, commit: ci_commit) }
let(:guest) { create_user(ProjectMember::GUEST) }
let(:reporter) { create_user(ProjectMember::REPORTER) }
let(:developer) { create_user(ProjectMember::DEVELOPER) }
describe "GET /projects/:id/repository/commits/:sha/statuses" do describe "GET /projects/:id/repository/commits/:sha/statuses" do
it_behaves_like 'a paginated resources' do it_behaves_like 'a paginated resources' do
let(:request) { get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", user) } let(:request) { get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", reporter) }
end end
context "reporter user" do context "reporter user" do
...@@ -29,7 +28,7 @@ describe API::CommitStatus, api: true do ...@@ -29,7 +28,7 @@ describe API::CommitStatus, api: true do
end end
it "should return latest commit statuses" do it "should return latest commit statuses" do
get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", user) get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", reporter)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response).to be_an Array expect(json_response).to be_an Array
...@@ -39,7 +38,7 @@ describe API::CommitStatus, api: true do ...@@ -39,7 +38,7 @@ describe API::CommitStatus, api: true do
end end
it "should return all commit statuses" do it "should return all commit statuses" do
get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?all=1", user) get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?all=1", reporter)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response).to be_an Array expect(json_response).to be_an Array
...@@ -47,7 +46,7 @@ describe API::CommitStatus, api: true do ...@@ -47,7 +46,7 @@ describe API::CommitStatus, api: true do
end end
it "should return latest commit statuses for specific ref" do it "should return latest commit statuses for specific ref" do
get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?ref=develop", user) get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?ref=develop", reporter)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response).to be_an Array expect(json_response).to be_an Array
...@@ -55,7 +54,7 @@ describe API::CommitStatus, api: true do ...@@ -55,7 +54,7 @@ describe API::CommitStatus, api: true do
end end
it "should return latest commit statuses for specific name" do it "should return latest commit statuses for specific name" do
get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?name=coverage", user) get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses?name=coverage", reporter)
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response).to be_an Array expect(json_response).to be_an Array
...@@ -65,7 +64,7 @@ describe API::CommitStatus, api: true do ...@@ -65,7 +64,7 @@ describe API::CommitStatus, api: true do
context "guest user" do context "guest user" do
it "should not return project commits" do it "should not return project commits" do
get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", user2) get api("/projects/#{project.id}/repository/commits/#{commit.id}/statuses", guest)
expect(response.status).to eq(403) expect(response.status).to eq(403)
end end
end end
...@@ -81,10 +80,10 @@ describe API::CommitStatus, api: true do ...@@ -81,10 +80,10 @@ describe API::CommitStatus, api: true do
describe 'POST /projects/:id/statuses/:sha' do describe 'POST /projects/:id/statuses/:sha' do
let(:post_url) { "/projects/#{project.id}/statuses/#{commit.id}" } let(:post_url) { "/projects/#{project.id}/statuses/#{commit.id}" }
context 'reporter user' do context 'developer user' do
context 'should create commit status' do context 'should create commit status' do
it 'with only required parameters' do it 'with only required parameters' do
post api(post_url, user), state: 'success' post api(post_url, developer), state: 'success'
expect(response.status).to eq(201) expect(response.status).to eq(201)
expect(json_response['sha']).to eq(commit.id) expect(json_response['sha']).to eq(commit.id)
expect(json_response['status']).to eq('success') expect(json_response['status']).to eq('success')
...@@ -95,7 +94,7 @@ describe API::CommitStatus, api: true do ...@@ -95,7 +94,7 @@ describe API::CommitStatus, api: true do
end end
it 'with all optional parameters' do it 'with all optional parameters' do
post api(post_url, user), state: 'success', context: 'coverage', ref: 'develop', target_url: 'url', description: 'test' post api(post_url, developer), state: 'success', context: 'coverage', ref: 'develop', target_url: 'url', description: 'test'
expect(response.status).to eq(201) expect(response.status).to eq(201)
expect(json_response['sha']).to eq(commit.id) expect(json_response['sha']).to eq(commit.id)
expect(json_response['status']).to eq('success') expect(json_response['status']).to eq('success')
...@@ -108,25 +107,32 @@ describe API::CommitStatus, api: true do ...@@ -108,25 +107,32 @@ describe API::CommitStatus, api: true do
context 'should not create commit status' do context 'should not create commit status' do
it 'with invalid state' do it 'with invalid state' do
post api(post_url, user), state: 'invalid' post api(post_url, developer), state: 'invalid'
expect(response.status).to eq(400) expect(response.status).to eq(400)
end end
it 'without state' do it 'without state' do
post api(post_url, user) post api(post_url, developer)
expect(response.status).to eq(400) expect(response.status).to eq(400)
end end
it 'invalid commit' do it 'invalid commit' do
post api("/projects/#{project.id}/statuses/invalid_sha", user), state: 'running' post api("/projects/#{project.id}/statuses/invalid_sha", developer), state: 'running'
expect(response.status).to eq(404) expect(response.status).to eq(404)
end end
end end
end end
context 'reporter user' do
it 'should not create commit status' do
post api(post_url, reporter)
expect(response.status).to eq(403)
end
end
context 'guest user' do context 'guest user' do
it 'should not create commit status' do it 'should not create commit status' do
post api(post_url, user2) post api(post_url, guest)
expect(response.status).to eq(403) expect(response.status).to eq(403)
end end
end end
...@@ -138,4 +144,10 @@ describe API::CommitStatus, api: true do ...@@ -138,4 +144,10 @@ describe API::CommitStatus, api: true do
end end
end end
end end
def create_user(access_level)
user = create(:user)
create(:project_member, user: user, project: project, access_level: access_level)
user
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment