Merge branch 'refactor/add-policies' into 'master'

Gemfile

@@ -97,9 +97,6 @@ gem 'fog-rackspace', '~> 0.1.1'
 # for aws storage
 gem 'unf', '~> 0.1.4'
 
-# Authorization
-gem 'six', '~> 0.2.0'
-
 # Seed data
 gem 'seed-fu', '~> 2.3.5'
 

Gemfile.lock

@@ -683,7 +683,6 @@ GEM
       rack (~> 1.5)
       rack-protection (~> 1.4)
       tilt (>= 1.3, < 3)
-    six (0.2.0)
     slack-notifier (1.2.1)
     slop (3.6.0)
     spinach (0.8.10)
@@ -954,7 +953,6 @@ DEPENDENCIES
   sidekiq-cron (~> 0.4.0)
   simplecov (= 0.12.0)
   sinatra (~> 1.4.4)
-  six (~> 0.2.0)
   slack-notifier (~> 1.2.0)
   spinach-rails (~> 0.2.1)
   spinach-rerun-reporter (~> 0.0.2)

app/controllers/application_controller.rb

@@ -24,7 +24,7 @@ class ApplicationController < ActionController::Base
 
   protect_from_forgery with: :exception
 
-  helper_method :abilities, :can?, :current_application_settings
+  helper_method :can?, :current_application_settings
   helper_method :import_sources_enabled?, :github_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled?, :gitlab_project_import_enabled?
 
   rescue_from Encoding::CompatibilityError do |exception|
@@ -97,12 +97,8 @@ class ApplicationController < ActionController::Base
     current_application_settings.after_sign_out_path.presence || new_user_session_path
   end
 
-  def abilities
-    Ability.abilities
-  end
-
   def can?(object, action, subject)
-    abilities.allowed?(object, action, subject)
+    Ability.allowed?(object, action, subject)
   end
 
   def access_denied!

app/controllers/namespaces_controller.rb

@@ -14,7 +14,7 @@ class NamespacesController < ApplicationController
 
     if user
       redirect_to user_path(user)
-    elsif group && can?(current_user, :read_group, namespace)
+    elsif group && can?(current_user, :read_group, group)
       redirect_to group_path(group)
     elsif current_user.nil?
       authenticate_user!

app/finders/issuable_finder.rb

@@ -64,7 +64,7 @@ class IssuableFinder
     if project?
       @project = Project.find(params[:project_id])
 
-      unless Ability.abilities.allowed?(current_user, :read_project, @project)
+      unless Ability.allowed?(current_user, :read_project, @project)
         @project = nil
       end
     else

app/finders/todos_finder.rb

@@ -83,7 +83,7 @@ class TodosFinder
     if project?
       @project = Project.find(params[:project_id])
 
-      unless Ability.abilities.allowed?(current_user, :read_project, @project)
+      unless Ability.allowed?(current_user, :read_project, @project)
         @project = nil
       end
     else

app/mailers/base_mailer.rb

@@ -9,7 +9,7 @@ class BaseMailer < ActionMailer::Base
   default reply_to: Proc.new { default_reply_to_address.format }
 
   def can?
-    Ability.abilities.allowed?(current_user, action, subject)
+    Ability.allowed?(current_user, action, subject)
   end
 
   private

app/models/event.rb

@@ -65,7 +65,7 @@ class Event < ActiveRecord::Base
     elsif created_project?
       true
     elsif issue? || issue_note?
-      Ability.abilities.allowed?(user, :read_issue, note? ? note_target : target)
+      Ability.allowed?(user, :read_issue, note? ? note_target : target)
     else
       ((merge_request? || note?) && target.present?) || milestone?
     end

app/models/merge_request.rb

@@ -411,7 +411,7 @@ class MergeRequest < ActiveRecord::Base
   def can_remove_source_branch?(current_user)
     !source_project.protected_branch?(source_branch) &&
       !source_project.root_ref?(source_branch) &&
-      Ability.abilities.allowed?(current_user, :push_code, source_project) &&
+      Ability.allowed?(current_user, :push_code, source_project) &&
       diff_head_commit == source_branch_head
   end
 

app/models/user.rb

@@ -460,16 +460,12 @@ class User < ActiveRecord::Base
     can?(:create_group, nil)
   end
 
-  def abilities
-    Ability.abilities
-  end
-
   def can_select_namespace?
     several_namespaces? || admin
   end
 
   def can?(action, subject)
-    abilities.allowed?(self, action, subject)
+    Ability.allowed?(self, action, subject)
   end
 
   def first_name

app/policies/base_policy.rb

+class BasePolicy
+  class RuleSet
+    attr_reader :can_set, :cannot_set
+    def initialize(can_set, cannot_set)
+      @can_set = can_set
+      @cannot_set = cannot_set
+    end
+
+    def size
+      to_set.size
+    end
+
+    def self.empty
+      new(Set.new, Set.new)
+    end
+
+    def can?(ability)
+      @can_set.include?(ability) && !@cannot_set.include?(ability)
+    end
+
+    def include?(ability)
+      can?(ability)
+    end
+
+    def to_set
+      @can_set - @cannot_set
+    end
+
+    def merge(other)
+      @can_set.merge(other.can_set)
+      @cannot_set.merge(other.cannot_set)
+    end
+
+    def can!(*abilities)
+      @can_set.merge(abilities)
+    end
+
+    def cannot!(*abilities)
+      @cannot_set.merge(abilities)
+    end
+
+    def freeze
+      @can_set.freeze
+      @cannot_set.freeze
+      super
+    end
+  end
+
+  def self.abilities(user, subject)
+    new(user, subject).abilities
+  end
+
+  def self.class_for(subject)
+    return GlobalPolicy if subject.nil?
+
+    subject.class.ancestors.each do |klass|
+      next unless klass.name
+
+      begin
+        policy_class = "#{klass.name}Policy".constantize
+
+        # NOTE: the < operator here tests whether policy_class
+        # inherits from BasePolicy
+        return policy_class if policy_class < BasePolicy
+      rescue NameError
+        nil
+      end
+    end
+
+    raise "no policy for #{subject.class.name}"
+  end
+
+  attr_reader :user, :subject
+  def initialize(user, subject)
+    @user = user
+    @subject = subject
+  end
+
+  def abilities
+    return RuleSet.empty if @user && @user.blocked?
+    return anonymous_abilities if @user.nil?
+    collect_rules { rules }
+  end
+
+  def anonymous_abilities
+    collect_rules { anonymous_rules }
+  end
+
+  def anonymous_rules
+    rules
+  end
+
+  def delegate!(new_subject)
+    @rule_set.merge(Ability.allowed(@user, new_subject))
+  end
+
+  def can?(rule)
+    @rule_set.can?(rule)
+  end
+
+  def can!(*rules)
+    @rule_set.can!(*rules)
+  end
+
+  def cannot!(*rules)
+    @rule_set.cannot!(*rules)
+  end
+
+  private
+
+  def collect_rules(&b)
+    @rule_set = RuleSet.empty
+    yield
+    @rule_set
+  end
+end

app/policies/ci/build_policy.rb

+module Ci
+  class BuildPolicy < CommitStatusPolicy
+    def rules
+      super
+
+      # 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|
+        cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build"
+      end
+    end
+  end
+end

app/policies/ci/runner_policy.rb

+module Ci
+  class RunnerPolicy < BasePolicy
+    def rules
+      return unless @user
+
+      can! :assign_runner if @user.is_admin?
+
+      return if @subject.is_shared? || @subject.locked?
+
+      can! :assign_runner if @user.ci_authorized_runners.include?(@subject)
+    end
+  end
+end

app/policies/commit_status_policy.rb

+class CommitStatusPolicy < BasePolicy
+  def rules
+    delegate! @subject.project
+  end
+end

app/policies/deployment_policy.rb

+class DeploymentPolicy < BasePolicy
+  def rules
+    delegate! @subject.project
+  end
+end

app/policies/environment_policy.rb

+class EnvironmentPolicy < BasePolicy
+  def rules
+    delegate! @subject.project
+  end
+end

app/policies/external_issue_policy.rb

+class ExternalIssuePolicy < BasePolicy
+  def rules
+    delegate! @subject.project
+  end
+end

app/policies/global_policy.rb

+class GlobalPolicy < BasePolicy
+  def rules
+    return unless @user
+
+    can! :create_group if @user.can_create_group
+    can! :read_users_list
+  end
+end

app/policies/group_member_policy.rb

+class GroupMemberPolicy < BasePolicy
+  def rules
+    return unless @user
+
+    target_user = @subject.user
+    group = @subject.group
+
+    return if group.last_owner?(target_user)
+
+    can_manage = Ability.allowed?(@user, :admin_group_member, group)
+
+    if can_manage
+      can! :update_group_member
+      can! :destroy_group_member
+    elsif @user == target_user
+      can! :destroy_group_member
+    end
+  end
+end

