Skip to content

G
gitlab-ce
Commit 8c5538be authored by Sean McGivern's avatar Sean McGivern
Browse files
Options

Merge branch 'refactor/declarative-policy' into 'master' 

Refactor/declarative policy

See merge request !10515
parents adf792f1 72dc16da
Expand all Show whitespace changes
Inline Side-by-side
Showing with 2285 additions and 971 deletions
app/models/ability.rb
View file @ 8c5538be
require 'declarative_policy'
class Ability class Ability
class << self class << self
# Given a list of users and a project this method returns the users that can # Given a list of users and a project this method returns the users that can
# read the given project. # read the given project.
def users_that_can_read_project(users, project) def users_that_can_read_project(users, project)
if project.public? DeclarativePolicy.subject_scope do
users users.select { |u| allowed?(u, :read_project, project) }
else
users.select do |user|
if user.admin?
true
elsif project.internal? && !user.external?
true
elsif project.owner == user
true
elsif project.team.members.include?(user)
true
else
false
end
end
end end
end end
# Given a list of users and a snippet this method returns the users that can # Given a list of users and a snippet this method returns the users that can
# read the given snippet. # read the given snippet.
def users_that_can_read_personal_snippet(users, snippet) def users_that_can_read_personal_snippet(users, snippet)
case snippet.visibility_level DeclarativePolicy.subject_scope do
when Snippet::INTERNAL, Snippet::PUBLIC users.select { |u| allowed?(u, :read_personal_snippet, snippet) }
users
when Snippet::PRIVATE
users.include?(snippet.author) ? [snippet.author] : []
end end
end end
...@@ -38,42 +23,35 @@ class Ability ...@@ -38,42 +23,35 @@ class Ability
# issues - The issues to reduce down to those readable by the user. # issues - The issues to reduce down to those readable by the user.
# user - The User for which to check the issues # user - The User for which to check the issues
def issues_readable_by_user(issues, user = nil) def issues_readable_by_user(issues, user = nil)
return issues if user && user.admin? DeclarativePolicy.user_scope do
issues.select { |issue| issue.visible_to_user?(user) } issues.select { |issue| issue.visible_to_user?(user) }
end end
end
# TODO: make this private and use the actual abilities stuff for this
def can_edit_note?(user, note) def can_edit_note?(user, note)
return false if !note.editable? || !user.present? allowed?(user, :edit_note, note)
return true if note.author == user || user.admin?
if note.project
max_access_level = note.project.team.max_member_access(user.id)
max_access_level >= Gitlab::Access::MASTER
else
false
end
end end
def allowed?(user, action, subject = :global) def allowed?(user, action, subject = :global, opts = {})
allowed(user, subject).include?(action) if subject.is_a?(Hash)
opts, subject = subject, :global
end end
def allowed(user, subject = :global) policy = policy_for(user, subject)
return BasePolicy::RuleSet.none if subject.nil?
return uncached_allowed(user, subject) unless RequestStore.active?
user_key = user ? user.id : 'anonymous' case opts[:scope]
subject_key = subject == :global ? 'global' : "#{subject.class.name}/#{subject.id}" when :user
key = "/ability/#{user_key}/#{subject_key}" DeclarativePolicy.user_scope { policy.can?(action) }
RequestStore[key] ||= uncached_allowed(user, subject).freeze when :subject
DeclarativePolicy.subject_scope { policy.can?(action) }
else
policy.can?(action)
end
end end
private def policy_for(user, subject = :global)
cache = RequestStore.active? ? RequestStore : {}
def uncached_allowed(user, subject) DeclarativePolicy.policy_for(user, subject, cache: cache)
BasePolicy.class_for(subject).abilities(user, subject)
end end
end end
end end
app/models/project_feature.rb
View file @ 8c5538be
...@@ -51,8 +51,11 @@ class ProjectFeature < ActiveRecord::Base ...@@ -51,8 +51,11 @@ class ProjectFeature < ActiveRecord::Base
default_value_for :repository_access_level, value: ENABLED, allows_nil: false default_value_for :repository_access_level, value: ENABLED, allows_nil: false
def feature_available?(feature, user) def feature_available?(feature, user)
access_level = public_send(ProjectFeature.access_level_attribute(feature)) get_permission(user, access_level(feature))
get_permission(user, access_level) end
def access_level(feature)
public_send(ProjectFeature.access_level_attribute(feature))
end end
def builds_enabled? def builds_enabled?
......
app/policies/base_policy.rb
View file @ 8c5538be
class BasePolicy require 'declarative_policy'
class RuleSet
attr_reader :can_set, :cannot_set
def initialize(can_set, cannot_set)
@can_set = can_set
@cannot_set = cannot_set
end
delegate :size, to: :to_set class BasePolicy < DeclarativePolicy::Base
desc "User is an instance admin"
with_options scope: :user, score: 0
condition(:admin) { @user&.admin? }
def self.empty with_options scope: :user, score: 0
new(Set.new, Set.new) condition(:external_user) { @user.nil? || @user.external? }
end
def self.none with_options scope: :user, score: 0
empty.freeze condition(:can_create_group) { @user&.can_create_group }
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 == :global
raise ArgumentError, 'no policy for nil' if subject.nil?
if subject.class.try(:presenter?)
subject = subject.subject
end
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.none 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 rules
raise NotImplementedError
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 end
app/policies/ci/build_policy.rb
View file @ 8c5538be
module Ci module Ci
class BuildPolicy < CommitStatusPolicy class BuildPolicy < CommitStatusPolicy
alias_method :build, :subject condition(:protected_action) do
next false unless @subject.action?
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
if can?(:update_build) && protected_action?
cannot! :update_build
end
end
private
def protected_action?
return false unless build.action?
!::Gitlab::UserAccess !::Gitlab::UserAccess
.new(user, project: build.project) .new(@user, project: @subject.project)
.can_merge_to_branch?(build.ref) .can_merge_to_branch?(@subject.ref)
end end
rule { protected_action }.prevent :update_build
end end
end end
app/policies/ci/pipeline_policy.rb
View file @ 8c5538be
module Ci module Ci
class PipelinePolicy < BasePolicy class PipelinePolicy < BasePolicy
def rules delegate { @subject.project }
delegate! @subject.project
end
end end
end end
app/policies/ci/runner_policy.rb
View file @ 8c5538be
module Ci module Ci
class RunnerPolicy < BasePolicy class RunnerPolicy < BasePolicy
def rules with_options scope: :subject, score: 0
return unless @user condition(:shared) { @subject.is_shared? }
can! :assign_runner if @user.admin? with_options scope: :subject, score: 0
condition(:locked, scope: :subject) { @subject.locked? }
return if @subject.is_shared? || @subject.locked? condition(:authorized_runner) { @user.ci_authorized_runners.include?(@subject) }
can! :assign_runner if @user.ci_authorized_runners.include?(@subject) rule { anonymous }.prevent_all
end rule { admin | authorized_runner }.enable :assign_runner
rule { ~admin & shared }.prevent :assign_runner
rule { ~admin & locked }.prevent :assign_runner
end end
end end
app/policies/ci/trigger_policy.rb
View file @ 8c5538be
module Ci module Ci
class TriggerPolicy < BasePolicy class TriggerPolicy < BasePolicy
def rules delegate { @subject.project }
delegate! @subject.project
if can?(:admin_build) with_options scope: :subject, score: 0
can! :admin_trigger if @subject.owner.blank? || condition(:legacy) { @subject.legacy? }
@subject.owner == @user
can! :manage_trigger with_score 0
end condition(:is_owner) { @user && @subject.owner_id == @user.id }
end
rule { ~can?(:admin_build) }.prevent :admin_trigger
rule { legacy | is_owner }.enable :admin_trigger
rule { can?(:admin_build) }.enable :manage_trigger
end end
end end
app/policies/commit_status_policy.rb
View file @ 8c5538be
class CommitStatusPolicy < BasePolicy class CommitStatusPolicy < BasePolicy
def rules delegate { @subject.project }
delegate! @subject.project
%w[read create update admin].each do |action|
rule { ~can?(:"#{action}_commit_status") }.prevent :"#{action}_build"
end end
end end
app/policies/deploy_key_policy.rb
View file @ 8c5538be
class DeployKeyPolicy < BasePolicy class DeployKeyPolicy < BasePolicy
def rules with_options scope: :subject, score: 0
return unless @user condition(:private_deploy_key) { @subject.private? }
can! :update_deploy_key if @user.admin? condition(:has_deploy_key) { @user.project_deploy_keys.exists?(id: @subject.id) }
if @subject.private? && @user.project_deploy_keys.exists?(id: @subject.id) rule { anonymous }.prevent_all
can! :update_deploy_key
end rule { admin }.enable :update_deploy_key
end rule { private_deploy_key & has_deploy_key }.enable :update_deploy_key
end end
app/policies/deployment_policy.rb
View file @ 8c5538be
class DeploymentPolicy < BasePolicy class DeploymentPolicy < BasePolicy
def rules delegate { @subject.project }
delegate! @subject.project
end
end end
app/policies/environment_policy.rb
View file @ 8c5538be
class EnvironmentPolicy < BasePolicy class EnvironmentPolicy < BasePolicy
alias_method :environment, :subject delegate { @subject.project }
def rules condition(:stop_action_allowed) do
delegate! environment.project @subject.stop_action? && can?(:update_build, @subject.stop_action)
if can?(:create_deployment) && environment.stop_action?
can! :stop_environment if can_play_stop_action?
end
end end
private rule { can?(:create_deployment) & stop_action_allowed }.enable :stop_environment
def can_play_stop_action?
Ability.allowed?(user, :update_build, environment.stop_action)
end
end end
app/policies/external_issue_policy.rb
View file @ 8c5538be
class ExternalIssuePolicy < BasePolicy class ExternalIssuePolicy < BasePolicy
def rules delegate { @subject.project }
delegate! @subject.project
end
end end
app/policies/global_policy.rb
View file @ 8c5538be
class GlobalPolicy < BasePolicy class GlobalPolicy < BasePolicy
def rules desc "User is blocked"
return unless @user with_options scope: :user, score: 0
condition(:blocked) { @user.blocked? }
can! :create_group if @user.can_create_group desc "User is an internal user"
can! :read_users_list with_options scope: :user, score: 0
condition(:internal) { @user.internal? }
unless @user.blocked? || @user.internal? desc "User's access has been locked"
can! :log_in unless @user.access_locked? with_options scope: :user, score: 0
can! :access_api condition(:access_locked) { @user.access_locked? }
can! :access_git
can! :receive_notifications rule { anonymous }.prevent_all
can! :use_quick_actions
rule { default }.policy do
enable :read_users_list
enable :log_in
enable :access_api
enable :access_git
enable :receive_notifications
enable :use_quick_actions
end
rule { blocked | internal }.policy do
prevent :log_in
prevent :access_api
prevent :access_git
prevent :receive_notifications
prevent :use_quick_actions
end end
rule { can_create_group }.policy do
enable :create_group
end
rule { access_locked }.policy do
prevent :log_in
end end
end end
app/policies/group_label_policy.rb
View file @ 8c5538be
class GroupLabelPolicy < BasePolicy class GroupLabelPolicy < BasePolicy
def rules delegate { @subject.group }
delegate! @subject.group
end
end end
app/policies/group_member_policy.rb
View file @ 8c5538be
class GroupMemberPolicy < BasePolicy class GroupMemberPolicy < BasePolicy
def rules delegate :group
return unless @user
target_user = @subject.user with_scope :subject
group = @subject.group condition(:last_owner) { @subject.group.last_owner?(@subject.user) }
return if group.last_owner?(target_user) desc "Membership is users' own"
with_score 0
condition(:is_target_user) { @user && @subject.user_id == @user.id }
can_manage = Ability.allowed?(@user, :admin_group_member, group) rule { anonymous }.prevent_all
rule { last_owner }.prevent_all
if can_manage rule { can?(:admin_group_member) }.policy do
can! :update_group_member enable :update_group_member
can! :destroy_group_member enable :destroy_group_member
elsif @user == target_user
can! :destroy_group_member
end end
additional_rules! rule { is_target_user }.policy do
end enable :destroy_group_member
def additional_rules!
# This is meant to be overriden in EE
end end
end end
app/policies/group_policy.rb
View file @ 8c5538be
class GroupPolicy < BasePolicy class GroupPolicy < BasePolicy
def rules desc "Group is public"
can! :read_group if @subject.public? with_options scope: :subject, score: 0
return unless @user condition(:public_group) { @subject.public? }
globally_viewable = @subject.public? || (@subject.internal? && !@user.external?)
access_level = @subject.max_member_access_for_user(@user)
owner = access_level >= GroupMember::OWNER
master = access_level >= GroupMember::MASTER
reporter = access_level >= GroupMember::REPORTER
can_read = false
can_read ||= globally_viewable
can_read ||= access_level >= GroupMember::GUEST
can_read ||= GroupProjectsFinder.new(group: @subject, current_user: @user).execute.any?
can! :read_group if can_read
if reporter
can! :admin_label
end
# Only group masters and group owners can create new projects with_score 0
if master condition(:logged_in_viewable) { @user && @subject.internal? && !@user.external? }
can! :create_projects
can! :admin_milestones condition(:has_access) { access_level != GroupMember::NO_ACCESS }
end
condition(:guest) { access_level >= GroupMember::GUEST }
condition(:owner) { access_level >= GroupMember::OWNER }
condition(:master) { access_level >= GroupMember::MASTER }
condition(:reporter) { access_level >= GroupMember::REPORTER }
# Only group owner and administrators can admin group condition(:has_projects) do
if owner GroupProjectsFinder.new(group: @subject, current_user: @user).execute.any?
can! :admin_group
can! :admin_namespace
can! :admin_group_member
can! :change_visibility_level
can! :create_subgroup if @user.can_create_group
end end
if globally_viewable && @subject.request_access_enabled && access_level == GroupMember::NO_ACCESS with_options scope: :subject, score: 0
can! :request_access condition(:request_access_enabled) { @subject.request_access_enabled }
rule { public_group } .enable :read_group
rule { logged_in_viewable }.enable :read_group
rule { guest } .enable :read_group
rule { admin } .enable :read_group
rule { has_projects } .enable :read_group
rule { reporter }.enable :admin_label
rule { master }.policy do
enable :create_projects
enable :admin_milestones
end end
rule { owner }.policy do
enable :admin_group
enable :admin_namespace
enable :admin_group_member
enable :change_visibility_level
end end
def can_read_group? rule { owner & can_create_group }.enable :create_subgroup
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(group: @subject, current_user: @user).execute.any? rule { public_group | logged_in_viewable }.enable :view_globally
rule { default }.enable(:request_access)
rule { ~request_access_enabled }.prevent :request_access
rule { ~can?(:view_globally) }.prevent :request_access
rule { has_access }.prevent :request_access
def access_level
return GroupMember::NO_ACCESS if @user.nil?
@access_level ||= @subject.max_member_access_for_user(@user)
end end
end end
app/policies/issuable_policy.rb
View file @ 8c5538be
class IssuablePolicy < BasePolicy class IssuablePolicy < BasePolicy
def action_name delegate { @subject.project }
@subject.class.name.underscore
end
def rules desc "User is the assignee or author"
if @user && @subject.assignee_or_author?(@user) condition(:assignee_or_author) do
can! :"read_#{action_name}" @user && @subject.assignee_or_author?(@user)
can! :"update_#{action_name}"
end end
delegate! @subject.project rule { assignee_or_author }.policy do
enable :read_issue
enable :update_issue
enable :read_merge_request
enable :update_merge_request
end end
end end
app/policies/issue_policy.rb
View file @ 8c5538be
...@@ -3,25 +3,17 @@ class IssuePolicy < IssuablePolicy ...@@ -3,25 +3,17 @@ class IssuePolicy < IssuablePolicy
# Make sure to sync this class checks with issue.rb to avoid security problems. # Make sure to sync this class checks with issue.rb to avoid security problems.
# Check commit 002ad215818450d2cbbc5fa065850a953dc7ada8 for more information. # Check commit 002ad215818450d2cbbc5fa065850a953dc7ada8 for more information.
def issue desc "User can read confidential issues"
@subject condition(:can_read_confidential) do
@user && IssueCollection.new([@subject]).visible_to(@user).any?
end end
def rules desc "Issue is confidential"
super condition(:confidential, scope: :subject) { @subject.confidential? }
if @subject.confidential? && !can_read_confidential? rule { confidential & ~can_read_confidential }.policy do
cannot! :read_issue prevent :read_issue
cannot! :update_issue prevent :update_issue
cannot! :admin_issue prevent :admin_issue
end
end
private
def can_read_confidential?
return false unless @user
IssueCollection.new([@subject]).visible_to(@user).any?
end end
end end
app/policies/namespace_policy.rb
View file @ 8c5538be
class NamespacePolicy < BasePolicy class NamespacePolicy < BasePolicy
def rules rule { anonymous }.prevent_all
return unless @user
if @subject.owner == @user || @user.admin? condition(:owner) { @subject.owner == @user }
can! :create_projects
can! :admin_namespace rule { owner | admin }.policy do
end enable :create_projects
enable :admin_namespace
end end
end end
app/policies/nil_policy.rb 0 → 100644
View file @ 8c5538be
class NilPolicy < BasePolicy
rule { default }.prevent_all
end
app/policies/note_policy.rb
View file @ 8c5538be
class NotePolicy < BasePolicy class NotePolicy < BasePolicy
def rules delegate { @subject.project }
delegate! @subject.project
return unless @user condition(:is_author) { @user && @subject.author == @user }
condition(:for_merge_request, scope: :subject) { @subject.for_merge_request? }
condition(:is_noteable_author) { @user && @subject.noteable.author_id == @user.id }
if @subject.author == @user condition(:editable, scope: :subject) { @subject.editable? }
can! :read_note
can! :update_note rule { ~editable | anonymous }.prevent :edit_note
can! :admin_note rule { is_author | admin }.enable :edit_note
can! :resolve_note rule { can?(:master_access) }.enable :edit_note
end
if @subject.for_merge_request? && rule { is_author }.policy do
@subject.noteable.author == @user enable :read_note
can! :resolve_note enable :update_note
enable :admin_note
enable :resolve_note
end end
rule { for_merge_request & is_noteable_author }.policy do
enable :resolve_note
end end
end end
app/policies/personal_snippet_policy.rb
View file @ 8c5538be
class PersonalSnippetPolicy < BasePolicy class PersonalSnippetPolicy < BasePolicy
def rules condition(:public_snippet, scope: :subject) { @subject.public? }
can! :read_personal_snippet if @subject.public? condition(:is_author) { @user && @subject.author == @user }
return unless @user condition(:internal_snippet, scope: :subject) { @subject.internal? }
if @subject.public? rule { public_snippet }.policy do
can! :comment_personal_snippet enable :read_personal_snippet
enable :comment_personal_snippet
end end
if @subject.author == @user rule { is_author }.policy do
can! :read_personal_snippet enable :read_personal_snippet
can! :update_personal_snippet enable :update_personal_snippet
can! :destroy_personal_snippet enable :destroy_personal_snippet
can! :admin_personal_snippet enable :admin_personal_snippet
can! :comment_personal_snippet enable :comment_personal_snippet
end end
unless @user.external? rule { ~anonymous }.enable :create_personal_snippet
can! :create_personal_snippet rule { external_user }.prevent :create_personal_snippet
end
if @subject.internal? && !@user.external? rule { internal_snippet & ~external_user }.policy do
can! :read_personal_snippet enable :read_personal_snippet
can! :comment_personal_snippet enable :comment_personal_snippet
end
end end
rule { anonymous }.prevent :comment_personal_snippet
end end
app/policies/project_label_policy.rb
View file @ 8c5538be
class ProjectLabelPolicy < BasePolicy class ProjectLabelPolicy < BasePolicy
def rules delegate { @subject.project }
delegate! @subject.project
end
end end
app/policies/project_member_policy.rb
View file @ 8c5538be
class ProjectMemberPolicy < BasePolicy class ProjectMemberPolicy < BasePolicy
def rules delegate { @subject.project }
# anonymous users have no abilities here
return unless @user
target_user = @subject.user condition(:target_is_owner, scope: :subject) { @subject.user == @subject.project.owner }
project = @subject.project condition(:target_is_self) { @user && @subject.user == @user }
return if target_user == project.owner rule { anonymous }.prevent_all
rule { target_is_owner }.prevent_all
can_manage = Ability.allowed?(@user, :admin_project_member, project) rule { can?(:admin_project_member) }.policy do
enable :update_project_member
if can_manage enable :destroy_project_member
can! :update_project_member
can! :destroy_project_member
end end
if @user == target_user rule { target_is_self }.enable :destroy_project_member
can! :destroy_project_member
end
end
end end
app/policies/project_policy.rb
View file @ 8c5538be
This diff is collapsed.
app/policies/project_snippet_policy.rb
View file @ 8c5538be
class ProjectSnippetPolicy < BasePolicy class ProjectSnippetPolicy < BasePolicy
def rules delegate :project
desc "Snippet is public"
condition(:public_snippet, scope: :subject) { @subject.public? }
condition(:private_snippet, scope: :subject) { @subject.private? }
condition(:public_project, scope: :subject) { @subject.project.public? }
condition(:is_author) { @user && @subject.author == @user }
condition(:internal, scope: :subject) { @subject.internal? }
# We have to check both project feature visibility and a snippet visibility and take the stricter one # We have to check both project feature visibility and a snippet visibility and take the stricter one
# This will be simplified - check https://gitlab.com/gitlab-org/gitlab-ce/issues/27573 # This will be simplified - check https://gitlab.com/gitlab-org/gitlab-ce/issues/27573
return unless @subject.project.feature_available?(:snippets, @user) rule { ~can?(:read_project) }.policy do
return unless Ability.allowed?(@user, :read_project, @subject.project) prevent :read_project_snippet
prevent :update_project_snippet
prevent :admin_project_snippet
end
can! :read_project_snippet if @subject.public? # we have to use this complicated prevent because the delegated project policy
return unless @user # is overly greedy in allowing :read_project_snippet, since it doesn't have any
# information about the snippet. However, :read_project_snippet on the *project*
# is used to hide/show various snippet-related controls, so we can't just move
# all of the handling here.
rule do
all?(private_snippet | (internal & external_user),
~project.guest,
~admin,
~is_author)
end.prevent :read_project_snippet
if @user && (@subject.author == @user || @user.admin?) rule { internal & ~is_author & ~admin }.policy do
can! :read_project_snippet prevent :update_project_snippet
can! :update_project_snippet prevent :admin_project_snippet
can! :admin_project_snippet
end end
if @subject.internal? && !@user.external? rule { public_snippet }.enable :read_project_snippet
can! :read_project_snippet
end
if @subject.project.team.member?(@user) rule { is_author | admin }.policy do
can! :read_project_snippet enable :read_project_snippet
end enable :update_project_snippet
enable :admin_project_snippet
end end
end end
app/policies/user_policy.rb
View file @ 8c5538be
class UserPolicy < BasePolicy class UserPolicy < BasePolicy
include Gitlab::CurrentSettings include Gitlab::CurrentSettings
def rules desc "The application is restricted from public visibility"
can! :read_user if @user || !restricted_public_level? condition(:restricted_public_level, scope: :global) do
current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC)
if @user
if @user.admin? || @subject == @user
can! :destroy_user
end end
cannot! :destroy_user if @subject.ghost? desc "The current user is the user in question"
end condition(:user_is_self, score: 0) { @subject == @user }
end
def restricted_public_level? desc "This is the ghost user"
current_application_settings.restricted_visibility_levels.include?(Gitlab::VisibilityLevel::PUBLIC) condition(:subject_ghost, scope: :subject, score: 0) { @subject.ghost? }
end
rule { ~restricted_public_level }.enable :read_user
rule { ~anonymous }.enable :read_user
rule { user_is_self | admin }.enable :destroy_user
rule { subject_ghost }.prevent :destroy_user
end end
doc/development/policies.md 0 → 100644
View file @ 8c5538be
# `DeclarativePolicy` framework
The DeclarativePolicy framework is designed to assist in performance of policy checks, and to enable ease of extension for EE. The DSL code in `app/policies` is what `Ability.allowed?` uses to check whether a particular action is allowed on a subject.
The policy used is based on the subject's class name - so `Ability.allowed?(user, :some_ability, project)` will create a `ProjectPolicy` and check permissions on that.
## Managing Permission Rules
Permissions are broken into two parts: `conditions` and `rules`. Conditions are boolean expressions that can access the database and the environment, while rules are statically configured combinations of expressions and other rules that enable or prevent certain abilities. For an ability to be allowed, it must be enabled by at least one rule, and not prevented by any.
### Conditions
Conditions are defined by the `condition` method, and are given a name and a block. The block will be executed in the context of the policy object - so it can access `@user` and `@subject`, as well as call any methods defined on the policy. Note that `@user` may be nil (in the anonymous case), but `@subject` is guaranteed to be a real instance of the subject class.
``` ruby
class FooPolicy < BasePolicy
condition(:is_public) do
# @subject guaranteed to be an instance of Foo
@subject.public?
end
# instance methods can be called from the condition as well
condition(:thing) { check_thing }
def check_thing
# ...
end
end
```
When you define a condition, a predicate method is defined on the policy to check whether that condition passes - so in the above example, an instance of `FooPolicy` will also respond to `#is_public?` and `#thing?`.
Conditions are cached according to their scope. Scope and ordering will be covered later.
### Rules
A `rule` is a logical combination of conditions and other rules, that are configured to enable or prevent certain abilities. It is important to note that the rule configuration is static - a rule's logic cannot touch the database or know about `@user` or `@subject`. This allows us to cache only at the condition level. Rules are specified through the `rule` method, which takes a block of DSL configuration, and returns an object that responds to `#enable` or `#prevent`:
``` ruby
class FooPolicy < BasePolicy
# ...
rule { is_public }.enable :read
rule { thing }.prevent :read
# equivalently,
rule { is_public }.policy do
enable :read
end
rule { ~thing }.policy do
prevent :read
end
end
```
Within the rule DSL, you can use:
* A regular word mentions a condition by name - a rule that is in effect when that condition is truthy.
* `~` indicates negation
* `&` and `|` are logical combinations, also available as `all?(...)` and `any?(...)`
* `can?(:other_ability)` delegates to the rules that apply to `:other_ability`. Note that this is distinct from the instance method `can?`, which can check dynamically - this only configures a delegation to another ability.
## Scores, Order, Performance
To see how the rules get evaluated into a judgment, it is useful in a console to use `policy.debug(:some_ability)`. This will print the rules in the order they are evaluated.
When a policy is asked whether a particular ability is allowed (`policy.allowed?(:some_ability)`), it does not necessarily have to compute all the conditions on the policy. First, only the rules relevant to that particular ability are selected. Then, the execution model takes advantage of short-circuiting, and attempts to sort rules based on a heuristic of how expensive they will be to calculate. The sorting is dynamic and cache-aware, so that previously calculated conditions will be considered first, before computing other conditions.
## Scope
Sometimes, a condition will only use data from `@user` or only from `@subject`. In this case, we want to change the scope of the caching, so that we don't recalculate conditions unnecessarily. For example, given:
``` ruby
class FooPolicy < BasePolicy
condition(:expensive_condition) { @subject.expensive_query? }
rule { expensive_condition }.enable :some_ability
end
```
Naively, if we call `Ability.can?(user1, :some_ability, foo)` and `Ability.can?(user2, :some_ability, foo)`, we would have to calculate the condition twice - since they are for different users. But if we use the `scope: :subject` option:
``` ruby
condition(:expensive_condition, scope: :subject) { @subject.expensive_query? }
```
then the result of the condition will be cached globally only based on the subject - so it will not be calculated repeatedly for different users. Similarly, `scope: :user` will cache only based on the user.
**DANGER**: If you use a `:scope` option when the condition actually uses data from
both user and subject (including a simple anonymous check!) your result will be cached at too global of a scope and will result in cache bugs.
Sometimes we are checking permissions for a lot of users for one subject, or a lot of subjects for one user. In this case, we want to set a *preferred scope* - i.e. tell the system that we prefer rules that can be cached on the repeated parameter. For example, in `Ability.users_that_can_read_project`:
``` ruby
def users_that_can_read_project(users, project)
DeclarativePolicy.subject_scope do
users.select { |u| allowed?(u, :read_project, project) }
end
end
```
This will, for example, prefer checking `project.public?` to checking `user.admin?`.
## Delegation
Delegation is the inclusion of rules from another policy, on a different subject. For example,
``` ruby
class FooPolicy < BasePolicy
delegate { @subject.project }
end
```
will include all rules from `ProjectPolicy`. The delegated conditions will be evaluated with the correct delegated subject, and will be sorted along with the regular rules in the policy. Note that only the relevant rules for a particular ability will actually be considered.
lib/api/projects.rb
View file @ 8c5538be
require 'declarative_policy'
module API module API
# Projects API # Projects API
class Projects < Grape::API class Projects < Grape::API
...@@ -396,7 +398,7 @@ module API ...@@ -396,7 +398,7 @@ module API
use :pagination use :pagination
end end
get ':id/users' do get ':id/users' do
users = user_project.team.users users = DeclarativePolicy.subject_scope { user_project.team.users }
users = users.search(params[:search]) if params[:search].present? users = users.search(params[:search]) if params[:search].present?
present paginate(users), with: Entities::UserBasic present paginate(users), with: Entities::UserBasic
......
lib/declarative_policy.rb 0 → 100644
View file @ 8c5538be
require_dependency 'declarative_policy/cache'
require_dependency 'declarative_policy/condition'
require_dependency 'declarative_policy/dsl'
require_dependency 'declarative_policy/preferred_scope'
require_dependency 'declarative_policy/rule'
require_dependency 'declarative_policy/runner'
require_dependency 'declarative_policy/step'
require_dependency 'declarative_policy/base'
module DeclarativePolicy
class << self
def policy_for(user, subject, opts = {})
cache = opts[:cache] || {}
key = Cache.policy_key(user, subject)
cache[key] ||= class_for(subject).new(user, subject, opts)
end
def class_for(subject)
return GlobalPolicy if subject == :global
return NilPolicy if subject.nil?
subject = find_delegate(subject)
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 Base. We can't use #is_a? because that
# tests for *instances*, not *subclasses*.
return policy_class if policy_class < Base
rescue NameError
nil
end
end
raise "no policy for #{subject.class.name}"
end
private
def find_delegate(subject)
seen = Set.new
while subject.respond_to?(:declarative_policy_delegate)
raise ArgumentError, "circular delegations" if seen.include?(subject.object_id)
seen << subject.object_id
subject = subject.declarative_policy_delegate
end
subject
end
end
end
lib/declarative_policy/base.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
class Base
# A map of ability => list of rules together with :enable
# or :prevent actions. Used to look up which rules apply to
# a given ability. See Base.ability_map
class AbilityMap
attr_reader :map
def initialize(map = {})
@map = map
end
# This merge behavior is different than regular hashes - if both
# share a key, the values at that key are concatenated, rather than
# overridden.
def merge(other)
conflict_proc = proc { |key, my_val, other_val| my_val + other_val }
AbilityMap.new(@map.merge(other.map, &conflict_proc))
end
def actions(key)
@map[key] ||= []
end
def enable(key, rule)
actions(key) << [:enable, rule]
end
def prevent(key, rule)
actions(key) << [:prevent, rule]
end
end
class << self
# The `own_ability_map` vs `ability_map` distinction is used so that
# the data structure is properly inherited - with subclasses recursively
# merging their parent class.
#
# This pattern is also used for conditions, global_actions, and delegations.
def ability_map
if self == Base
own_ability_map
else
superclass.ability_map.merge(own_ability_map)
end
end
def own_ability_map
@own_ability_map ||= AbilityMap.new
end
# an inheritable map of conditions, by name
def conditions
if self == Base
own_conditions
else
superclass.conditions.merge(own_conditions)
end
end
def own_conditions
@own_conditions ||= {}
end
# a list of global actions, generated by `prevent_all`. these aren't
# stored in `ability_map` because they aren't indexed by a particular
# ability.
def global_actions
if self == Base
own_global_actions
else
superclass.global_actions + own_global_actions
end
end
def own_global_actions
@own_global_actions ||= []
end
# an inheritable map of delegations, indexed by name (which may be
# autogenerated)
def delegations
if self == Base
own_delegations
else
superclass.delegations.merge(own_delegations)
end
end
def own_delegations
@own_delegations ||= {}
end
# all the [rule, action] pairs that apply to a particular ability.
# we combine the specific ones looked up in ability_map with the global
# ones.
def configuration_for(ability)
ability_map.actions(ability) + global_actions
end
### declaration methods ###
def delegate(name = nil, &delegation_block)
if name.nil?
@delegate_name_counter ||= 0
@delegate_name_counter += 1
name = :"anonymous_#{@delegate_name_counter}"
end
name = name.to_sym
if delegation_block.nil?
delegation_block = proc { @subject.__send__(name) }
end
own_delegations[name] = delegation_block
end
# Declares a rule, constructed using RuleDsl, and returns
# a PolicyDsl which is used for registering the rule with
# this class. PolicyDsl will call back into Base.enable_when,
# Base.prevent_when, and Base.prevent_all_when.
def rule(&b)
rule = RuleDsl.new(self).instance_eval(&b)
PolicyDsl.new(self, rule)
end
# A hash in which to store calls to `desc` and `with_scope`, etc.
def last_options
@last_options ||= {}.with_indifferent_access
end
# retrieve and zero out the previously set options (used in .condition)
def last_options!
last_options.tap { @last_options = nil }
end
# Declare a description for the following condition. Currently unused,
# but opens the potential for explaining to users why they were or were
# not able to do something.
def desc(description)
last_options[:description] = description
end
def with_options(opts = {})
last_options.merge!(opts)
end
def with_scope(scope)
with_options scope: scope
end
def with_score(score)
with_options score: score
end
# Declares a condition. It gets stored in `own_conditions`, and generates
# a query method based on the condition's name.
def condition(name, opts = {}, &value)
name = name.to_sym
opts = last_options!.merge(opts)
opts[:context_key] ||= self.name
condition = Condition.new(name, opts, &value)
self.own_conditions[name] = condition
define_method(:"#{name}?") { condition(name).pass? }
end
# These next three methods are mainly called from PolicyDsl,
# and are responsible for "inverting" the relationship between
# an ability and a rule. We store in `ability_map` a map of
# abilities to rules that affect them, together with a
# symbol indicating :prevent or :enable.
def enable_when(abilities, rule)
abilities.each { |a| own_ability_map.enable(a, rule) }
end
def prevent_when(abilities, rule)
abilities.each { |a| own_ability_map.prevent(a, rule) }
end
# we store global prevents (from `prevent_all`) separately,
# so that they can be combined into every decision made.
def prevent_all_when(rule)
own_global_actions << [:prevent, rule]
end
end
# A policy object contains a specific user and subject on which
# to compute abilities. For this reason it's sometimes called
# "context" within the framework.
#
# It also stores a reference to the cache, so it can be used
# to cache computations by e.g. ManifestCondition.
attr_reader :user, :subject, :cache
def initialize(user, subject, opts = {})
@user = user
@subject = subject
@cache = opts[:cache] || {}
end
# helper for checking abilities on this and other subjects
# for the current user.
def can?(ability, new_subject = :_self)
return allowed?(ability) if new_subject == :_self
policy_for(new_subject).allowed?(ability)
end
# This is the main entry point for permission checks. It constructs
# or looks up a Runner for the given ability and asks it if it passes.
def allowed?(*abilities)
abilities.all? { |a| runner(a).pass? }
end
# The inverse of #allowed?, used mainly in specs.
def disallowed?(*abilities)
abilities.all? { |a| !runner(a).pass? }
end
# computes the given ability and prints a helpful debugging output
# showing which
def debug(ability, *a)
runner(ability).debug(*a)
end
desc "Unknown user"
condition(:anonymous, scope: :user, score: 0) { @user.nil? }
desc "By default"
condition(:default, scope: :global, score: 0) { true }
def repr
subject_repr =
if @subject.respond_to?(:id)
"#{@subject.class.name}/#{@subject.id}"
else
@subject.inspect
end
user_repr =
if @user
@user.to_reference
else
"<anonymous>"
end
"(#{user_repr} : #{subject_repr})"
end
def inspect
"#<#{self.class.name} #{repr}>"
end
# returns a Runner for the given ability, capable of computing whether
# the ability is allowed. Runners are cached on the policy (which itself
# is cached on @cache), and caches its result. This is how we perform caching
# at the ability level.
def runner(ability)
ability = ability.to_sym
@runners ||= {}
@runners[ability] ||=
begin
delegated_runners = delegated_policies.values.compact.map { |p| p.runner(ability) }
own_runner = Runner.new(own_steps(ability))
delegated_runners.inject(own_runner, &:merge_runner)
end
end
# Helpers for caching. Used by ManifestCondition in performing condition
# computation.
#
# NOTE we can't use ||= here because the value might be the
# boolean `false`
def cache(key, &b)
return @cache[key] if cached?(key)
@cache[key] = yield
end
def cached?(key)
!@cache[key].nil?
end
# returns a ManifestCondition capable of computing itself. The computation
# will use our own @cache.
def condition(name)
name = name.to_sym
@_conditions ||= {}
@_conditions[name] ||=
begin
raise "invalid condition #{name}" unless self.class.conditions.key?(name)
ManifestCondition.new(self.class.conditions[name], self)
end
end
# used in specs - returns true if there is no possible way for any action
# to be allowed, determined only by the global :prevent_all rules.
def banned?
global_steps = self.class.global_actions.map { |(action, rule)| Step.new(self, rule, action) }
!Runner.new(global_steps).pass?
end
# A list of other policies that we've delegated to (see `Base.delegate`)
def delegated_policies
@delegated_policies ||= self.class.delegations.transform_values do |block|
new_subject = instance_eval(&block)
# never delegate to nil, as that would immediately prevent_all
next if new_subject.nil?
policy_for(new_subject)
end
end
def policy_for(other_subject)
DeclarativePolicy.policy_for(@user, other_subject, cache: @cache)
end
protected
# constructs steps that come from this policy and not from any delegations
def own_steps(ability)
rules = self.class.configuration_for(ability)
rules.map { |(action, rule)| Step.new(self, rule, action) }
end
end
end
lib/declarative_policy/cache.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
module Cache
class << self
def user_key(user)
return '<anonymous>' if user.nil?
id_for(user)
end
def policy_key(user, subject)
u = user_key(user)
s = subject_key(subject)
"/dp/policy/#{u}/#{s}"
end
def subject_key(subject)
return '<nil>' if subject.nil?
return subject.inspect if subject.is_a?(Symbol)
"#{subject.class.name}:#{id_for(subject)}"
end
private
def id_for(obj)
if obj.respond_to?(:id) && obj.id
obj.id.to_s
else
"##{obj.object_id}"
end
end
end
end
end
lib/declarative_policy/condition.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
# A Condition is the data structure that is created by the
# `condition` declaration on DeclarativePolicy::Base. It is
# more or less just a struct of the data passed to that
# declaration. It holds on to the block to be instance_eval'd
# on a context (instance of Base) later, via #compute.
class Condition
attr_reader :name, :description, :scope
attr_reader :manual_score
attr_reader :context_key
def initialize(name, opts = {}, &compute)
@name = name
@compute = compute
@scope = opts.fetch(:scope, :normal)
@description = opts.delete(:description)
@context_key = opts[:context_key]
@manual_score = opts.fetch(:score, nil)
end
def compute(context)
!!context.instance_eval(&@compute)
end
def key
"#{@context_key}/#{@name}"
end
end
# In contrast to a Condition, a ManifestCondition contains
# a Condition and a context object, and is capable of calculating
# a result itself. This is the return value of Base#condition.
class ManifestCondition
def initialize(condition, context)
@condition = condition
@context = context
end
# The main entry point - does this condition pass? We reach into
# the context's cache here so that we can share in the global
# cache (often RequestStore or similar).
def pass?
@context.cache(cache_key) { @condition.compute(@context) }
end
# Whether we've already computed this condition.
def cached?
@context.cached?(cache_key)
end
# This is used to score Rule::Condition. See Rule::Condition#score
# and Runner#steps_by_score for how scores are used.
#
# The number here is intended to represent, abstractly, how
# expensive it would be to calculate this condition.
#
# See #cache_key for info about @condition.scope.
def score
# If we've been cached, no computation is necessary.
return 0 if cached?
# Use the override from condition(score: ...) if present
return @condition.manual_score if @condition.manual_score
# Global scope rules are cheap due to max cache sharing
return 2 if @condition.scope == :global
# "Normal" rules can't share caches with any other policies
return 16 if @condition.scope == :normal
# otherwise, we're :user or :subject scope, so it's 4 if
# the caller has declared a preference
return 4 if @condition.scope == DeclarativePolicy.preferred_scope
# and 8 for all other :user or :subject scope conditions.
8
end
private
# This method controls the caching for the condition. This is where
# the condition(scope: ...) option comes into play. Notice that
# depending on the scope, we may cache only by the user or only by
# the subject, resulting in sharing across different policy objects.
def cache_key
case @condition.scope
when :normal then "/dp/condition/#{@condition.key}/#{user_key},#{subject_key}"
when :user then "/dp/condition/#{@condition.key}/#{user_key}"
when :subject then "/dp/condition/#{@condition.key}/#{subject_key}"
when :global then "/dp/condition/#{@condition.key}"
else raise 'invalid scope'
end
end
def user_key
Cache.user_key(@context.user)
end
def subject_key
Cache.subject_key(@context.subject)
end
end
end
lib/declarative_policy/dsl.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
# The DSL evaluation context inside rule { ... } blocks.
# Responsible for creating and combining Rule objects.
#
# See Base.rule
class RuleDsl
def initialize(context_class)
@context_class = context_class
end
def can?(ability)
Rule::Ability.new(ability)
end
def all?(*rules)
Rule::And.make(rules)
end
def any?(*rules)
Rule::Or.make(rules)
end
def none?(*rules)
~Rule::Or.new(rules)
end
def cond(condition)
Rule::Condition.new(condition)
end
def delegate(delegate_name, condition)
Rule::DelegatedCondition.new(delegate_name, condition)
end
def method_missing(m, *a, &b)
return super unless a.size == 0 && !block_given?
if @context_class.delegations.key?(m)
DelegateDsl.new(self, m)
else
cond(m.to_sym)
end
end
end
# Used when the name of a delegate is mentioned in
# the rule DSL.
class DelegateDsl
def initialize(rule_dsl, delegate_name)
@rule_dsl = rule_dsl
@delegate_name = delegate_name
end
def method_missing(m, *a, &b)
return super unless a.size == 0 && !block_given?
@rule_dsl.delegate(@delegate_name, m)
end
end
# The return value of a rule { ... } declaration.
# Can call back to register rules with the containing
# Policy class (context_class here). See Base.rule
#
# Note that the #policy method just performs an #instance_eval,
# which is useful for multiple #enable or #prevent callse.
#
# Also provides a #method_missing proxy to the context
# class's class methods, so that helper methods can be
# defined and used in a #policy { ... } block.
class PolicyDsl
def initialize(context_class, rule)
@context_class = context_class
@rule = rule
end
def policy(&b)
instance_eval(&b)
end
def enable(*abilities)
@context_class.enable_when(abilities, @rule)
end
def prevent(*abilities)
@context_class.prevent_when(abilities, @rule)
end
def prevent_all
@context_class.prevent_all_when(@rule)
end
def method_missing(m, *a, &b)
return super unless @context_class.respond_to?(m)
@context_class.__send__(m, *a, &b)
end
def respond_to_missing?(m)
@context_class.respond_to?(m) || super
end
end
end
lib/declarative_policy/preferred_scope.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
PREFERRED_SCOPE_KEY = :"DeclarativePolicy.preferred_scope"
class << self
def with_preferred_scope(scope, &b)
Thread.current[PREFERRED_SCOPE_KEY], old_scope = scope, Thread.current[PREFERRED_SCOPE_KEY]
yield
ensure
Thread.current[PREFERRED_SCOPE_KEY] = old_scope
end
def preferred_scope
Thread.current[PREFERRED_SCOPE_KEY]
end
def user_scope(&b)
with_preferred_scope(:user, &b)
end
def subject_scope(&b)
with_preferred_scope(:subject, &b)
end
def preferred_scope=(scope)
Thread.current[PREFERRED_SCOPE_KEY] = scope
end
end
end
lib/declarative_policy/rule.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
module Rule
# A Rule is the object that results from the `rule` declaration,
# usually built using the DSL in `RuleDsl`. It is a basic logical
# combination of building blocks, and is capable of deciding,
# given a context (instance of DeclarativePolicy::Base) whether it
# passes or not. Note that this decision doesn't by itself know
# how that affects the actual ability decision - for that, a
# `Step` is used.
class Base
def self.make(*a)
new(*a).simplify
end
# true or false whether this rule passes.
# `context` is a policy - an instance of
# DeclarativePolicy::Base.
def pass?(context)
raise 'abstract'
end
# same as #pass? except refuses to do any I/O,
# returning nil if the result is not yet cached.
# used for accurately scoring And/Or
def cached_pass?(context)
raise 'abstract'
end
# abstractly, how long would it take to compute
# this rule? lower-scored rules are tried first.
def score(context)
raise 'abstract'
end
# unwrap double negatives and nested and/or
def simplify
self
end
# convenience combination methods
def or(other)
Or.make([self, other])
end
def and(other)
And.make([self, other])
end
def negate
Not.make(self)
end
alias_method :|, :or
alias_method :&, :and
alias_method :~@, :negate
def inspect
"#<Rule #{repr}>"
end
end
# A rule that checks a condition. This is the
# type of rule that results from a basic bareword
# in the rule dsl (see RuleDsl#method_missing).
class Condition < Base
def initialize(name)
@name = name
end
# we delegate scoring to the condition. See
# ManifestCondition#score.
def score(context)
context.condition(@name).score
end
# Let the ManifestCondition from the context
# decide whether we pass.
def pass?(context)
context.condition(@name).pass?
end
# returns nil unless it's already cached
def cached_pass?(context)
condition = context.condition(@name)
return nil unless condition.cached?
condition.pass?
end
def description(context)
context.class.conditions[@name].description
end
def repr
@name.to_s
end
end
# A rule constructed from DelegateDsl - using a condition from a
# delegated policy.
class DelegatedCondition < Base
# Internal use only - this is rescued each time it's raised.
MissingDelegate = Class.new(StandardError)
def initialize(delegate_name, name)
@delegate_name = delegate_name
@name = name
end
def delegated_context(context)
policy = context.delegated_policies[@delegate_name]
raise MissingDelegate if policy.nil?
policy
end
def score(context)
delegated_context(context).condition(@name).score
rescue MissingDelegate
0
end
def cached_pass?(context)
condition = delegated_context(context).condition(@name)
return nil unless condition.cached?
condition.pass?
rescue MissingDelegate
false
end
def pass?(context)
delegated_context(context).condition(@name).pass?
rescue MissingDelegate
false
end
def repr
"#{@delegate_name}.#{@name}"
end
end
# A rule constructed from RuleDsl#can?. Computes a different ability
# on the same subject.
class Ability < Base
attr_reader :ability
def initialize(ability)
@ability = ability
end
# We ask the ability's runner for a score
def score(context)
context.runner(@ability).score
end
def pass?(context)
context.allowed?(@ability)
end
def cached_pass?(context)
runner = context.runner(@ability)
return nil unless runner.cached?
runner.pass?
end
def description(context)
"User can #{@ability.inspect}"
end
def repr
"can?(#{@ability.inspect})"
end
end
# Logical `and`, containing a list of rules. Only passes
# if all of them do.
class And < Base
attr_reader :rules
def initialize(rules)
@rules = rules
end
def simplify
simplified_rules = @rules.flat_map do |rule|
simplified = rule.simplify
case simplified
when And then simplified.rules
else [simplified]
end
end
And.new(simplified_rules)
end
def score(context)
return 0 unless cached_pass?(context).nil?
# note that cached rules will have score 0 anyways.
@rules.map { |r| r.score(context) }.inject(0, :+)
end
def pass?(context)
# try to find a cached answer before
# checking in order
cached = cached_pass?(context)
return cached unless cached.nil?
@rules.all? { |r| r.pass?(context) }
end
def cached_pass?(context)
passes = @rules.map { |r| r.cached_pass?(context) }
return false if passes.any? { |p| p == false }
return true if passes.all? { |p| p == true }
nil
end
def repr
"all?(#{rules.map(&:repr).join(', ')})"
end
end
# Logical `or`. Mirrors And.
class Or < Base
attr_reader :rules
def initialize(rules)
@rules = rules
end
def pass?(context)
cached = cached_pass?(context)
return cached unless cached.nil?
@rules.any? { |r| r.pass?(context) }
end
def simplify
simplified_rules = @rules.flat_map do |rule|
simplified = rule.simplify
case simplified
when Or then simplified.rules
else [simplified]
end
end
Or.new(simplified_rules)
end
def cached_pass?(context)
passes = @rules.map { |r| r.cached_pass?(context) }
return true if passes.any? { |p| p == true }
return false if passes.all? { |p| p == false }
nil
end
def score(context)
return 0 unless cached_pass?(context).nil?
@rules.map { |r| r.score(context) }.inject(0, :+)
end
def repr
"any?(#{@rules.map(&:repr).join(', ')})"
end
end
class Not < Base
attr_reader :rule
def initialize(rule)
@rule = rule
end
def simplify
case @rule
when And then Or.new(@rule.rules.map(&:negate)).simplify
when Or then And.new(@rule.rules.map(&:negate)).simplify
when Not then @rule.rule.simplify
else Not.new(@rule.simplify)
end
end
def pass?(context)
!@rule.pass?(context)
end
def cached_pass?(context)
case @rule.cached_pass?(context)
when nil then nil
when true then false
when false then true
end
end
def score(context)
@rule.score(context)
end
def repr
"~#{@rule.repr}"
end
end
end
end
lib/declarative_policy/runner.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
class Runner
class State
def initialize
@enabled = false
@prevented = false
end
def enable!
@enabled = true
end
def enabled?
@enabled
end
def prevent!
@prevented = true
end
def prevented?
@prevented
end
def pass?
!prevented? && enabled?
end
end
# a Runner contains a list of Steps to be run.
attr_reader :steps
def initialize(steps)
@steps = steps
end
# We make sure only to run any given Runner once,
# and just continue to use the resulting @state
# that's left behind.
def cached?
!!@state
end
# used by Rule::Ability. See #steps_by_score
def score
return 0 if cached?
steps.map(&:score).inject(0, :+)
end
def merge_runner(other)
Runner.new(@steps + other.steps)
end
# The main entry point, called for making an ability decision.
# See #run and DeclarativePolicy::Base#can?
def pass?
run unless cached?
@state.pass?
end
# see DeclarativePolicy::Base#debug
def debug(out = $stderr)
run(out)
end
private
def flatten_steps!
@steps = @steps.flat_map { |s| s.flattened(@steps) }
end
# This method implements the semantic of "one enable and no prevents".
# It relies on #steps_by_score for the main loop, and updates @state
# with the result of the step.
def run(debug = nil)
@state = State.new
steps_by_score do |step, score|
passed = nil
case step.action
when :enable then
# we only check :enable actions if they have a chance of
# changing the outcome - if no other rule has enabled or
# prevented.
unless @state.enabled? || @state.prevented?
passed = step.pass?
@state.enable! if passed
end
debug << inspect_step(step, score, passed) if debug
when :prevent then
# we only check :prevent actions if the state hasn't already
# been prevented.
unless @state.prevented?
passed = step.pass?
if passed
@state.prevent!
return unless debug
end
end
debug << inspect_step(step, score, passed) if debug
else raise "invalid action #{step.action.inspect}"
end
end
@state
end
# This is the core spot where all those `#score` methods matter.
# It is critcal for performance to run steps in the correct order,
# so that we don't compute expensive conditions (potentially n times
# if we're called on, say, a large list of users).
#
# In order to determine the cheapest step to run next, we rely on
# Step#score, which returns a numerical rating of how expensive
# it would be to calculate - the lower the better. It would be
# easy enough to statically sort by these scores, but we can do
# a little better - the scores are cache-aware (conditions that
# are already in the cache have score 0), which means that running
# a step can actually change the scores of other steps.
#
# So! The way we sort here involves re-scoring at every step. This
# is by necessity quadratic, but most of the time the number of steps
# will be low. But just in case, if the number of steps exceeds 50,
# we print a warning and fall back to a static sort.
#
# For each step, we yield the step object along with the computed score
# for debugging purposes.
def steps_by_score(&b)
flatten_steps!
if @steps.size > 50
warn "DeclarativePolicy: large number of steps (#{steps.size}), falling back to static sort"
@steps.map { |s| [s.score, s] }.sort_by { |(score, _)| score }.each do |(score, step)|
yield step, score
end
return
end
steps = Set.new(@steps)
loop do
return if steps.empty?
# if the permission hasn't yet been enabled and we only have
# prevent steps left, we short-circuit the state here
@state.prevent! if !@state.enabled? && steps.all?(&:prevent?)
lowest_score = Float::INFINITY
next_step = nil
steps.each do |step|
score = step.score
if score < lowest_score
next_step = step
lowest_score = score
end
end
steps.delete(next_step)
yield next_step, lowest_score
end
end
# Formatter for debugging output.
def inspect_step(step, original_score, passed)
symbol =
case passed
when true then '+'
when false then '-'
when nil then ' '
end
"#{symbol} [#{original_score.to_i}] #{step.repr}\n"
end
end
end
lib/declarative_policy/step.rb 0 → 100644
View file @ 8c5538be
module DeclarativePolicy
# This object represents one step in the runtime decision of whether
# an ability is allowed. It contains a Rule and a context (instance
# of DeclarativePolicy::Base), which contains the user, the subject,
# and the cache. It also contains an "action", which is the symbol
# :prevent or :enable.
class Step
attr_reader :context, :rule, :action
def initialize(context, rule, action)
@context = context
@rule = rule
@action = action
end
# In the flattening process, duplicate steps may be generated in the
# same rule. This allows us to eliminate those (see Runner#steps_by_score
# and note its use of a Set)
def ==(other)
@context == other.context && @rule == other.rule && @action == other.action
end
# In the runner, steps are sorted dynamically by score, so that
# we are sure to compute them in close to the optimal order.
#
# See also Rule#score, ManifestCondition#score, and Runner#steps_by_score.
def score
# we slightly prefer the preventative actions
# since they are more likely to short-circuit
case @action
when :prevent
@rule.score(@context) * (7.0 / 8)
when :enable
@rule.score(@context)
end
end
def with_action(action)
Step.new(@context, @rule, action)
end
def enable?
@action == :enable
end
def prevent?
@action == :prevent
end
# This rather complex method allows us to split rules into parts so that
# they can be sorted independently for better optimization
def flattened(roots)
case @rule
when Rule::Or
# A single `Or` step is the same as each of its elements as separate steps
@rule.rules.flat_map { |r| Step.new(@context, r, @action).flattened(roots) }
when Rule::Ability
# This looks like a weird micro-optimization but it buys us quite a lot
# in some cases. If we depend on an Ability (i.e. a `can?(...)` rule),
# and that ability *only* has :enable actions (modulo some actions that
# we already have taken care of), then its rules can be safely inlined.
steps = @context.runner(@rule.ability).steps.reject { |s| roots.include?(s) }
if steps.all?(&:enable?)
# in the case that we are a :prevent step, each inlined step becomes
# an independent :prevent, even though it was an :enable in its initial
# context.
steps.map! { |s| s.with_action(:prevent) } if prevent?
steps.flat_map { |s| s.flattened(roots) }
else
[self]
end
else
[self]
end
end
def pass?
@rule.pass?(@context)
end
def repr
"#{@action} when #{@rule.repr} (#{@context.repr})"
end
end
end
lib/gitlab/allowable.rb
View file @ 8c5538be
module Gitlab module Gitlab
module Allowable module Allowable
def can?(user, action, subject = :global) def can?(*args)
Ability.allowed?(user, action, subject) Ability.allowed?(*args)
end end
end end
end end
lib/gitlab/view/presenter/base.rb
View file @ 8c5538be
...@@ -15,6 +15,11 @@ module Gitlab ...@@ -15,6 +15,11 @@ module Gitlab
super(user, action, overriden_subject || subject) super(user, action, overriden_subject || subject)
end end
# delegate all #can? queries to the subject
def declarative_policy_delegate
subject
end
class_methods do class_methods do
def presenter? def presenter?
true true
......
spec/models/ability_spec.rb
View file @ 8c5538be
...@@ -2,8 +2,8 @@ require 'spec_helper' ...@@ -2,8 +2,8 @@ require 'spec_helper'
describe Ability, lib: true do describe Ability, lib: true do
context 'using a nil subject' do context 'using a nil subject' do
it 'is always empty' do it 'has no permissions' do
expect(Ability.allowed(nil, nil).to_set).to be_empty expect(Ability.policy_for(nil, nil)).to be_banned
end end
end end
...@@ -255,12 +255,15 @@ describe Ability, lib: true do ...@@ -255,12 +255,15 @@ describe Ability, lib: true do
describe '.project_disabled_features_rules' do describe '.project_disabled_features_rules' do
let(:project) { create(:empty_project, :wiki_disabled) } let(:project) { create(:empty_project, :wiki_disabled) }
subject { described_class.allowed(project.owner, project) } subject { described_class.policy_for(project.owner, project) }
context 'wiki named abilities' do context 'wiki named abilities' do
it 'disables wiki abilities if the project has no wiki' do it 'disables wiki abilities if the project has no wiki' do
expect(project).to receive(:has_external_wiki?).and_return(false) expect(project).to receive(:has_external_wiki?).and_return(false)
expect(subject).not_to include(:read_wiki, :create_wiki, :update_wiki, :admin_wiki) expect(subject).not_to be_allowed(:read_wiki)
expect(subject).not_to be_allowed(:create_wiki)
expect(subject).not_to be_allowed(:update_wiki)
expect(subject).not_to be_allowed(:admin_wiki)
end end
end end
end end
......
spec/policies/base_policy_spec.rb
View file @ 8c5538be
...@@ -3,17 +3,17 @@ require 'spec_helper' ...@@ -3,17 +3,17 @@ require 'spec_helper'
describe BasePolicy, models: true do describe BasePolicy, models: true do
describe '.class_for' do describe '.class_for' do
it 'detects policy class based on the subject ancestors' do it 'detects policy class based on the subject ancestors' do
expect(described_class.class_for(GenericCommitStatus.new)).to eq(CommitStatusPolicy) expect(DeclarativePolicy.class_for(GenericCommitStatus.new)).to eq(CommitStatusPolicy)
end end
it 'detects policy class for a presented subject' do it 'detects policy class for a presented subject' do
presentee = Ci::BuildPresenter.new(Ci::Build.new) presentee = Ci::BuildPresenter.new(Ci::Build.new)
expect(described_class.class_for(presentee)).to eq(Ci::BuildPolicy) expect(DeclarativePolicy.class_for(presentee)).to eq(Ci::BuildPolicy)
end end
it 'uses GlobalPolicy when :global is given' do it 'uses GlobalPolicy when :global is given' do
expect(described_class.class_for(:global)).to eq(GlobalPolicy) expect(DeclarativePolicy.class_for(:global)).to eq(GlobalPolicy)
end end
end end
end end
spec/policies/ci/build_policy_spec.rb
View file @ 8c5538be
...@@ -5,8 +5,8 @@ describe Ci::BuildPolicy, :models do ...@@ -5,8 +5,8 @@ describe Ci::BuildPolicy, :models do
let(:build) { create(:ci_build, pipeline: pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) }
let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:pipeline) { create(:ci_empty_pipeline, project: project) }
let(:policies) do let(:policy) do
described_class.abilities(user, build).to_set described_class.new(user, build)
end end
shared_context 'public pipelines disabled' do shared_context 'public pipelines disabled' do
...@@ -21,7 +21,7 @@ describe Ci::BuildPolicy, :models do ...@@ -21,7 +21,7 @@ describe Ci::BuildPolicy, :models do
context 'when public builds are enabled' do context 'when public builds are enabled' do
it 'does not include ability to read build' do it 'does not include ability to read build' do
expect(policies).not_to include :read_build expect(policy).not_to be_allowed :read_build
end end
end end
...@@ -29,7 +29,7 @@ describe Ci::BuildPolicy, :models do ...@@ -29,7 +29,7 @@ describe Ci::BuildPolicy, :models do
include_context 'public pipelines disabled' include_context 'public pipelines disabled'
it 'does not include ability to read build' do it 'does not include ability to read build' do
expect(policies).not_to include :read_build expect(policy).not_to be_allowed :read_build
end end
end end
end end
...@@ -39,7 +39,7 @@ describe Ci::BuildPolicy, :models do ...@@ -39,7 +39,7 @@ describe Ci::BuildPolicy, :models do
context 'when public builds are enabled' do context 'when public builds are enabled' do
it 'includes ability to read build' do it 'includes ability to read build' do
expect(policies).to include :read_build expect(policy).to be_allowed :read_build
end end
end end
...@@ -47,7 +47,7 @@ describe Ci::BuildPolicy, :models do ...@@ -47,7 +47,7 @@ describe Ci::BuildPolicy, :models do
include_context 'public pipelines disabled' include_context 'public pipelines disabled'
it 'does not include ability to read build' do it 'does not include ability to read build' do
expect(policies).not_to include :read_build expect(policy).not_to be_allowed :read_build
end end
end end
end end
...@@ -62,7 +62,7 @@ describe Ci::BuildPolicy, :models do ...@@ -62,7 +62,7 @@ describe Ci::BuildPolicy, :models do
context 'when public builds are enabled' do context 'when public builds are enabled' do
it 'includes ability to read build' do it 'includes ability to read build' do
expect(policies).to include :read_build expect(policy).to be_allowed :read_build
end end
end end
...@@ -70,7 +70,7 @@ describe Ci::BuildPolicy, :models do ...@@ -70,7 +70,7 @@ describe Ci::BuildPolicy, :models do
include_context 'public pipelines disabled' include_context 'public pipelines disabled'
it 'does not include ability to read build' do it 'does not include ability to read build' do
expect(policies).not_to include :read_build expect(policy).not_to be_allowed :read_build
end end
end end
end end
...@@ -82,7 +82,7 @@ describe Ci::BuildPolicy, :models do ...@@ -82,7 +82,7 @@ describe Ci::BuildPolicy, :models do
context 'when public builds are enabled' do context 'when public builds are enabled' do
it 'includes ability to read build' do it 'includes ability to read build' do
expect(policies).to include :read_build expect(policy).to be_allowed :read_build
end end
end end
...@@ -90,7 +90,7 @@ describe Ci::BuildPolicy, :models do ...@@ -90,7 +90,7 @@ describe Ci::BuildPolicy, :models do
include_context 'public pipelines disabled' include_context 'public pipelines disabled'
it 'does not include ability to read build' do it 'does not include ability to read build' do
expect(policies).to include :read_build expect(policy).to be_allowed :read_build
end end
end end
end end
...@@ -115,7 +115,7 @@ describe Ci::BuildPolicy, :models do ...@@ -115,7 +115,7 @@ describe Ci::BuildPolicy, :models do
end end
it 'does not include ability to update build' do it 'does not include ability to update build' do
expect(policies).not_to include :update_build expect(policy).to be_disallowed :update_build
end end
end end
...@@ -125,7 +125,7 @@ describe Ci::BuildPolicy, :models do ...@@ -125,7 +125,7 @@ describe Ci::BuildPolicy, :models do
end end
it 'includes ability to update build' do it 'includes ability to update build' do
expect(policies).to include :update_build expect(policy).to be_allowed :update_build
end end
end end
end end
...@@ -135,7 +135,7 @@ describe Ci::BuildPolicy, :models do ...@@ -135,7 +135,7 @@ describe Ci::BuildPolicy, :models do
let(:build) { create(:ci_build, :manual, pipeline: pipeline) } let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
it 'includes ability to update build' do it 'includes ability to update build' do
expect(policies).to include :update_build expect(policy).to be_allowed :update_build
end end
end end
...@@ -143,7 +143,7 @@ describe Ci::BuildPolicy, :models do ...@@ -143,7 +143,7 @@ describe Ci::BuildPolicy, :models do
let(:build) { create(:ci_build, pipeline: pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) }
it 'includes ability to update build' do it 'includes ability to update build' do
expect(policies).to include :update_build expect(policy).to be_allowed :update_build
end end
end end
end end
......
spec/policies/ci/trigger_policy_spec.rb
View file @ 8c5538be
...@@ -6,36 +6,36 @@ describe Ci::TriggerPolicy, :models do ...@@ -6,36 +6,36 @@ describe Ci::TriggerPolicy, :models do
let(:trigger) { create(:ci_trigger, project: project, owner: owner) } let(:trigger) { create(:ci_trigger, project: project, owner: owner) }
let(:policies) do let(:policies) do
described_class.abilities(user, trigger).to_set described_class.new(user, trigger)
end end
shared_examples 'allows to admin and manage trigger' do shared_examples 'allows to admin and manage trigger' do
it 'does include ability to admin trigger' do it 'does include ability to admin trigger' do
expect(policies).to include :admin_trigger expect(policies).to be_allowed :admin_trigger
end end
it 'does include ability to manage trigger' do it 'does include ability to manage trigger' do
expect(policies).to include :manage_trigger expect(policies).to be_allowed :manage_trigger
end end
end end
shared_examples 'allows to manage trigger' do shared_examples 'allows to manage trigger' do
it 'does not include ability to admin trigger' do it 'does not include ability to admin trigger' do
expect(policies).not_to include :admin_trigger expect(policies).not_to be_allowed :admin_trigger
end end
it 'does include ability to manage trigger' do it 'does include ability to manage trigger' do
expect(policies).to include :manage_trigger expect(policies).to be_allowed :manage_trigger
end end
end end
shared_examples 'disallows to admin and manage trigger' do shared_examples 'disallows to admin and manage trigger' do
it 'does not include ability to admin trigger' do it 'does not include ability to admin trigger' do
expect(policies).not_to include :admin_trigger expect(policies).not_to be_allowed :admin_trigger
end end
it 'does not include ability to manage trigger' do it 'does not include ability to manage trigger' do
expect(policies).not_to include :manage_trigger expect(policies).not_to be_allowed :manage_trigger
end end
end end
......
spec/policies/deploy_key_policy_spec.rb
View file @ 8c5538be
require 'spec_helper' require 'spec_helper'
describe DeployKeyPolicy, models: true do describe DeployKeyPolicy, models: true do
subject { described_class.abilities(current_user, deploy_key).to_set } subject { described_class.new(current_user, deploy_key) }
describe 'updating a deploy_key' do describe 'updating a deploy_key' do
context 'when a regular user' do context 'when a regular user' do
...@@ -16,7 +16,7 @@ describe DeployKeyPolicy, models: true do ...@@ -16,7 +16,7 @@ describe DeployKeyPolicy, models: true do
project.deploy_keys << deploy_key project.deploy_keys << deploy_key
end end
it { is_expected.to include(:update_deploy_key) } it { is_expected.to be_allowed(:update_deploy_key) }
end end
context 'tries to update private deploy key attached to other project' do context 'tries to update private deploy key attached to other project' do
...@@ -27,13 +27,13 @@ describe DeployKeyPolicy, models: true do ...@@ -27,13 +27,13 @@ describe DeployKeyPolicy, models: true do
other_project.deploy_keys << deploy_key other_project.deploy_keys << deploy_key
end end
it { is_expected.not_to include(:update_deploy_key) } it { is_expected.to be_disallowed(:update_deploy_key) }
end end
context 'tries to update public deploy key' do context 'tries to update public deploy key' do
let(:deploy_key) { create(:another_deploy_key, public: true) } let(:deploy_key) { create(:another_deploy_key, public: true) }
it { is_expected.not_to include(:update_deploy_key) } it { is_expected.to be_disallowed(:update_deploy_key) }
end end
end end
...@@ -43,13 +43,13 @@ describe DeployKeyPolicy, models: true do ...@@ -43,13 +43,13 @@ describe DeployKeyPolicy, models: true do
context ' tries to update private deploy key' do context ' tries to update private deploy key' do
let(:deploy_key) { create(:deploy_key, public: false) } let(:deploy_key) { create(:deploy_key, public: false) }
it { is_expected.to include(:update_deploy_key) } it { is_expected.to be_allowed(:update_deploy_key) }
end end
context 'when an admin user tries to update public deploy key' do context 'when an admin user tries to update public deploy key' do
let(:deploy_key) { create(:another_deploy_key, public: true) } let(:deploy_key) { create(:another_deploy_key, public: true) }
it { is_expected.to include(:update_deploy_key) } it { is_expected.to be_allowed(:update_deploy_key) }
end end
end end
end end
......
spec/policies/environment_policy_spec.rb
View file @ 8c5538be
...@@ -8,8 +8,8 @@ describe EnvironmentPolicy do ...@@ -8,8 +8,8 @@ describe EnvironmentPolicy do
create(:environment, :with_review_app, project: project) create(:environment, :with_review_app, project: project)
end end
let(:policies) do let(:policy) do
described_class.abilities(user, environment).to_set described_class.new(user, environment)
end end
describe '#rules' do describe '#rules' do
...@@ -17,7 +17,7 @@ describe EnvironmentPolicy do ...@@ -17,7 +17,7 @@ describe EnvironmentPolicy do
let(:project) { create(:project, :private) } let(:project) { create(:project, :private) }
it 'does not include ability to stop environment' do it 'does not include ability to stop environment' do
expect(policies).not_to include :stop_environment expect(policy).to be_disallowed :stop_environment
end end
end end
...@@ -25,7 +25,7 @@ describe EnvironmentPolicy do ...@@ -25,7 +25,7 @@ describe EnvironmentPolicy do
let(:project) { create(:project, :public) } let(:project) { create(:project, :public) }
it 'does not include ability to stop environment' do it 'does not include ability to stop environment' do
expect(policies).not_to include :stop_environment expect(policy).to be_disallowed :stop_environment
end end
end end
...@@ -38,7 +38,7 @@ describe EnvironmentPolicy do ...@@ -38,7 +38,7 @@ describe EnvironmentPolicy do
context 'when team member has ability to stop environment' do context 'when team member has ability to stop environment' do
it 'does includes ability to stop environment' do it 'does includes ability to stop environment' do
expect(policies).to include :stop_environment expect(policy).to be_allowed :stop_environment
end end
end end
...@@ -49,7 +49,7 @@ describe EnvironmentPolicy do ...@@ -49,7 +49,7 @@ describe EnvironmentPolicy do
end end
it 'does not include ability to stop environment' do it 'does not include ability to stop environment' do
expect(policies).not_to include :stop_environment expect(policy).to be_disallowed :stop_environment
end end
end end
end end
......
spec/policies/group_policy_spec.rb
View file @ 8c5538be
...@@ -36,16 +36,24 @@ describe GroupPolicy, models: true do ...@@ -36,16 +36,24 @@ describe GroupPolicy, models: true do
group.add_owner(owner) group.add_owner(owner)
end end
subject { described_class.abilities(current_user, group).to_set } subject { described_class.new(current_user, group) }
def expect_allowed(*permissions)
permissions.each { |p| is_expected.to be_allowed(p) }
end
def expect_disallowed(*permissions)
permissions.each { |p| is_expected.not_to be_allowed(p) }
end
context 'with no user' do context 'with no user' do
let(:current_user) { nil } let(:current_user) { nil }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.not_to include(*reporter_permissions) expect_disallowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -53,10 +61,10 @@ describe GroupPolicy, models: true do ...@@ -53,10 +61,10 @@ describe GroupPolicy, models: true do
let(:current_user) { guest } let(:current_user) { guest }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.not_to include(*reporter_permissions) expect_disallowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -64,10 +72,10 @@ describe GroupPolicy, models: true do ...@@ -64,10 +72,10 @@ describe GroupPolicy, models: true do
let(:current_user) { reporter } let(:current_user) { reporter }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -75,10 +83,10 @@ describe GroupPolicy, models: true do ...@@ -75,10 +83,10 @@ describe GroupPolicy, models: true do
let(:current_user) { developer } let(:current_user) { developer }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -86,10 +94,10 @@ describe GroupPolicy, models: true do ...@@ -86,10 +94,10 @@ describe GroupPolicy, models: true do
let(:current_user) { master } let(:current_user) { master }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -97,10 +105,10 @@ describe GroupPolicy, models: true do ...@@ -97,10 +105,10 @@ describe GroupPolicy, models: true do
let(:current_user) { owner } let(:current_user) { owner }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.to include(*owner_permissions) expect_allowed(*owner_permissions)
end end
end end
...@@ -108,10 +116,10 @@ describe GroupPolicy, models: true do ...@@ -108,10 +116,10 @@ describe GroupPolicy, models: true do
let(:current_user) { admin } let(:current_user) { admin }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.to include(*owner_permissions) expect_allowed(*owner_permissions)
end end
end end
...@@ -130,16 +138,16 @@ describe GroupPolicy, models: true do ...@@ -130,16 +138,16 @@ describe GroupPolicy, models: true do
nested_group.add_owner(owner) nested_group.add_owner(owner)
end end
subject { described_class.abilities(current_user, nested_group).to_set } subject { described_class.new(current_user, nested_group) }
context 'with no user' do context 'with no user' do
let(:current_user) { nil } let(:current_user) { nil }
it do it do
is_expected.not_to include(:read_group) expect_disallowed(:read_group)
is_expected.not_to include(*reporter_permissions) expect_disallowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -147,10 +155,10 @@ describe GroupPolicy, models: true do ...@@ -147,10 +155,10 @@ describe GroupPolicy, models: true do
let(:current_user) { guest } let(:current_user) { guest }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.not_to include(*reporter_permissions) expect_disallowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -158,10 +166,10 @@ describe GroupPolicy, models: true do ...@@ -158,10 +166,10 @@ describe GroupPolicy, models: true do
let(:current_user) { reporter } let(:current_user) { reporter }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -169,10 +177,10 @@ describe GroupPolicy, models: true do ...@@ -169,10 +177,10 @@ describe GroupPolicy, models: true do
let(:current_user) { developer } let(:current_user) { developer }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -180,10 +188,10 @@ describe GroupPolicy, models: true do ...@@ -180,10 +188,10 @@ describe GroupPolicy, models: true do
let(:current_user) { master } let(:current_user) { master }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -191,10 +199,10 @@ describe GroupPolicy, models: true do ...@@ -191,10 +199,10 @@ describe GroupPolicy, models: true do
let(:current_user) { owner } let(:current_user) { owner }
it do it do
is_expected.to include(:read_group) expect_allowed(:read_group)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.to include(*owner_permissions) expect_allowed(*owner_permissions)
end end
end end
end end
......
spec/policies/issue_policy_spec.rb
View file @ 8c5538be
This diff is collapsed.
spec/policies/personal_snippet_policy_spec.rb
View file @ 8c5538be
...@@ -14,7 +14,7 @@ describe PersonalSnippetPolicy, models: true do ...@@ -14,7 +14,7 @@ describe PersonalSnippetPolicy, models: true do
end end
def permissions(user) def permissions(user)
described_class.abilities(user, snippet).to_set described_class.new(user, snippet)
end end
context 'public snippet' do context 'public snippet' do
...@@ -24,9 +24,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -24,9 +24,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(nil) } subject { permissions(nil) }
it do it do
is_expected.to include(:read_personal_snippet) is_expected.to be_allowed(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet) is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -34,9 +34,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -34,9 +34,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(regular_user) } subject { permissions(regular_user) }
it do it do
is_expected.to include(:read_personal_snippet) is_expected.to be_allowed(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet) is_expected.to be_allowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -44,9 +44,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -44,9 +44,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(snippet.author) } subject { permissions(snippet.author) }
it do it do
is_expected.to include(:read_personal_snippet) is_expected.to be_allowed(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet) is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to include(*author_permissions) is_expected.to be_allowed(*author_permissions)
end end
end end
end end
...@@ -58,9 +58,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -58,9 +58,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(nil) } subject { permissions(nil) }
it do it do
is_expected.not_to include(:read_personal_snippet) is_expected.to be_disallowed(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet) is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -68,9 +68,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -68,9 +68,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(regular_user) } subject { permissions(regular_user) }
it do it do
is_expected.to include(:read_personal_snippet) is_expected.to be_allowed(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet) is_expected.to be_allowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -78,9 +78,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -78,9 +78,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(external_user) } subject { permissions(external_user) }
it do it do
is_expected.not_to include(:read_personal_snippet) is_expected.to be_disallowed(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet) is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -88,9 +88,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -88,9 +88,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(snippet.author) } subject { permissions(snippet.author) }
it do it do
is_expected.to include(:read_personal_snippet) is_expected.to be_allowed(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet) is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to include(*author_permissions) is_expected.to be_allowed(*author_permissions)
end end
end end
end end
...@@ -102,9 +102,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -102,9 +102,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(nil) } subject { permissions(nil) }
it do it do
is_expected.not_to include(:read_personal_snippet) is_expected.to be_disallowed(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet) is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -112,9 +112,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -112,9 +112,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(regular_user) } subject { permissions(regular_user) }
it do it do
is_expected.not_to include(:read_personal_snippet) is_expected.to be_disallowed(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet) is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -122,9 +122,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -122,9 +122,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(external_user) } subject { permissions(external_user) }
it do it do
is_expected.not_to include(:read_personal_snippet) is_expected.to be_disallowed(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet) is_expected.to be_disallowed(:comment_personal_snippet)
is_expected.not_to include(*author_permissions) is_expected.to be_disallowed(*author_permissions)
end end
end end
...@@ -132,9 +132,9 @@ describe PersonalSnippetPolicy, models: true do ...@@ -132,9 +132,9 @@ describe PersonalSnippetPolicy, models: true do
subject { permissions(snippet.author) } subject { permissions(snippet.author) }
it do it do
is_expected.to include(:read_personal_snippet) is_expected.to be_allowed(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet) is_expected.to be_allowed(:comment_personal_snippet)
is_expected.to include(*author_permissions) is_expected.to be_allowed(*author_permissions)
end end
end end
end end
......
spec/policies/project_policy_spec.rb
View file @ 8c5538be
...@@ -73,37 +73,45 @@ describe ProjectPolicy, models: true do ...@@ -73,37 +73,45 @@ describe ProjectPolicy, models: true do
project.team << [reporter, :reporter] project.team << [reporter, :reporter]
end end
def expect_allowed(*permissions)
permissions.each { |p| is_expected.to be_allowed(p) }
end
def expect_disallowed(*permissions)
permissions.each { |p| is_expected.not_to be_allowed(p) }
end
it 'does not include the read_issue permission when the issue author is not a member of the private project' do it 'does not include the read_issue permission when the issue author is not a member of the private project' do
project = create(:empty_project, :private) project = create(:empty_project, :private)
issue = create(:issue, project: project) issue = create(:issue, project: project)
user = issue.author user = issue.author
expect(project.team.member?(issue.author)).to eq(false) expect(project.team.member?(issue.author)).to be false
expect(BasePolicy.class_for(project).abilities(user, project).can_set)
.not_to include(:read_issue)
expect(Ability.allowed?(user, :read_issue, project)).to be_falsy expect(Ability).not_to be_allowed(user, :read_issue, project)
end end
it 'does not include the wiki permissions when the feature is disabled' do context 'when the feature is disabled' do
project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) subject { described_class.new(owner, project) }
wiki_permissions = [:read_wiki, :create_wiki, :update_wiki, :admin_wiki, :download_wiki_code]
permissions = described_class.abilities(owner, project).to_set before do
project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED)
end
expect(permissions).not_to include(*wiki_permissions) it 'does not include the wiki permissions' do
expect_disallowed :read_wiki, :create_wiki, :update_wiki, :admin_wiki, :download_wiki_code
end
end end
context 'abilities for non-public projects' do context 'abilities for non-public projects' do
let(:project) { create(:empty_project, namespace: owner.namespace) } let(:project) { create(:empty_project, namespace: owner.namespace) }
subject { described_class.abilities(current_user, project).to_set } subject { described_class.new(current_user, project) }
context 'with no user' do context 'with no user' do
let(:current_user) { nil } let(:current_user) { nil }
it { is_expected.to be_empty } it { is_expected.to be_banned }
end end
context 'guests' do context 'guests' do
...@@ -114,18 +122,18 @@ describe ProjectPolicy, models: true do ...@@ -114,18 +122,18 @@ describe ProjectPolicy, models: true do
end end
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.not_to include(*reporter_public_build_permissions) expect_disallowed(*reporter_public_build_permissions)
is_expected.not_to include(*team_member_reporter_permissions) expect_disallowed(*team_member_reporter_permissions)
is_expected.not_to include(*developer_permissions) expect_disallowed(*developer_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
context 'public builds enabled' do context 'public builds enabled' do
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.to include(:read_build, :read_pipeline) expect_allowed(:read_build, :read_pipeline)
end end
end end
...@@ -135,8 +143,8 @@ describe ProjectPolicy, models: true do ...@@ -135,8 +143,8 @@ describe ProjectPolicy, models: true do
end end
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.not_to include(:read_build, :read_pipeline) expect_disallowed(:read_build, :read_pipeline)
end end
end end
...@@ -147,8 +155,8 @@ describe ProjectPolicy, models: true do ...@@ -147,8 +155,8 @@ describe ProjectPolicy, models: true do
end end
it do it do
is_expected.not_to include(:read_build) expect_disallowed(:read_build)
is_expected.to include(:read_pipeline) expect_allowed(:read_pipeline)
end end
end end
end end
...@@ -157,12 +165,13 @@ describe ProjectPolicy, models: true do ...@@ -157,12 +165,13 @@ describe ProjectPolicy, models: true do
let(:current_user) { reporter } let(:current_user) { reporter }
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*team_member_reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.not_to include(*developer_permissions) expect_allowed(*team_member_reporter_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*developer_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*master_permissions)
expect_disallowed(*owner_permissions)
end end
end end
...@@ -170,12 +179,12 @@ describe ProjectPolicy, models: true do ...@@ -170,12 +179,12 @@ describe ProjectPolicy, models: true do
let(:current_user) { dev } let(:current_user) { dev }
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*team_member_reporter_permissions) expect_allowed(*team_member_reporter_permissions)
is_expected.to include(*developer_permissions) expect_allowed(*developer_permissions)
is_expected.not_to include(*master_permissions) expect_disallowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -183,12 +192,12 @@ describe ProjectPolicy, models: true do ...@@ -183,12 +192,12 @@ describe ProjectPolicy, models: true do
let(:current_user) { master } let(:current_user) { master }
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*team_member_reporter_permissions) expect_allowed(*team_member_reporter_permissions)
is_expected.to include(*developer_permissions) expect_allowed(*developer_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.not_to include(*owner_permissions) expect_disallowed(*owner_permissions)
end end
end end
...@@ -196,12 +205,12 @@ describe ProjectPolicy, models: true do ...@@ -196,12 +205,12 @@ describe ProjectPolicy, models: true do
let(:current_user) { owner } let(:current_user) { owner }
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.to include(*team_member_reporter_permissions) expect_allowed(*team_member_reporter_permissions)
is_expected.to include(*developer_permissions) expect_allowed(*developer_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.to include(*owner_permissions) expect_allowed(*owner_permissions)
end end
end end
...@@ -209,12 +218,12 @@ describe ProjectPolicy, models: true do ...@@ -209,12 +218,12 @@ describe ProjectPolicy, models: true do
let(:current_user) { admin } let(:current_user) { admin }
it do it do
is_expected.to include(*guest_permissions) expect_allowed(*guest_permissions)
is_expected.to include(*reporter_permissions) expect_allowed(*reporter_permissions)
is_expected.not_to include(*team_member_reporter_permissions) expect_disallowed(*team_member_reporter_permissions)
is_expected.to include(*developer_permissions) expect_allowed(*developer_permissions)
is_expected.to include(*master_permissions) expect_allowed(*master_permissions)
is_expected.to include(*owner_permissions) expect_allowed(*owner_permissions)
end end
end end
end end
......
spec/policies/project_snippet_policy_spec.rb
View file @ 8c5538be
...@@ -15,7 +15,15 @@ describe ProjectSnippetPolicy, models: true do ...@@ -15,7 +15,15 @@ describe ProjectSnippetPolicy, models: true do
def abilities(user, snippet_visibility) def abilities(user, snippet_visibility)
snippet = create(:project_snippet, snippet_visibility, project: project) snippet = create(:project_snippet, snippet_visibility, project: project)
described_class.abilities(user, snippet).to_set described_class.new(user, snippet)
end
def expect_allowed(*permissions)
permissions.each { |p| is_expected.to be_allowed(p) }
end
def expect_disallowed(*permissions)
permissions.each { |p| is_expected.not_to be_allowed(p) }
end end
context 'public snippet' do context 'public snippet' do
...@@ -23,8 +31,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -23,8 +31,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(nil, :public) } subject { abilities(nil, :public) }
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -32,8 +40,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -32,8 +40,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(regular_user, :public) } subject { abilities(regular_user, :public) }
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -41,8 +49,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -41,8 +49,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(external_user, :public) } subject { abilities(external_user, :public) }
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
end end
...@@ -52,8 +60,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -52,8 +60,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(nil, :internal) } subject { abilities(nil, :internal) }
it do it do
is_expected.not_to include(:read_project_snippet) expect_disallowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -61,8 +69,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -61,8 +69,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(regular_user, :internal) } subject { abilities(regular_user, :internal) }
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -70,8 +78,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -70,8 +78,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(external_user, :internal) } subject { abilities(external_user, :internal) }
it do it do
is_expected.not_to include(:read_project_snippet) expect_disallowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -83,8 +91,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -83,8 +91,8 @@ describe ProjectSnippetPolicy, models: true do
end end
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
end end
...@@ -94,8 +102,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -94,8 +102,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(nil, :private) } subject { abilities(nil, :private) }
it do it do
is_expected.not_to include(:read_project_snippet) expect_disallowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -103,19 +111,19 @@ describe ProjectSnippetPolicy, models: true do ...@@ -103,19 +111,19 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(regular_user, :private) } subject { abilities(regular_user, :private) }
it do it do
is_expected.not_to include(:read_project_snippet) expect_disallowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
context 'snippet author' do context 'snippet author' do
let(:snippet) { create(:project_snippet, :private, author: regular_user, project: project) } let(:snippet) { create(:project_snippet, :private, author: regular_user, project: project) }
subject { described_class.abilities(regular_user, snippet).to_set } subject { described_class.new(regular_user, snippet) }
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.to include(*author_permissions) expect_allowed(*author_permissions)
end end
end end
...@@ -127,8 +135,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -127,8 +135,8 @@ describe ProjectSnippetPolicy, models: true do
end end
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -140,8 +148,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -140,8 +148,8 @@ describe ProjectSnippetPolicy, models: true do
end end
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.not_to include(*author_permissions) expect_disallowed(*author_permissions)
end end
end end
...@@ -149,8 +157,8 @@ describe ProjectSnippetPolicy, models: true do ...@@ -149,8 +157,8 @@ describe ProjectSnippetPolicy, models: true do
subject { abilities(create(:admin), :private) } subject { abilities(create(:admin), :private) }
it do it do
is_expected.to include(:read_project_snippet) expect_allowed(:read_project_snippet)
is_expected.to include(*author_permissions) expect_allowed(*author_permissions)
end end
end end
end end
......
spec/policies/user_policy_spec.rb
View file @ 8c5538be
...@@ -4,34 +4,34 @@ describe UserPolicy, models: true do ...@@ -4,34 +4,34 @@ describe UserPolicy, models: true do
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
let(:user) { create(:user) } let(:user) { create(:user) }
subject { described_class.abilities(current_user, user).to_set } subject { UserPolicy.new(current_user, user) }
describe "reading a user's information" do describe "reading a user's information" do
it { is_expected.to include(:read_user) } it { is_expected.to be_allowed(:read_user) }
end end
describe "destroying a user" do describe "destroying a user" do
context "when a regular user tries to destroy another regular user" do context "when a regular user tries to destroy another regular user" do
it { is_expected.not_to include(:destroy_user) } it { is_expected.not_to be_allowed(:destroy_user) }
end end
context "when a regular user tries to destroy themselves" do context "when a regular user tries to destroy themselves" do
let(:current_user) { user } let(:current_user) { user }
it { is_expected.to include(:destroy_user) } it { is_expected.to be_allowed(:destroy_user) }
end end
context "when an admin user tries to destroy a regular user" do context "when an admin user tries to destroy a regular user" do
let(:current_user) { create(:user, :admin) } let(:current_user) { create(:user, :admin) }
it { is_expected.to include(:destroy_user) } it { is_expected.to be_allowed(:destroy_user) }
end end
context "when an admin user tries to destroy a ghost user" do context "when an admin user tries to destroy a ghost user" do
let(:current_user) { create(:user, :admin) } let(:current_user) { create(:user, :admin) }
let(:user) { create(:user, :ghost) } let(:user) { create(:user, :ghost) }
it { is_expected.not_to include(:destroy_user) } it { is_expected.not_to be_allowed(:destroy_user) }
end end
end 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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7