Merge remote-tracking branch 'dev/master' into 'master'

18 files changed, 93 insertions, 114 deletions

diff --git a/app/controllers/ci/application_controller.rb b/app/controllers/ci/application_controller.rb
index c420b59c3a2..5bb7d499cdc 100644
--- a/app/controllers/ci/application_controller.rb
+++ b/app/controllers/ci/application_controller.rb
@@ -3,52 +3,5 @@ module Ci
     def self.railtie_helpers_paths
       "app/helpers/ci"
     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
diff --git a/app/controllers/ci/projects_controller.rb b/app/controllers/ci/projects_controller.rb
index 3004c2d27f0..711c2847d5e 100644
--- a/app/controllers/ci/projects_controller.rb
+++ b/app/controllers/ci/projects_controller.rb
@@ -1,8 +1,7 @@
 module Ci
   class ProjectsController < Ci::ApplicationController
-    before_action :project, except: [:index]
-    before_action :authenticate_user!, except: [:index, :build, :badge]
-    before_action :authorize_access_project!, except: [:index, :badge]
+    before_action :project
+    before_action :authorize_read_project!, except: [:badge]
     before_action :no_cache, only: [:badge]
     protect_from_forgery
 
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index f159a6d6dc6..cfea1266516 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -1,6 +1,6 @@
 class Projects::ArtifactsController < Projects::ApplicationController
   layout 'project'
-  before_action :authorize_read_build_artifacts!
+  before_action :authorize_read_build!
 
   def download
     unless artifacts_file.file_storage?
@@ -43,14 +43,4 @@ class Projects::ArtifactsController < Projects::ApplicationController
   def artifacts_file
     @artifacts_file ||= build.artifacts_file
   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
diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index 92d9699fe84..9e89296e71d 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -1,7 +1,8 @@
 class Projects::BuildsController < Projects::ApplicationController
   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"
 
@@ -69,10 +70,4 @@ class Projects::BuildsController < Projects::ApplicationController
   def build_path(build)
     namespace_project_build_path(build.project.namespace, build.project, build)
   end
-
-  def authorize_manage_builds!
-    unless can?(current_user, :manage_builds, project)
-      return render_404
-    end
-  end
 end
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index 5a084e123a1..21f4d9f44ec 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -4,10 +4,10 @@
 class Projects::CommitController < Projects::ApplicationController
   # Authorize
   before_action :require_non_empty_project
-  before_action :authorize_download_code!, except: [:cancel_builds]
-  before_action :authorize_manage_builds!, only: [:cancel_builds]
+  before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds]
+  before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds]
+  before_action :authorize_read_commit_status!, only: [:builds]
   before_action :commit
-  before_action :authorize_manage_builds!, only: [:cancel_builds, :retry_builds]
   before_action :define_show_vars, only: [:show, :builds]
 
   def show
@@ -79,10 +79,4 @@ class Projects::CommitController < Projects::ApplicationController
 
     @statuses = ci_commit.statuses if ci_commit
   end
-
-  def authorize_manage_builds!
-    unless can?(current_user, :manage_builds, project)
-      return render_404
-    end
-  end
 end
diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb
index e2785caa2fb..bedeb4a295c 100644
--- a/app/controllers/projects/runner_projects_controller.rb
+++ b/app/controllers/projects/runner_projects_controller.rb
@@ -1,5 +1,5 @@
 class Projects::RunnerProjectsController < Projects::ApplicationController
-  before_action :authorize_admin_project!
+  before_action :authorize_admin_build!
 
   layout 'project_settings'
 
diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb
index 4993b2648a5..0dd2d6a99be 100644
--- a/app/controllers/projects/runners_controller.rb
+++ b/app/controllers/projects/runners_controller.rb
@@ -1,6 +1,6 @@
 class Projects::RunnersController < Projects::ApplicationController
+  before_action :authorize_admin_build!
   before_action :set_runner, only: [:edit, :update, :destroy, :pause, :resume, :show]
-  before_action :authorize_admin_project!
 
   layout 'project_settings'
 
diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb
index 30adfad1daa..92359745cec 100644
--- a/app/controllers/projects/triggers_controller.rb
+++ b/app/controllers/projects/triggers_controller.rb
@@ -1,5 +1,5 @@
 class Projects::TriggersController < Projects::ApplicationController
-  before_action :authorize_admin_project!
+  before_action :authorize_admin_build!
 
   layout 'project_settings'
 
diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb
index 10efafea9db..00234654578 100644
--- a/app/controllers/projects/variables_controller.rb
+++ b/app/controllers/projects/variables_controller.rb
@@ -1,5 +1,5 @@
 class Projects::VariablesController < Projects::ApplicationController
-  before_action :authorize_admin_project!
+  before_action :authorize_admin_build!
 
   layout 'project_settings'
 
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 4df5095bd94..14ca7426c2f 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -227,6 +227,7 @@ class ProjectsController < ApplicationController
       :issues_enabled, :merge_requests_enabled, :snippets_enabled, :issues_tracker_id, :default_branch,
       :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,
+      :public_builds,
     )
   end
 
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index dd49283089d..cc1c61e9ec0 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -141,7 +141,7 @@ module ProjectsHelper
       nav_tabs << :merge_requests
     end
 
-    if project.builds_enabled? && can?(current_user, :read_build, project)
+    if can?(current_user, :read_build, project)
       nav_tabs << :builds
     end
 
diff --git a/app/models/ability.rb b/app/models/ability.rb
index ab59a3506a2..a866eadeebb 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -5,17 +5,18 @@ class Ability
       return [] unless user.is_a?(User)
       return [] if user.blocked?
 
-      case subject.class.name
-      when "Project" then project_abilities(user, subject)
-      when "Issue" then issue_abilities(user, subject)
-      when "Note" then note_abilities(user, subject)
-      when "ProjectSnippet" then project_snippet_abilities(user, subject)
-      when "PersonalSnippet" then personal_snippet_abilities(user, subject)
-      when "MergeRequest" then merge_request_abilities(user, subject)
-      when "Group" then group_abilities(user, subject)
-      when "Namespace" then namespace_abilities(user, subject)
-      when "GroupMember" then group_member_abilities(user, subject)
-      when "ProjectMember" then project_member_abilities(user, subject)
+      case subject
+      when CommitStatus then commit_status_abilities(user, subject)
+      when Project then project_abilities(user, subject)
+      when Issue then issue_abilities(user, subject)
+      when Note then note_abilities(user, subject)
+      when ProjectSnippet then project_snippet_abilities(user, subject)
+      when PersonalSnippet then personal_snippet_abilities(user, subject)
+      when MergeRequest then merge_request_abilities(user, subject)
+      when Group then group_abilities(user, subject)
+      when Namespace then namespace_abilities(user, subject)
+      when GroupMember then group_member_abilities(user, subject)
+      when ProjectMember then project_member_abilities(user, subject)
       else []
       end.concat(global_abilities(user))
     end
@@ -25,6 +26,8 @@ class Ability
       case true
       when subject.is_a?(PersonalSnippet)
         anonymous_personal_snippet_abilities(subject)
+      when subject.is_a?(CommitStatus)
+        anonymous_commit_status_abilities(subject)
       when subject.is_a?(Project) || subject.respond_to?(:project)
         anonymous_project_abilities(subject)
       when subject.is_a?(Group) || subject.respond_to?(:group)
@@ -52,16 +55,26 @@ class Ability
           :read_project_member,
           :read_merge_request,
           :read_note,
-          :read_build,
+          :read_commit_status,
           :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)
       else
         []
       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)
       group = if subject.is_a?(Group)
                 subject
@@ -113,6 +126,9 @@ class Ability
 
         if project.public? || project.internal?
           rules.push(*public_project_rules)
+
+          # Allow to read builds for internal projects
+          rules << :read_build if project.public_builds?
         end
 
         if project.owner == user || user.admin?
@@ -134,7 +150,8 @@ class Ability
     def public_project_rules
       @public_project_rules ||= project_guest_rules + [
         :download_code,
-        :fork_project
+        :fork_project,
+        :read_commit_status,
       ]
     end
 
@@ -149,7 +166,6 @@ class Ability
         :read_project_member,
         :read_merge_request,
         :read_note,
-        :read_build,
         :create_project,
         :create_issue,
         :create_note
