Commit ecb83afa authored by Douwe Maan's avatar Douwe Maan

Refactor ability changes

parent e849b51c
...@@ -240,11 +240,11 @@ class Ability ...@@ -240,11 +240,11 @@ class Ability
# Only group owner and administrators can admin group # Only group owner and administrators can admin group
if group.has_owner?(user) || user.admin? if group.has_owner?(user) || user.admin?
rules.push(*[ rules += [
:admin_group, :admin_group,
:admin_namespace, :admin_namespace,
:admin_group_member :admin_group_member
]) ]
end end
rules.flatten rules.flatten
...@@ -255,16 +255,15 @@ class Ability ...@@ -255,16 +255,15 @@ class Ability
# Only namespace owner and administrators can admin it # Only namespace owner and administrators can admin it
if namespace.owner == user || user.admin? if namespace.owner == user || user.admin?
rules.push(*[ rules += [
:create_projects, :create_projects,
:admin_namespace :admin_namespace
]) ]
end end
rules.flatten rules.flatten
end end
[:issue, :merge_request].each do |name| [:issue, :merge_request].each do |name|
define_method "#{name}_abilities" do |user, subject| define_method "#{name}_abilities" do |user, subject|
rules = [] rules = []
...@@ -305,16 +304,19 @@ class Ability ...@@ -305,16 +304,19 @@ class Ability
rules = [] rules = []
target_user = subject.user target_user = subject.user
group = subject.group group = subject.group
unless group.last_owner?(target_user)
can_manage = group_abilities(user, group).include?(:admin_group_member) can_manage = group_abilities(user, group).include?(:admin_group_member)
if can_manage && (user != target_user) if can_manage && user != target_user
rules << :update_group_member rules << :update_group_member
rules << :destroy_group_member rules << :destroy_group_member
end end
if !group.last_owner?(user) && (can_manage || (user == target_user)) if user == target_user
rules << :destroy_group_member rules << :destroy_group_member
end end
end
rules rules
end end
...@@ -323,16 +325,20 @@ class Ability ...@@ -323,16 +325,20 @@ class Ability
rules = [] rules = []
target_user = subject.user target_user = subject.user
project = subject.project project = subject.project
unless target_user == project.owner
can_manage = project_abilities(user, project).include?(:admin_project_member) can_manage = project_abilities(user, project).include?(:admin_project_member)
if can_manage && user != target_user && target_user != project.owner if can_manage && user != target_user
rules << :update_project_member rules << :update_project_member
rules << :destroy_project_member rules << :destroy_project_member
end end
if user == target_user && target_user != project.owner if user == target_user
rules << :destroy_project_member rules << :destroy_project_member
end end
end
rules rules
end end
......
# == Owners concern
#
# Contains owners functionality for groups
#
module HasOwners
extend ActiveSupport::Concern
def owners
@owners ||= members.owners.includes(:user).map(&:user)
end
def members
raise NotImplementedError, "Expected members to be defined in #{self.class.name}"
end
def add_owner(user, current_user = nil)
add_user(user, Gitlab::Access::OWNER, current_user)
end
def has_owner?(user)
owners.include?(user)
end
def has_master?(user)
members.masters.where(user_id: user).any?
end
def last_owner?(user)
has_owner?(user) && owners.size == 1
end
end
...@@ -20,7 +20,6 @@ require 'file_size_validator' ...@@ -20,7 +20,6 @@ require 'file_size_validator'
class Group < Namespace class Group < Namespace
include Gitlab::ConfigHelper include Gitlab::ConfigHelper
include Referable include Referable
include HasOwners
has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
alias_method :members, :group_members alias_method :members, :group_members
...@@ -66,6 +65,10 @@ class Group < Namespace ...@@ -66,6 +65,10 @@ class Group < Namespace
end end
end end
def owners
@owners ||= group_members.owners.includes(:user).map(&:user)
end
def add_users(user_ids, access_level, current_user = nil) def add_users(user_ids, access_level, current_user = nil)
user_ids.each do |user_id| user_ids.each do |user_id|
Member.add_user(self.group_members, user_id, access_level, current_user) Member.add_user(self.group_members, user_id, access_level, current_user)
...@@ -92,6 +95,22 @@ class Group < Namespace ...@@ -92,6 +95,22 @@ class Group < Namespace
add_user(user, Gitlab::Access::MASTER, current_user) add_user(user, Gitlab::Access::MASTER, current_user)
end end
def add_owner(user, current_user = nil)
add_user(user, Gitlab::Access::OWNER, current_user)
end
def has_owner?(user)
owners.include?(user)
end
def has_master?(user)
members.masters.where(user_id: user).any?
end
def last_owner?(user)
has_owner?(user) && owners.size == 1
end
def avatar_type def avatar_type
unless self.avatar.image? unless self.avatar.image?
self.errors.add :avatar, "only images allowed" self.errors.add :avatar, "only images allowed"
......
...@@ -34,14 +34,16 @@ class Member < ActiveRecord::Base ...@@ -34,14 +34,16 @@ class Member < ActiveRecord::Base
message: "already exists in source", message: "already exists in source",
allow_nil: true } allow_nil: true }
validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true
validates :invite_email, presence: { if: :invite? }, validates :invite_email,
presence: {
if: :invite?
},
email: { email: {
strict_mode: true, strict_mode: true,
allow_nil: true allow_nil: true
}, },
uniqueness: { uniqueness: {
scope: [:source_type, scope: [:source_type, :source_id],
:source_id],
allow_nil: true allow_nil: true
} }
...@@ -100,7 +102,9 @@ class Member < ActiveRecord::Base ...@@ -100,7 +102,9 @@ class Member < ActiveRecord::Base
private private
def can_update_member?(current_user, member) def can_update_member?(current_user, member)
!current_user || current_user.can?(:update_group_member, member) || # There is no current user for bulk actions, in which case anything is allowed
!current_user ||
current_user.can?(:update_group_member, member) ||
current_user.can?(:update_project_member, member) current_user.can?(:update_project_member, member)
end end
end end
......
...@@ -42,7 +42,6 @@ class Project < ActiveRecord::Base ...@@ -42,7 +42,6 @@ class Project < ActiveRecord::Base
include Sortable include Sortable
include AfterCommitQueue include AfterCommitQueue
include CaseSensitivity include CaseSensitivity
include HasOwners
extend Gitlab::ConfigHelper extend Gitlab::ConfigHelper
extend Enumerize extend Enumerize
...@@ -117,7 +116,6 @@ class Project < ActiveRecord::Base ...@@ -117,7 +116,6 @@ class Project < ActiveRecord::Base
has_many :hooks, dependent: :destroy, class_name: 'ProjectHook' has_many :hooks, dependent: :destroy, class_name: 'ProjectHook'
has_many :protected_branches, dependent: :destroy has_many :protected_branches, dependent: :destroy
has_many :project_members, dependent: :destroy, as: :source, class_name: 'ProjectMember' has_many :project_members, dependent: :destroy, as: :source, class_name: 'ProjectMember'
alias_method :my_members, :project_members
has_many :users, through: :project_members has_many :users, through: :project_members
has_many :deploy_keys_projects, dependent: :destroy has_many :deploy_keys_projects, dependent: :destroy
has_many :deploy_keys, through: :deploy_keys_projects has_many :deploy_keys, through: :deploy_keys_projects
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment