diff --git a/app/controllers/api/v1/build_lists_controller.rb b/app/controllers/api/v1/build_lists_controller.rb index 8d1a1a67c..cba55c685 100644 --- a/app/controllers/api/v1/build_lists_controller.rb +++ b/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 diff --git a/app/controllers/api/v1/pull_requests_controller.rb b/app/controllers/api/v1/pull_requests_controller.rb index 5cc436a92..8b4e2f1e2 100644 --- a/app/controllers/api/v1/pull_requests_controller.rb +++ b/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] diff --git a/app/controllers/groups/members_controller.rb b/app/controllers/groups/members_controller.rb index 4dc0c5d0f..328f76a7a 100644 --- a/app/controllers/groups/members_controller.rb +++ b/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] diff --git a/app/controllers/groups/profile_controller.rb b/app/controllers/groups/profile_controller.rb index 5c5d7b40b..c2db64d8a 100644 --- a/app/controllers/groups/profile_controller.rb +++ b/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 diff --git a/app/controllers/projects/build_lists_controller.rb b/app/controllers/projects/build_lists_controller.rb index 7637c3109..22b8cd1ee 100644 --- a/app/controllers/projects/build_lists_controller.rb +++ b/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| diff --git a/app/controllers/projects/collaborators_controller.rb b/app/controllers/projects/collaborators_controller.rb index 8ba508a6d..485d8faf4 100644 --- a/app/controllers/projects/collaborators_controller.rb +++ b/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 diff --git a/app/controllers/projects/git/blobs_controller.rb b/app/controllers/projects/git/blobs_controller.rb index 76d199b88..20979b566 100644 --- a/app/controllers/projects/git/blobs_controller.rb +++ b/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 diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index d580e7c08..254a2cabc 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/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? diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index dcd7d61e0..ad3594fbd 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/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 diff --git a/app/controllers/projects/projects_controller.rb b/app/controllers/projects/projects_controller.rb index a9151799e..1f19fa971 100644 --- a/app/controllers/projects/projects_controller.rb +++ b/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 diff --git a/app/controllers/projects/pull_requests_controller.rb b/app/controllers/projects/pull_requests_controller.rb index 861f431cf..254391513 100644 --- a/app/controllers/projects/pull_requests_controller.rb +++ b/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? diff --git a/app/controllers/projects/wiki_controller.rb b/app/controllers/projects/wiki_controller.rb index c3bc10a48..26b21d603 100644 --- a/app/controllers/projects/wiki_controller.rb +++ b/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 diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 220cf2085..7175d00a2 100644 --- a/app/helpers/projects_helper.rb +++ b/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) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index f5ffa00b7..542aab4b8 100644 --- a/app/policies/application_policy.rb +++ b/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 \ No newline at end of file diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 6c35b1e29..06db19b26 100644 --- a/app/policies/group_policy.rb +++ b/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 diff --git a/app/policies/platform_policy.rb b/app/policies/platform_policy.rb index 00376f59e..20104f642 100644 --- a/app/policies/platform_policy.rb +++ b/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 diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 797194261..559047096 100644 --- a/app/policies/project_policy.rb +++ b/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 diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 5eea7174f..5f8bd4fc4 100644 --- a/app/policies/user_policy.rb +++ b/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 diff --git a/app/views/groups/base/_submenu.html.slim b/app/views/groups/base/_submenu.html.slim index 579cb80a0..0f147e1a0 100644 --- a/app/views/groups/base/_submenu.html.slim +++ b/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?