app/policies/group_policy.rb

+class GroupPolicy < BasePolicy
+  def rules
+    can! :read_group if @subject.public?
+    return unless @user
+
+    globally_viewable = @subject.public? || (@subject.internal? && !@user.external?)
+    member = @subject.users.include?(@user)
+    owner = @user.admin? || @subject.has_owner?(@user)
+    master = owner || @subject.has_master?(@user)
+
+    can_read = false
+    can_read ||= globally_viewable
+    can_read ||= member
+    can_read ||= @user.admin?
+    can_read ||= GroupProjectsFinder.new(@subject).execute(@user).any?
+    can! :read_group if can_read
+
+    # Only group masters and group owners can create new projects
+    if master
+      can! :create_projects
+      can! :admin_milestones
+    end
+
+    # Only group owner and administrators can admin group
+    if owner
+      can! :admin_group
+      can! :admin_namespace
+      can! :admin_group_member
+      can! :change_visibility_level
+    end
+
+    if globally_viewable && @subject.request_access_enabled && !member
+      can! :request_access
+    end
+  end
+
+  def can_read_group?
+    return true if @subject.public?
+    return true if @user.admin?
+    return true if @subject.internal? && !@user.external?
+    return true if @subject.users.include?(@user)
+
+    GroupProjectsFinder.new(@subject).execute(@user).any?
+  end
+end

app/policies/issuable_policy.rb

+class IssuablePolicy < BasePolicy
+  def action_name
+    @subject.class.name.underscore
+  end
+
+  def rules
+    if @user && (@subject.author == @user || @subject.assignee == @user)
+      can! :"read_#{action_name}"
+      can! :"update_#{action_name}"
+    end
+
+    delegate! @subject.project
+  end
+end

app/policies/issue_policy.rb

+class IssuePolicy < IssuablePolicy
+  def issue
+    @subject
+  end
+
+  def rules
+    super
+
+    if @subject.confidential? && !can_read_confidential?
+      cannot! :read_issue
+      cannot! :admin_issue
+      cannot! :update_issue
+      cannot! :read_issue
+    end
+  end
+
+  private
+
+  def can_read_confidential?
+    return false unless @user
+    return true if @user.admin?
+    return true if @subject.author == @user
+    return true if @subject.assignee == @user
+    return true if @subject.project.team.member?(@user, Gitlab::Access::REPORTER)
+
+    false
+  end
+end

app/policies/merge_request_policy.rb

+class MergeRequestPolicy < IssuablePolicy
+  # pass
+end

app/policies/namespace_policy.rb

+class NamespacePolicy < BasePolicy
+  def rules
+    return unless @user
+
+    if @subject.owner == @user || @user.admin?
+      can! :create_projects
+      can! :admin_namespace
+    end
+  end
+end

app/policies/note_policy.rb

+class NotePolicy < BasePolicy
+  def rules
+    delegate! @subject.project
+
+    return unless @user
+
+    if @subject.author == @user
+      can! :read_note
+      can! :update_note
+      can! :admin_note
+      can! :resolve_note
+    end
+
+    if @subject.for_merge_request? &&
+       @subject.noteable.author == @user
+      can! :resolve_note
+    end
+  end
+end

app/policies/personal_snippet_policy.rb

+class PersonalSnippetPolicy < BasePolicy
+  def rules
+    can! :read_personal_snippet if @subject.public?
+    return unless @user
+
+    if @subject.author == @user
+      can! :read_personal_snippet
+      can! :update_personal_snippet
+      can! :admin_personal_snippet
+    end
+
+    if @subject.internal? && !@user.external?
+      can! :read_personal_snippet
+    end
+  end
+end

app/policies/project_member_policy.rb

+class ProjectMemberPolicy < BasePolicy
+  def rules
+    # anonymous users have no abilities here
+    return unless @user
+
+    target_user = @subject.user
+    project = @subject.project
+
+    return if target_user == project.owner
+
+    can_manage = Ability.allowed?(@user, :admin_project_member, project)
+
+    if can_manage
+      can! :update_project_member
+      can! :destroy_project_member
+    end
+
+    if @user == target_user
+      can! :destroy_project_member
+    end
+  end
+end

app/policies/project_policy.rb

