From 846e581732e291f8927d04a5b1b40fe8f2688885 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 28 Feb 2017 13:08:07 -0800 Subject: [PATCH] use a magic default :global symbol instead of nil to make sure we mean the global permissions --- app/controllers/application_controller.rb | 2 +- app/controllers/groups_controller.rb | 2 +- app/models/ability.rb | 7 ++++--- app/models/guest.rb | 2 +- app/models/user.rb | 4 ++-- app/policies/base_policy.rb | 9 +++++++-- lib/api/helpers.rb | 4 ++-- lib/api/users.rb | 2 +- lib/banzai/reference_parser/base_parser.rb | 2 +- lib/gitlab/allowable.rb | 2 +- 10 files changed, 21 insertions(+), 15 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1c66c530cd2..9b381336fab 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -90,7 +90,7 @@ class ApplicationController < ActionController::Base current_application_settings.after_sign_out_path.presence || new_user_session_path end - def can?(object, action, subject) + def can?(object, action, subject = :global) Ability.allowed?(object, action, subject) end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 4663b6e7fc6..05f9ee1ee90 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -118,7 +118,7 @@ class GroupsController < Groups::ApplicationController end def authorize_create_group! - unless can?(current_user, :create_group, nil) + unless can?(current_user, :create_group) return render_404 end end diff --git a/app/models/ability.rb b/app/models/ability.rb index ad6c588202e..f3692a5a067 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -56,15 +56,16 @@ class Ability end end - def allowed?(user, action, subject) + def allowed?(user, action, subject = :global) allowed(user, subject).include?(action) end - def allowed(user, subject) + def allowed(user, subject = :global) + return BasePolicy::RuleSet.none if subject.nil? return uncached_allowed(user, subject) unless RequestStore.active? user_key = user ? user.id : 'anonymous' - subject_key = subject ? "#{subject.class.name}/#{subject.id}" : 'global' + subject_key = subject == :global ? 'global' : "#{subject.class.name}/#{subject.id}" key = "/ability/#{user_key}/#{subject_key}" RequestStore[key] ||= uncached_allowed(user, subject).freeze end diff --git a/app/models/guest.rb b/app/models/guest.rb index 01285ca1264..df287c277a7 100644 --- a/app/models/guest.rb +++ b/app/models/guest.rb @@ -1,6 +1,6 @@ class Guest class << self - def can?(action, subject) + def can?(action, subject = :global) Ability.allowed?(nil, action, subject) end end diff --git a/app/models/user.rb b/app/models/user.rb index 76fb4cd470e..db3837ecdf2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -563,14 +563,14 @@ class User < ActiveRecord::Base end def can_create_group? - can?(:create_group, nil) + can?(:create_group) end def can_select_namespace? several_namespaces? || admin end - def can?(action, subject) + def can?(action, subject = :global) Ability.allowed?(self, action, subject) end diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index e07b144355a..8890409d056 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -12,6 +12,10 @@ class BasePolicy new(Set.new, Set.new) end + def self.none + empty.freeze + end + def can?(ability) @can_set.include?(ability) && !@cannot_set.include?(ability) end @@ -49,7 +53,8 @@ class BasePolicy end def self.class_for(subject) - return GlobalPolicy if subject.nil? + return GlobalPolicy if subject == :global + raise ArgumentError, 'no policy for nil' if subject.nil? if subject.class.try(:presenter?) subject = subject.subject @@ -79,7 +84,7 @@ class BasePolicy end def abilities - return RuleSet.empty if @user && @user.blocked? + return RuleSet.none if @user && @user.blocked? return anonymous_abilities if @user.nil? collect_rules { rules } end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index a9b364da9e1..e5f5de2af57 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -116,7 +116,7 @@ module API forbidden! unless current_user.is_admin? end - def authorize!(action, subject = nil) + def authorize!(action, subject = :global) forbidden! unless can?(current_user, action, subject) end @@ -134,7 +134,7 @@ module API end end - def can?(object, action, subject) + def can?(object, action, subject = :global) Ability.allowed?(object, action, subject) end diff --git a/lib/api/users.rb b/lib/api/users.rb index 549003f576a..2d4d5a25221 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -45,7 +45,7 @@ module API use :pagination end get do - unless can?(current_user, :read_users_list, nil) + unless can?(current_user, :read_users_list) render_api_error!("Not authorized.", 403) end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index 2058a58d0ae..b121c37c5d0 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -210,7 +210,7 @@ module Banzai grouped_objects_for_nodes(nodes, Project, 'data-project') end - def can?(user, permission, subject) + def can?(user, permission, subject = :global) Ability.allowed?(user, permission, subject) end diff --git a/lib/gitlab/allowable.rb b/lib/gitlab/allowable.rb index f48abcc86d5..e4f7cad2b79 100644 --- a/lib/gitlab/allowable.rb +++ b/lib/gitlab/allowable.rb @@ -1,6 +1,6 @@ module Gitlab module Allowable - def can?(user, action, subject) + def can?(user, action, subject = :global) Ability.allowed?(user, action, subject) end end