@@ -158,24 +174,26 @@ class Ability
 
     def project_report_rules
       @project_report_rules ||= project_guest_rules + [
-        :create_commit_status,
-        :read_commit_statuses,
-        :read_build_artifacts,
         :download_code,
         :fork_project,
         :create_project_snippet,
         :update_issue,
         :admin_issue,
-        :admin_label
+        :admin_label,
+        :read_commit_status,
+        :read_build,
       ]
     end
 
     def project_dev_rules
       @project_dev_rules ||= project_report_rules + [
         :admin_merge_request,
+        :create_commit_status,
+        :update_commit_status,
+        :create_build,
+        :update_build,
         :create_merge_request,
         :create_wiki,
-        :manage_builds,
         :push_code
       ]
     end
@@ -201,7 +219,9 @@ class Ability
         :admin_merge_request,
         :admin_note,
         :admin_wiki,
-        :admin_project
+        :admin_project,
+        :admin_commit_status,
+        :admin_build
       ]
     end
 
@@ -240,6 +260,10 @@ class Ability
         rules += named_abilities('wiki')
       end
 
+      unless project.builds_enabled
+        rules += named_abilities('build')
+      end
+
       rules
     end
 
@@ -376,6 +400,22 @@ class Ability
       rules
     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
       @abilities ||= begin
         abilities = Six.new
diff --git a/app/views/admin/builds/_build.html.haml b/app/views/admin/builds/_build.html.haml
index c395bd908c3..34d955568f2 100644
--- a/app/views/admin/builds/_build.html.haml
+++ b/app/views/admin/builds/_build.html.haml
@@ -4,7 +4,7 @@
     = ci_status_with_icon(build.status)
 
   %td.build-link
-    - if build.target_url
+    - if can?(current_user, :read_build, project) && build.target_url
       = link_to build.target_url do
         %strong Build ##{build.id}
     - else
@@ -60,10 +60,10 @@
 
   %td
     .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
           %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.cancel_url
             = link_to build.cancel_url, method: :post, title: 'Cancel' do
diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml
index 630d0286f25..5e3bd14565e 100644
--- a/app/views/projects/builds/index.html.haml
+++ b/app/views/projects/builds/index.html.haml
@@ -22,7 +22,7 @@
           = number_with_delimiter(@all_builds.finished.count(:id))
 
   .nav-controls
-    - if can?(current_user, :manage_builds, @project)
+    - if can?(current_user, :update_build, @project)
       - 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
diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml
index ba1fdc6f0e7..ca1441a20d8 100644
--- a/app/views/projects/builds/show.html.haml
+++ b/app/views/projects/builds/show.html.haml
@@ -89,8 +89,7 @@
             Test 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
           %h4.title Build artifacts
           .center
@@ -102,7 +101,7 @@
       .build-widget
         %h4.title
           Build ##{@build.id}
-          - if current_user && can?(current_user, :manage_builds, @project)
+          - if can?(current_user, :update_build, @project)
             .pull-right
               - if @build.cancel_url
                 = link_to "Cancel", @build.cancel_url, class: 'btn btn-sm btn-danger', method: :post
diff --git a/app/views/projects/commit/_builds.html.haml b/app/views/projects/commit/_builds.html.haml
index 329aaa0bb8b..befad27666c 100644
--- a/app/views/projects/commit/_builds.html.haml
+++ b/app/views/projects/commit/_builds.html.haml
@@ -1,6 +1,6 @@
 .gray-content-block.middle-block
   .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?)
         = 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
 
diff --git a/app/views/projects/commit_statuses/_commit_status.html.haml b/app/views/projects/commit_statuses/_commit_status.html.haml
index 943b025c80f..a3449d1ae05 100644
--- a/app/views/projects/commit_statuses/_commit_status.html.haml
+++ b/app/views/projects/commit_statuses/_commit_status.html.haml
@@ -1,6 +1,6 @@
 %tr.commit_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
         = ci_icon_for_status(commit_status.status)
         = commit_status.status
@@ -8,7 +8,7 @@
       = ci_status_with_icon(commit_status.status)
 
   %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
         %strong ##{commit_status.id}
     - else
@@ -66,10 +66,10 @@
 
   %td
     .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
           %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.cancel_url
             = link_to commit_status.cancel_url, method: :post, title: 'Cancel' do
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index 8a99aceef7f..fdcb6987471 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -130,6 +130,7 @@
                     %strong git fetch
                     %br
                     %span.descr Faster
+
             .form-group
               = f.label :build_timeout_in_minutes, 'Timeout', class: 'control-label'
               .col-sm-10
@@ -158,6 +159,13 @@
                       phpunit --coverage-text --colors=never (PHP) -
                       %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
             %legend