+class ProjectPolicy < BasePolicy
+  def rules
+    team_access!(user)
+
+    owner = user.admin? ||
+            project.owner == user ||
+            (project.group && project.group.has_owner?(user))
+
+    owner_access! if owner
+
+    if project.public? || (project.internal? && !user.external?)
+      guest_access!
+      public_access!
+
+      # Allow to read builds for internal projects
+      can! :read_build if project.public_builds?
+
+      if project.request_access_enabled &&
+         !(owner || project.team.member?(user) || project_group_member?(user))
+        can! :request_access
+      end
+    end
+
+    archived_access! if project.archived?
+
+    disabled_features!
+  end
+
+  def project
+    @subject
+  end
+
+  def guest_access!
+    can! :read_project
+    can! :read_board
+    can! :read_list
+    can! :read_wiki
+    can! :read_issue
+    can! :read_label
+    can! :read_milestone
+    can! :read_project_snippet
+    can! :read_project_member
+    can! :read_merge_request
+    can! :read_note
+    can! :create_project
+    can! :create_issue
+    can! :create_note
+    can! :upload_file
+  end
+
+  def reporter_access!
+    can! :download_code
+    can! :fork_project
+    can! :create_project_snippet
+    can! :update_issue
+    can! :admin_issue
+    can! :admin_label
+    can! :admin_list
+    can! :read_commit_status
+    can! :read_build
+    can! :read_container_image
+    can! :read_pipeline
+    can! :read_environment
+    can! :read_deployment
+  end
+
+  def developer_access!
+    can! :admin_merge_request
+    can! :update_merge_request
+    can! :create_commit_status
+    can! :update_commit_status
+    can! :create_build
+    can! :update_build
+    can! :create_pipeline
+    can! :update_pipeline
+    can! :create_merge_request
+    can! :create_wiki
+    can! :push_code
+    can! :resolve_note
+    can! :create_container_image
+    can! :update_container_image
+    can! :create_environment
+    can! :create_deployment
+  end
+
+  def master_access!
+    can! :push_code_to_protected_branches
+    can! :update_project_snippet
+    can! :update_environment
+    can! :update_deployment
+    can! :admin_milestone
+    can! :admin_project_snippet
+    can! :admin_project_member
+    can! :admin_merge_request
+    can! :admin_note
+    can! :admin_wiki
+    can! :admin_project
+    can! :admin_commit_status
+    can! :admin_build
+    can! :admin_container_image
+    can! :admin_pipeline
+    can! :admin_environment
+    can! :admin_deployment
+  end
+
+  def public_access!
+    can! :download_code
+    can! :fork_project
+    can! :read_commit_status
+    can! :read_pipeline
+    can! :read_container_image
+  end
+
+  def owner_access!
+    guest_access!
+    reporter_access!
+    developer_access!
+    master_access!
+    can! :change_namespace
+    can! :change_visibility_level
+    can! :rename_project
+    can! :remove_project
+    can! :archive_project
+    can! :remove_fork_project
+    can! :destroy_merge_request
+    can! :destroy_issue
+  end
+
+  # Push abilities on the users team role
+  def team_access!(user)
+    access = project.team.max_member_access(user.id)
+
+    guest_access!     if access >= Gitlab::Access::GUEST
+    reporter_access!  if access >= Gitlab::Access::REPORTER
+    developer_access! if access >= Gitlab::Access::DEVELOPER
+    master_access!    if access >= Gitlab::Access::MASTER
+  end
+
+  def archived_access!
+    cannot! :create_merge_request
+    cannot! :push_code
+    cannot! :push_code_to_protected_branches
+    cannot! :update_merge_request
+    cannot! :admin_merge_request
+  end
+
+  def disabled_features!
+    unless project.issues_enabled
+      cannot!(*named_abilities(:issue))
+    end
+
+    unless project.merge_requests_enabled
+      cannot!(*named_abilities(:merge_request))
+    end
+
+    unless project.issues_enabled || project.merge_requests_enabled
+      cannot!(*named_abilities(:label))
+      cannot!(*named_abilities(:milestone))
+    end
+
+    unless project.snippets_enabled
+      cannot!(*named_abilities(:project_snippet))
+    end
+
+    unless project.has_wiki?
+      cannot!(*named_abilities(:wiki))
+    end
+
+    unless project.builds_enabled
+      cannot!(*named_abilities(:build))
+      cannot!(*named_abilities(:pipeline))
+      cannot!(*named_abilities(:environment))
+      cannot!(*named_abilities(:deployment))
+    end
+
+    unless project.container_registry_enabled
+      cannot!(*named_abilities(:container_image))
+    end
+  end
+
+  def anonymous_rules
+    return unless project.public?
+
+    can! :read_project
+    can! :read_board
+    can! :read_list
+    can! :read_wiki
+    can! :read_label
+    can! :read_milestone
+    can! :read_project_snippet
+    can! :read_project_member
+    can! :read_merge_request
+    can! :read_note
+    can! :read_pipeline
+    can! :read_commit_status
+    can! :read_container_image
+    can! :download_code
+
+    # NOTE: may be overridden by IssuePolicy
+    can! :read_issue
+
+    # Allow to read builds by anonymous user if guests are allowed
+    can! :read_build if project.public_builds?
+
+    disabled_features!
+  end
+
+  def project_group_member?(user)
+    project.group &&
+    (
+      project.group.members.exists?(user_id: user.id) ||
+      project.group.requesters.exists?(user_id: user.id)
+    )
+  end
+
+  def named_abilities(name)
+    [
+      :"read_#{name}",
+      :"create_#{name}",
+      :"update_#{name}",
+      :"admin_#{name}"
+    ]
+  end
+end

