diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb index f614e2ead..690c31cbe 100644 --- a/app/controllers/api/v1/base_controller.rb +++ b/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" diff --git a/app/controllers/api/v1/projects_controller.rb b/app/controllers/api/v1/projects_controller.rb index 541121b9d..1a34bcc25 100644 --- a/app/controllers/api/v1/projects_controller.rb +++ b/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 diff --git a/app/controllers/projects/git/trees_controller.rb b/app/controllers/projects/git/trees_controller.rb index 03a7b6086..c8d627fb9 100644 --- a/app/controllers/projects/git/trees_controller.rb +++ b/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 diff --git a/app/controllers/projects/projects_controller.rb b/app/controllers/projects/projects_controller.rb index 84a1ba68b..a9151799e 100644 --- a/app/controllers/projects/projects_controller.rb +++ b/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 diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index a57b1fe40..f5ffa00b7 100644 --- a/app/policies/application_policy.rb +++ b/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? - best_role == 'admin' + def local_admin?(r = record) + 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? - %w(reader writer admin).include?(best_role) + def local_reader?(r = record) + %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? - %w(writer admin).include?(best_role) + def local_writer?(r = record) + %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 - Rails.cache.fetch(['ApplicationPolicy#best_role', record, user]) do - user.best_role(record) + def best_role(r = record) + Rails.cache.fetch(['ApplicationPolicy#best_role', r, user]) do + 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 diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index b8a47f4a7..797194261 100644 --- a/app/policies/project_policy.rb +++ b/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? - show? - end - - def archive? - show? - end - - def fork? - show? + def possible_forks + true end end diff --git a/app/views/projects/base/_settings_menu.html.slim b/app/views/projects/base/_settings_menu.html.slim index 22dcb9203..0ab7a53f4 100644 --- a/app/views/projects/base/_settings_menu.html.slim +++ b/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)