#465: Updated some controllers, logic of polices

app/controllers/api/v1/build_lists_controller.rb

@@ -2,15 +2,17 @@ class Api::V1::BuildListsController < Api::V1::BaseController
   before_action :authenticate_user!
   skip_before_action :authenticate_user!, only: [:show, :index] if APP_CONFIG['anonymous_access']
 
-  load_and_authorize_resource :build_list, only: [:show, :create, :cancel, :publish, :reject_publish, :create_container, :publish_into_testing, :rerun_tests]
+  # load_and_authorize_resource :build_list, only: [:show, :create, :cancel, :publish, :reject_publish, :create_container, :publish_into_testing, :rerun_tests]
 
   def show
+    authorize @build_list
     respond_to :json
   end
 
   def index
+    authorize :build_list
     @project = Project.find(params[:project_id]) if params[:project_id].present?
-    authorize!(:show, @project) if @project
+    authorize @project, :show? if @project
     filter = BuildList::Filter.new(@project, current_user, current_ability, params[:filter] || {})
     @build_lists = filter.find.includes(:build_for_platform,
                                         :save_to_repository,
@@ -36,6 +38,7 @@ class Api::V1::BuildListsController < Api::V1::BaseController
   end
 
   def cancel
+    authorize @build_list, :create?
     render_json :cancel
   end
 

app/controllers/api/v1/pull_requests_controller.rb

@@ -45,7 +45,7 @@ class Api::V1::PullRequestsController < Api::V1::BaseController
   def create
     from_project = Project.find(pull_params[:from_project_id]) if pull_params[:from_project_id].present?
     from_project ||= @project
-    authorize! :read, from_project
+    authorize from_project, :show?
 
     @pull = @project.pull_requests.new
     @pull.build_issue title: pull_params[:title], body: pull_params[:body]

app/controllers/groups/members_controller.rb

@@ -1,5 +1,5 @@
 class Groups::MembersController < Groups::BaseController
-  before_action -> { authorize! :manage_members, @group }
+  before_action -> { authorize @group, :update?  }
 
   def index
     @members = @group.members.order(:uname) - [@group.owner]

app/controllers/groups/profile_controller.rb

@@ -2,7 +2,7 @@ class Groups::ProfileController < Groups::BaseController
   include AvatarHelper
   include PaginateHelper
 
-  load_and_authorize_resource class: Group, instance_name: 'group'
+  # load_and_authorize_resource class: Group, instance_name: 'group'
   skip_before_action :authenticate_user!, only: :show if APP_CONFIG['anonymous_access']
 
   def index
@@ -21,9 +21,10 @@ class Groups::ProfileController < Groups::BaseController
         when 'open'
           @projects = @projects.opened
         when 'hidden'
-          @projects = @projects.by_visibilities('hidden').accessible_by(current_ability, :read)
+          @projects = @projects.by_visibilities('hidden')
+          @projects = @projects.none unless policy(@group).reader?
         else
-          @projects = @projects.accessible_by(current_ability, :read)
+          @projects = @projects.opened unless policy(@group).reader?
         end
         @total_items  = @projects.count
         @projects     = @projects.paginate(paginate_params)
@@ -33,6 +34,7 @@ class Groups::ProfileController < Groups::BaseController
   end
 
   def new
+    @group = current_user.own_groups.build
   end
 
   def edit

app/controllers/projects/build_lists_controller.rb

@@ -17,7 +17,8 @@ class Projects::BuildListsController < Projects::BaseController
   before_action :create_from_build_list, only: :new
 
   def index
-    authorize!(:show, @project) if @project
+    authorize :build_list
+    authorize @project, :show? if @project
     params[:filter].each{|k,v| params[:filter].delete(k) if v.blank? } if params[:filter]
 
     respond_to do |format|

app/controllers/projects/collaborators_controller.rb

@@ -74,6 +74,6 @@ class Projects::CollaboratorsController < Projects::BaseController
   end
 
   def authorize_collaborators
-    authorize! :update, @project
+    authorize @project, :update?
   end
 end

app/controllers/projects/git/blobs_controller.rb

@@ -1,6 +1,6 @@
 class Projects::Git::BlobsController < Projects::Git::BaseController
   before_action :set_blob
-  before_action -> {authorize! :write, @project}, only: [:edit, :update]
+  before_action -> {authorize @project, :write? }, only: [:edit, :update]
 
   def show
   end

app/controllers/projects/hooks_controller.rb

@@ -1,10 +1,10 @@
 class Projects::HooksController < Projects::BaseController
   before_action :authenticate_user!
-  load_and_authorize_resource :project
-  load_and_authorize_resource :hook, through: :project
+  before_action -> { authorize @project, :update? }
+  # load_and_authorize_resource :project
+  # load_and_authorize_resource :hook, through: :project
 
   def index
-    authorize! :edit, @project
     @name = params[:name]
     @hooks = @project.hooks.for_name(@name).order('name asc, created_at desc')
     render(:show) if @name.present?

app/controllers/projects/issues_controller.rb

@@ -166,7 +166,7 @@ class Projects::IssuesController < Projects::BaseController
   private
 
   def load_and_authorize_label
-    authorize! :write, @project
+    authorize @project, :write?
     @label = Label.find(params[:label_id]) if params[:label_id]
   end
 end

app/controllers/projects/projects_controller.rb

@@ -7,7 +7,8 @@ class Projects::ProjectsController < Projects::BaseController
   before_action :who_owns, only: [:new, :create, :mass_import, :run_mass_import]
 
   def index
-    @projects = Project.accessible_by(current_ability, :membered).search(params[:search])
+    authorize :project
+    @projects = ProjectPolicy::Scope.new(current_user, Project).membered.search(params[:search])
     respond_to do |format|
       format.html {
         @groups = current_user.groups
@@ -24,18 +25,19 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def new
+    authorize :project
     @project = Project.new
   end
 
   def mass_import
+    authorize :project
     @project = Project.new(mass_import: true)
   end
 
   def run_mass_import
     @project = Project.new params[:project]
     @project.owner = choose_owner
-    authorize! :write, @project.owner if @project.owner.class == Group
-    authorize! :add_project, Repository.find(params[:project][:add_to_repository_id])
+    authorize @project
     @project.valid?
     @project.errors.messages.slice! :url
     if @project.errors.messages.blank? # We need only url validation
@@ -48,13 +50,14 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def edit
+    authorize @project
     @project_aliases = Project.project_aliases(@project).paginate(page: current_page)
   end
 
   def create
     @project = Project.new params[:project]
     @project.owner = choose_owner
-    authorize! :write, @project.owner if @project.owner.class == Group
+    authorize @project
 
     if @project.save
       flash[:notice] = t('flash.project.saved')
@@ -67,6 +70,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def update
+    authorize @project
     params[:project].delete(:maintainer_id) if params[:project][:maintainer_id].blank?
     respond_to do |format|
       format.html do
@@ -82,18 +86,19 @@ class Projects::ProjectsController < Projects::BaseController
       end
       format.json do
         if @project.update_attributes(params[:project])
-          render json: { notice: I18n.t('flash.project.saved') }.to_json
+          render json: { notice: I18n.t('flash.project.saved') }
         else
-          render json: { error: I18n.t('flash.project.save_error') }.to_json, status: 422
+          render json: { error: I18n.t('flash.project.save_error') }, status: 422
         end
       end
     end
   end
 
   def schedule
+    authorize @project, :update?
     p_to_r = @project.project_to_repositories.where(repository_id: params[:repository_id]).first
     unless p_to_r.repository.publish_without_qa
-      authorize! :local_admin_manage, p_to_r.repository.platform
+      authorize p_to_r.repository.platform, :update?
     end
     p_to_r.user_id      = current_user.id
     p_to_r.enabled      = params[:enabled].present?
@@ -107,6 +112,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def destroy
+    authorize @project
     @project.destroy
     flash[:notice] = t("flash.project.destroyed")
     redirect_to @project.owner
@@ -114,8 +120,7 @@ class Projects::ProjectsController < Projects::BaseController
 
   def fork(is_alias = false)
     owner = (Group.find params[:group] if params[:group].present?) || current_user
-    authorize! :write, owner if owner.class == Group
-
+    authorize owner, :write?
     if forked = @project.fork(owner, new_name: params[:fork_name], is_alias: is_alias) and forked.valid?
       redirect_to forked, notice: t("flash.project.forked")
     else
@@ -136,6 +141,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def sections
+    authorize @project, :update?
     if request.patch?
       if @project.update_attributes(params[:project])
         flash[:notice] = t('flash.project.saved')
@@ -148,6 +154,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def remove_user
+    authorize @project, :update?
     @project.relations.by_actor(current_user).destroy_all
     respond_to do |format|
       format.html do
@@ -159,6 +166,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def autocomplete_maintainers
+    authorize @project, :update?
     term, limit = params[:query], params[:limit] || 10
     items = User.member_of_project(@project)
                 .where("users.name ILIKE ? OR users.uname ILIKE ?", "%#{term}%", "%#{term}%")
@@ -167,6 +175,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def preview
+    authorize @project, :show?
     respond_to do |format|
       format.json {}
       format.html {render inline: view_context.markdown(params[:text]), layout: false}
@@ -174,6 +183,7 @@ class Projects::ProjectsController < Projects::BaseController
   end
 
   def refs_list
+    authorize @project, :show?
     refs = @project.repo.branches_and_tags.map(&:name)
     @selected   = params[:selected] if refs.include?(params[:selected])
     @selected ||= @project.resolve_default_branch

app/controllers/projects/pull_requests_controller.rb

@@ -9,7 +9,7 @@ class Projects::PullRequestsController < Projects::BaseController
 
   def new
     to_project = find_destination_project(false)
-    authorize! :read, to_project
+    authorize to_project, :show?
 
     @pull = to_project.pull_requests.new
     @pull.issue = to_project.issues.new
@@ -33,7 +33,7 @@ class Projects::PullRequestsController < Projects::BaseController
       redirect :back
     end
     to_project = find_destination_project
-    authorize! :read, to_project
+    authorize to_project, :show?
 
     @pull = to_project.pull_requests.new pull_params
     @pull.issue.assignee_id = (params[:issue] || {})[:assignee_id] if policy(to_project).write?

app/controllers/projects/wiki_controller.rb

@@ -279,11 +279,11 @@ class Projects::WikiController < Projects::BaseController
     end
 
     def authorize_read_actions
-      authorize! :show, @project
+      authorize @project, :show?
     end
 
     def authorize_write_actions
-      authorize! :write, @project
+      authorize @project, :write?
     end
 end
 

app/helpers/projects_helper.rb

@@ -70,7 +70,9 @@ module ProjectsHelper
   end
 
   def alone_member?(project)
-    Relation.by_target(project).by_actor(current_user).size > 0
+    Rails.cache.fetch(['ProjectsHelper#alone_member?', project, current_user]) do
+      Relation.by_target(project).by_actor(current_user).exists?
+    end
   end
 
   def participant_path(participant)

app/policies/application_policy.rb

@@ -54,6 +54,15 @@ class ApplicationPolicy
     end
   end
 
+  # Public: Get user's group ids.
+  #
+  # Returns the Array of group ids.
+  def user_group_ids
+    Rails.cache.fetch(['ApplicationPolicy#user_group_ids', user]) do
+      user.group_ids
+    end
+  end
+
   protected
 
   # Public: Check if provided user is the current user.
@@ -131,9 +140,11 @@ class ApplicationPolicy
   # Returns true if he is, false otherwise.
   def owner?
     (
-      record.owner_type == 'User'  && record.owner_id == user.id
+      !record.try(:owner_type) && record.owner_id == user.id
     ) || (
-      record.owner_type == 'Group' && user_own_group_ids.include?(record.owner_id)
+      record.try(:owner_type) == 'User'  && record.owner_id == user.id
+    ) || (
+      record.try(:owner_type) == 'Group' && user_own_group_ids.include?(record.owner_id)
     )
   end
 
@@ -155,13 +166,4 @@ class ApplicationPolicy
     end
   end
 
-  # Public: Get user's group ids.
-  #
-  # Returns the Array of group ids.
-  def user_group_ids
-    Rails.cache.fetch(['ApplicationPolicy#user_group_ids', user]) do
-      user.group_ids
-    end
-  end
-
 end

app/policies/group_policy.rb

@@ -1,3 +1,30 @@
 class GroupPolicy < ApplicationPolicy
 
+  def index?
+    !user.guest?
+  end
+
+  def show?
+    true
+  end
+
+  def create?
+    !user.guest?
+  end
+
+  def reader?
+    local_reader?
+  end
+
+  def write?
+    owner? || local_writer?
+  end
+
+  def update?
+    owner? || local_admin?
+  end
+  alias_method :manage_members?, :update?
+  alias_method :remove_members?, :update?
+  alias_method :add_member?, :update?
+
 end

app/policies/platform_policy.rb

@@ -14,7 +14,7 @@ class PlatformPolicy < ApplicationPolicy
     is_admin?
   end
 
-  def members?
+  def update?
     owner? || local_admin?
   end
 
@@ -23,10 +23,14 @@ class PlatformPolicy < ApplicationPolicy
     owner? || local_admin?
   end
 
+  def add_project?
+    owner? || local_admin?
+  end
+
   class Scope < Scope
 
     def related
-      policy = Pundit.policy!(user_context, :platform)
+      policy = Pundit.policy!(user, :platform)
       scope.where <<-SQL, { user_id: user.id, user_group_ids: policy.user_group_ids, platform_ids: related_platform_ids }
         (
           platforms.id IN (:platform_ids)
@@ -41,7 +45,7 @@ class PlatformPolicy < ApplicationPolicy
     protected
 
     def related_platform_ids
-      Rails.cache.fetch(['PlatformPolicy::Scope#related_platform_ids', user.id]) do
+      Rails.cache.fetch(['PlatformPolicy::Scope#related_platform_ids', user]) do
         user.repositories.pluck(:platform_id)
       end
     end

app/policies/project_policy.rb

@@ -11,7 +11,7 @@ class ProjectPolicy < ApplicationPolicy
   alias_method :fork?, :show?
 
   def create?
-    !user.guest?
+    !user.guest? && (!record.try(:owner) || policy(record.owner).write?)
   end
 
   def update?
@@ -26,7 +26,12 @@ class ProjectPolicy < ApplicationPolicy
   def mass_import?
     user.platforms.main.find{ |p| local_admin?(p) }.present?
   end
-  alias_method :run_mass_import?, :mass_import?
+
+  def run_mass_import?
+    return false unless policy(record.owner).write?
+    repo = Repository.find(record.add_to_repository_id)
+    repo.platform.main? && policy(repo.platform).add_project?
+  end
 
   # for grack
   def write?
@@ -37,4 +42,34 @@ class ProjectPolicy < ApplicationPolicy
     true
   end
 
+  class Scope < Scope
+
+    def membered
+      policy = Pundit.policy!(user, :project)
+      scope.where <<-SQL, { user_id: user.id, user_group_ids: policy.user_group_ids }
+        (
+          projects.owner_type = 'User'  AND projects.owner_id = :user_id
+        ) OR (
+          projects.owner_type = 'Group' AND projects.owner_id IN (:user_group_ids)
+        ) OR (
+          projects.id = ANY (
+            ARRAY (
+              SELECT target_id
+              FROM relations
+              INNER JOIN projects ON projects.id = relations.target_id
+              WHERE relations.target_type = 'Project' AND
+              (
+                projects.owner_type = 'User' AND projects.owner_id != :user_id OR
+                projects.owner_type = 'Group' AND projects.owner_id NOT IN (:user_group_ids)
+              ) AND (
+                relations.actor_type = 'User' AND relations.actor_id = :user_id OR
+                relations.actor_type = 'Group' AND relations.actor_id IN (:user_group_ids)
+              )
+            )
+          )
+        )
+      SQL
+    end
+  end
+
 end

app/policies/user_policy.rb

@@ -1,5 +1,13 @@
 class UserPolicy < ApplicationPolicy
 
+  def write?
+    record == user
+  end
+
+  def update?
+    record == user
+  end
+
   def banned?
     !is_banned?
   end

app/views/groups/base/_submenu.html.slim

@@ -15,7 +15,7 @@
       / Collect the nav links, forms, and other content for toggling
       #submenu-navbar-collapse.collapse.navbar-collapse
         ul.nav.navbar-nav.left-border
-          - if policy(@group).edit?
+          - if policy(@group).update?
             li class=('active' if act == :edit && contr == :profile)
               = link_to t('layout.groups.edit'), edit_group_path(@group)
           - if policy(@group).manage_members?