app/policies/project_snippet_policy.rb

+class ProjectSnippetPolicy < BasePolicy
+  def rules
+    can! :read_project_snippet if @subject.public?
+    return unless @user
+
+    if @user && @subject.author == @user || @user.admin?
+      can! :read_project_snippet
+      can! :update_project_snippet
+      can! :admin_project_snippet
+    end
+
+    if @subject.internal? && !@user.external?
+      can! :read_project_snippet
+    end
+
+    if @subject.private? && @subject.project.team.member?(@user)
+      can! :read_project_snippet
+    end
+  end
+end

app/policies/user_policy.rb

+class UserPolicy < BasePolicy
+  include Gitlab::CurrentSettings
+
+  def rules
+    can! :read_user if @user || !restricted_public_level?
+  end
+
+  def restricted_public_level?
+    current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
+  end
+end

app/services/base_service.rb

@@ -7,12 +7,8 @@ class BaseService
     @project, @current_user, @params = project, user, params.dup
   end
 
-  def abilities
-    Ability.abilities
-  end
-
   def can?(object, action, subject)
-    abilities.allowed?(object, action, subject)
+    Ability.allowed?(object, action, subject)
   end
 
   def notification_service

lib/api/groups.rb

@@ -30,7 +30,7 @@ module API
       # Example Request:
       #   POST /groups
       post do
-        authorize! :create_group, current_user
+        authorize! :create_group
         required_attributes! [:name, :path]
 
         attrs = attributes_for_keys [:name, :path, :description, :visibility_level]

lib/api/helpers.rb

@@ -129,7 +129,7 @@ module API
       forbidden! unless current_user.is_admin?
     end
 
-    def authorize!(action, subject)
+    def authorize!(action, subject = nil)
       forbidden! unless can?(current_user, action, subject)
     end
 
@@ -148,7 +148,7 @@ module API
     end
 
     def can?(object, action, subject)
-      abilities.allowed?(object, action, subject)
+      Ability.allowed?(object, action, subject)
     end
 
     # Checks the occurrences of required attributes, each attribute must be present in the params hash
@@ -408,14 +408,6 @@ module API
       links.join(', ')
     end
 
-    def abilities
-      @abilities ||= begin
-                       abilities = Six.new
-                       abilities << Ability
-                       abilities
-                     end
-    end
-
     def secret_token
       File.read(Gitlab.config.gitlab_shell.secret_file).chomp
     end

lib/banzai/reference_parser/base_parser.rb

@@ -211,7 +211,7 @@ module Banzai
       end
 
       def can?(user, permission, subject)
-        Ability.abilities.allowed?(user, permission, subject)
+        Ability.allowed?(user, permission, subject)
       end
 
       def find_projects_for_hash_keys(hash)

spec/controllers/projects/boards/issues_controller_spec.rb

@@ -41,8 +41,8 @@ describe Projects::Boards::IssuesController do
 
     context 'with unauthorized user' do
       before do
-        allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability.abilities).to receive(:allowed?).with(user, :read_issue, project).and_return(false)
+        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+        allow(Ability).to receive(:allowed?).with(user, :read_issue, project).and_return(false)
       end
 
       it 'returns a successful 403 response' do

spec/controllers/projects/boards/lists_controller_spec.rb

@@ -35,8 +35,8 @@ describe Projects::Boards::ListsController do
 
     context 'with unauthorized user' do
       before do
