#465: updated ProjectPolicy

app/controllers/api/v1/base_controller.rb

@@ -35,6 +35,7 @@ class Api::V1::BaseController < ApplicationController
   end
 
   def create_subject(subject)
+    authorize subject, :create?
     class_name = subject.class.name
     if subject.save
       render_json_response subject, "#{class_name} has been created successfully"
@@ -44,6 +45,7 @@ class Api::V1::BaseController < ApplicationController
   end
 
   def update_member_in_subject(subject, relation = :relations)
+    authorize subject, :update_member?
     role = params[:role]
     class_name = subject.class.name.downcase
     if member.present? && role.present? && subject.respond_to?(:owner) && subject.owner != member &&
@@ -55,6 +57,7 @@ class Api::V1::BaseController < ApplicationController
   end
 
   def add_member_to_subject(subject, role = 'admin')
+    authorize subject, :add_member?
     class_name = subject.class.name.downcase
     if member.present? && subject.add_member(member, role)
       render_json_response subject, "#{member.class.to_s} '#{member.id}' has been added to #{class_name} successfully"
@@ -64,6 +67,7 @@ class Api::V1::BaseController < ApplicationController
   end
 
   def remove_member_from_subject(subject)
+    authorize subject, :remove_member?
     class_name = subject.class.name.downcase
     if member.present? && subject.remove_member(member)
       render_json_response subject, "#{member.class.to_s} '#{member.id}' has been removed from #{class_name} successfully"
@@ -73,11 +77,13 @@ class Api::V1::BaseController < ApplicationController
   end
 
   def destroy_subject(subject)
+    authorize subject, :destroy?
     subject.destroy # later with resque
     render_json_response subject, "#{subject.class.name} has been destroyed successfully"
   end
 
   def update_subject(subject)
+    authorize subject, :update?
     class_name = subject.class.name
     if subject.update_attributes(params[class_name.underscore.to_sym] || {})
       render_json_response subject, "#{class_name} has been updated successfully"

app/controllers/api/v1/projects_controller.rb

@@ -13,7 +13,7 @@ class Api::V1::ProjectsController < Api::V1::BaseController
 
   def get_id
     if @project = Project.find_by_owner_and_name(params[:owner], params[:name])
-      authorize! :show, @project
+      authorize @project, :show?
     else
       raise ActiveRecord::RecordNotFound
     end
@@ -21,10 +21,12 @@ class Api::V1::ProjectsController < Api::V1::BaseController
   end
 
   def show
+    authorize @project, :show?
     respond_to :json
   end
 
   def refs_list
+    authorize @project, :show?
     @refs = @project.repo.branches + @project.repo.tags.select{ |t| t.commit }
     respond_to :json
   end
@@ -46,7 +48,7 @@ class Api::V1::ProjectsController < Api::V1::BaseController
     else
       @project.owner = nil
     end
-    authorize! :write, @project.owner if @project.owner != current_user
+    authorize @project.owner, :write? if @project.owner != current_user
     create_subject @project
   end
 
@@ -69,7 +71,7 @@ class Api::V1::ProjectsController < Api::V1::BaseController
 
   def fork(is_alias = false)
     owner = (Group.find params[:group_id] if params[:group_id].present?) || current_user
-    authorize! :write, owner if owner.class == Group
+    authorize owner, :write? if owner.class == Group
     if forked = @project.fork(owner, new_name: params[:fork_name], is_alias: is_alias) and forked.valid?
       render_json_response forked, 'Project has been forked successfully'
     else

app/controllers/projects/git/trees_controller.rb

@@ -6,7 +6,7 @@ class Projects::Git::TreesController < Projects::Git::BaseController
   before_action      :resolve_treeish,          only: [:branch, :destroy]
 
   # skip_authorize_resource :project,                 only: [:destroy, :restore_branch, :create]
-  before_action -> { authorize(@project, :write?) },  only: [:destroy, :restore_branch, :create]
+  before_action -> { authorize(@project, :show?)  },  only: [:show, :archive, :tags, :branches]
 
   def show
     unless request.xhr?
@@ -54,16 +54,19 @@ class Projects::Git::TreesController < Projects::Git::BaseController
   end
 
   def restore_branch
+    authorize @project, :write?
     status = @project.create_branch(@treeish, params[:sha], current_user) ? 200 : 422
     render nothing: true, status: status
   end
 
   def create
+    authorize @project
     status = @project.create_branch(params[:new_ref], params[:from_ref], current_user) ? 200 : 422
     render nothing: true, status: status
   end
 
   def destroy
+    authorize @project
     status = @branch && @project.delete_branch(@branch, current_user) ? 200 : 422
     render nothing: true, status: status
   end

app/controllers/projects/projects_controller.rb

@@ -3,7 +3,7 @@ class Projects::ProjectsController < Projects::BaseController
   include ProjectsHelper
 
   before_action :authenticate_user!
-  load_and_authorize_resource id_param: :name_with_owner # to force member actions load
+  # load_and_authorize_resource id_param: :name_with_owner # to force member actions load
   before_action :who_owns, only: [:new, :create, :mass_import, :run_mass_import]
 
   def index
@@ -130,6 +130,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def possible_forks
+    authorize @project
     render partial: 'projects/git/base/forks', layout: false,
       locals: { owner: current_user, name: (params[:name].presence || @project.name) }
   end

app/policies/application_policy.rb

@@ -108,22 +108,22 @@ class ApplicationPolicy
   # Private: Check if provided user is at least record admin.
   #
   # Returns true if he is, false otherwise.
-  def local_admin?
+  def local_admin?(r = record)
-    best_role == 'admin'
+    best_role(r) == 'admin'
   end
 
   # Private: Check if provided user is at least record reader.
   #
   # Returns true if he is, false otherwise.
-  def local_reader?
+  def local_reader?(r = record)
-    %w(reader writer admin).include?(best_role)
+    %w(reader writer admin).include? best_role(r)
   end
 
   # Private: Check if provided user is at least record writer.
   #
   # Returns true if he is, false otherwise.
-  def local_writer?
+  def local_writer?(r = record)
-    %w(writer admin).include?(best_role)
+    %w(writer admin).include? best_role(r)
   end
 
   # Private: Check if provided user is record owner.
@@ -133,16 +133,25 @@ class ApplicationPolicy
     (
       record.owner_type == 'User'  && record.owner_id == user.id
     ) || (
-      record.owner_type == 'Group' && user_group_ids.include?(record.owner_id)
+      record.owner_type == 'Group' && user_own_group_ids.include?(record.owner_id)
     )
   end
 
   # Private: Get the best role of user for record.
   #
   # Returns the String role or nil.
-  def best_role
+  def best_role(r = record)
-    Rails.cache.fetch(['ApplicationPolicy#best_role', record, user]) do
+    Rails.cache.fetch(['ApplicationPolicy#best_role', r, user]) do
-      user.best_role(record)
+      user.best_role(r)
+    end
+  end
+
+  # Public: Get own user's group ids.
+  #
+  # Returns the Array of own group ids.
+  def user_own_group_ids
+    Rails.cache.fetch(['ApplicationPolicy#user_own_group_ids', user]) do
+      user.own_group_ids
     end
   end
 
@@ -150,7 +159,7 @@ class ApplicationPolicy
   #
   # Returns the Array of group ids.
   def user_group_ids
-    Rails.cache.fetch(['ApplicationPolicy#user_group_ids', user.id]) do
+    Rails.cache.fetch(['ApplicationPolicy#user_group_ids', user]) do
       user.group_ids
     end
   end

app/policies/project_policy.rb

@@ -1,28 +1,40 @@
 class ProjectPolicy < ApplicationPolicy
 
   def index?
-    true
+    !user.guest?
   end
 
   def show?
-    return true if record.public?
+    record.public? || local_reader?
   end
+  alias_method :read?, :show?
+  alias_method :fork?, :show?
+
+  def create?
+    !user.guest?
+  end
+
+  def update?
+    local_admin?
+  end
+  alias_method :alias?, :update?
+
+  def destroy?
+    owner? || record.owner.is_a?(Group) && record.owner.actors.exists?(actor_type: 'User', actor_id: user.id, role: 'admin')
+  end
+
+  def mass_import?
+    user.platforms.main.find{ |p| local_admin?(p) }.present?
+  end
+  alias_method :run_mass_import?, :mass_import?
 
   # for grack
   def write?
     local_writer?
   end
 
-  def read?
+  def possible_forks
-    show?
+    true
-  end
-
-  def archive?
-    show?
-  end
-
-  def fork?
-    show?
   end
 
 end

app/views/projects/base/_settings_menu.html.slim

@@ -9,6 +9,5 @@ ul.nav.nav-tabs.nav-justified.boffset10[ role = 'tablist' ]
       = link_to t("layout.projects.sections"), sections_project_path(@project)
     li[ class = "#{(contr == :hooks) ? 'active' : ''}" ]
       = link_to t("layout.projects.hooks"), project_hooks_path(@project)
-  - if policy(@project).manage_collaborators?
     li[ class = "#{(act == :index && contr == :collaborators) ? 'active' : ''}" ]
       = link_to t("layout.projects.edit_collaborators"), project_collaborators_path(@project)