diff --git a/app/models/ability.rb b/app/models/ability.rb
index eef481c8f8a594ca84e76ae80eb3bbf2b3af4587..500af08d20948dd441d2d1fc2212a64d0b97b637 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -240,11 +240,11 @@ class Ability
# Only group owner and administrators can admin group
if group.has_owner?(user) || user.admin?
- rules.push(*[
- :admin_group,
- :admin_namespace,
- :admin_group_member
- ])
+ rules += [
+ :admin_group,
+ :admin_namespace,
+ :admin_group_member
+ ]
end
rules.flatten
@@ -255,16 +255,15 @@ class Ability
# Only namespace owner and administrators can admin it
if namespace.owner == user || user.admin?
- rules.push(*[
- :create_projects,
- :admin_namespace
- ])
+ rules += [
+ :create_projects,
+ :admin_namespace
+ ]
end
rules.flatten
end
-
[:issue, :merge_request].each do |name|
define_method "#{name}_abilities" do |user, subject|
rules = []
@@ -305,15 +304,18 @@ class Ability
rules = []
target_user = subject.user
group = subject.group
- can_manage = group_abilities(user, group).include?(:admin_group_member)
- if can_manage && (user != target_user)
- rules << :update_group_member
- rules << :destroy_group_member
- end
+ unless group.last_owner?(target_user)
+ can_manage = group_abilities(user, group).include?(:admin_group_member)
- if !group.last_owner?(user) && (can_manage || (user == target_user))
- rules << :destroy_group_member
+ if can_manage && user != target_user
+ rules << :update_group_member
+ rules << :destroy_group_member
+ end
+
+ if user == target_user
+ rules << :destroy_group_member
+ end
end
rules
@@ -323,16 +325,20 @@ class Ability
rules = []
target_user = subject.user
project = subject.project
- can_manage = project_abilities(user, project).include?(:admin_project_member)
- if can_manage && user != target_user && target_user != project.owner
- rules << :update_project_member
- rules << :destroy_project_member
- end
+ unless target_user == project.owner
+ can_manage = project_abilities(user, project).include?(:admin_project_member)
- if user == target_user && target_user != project.owner
- rules << :destroy_project_member
+ if can_manage && user != target_user
+ rules << :update_project_member
+ rules << :destroy_project_member
+ end
+
+ if user == target_user
+ rules << :destroy_project_member
+ end
end
+
rules
end
diff --git a/app/models/concerns/has_owners.rb b/app/models/concerns/has_owners.rb
deleted file mode 100644
index 53ef6e939dd4bd7e32402f8abaaf06f270d707bd..0000000000000000000000000000000000000000
--- a/app/models/concerns/has_owners.rb
+++ /dev/null
@@ -1,31 +0,0 @@
-# == 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
diff --git a/app/models/group.rb b/app/models/group.rb
index 11fde7ba6cd3a0802e14a0bdc0dc600a88cc5733..2c9e75496b9c6353de48279b27be17c748bf5a02 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -20,8 +20,7 @@ require 'file_size_validator'
class Group < Namespace
include Gitlab::ConfigHelper
include Referable
- include HasOwners
-
+
has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember'
alias_method :members, :group_members
has_many :users, through: :group_members
@@ -66,6 +65,10 @@ class Group < Namespace
end
end
+ def owners
+ @owners ||= group_members.owners.includes(:user).map(&:user)
+ end
+
def add_users(user_ids, access_level, current_user = nil)
user_ids.each do |user_id|
Member.add_user(self.group_members, user_id, access_level, current_user)
@@ -92,6 +95,22 @@ class Group < Namespace
add_user(user, Gitlab::Access::MASTER, current_user)
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
unless self.avatar.image?
self.errors.add :avatar, "only images allowed"
diff --git a/app/models/member.rb b/app/models/member.rb
index eed9f2537e9e75c117ea2cbdcef5472056710d9e..28aee2e379963224b4459c4b5251d10464528d63 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -34,16 +34,18 @@ class Member < ActiveRecord::Base
message: "already exists in source",
allow_nil: true }
validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true
- validates :invite_email, presence: { if: :invite? },
- email: {
- strict_mode: true,
- allow_nil: true
- },
- uniqueness: {
- scope: [:source_type,
- :source_id],
- allow_nil: true
- }
+ validates :invite_email,
+ presence: {
+ if: :invite?
+ },
+ email: {
+ strict_mode: true,
+ allow_nil: true
+ },
+ uniqueness: {
+ scope: [:source_type, :source_id],
+ allow_nil: true
+ }
scope :invite, -> { where(user_id: nil) }
scope :non_invite, -> { where("user_id IS NOT NULL") }
@@ -100,7 +102,9 @@ class Member < ActiveRecord::Base
private
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)
end
end
diff --git a/app/models/project.rb b/app/models/project.rb
index 09465775786003cf0e103c60bcefbbc303700dbf..a099a67cf639298fe54366803338c8227e9206c2 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -42,8 +42,7 @@ class Project < ActiveRecord::Base
include Sortable
include AfterCommitQueue
include CaseSensitivity
- include HasOwners
-
+
extend Gitlab::ConfigHelper
extend Enumerize
@@ -117,7 +116,6 @@ class Project < ActiveRecord::Base
has_many :hooks, dependent: :destroy, class_name: 'ProjectHook'
has_many :protected_branches, dependent: :destroy
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 :deploy_keys_projects, dependent: :destroy
has_many :deploy_keys, through: :deploy_keys_projects