-        allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability.abilities).to receive(:allowed?).with(user, :read_list, project).and_return(false)
+        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+        allow(Ability).to receive(:allowed?).with(user, :read_list, project).and_return(false)
       end
 
       it 'returns a forbidden 403 response' do

spec/controllers/projects/boards_controller_spec.rb

@@ -23,8 +23,8 @@ describe Projects::BoardsController do
 
     context 'with unauthorized user' do
       before do
-        allow(Ability.abilities).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability.abilities).to receive(:allowed?).with(user, :read_board, project).and_return(false)
+        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+        allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false)
       end
 
       it 'returns a successful 404 response' do

spec/lib/banzai/reference_parser/base_parser_spec.rb

@@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
       it 'returns the nodes if the attribute value equals the current project ID' do
         link['data-project'] = project.id.to_s
 
-        expect(Ability.abilities).not_to receive(:allowed?)
+        expect(Ability).not_to receive(:allowed?)
         expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
       end
 
@@ -39,7 +39,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
 
         link['data-project'] = other_project.id.to_s
 
-        expect(Ability.abilities).to receive(:allowed?).
+        expect(Ability).to receive(:allowed?).
           with(user, :read_project, other_project).
           and_return(true)
 
@@ -57,7 +57,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
 
         link['data-project'] = other_project.id.to_s
 
-        expect(Ability.abilities).to receive(:allowed?).
+        expect(Ability).to receive(:allowed?).
           with(user, :read_project, other_project).
           and_return(false)
 
@@ -221,7 +221,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
     it 'delegates the permissions check to the Ability class' do
       user = double(:user)
 
-      expect(Ability.abilities).to receive(:allowed?).
+      expect(Ability).to receive(:allowed?).
         with(user, :read_project, project)
 
       subject.can?(user, :read_project, project)

spec/lib/banzai/reference_parser/user_parser_spec.rb

@@ -82,7 +82,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do
         end
 
         it 'returns the nodes if the user can read the group' do
-          expect(Ability.abilities).to receive(:allowed?).
+          expect(Ability).to receive(:allowed?).
             with(user, :read_group, group).
             and_return(true)
 
@@ -90,7 +90,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do
         end
 
         it 'returns an empty Array if the user can not read the group' do
-          expect(Ability.abilities).to receive(:allowed?).
+          expect(Ability).to receive(:allowed?).
             with(user, :read_group, group).
             and_return(false)
 
@@ -103,7 +103,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do
           it 'returns the nodes if the attribute value equals the current project ID' do
             link['data-project'] = project.id.to_s
 
-            expect(Ability.abilities).not_to receive(:allowed?)
+            expect(Ability).not_to receive(:allowed?)
 
             expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
           end
@@ -113,7 +113,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do
 
             link['data-project'] = other_project.id.to_s
 
-            expect(Ability.abilities).to receive(:allowed?).
+            expect(Ability).to receive(:allowed?).
               with(user, :read_project, other_project).
               and_return(true)
 
@@ -125,7 +125,7 @@ describe Banzai::ReferenceParser::UserParser, lib: true do
 
             link['data-project'] = other_project.id.to_s
 
-            expect(Ability.abilities).to receive(:allowed?).
+            expect(Ability).to receive(:allowed?).
               with(user, :read_project, other_project).
               and_return(false)
 

spec/models/ability_spec.rb

@@ -171,70 +171,6 @@ describe Ability, lib: true do
     end
   end
 
-  shared_examples_for ".project_abilities" do |enable_request_store|
-    before do
-      RequestStore.begin! if enable_request_store
-    end
-
-    after do
-      if enable_request_store
-        RequestStore.end!
-        RequestStore.clear!
-      end
-    end
-
-    describe '.project_abilities' do
-      let!(:project) { create(:empty_project, :public) }
-      let!(:user) { create(:user) }
-
-      it 'returns permissions for admin user' do
-        admin = create(:admin)
-
-        results = described_class.project_abilities(admin, project)
-
-        expect(results.count).to eq(68)
-      end
-
-      it 'returns permissions for an owner' do
-        results = described_class.project_abilities(project.owner, project)
-
-        expect(results.count).to eq(68)
-      end
-
-      it 'returns permissions for a master' do
-        project.team << [user, :master]
-
-        results = described_class.project_abilities(user, project)
-
-        expect(results.count).to eq(60)
-      end
-
-      it 'returns permissions for a developer' do
-        project.team << [user, :developer]
-
-        results = described_class.project_abilities(user, project)
-
-        expect(results.count).to eq(44)
-      end
-
-      it 'returns permissions for a guest' do
-        project.team << [user, :guest]
-
-        results = described_class.project_abilities(user, project)
-
-        expect(results.count).to eq(21)
-      end
-    end
-  end
-
-  describe '.project_abilities with RequestStore' do
-    it_behaves_like ".project_abilities", true
-  end
-
-  describe '.project_abilities without RequestStore' do
-    it_behaves_like ".project_abilities", false
-  end
-
   describe '.issues_readable_by_user' do
     context 'with an admin user' do
       it 'returns all given issues' do
