use a magic default :global symbol instead of nil

to make sure we mean the global permissions

6 files changed, 16 insertions, 10 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