@@ -286,12 +222,12 @@ describe Ability, lib: true do
   describe '.project_disabled_features_rules' do
     let(:project) { build(:project) }
 
-    subject { described_class.project_disabled_features_rules(project) }
+    subject { described_class.allowed(project.owner, project) }
 
     context 'wiki named abilities' do
       it 'disables wiki abilities if the project has no wiki' do
         expect(project).to receive(:has_wiki?).and_return(false)
-        expect(subject).to include(:read_wiki, :create_wiki, :update_wiki, :admin_wiki)
+        expect(subject).not_to include(:read_wiki, :create_wiki, :update_wiki, :admin_wiki)
       end
     end
   end

spec/models/members/project_member_spec.rb

@@ -71,9 +71,6 @@ describe ProjectMember, models: true do
 
   describe :import_team do
     before do
-      @abilities = Six.new
-      @abilities << Ability
-
       @project_1 = create :project
       @project_2 = create :project
 
@@ -92,8 +89,8 @@ describe ProjectMember, models: true do
       it { expect(@project_2.users).to include(@user_1) }
       it { expect(@project_2.users).to include(@user_2) }
 
-      it { expect(@abilities.allowed?(@user_1, :create_project, @project_2)).to be_truthy }
-      it { expect(@abilities.allowed?(@user_2, :read_project, @project_2)).to be_truthy }
+      it { expect(Ability.allowed?(@user_1, :create_project, @project_2)).to be_truthy }
+      it { expect(Ability.allowed?(@user_2, :read_project, @project_2)).to be_truthy }
     end
 
     describe 'project 1 should not be changed' do

spec/models/note_spec.rb

@@ -85,8 +85,6 @@ describe Note, models: true do
       @u1 = create(:user)
       @u2 = create(:user)
       @u3 = create(:user)
-      @abilities = Six.new
-      @abilities << Ability
     end
 
     describe 'read' do
@@ -95,9 +93,9 @@ describe Note, models: true do
         @p2.project_members.create(user: @u3, access_level: ProjectMember::GUEST)
       end
 
-      it { expect(@abilities.allowed?(@u1, :read_note, @p1)).to be_falsey }
-      it { expect(@abilities.allowed?(@u2, :read_note, @p1)).to be_truthy }
-      it { expect(@abilities.allowed?(@u3, :read_note, @p1)).to be_falsey }
+      it { expect(Ability.allowed?(@u1, :read_note, @p1)).to be_falsey }
+      it { expect(Ability.allowed?(@u2, :read_note, @p1)).to be_truthy }
+      it { expect(Ability.allowed?(@u3, :read_note, @p1)).to be_falsey }
     end
 
     describe 'write' do
@@ -106,9 +104,9 @@ describe Note, models: true do
         @p2.project_members.create(user: @u3, access_level: ProjectMember::DEVELOPER)
       end
 
-      it { expect(@abilities.allowed?(@u1, :create_note, @p1)).to be_falsey }
-      it { expect(@abilities.allowed?(@u2, :create_note, @p1)).to be_truthy }
-      it { expect(@abilities.allowed?(@u3, :create_note, @p1)).to be_falsey }
+      it { expect(Ability.allowed?(@u1, :create_note, @p1)).to be_falsey }
+      it { expect(Ability.allowed?(@u2, :create_note, @p1)).to be_truthy }
+      it { expect(Ability.allowed?(@u3, :create_note, @p1)).to be_falsey }
     end
 
     describe 'admin' do
@@ -118,9 +116,9 @@ describe Note, models: true do
         @p2.project_members.create(user: @u3, access_level: ProjectMember::MASTER)
       end
 
-      it { expect(@abilities.allowed?(@u1, :admin_note, @p1)).to be_falsey }
-      it { expect(@abilities.allowed?(@u2, :admin_note, @p1)).to be_truthy }
-      it { expect(@abilities.allowed?(@u3, :admin_note, @p1)).to be_falsey }
+      it { expect(Ability.allowed?(@u1, :admin_note, @p1)).to be_falsey }
+      it { expect(Ability.allowed?(@u2, :admin_note, @p1)).to be_truthy }
+      it { expect(Ability.allowed?(@u3, :admin_note, @p1)).to be_falsey }
     end
   end
 

spec/models/project_security_spec.rb

-require 'spec_helper'
-
-describe Project, models: true do
-  describe 'authorization' do
-    before do
-      @p1 = create(:project)
-
-      @u1 = create(:user)
-      @u2 = create(:user)
-      @u3 = create(:user)
-      @u4 = @p1.owner
-
-      @abilities = Six.new
-      @abilities << Ability
-    end
-
-    let(:guest_actions) { Ability.project_guest_rules }
-    let(:report_actions) { Ability.project_report_rules }
-    let(:dev_actions) { Ability.project_dev_rules }
-    let(:master_actions) { Ability.project_master_rules }
-    let(:owner_actions) { Ability.project_owner_rules }
-
-    describe "Non member rules" do
-      it "denies for non-project users any actions" do
-        owner_actions.each do |action|
-          expect(@abilities.allowed?(@u1, action, @p1)).to be_falsey
-        end
-      end
-    end
-
-    describe "Guest Rules" do
-      before do
-        @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::GUEST)
-      end
-
-      it "allows for project user any guest actions" do
-        guest_actions.each do |action|
-          expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy
-        end
-      end
-    end
-
-    describe "Report Rules" do
-      before do
-        @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER)
-      end
-
-      it "allows for project user any report actions" do
-        report_actions.each do |action|
-          expect(@abilities.allowed?(@u2, action, @p1)).to be_truthy
-        end
-      end
-    end
-
-    describe "Developer Rules" do
-      before do
-        @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::REPORTER)
-        @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::DEVELOPER)
-      end
-
-      it "denies for developer master-specific actions" do
-        [dev_actions - report_actions].each do |action|
-          expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey
-        end
-      end
-
-      it "allows for project user any dev actions" do
-        dev_actions.each do |action|
-          expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy
-        end
-      end
-    end
-
-    describe "Master Rules" do
-      before do
-        @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::DEVELOPER)
-        @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER)
-      end
-
-      it "denies for developer master-specific actions" do
-        [master_actions - dev_actions].each do |action|
-          expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey
-        end
-      end
-
-      it "allows for project user any master actions" do
-        master_actions.each do |action|
-          expect(@abilities.allowed?(@u3, action, @p1)).to be_truthy
-        end
-      end
-    end
-
-    describe "Owner Rules" do
-      before do
-        @p1.project_members.create(project: @p1, user: @u2, access_level: ProjectMember::DEVELOPER)
-        @p1.project_members.create(project: @p1, user: @u3, access_level: ProjectMember::MASTER)
-      end
-
-      it "denies for masters admin-specific actions" do
-        [owner_actions - master_actions].each do |action|
-          expect(@abilities.allowed?(@u2, action, @p1)).to be_falsey
-        end
-      end
-
-      it "allows for project owner any admin actions" do
-        owner_actions.each do |action|
-          expect(@abilities.allowed?(@u4, action, @p1)).to be_truthy
-        end
-      end
-    end
-  end
-end

spec/policies/project_policy_spec.rb

+require 'spec_helper'
+
+describe ProjectPolicy, models: true do
+  let(:project) { create(:empty_project, :public) }
+  let(:guest) { create(:user) }
+  let(:reporter) { create(:user) }
+  let(:dev) { create(:user) }
+  let(:master) { create(:user) }
+  let(:owner) { create(:user) }
+  let(:admin) { create(:admin) }
+
+  let(:users_ordered_by_permissions) do
+    [nil, guest, reporter, dev, master, owner, admin]
+  end
+
+  let(:users_permissions) do
+    users_ordered_by_permissions.map { |u| Ability.allowed(u, project).size }
+  end
+
+  before do
+    project.team << [guest, :guest]
+    project.team << [master, :master]
+    project.team << [dev, :developer]
+    project.team << [reporter, :reporter]
+
+    group = create(:group)
+    project.project_group_links.create(
+      group: group,
+      group_access: Gitlab::Access::MASTER)
+    group.add_owner(owner)
+  end
+
+  it 'returns increasing permissions for each level' do
+    expect(users_permissions).to eq(users_permissions.sort.uniq)
+  end
+end