diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2016-11-23 12:23:08 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2016-11-23 12:23:08 +0000 |
commit | ba0d4877934349603e24125864c3a46eba271e0e (patch) | |
tree | 3debde710f9d85bc978001c245260ec826c0fe82 /app/models | |
parent | 6f7ce372921a322c48e659d8382a04895fd29b1f (diff) | |
parent | 2ea5ef0ba4ee00b5551b88a6b9a68e045bf4b3f4 (diff) | |
download | gitlab-ce-ba0d4877934349603e24125864c3a46eba271e0e.tar.gz |
Merge branch 'fix/drop-project-authorized-for-user' into 'master'
Use authorized projects in ProjectTeam
Closes #23938 and #23636
See merge request !7586
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/group.rb | 4 | ||||
-rw-r--r-- | app/models/issue.rb | 2 | ||||
-rw-r--r-- | app/models/member.rb | 2 | ||||
-rw-r--r-- | app/models/namespace.rb | 8 | ||||
-rw-r--r-- | app/models/project.rb | 10 | ||||
-rw-r--r-- | app/models/project_team.rb | 117 | ||||
-rw-r--r-- | app/models/user.rb | 2 |
7 files changed, 35 insertions, 110 deletions
diff --git a/app/models/group.rb b/app/models/group.rb index 73b0f1c6572..4248e1162d8 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -65,7 +65,9 @@ class Group < Namespace def select_for_project_authorization if current_scope.joins_values.include?(:shared_projects) - select("members.user_id, projects.id AS project_id, project_group_links.group_access") + joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id') + .where('project_namespace.share_with_group_lock = ?', false) + .select("members.user_id, projects.id AS project_id, LEAST(project_group_links.group_access, members.access_level) AS access_level") else super end diff --git a/app/models/issue.rb b/app/models/issue.rb index 6e8f5d3c422..dd0cb75f9a8 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -93,7 +93,7 @@ class Issue < ActiveRecord::Base # Check if we are scoped to a specific project's issues if owner_project - if owner_project.authorized_for_user?(user, Gitlab::Access::REPORTER) + if owner_project.team.member?(user, Gitlab::Access::REPORTER) # If the project is authorized for the user, they can see all issues in the project return all else diff --git a/app/models/member.rb b/app/models/member.rb index 7be2665bf48..df93aaee847 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -246,7 +246,7 @@ class Member < ActiveRecord::Base end def post_update_hook - # override in subclass + UserProjectAccessChangedService.new(user.id).execute if access_level_changed? end def post_destroy_hook diff --git a/app/models/namespace.rb b/app/models/namespace.rb index b67049f0f55..99da26a89fb 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -27,6 +27,7 @@ class Namespace < ActiveRecord::Base delegate :name, to: :owner, allow_nil: true, prefix: true after_update :move_dir, if: :path_changed? + after_commit :refresh_access_of_projects_invited_groups, on: :update, if: -> { previous_changes.key?('share_with_group_lock') } # Save the storage paths before the projects are destroyed to use them on after destroy before_destroy(prepend: true) { @old_repository_storage_paths = repository_storage_paths } @@ -175,4 +176,11 @@ class Namespace < ActiveRecord::Base end end end + + def refresh_access_of_projects_invited_groups + Group. + joins(project_group_links: :project). + where(projects: { namespace_id: id }). + find_each(&:refresh_members_authorized_projects) + end end diff --git a/app/models/project.rb b/app/models/project.rb index 76c1fc4945d..bd9fcb2f3b7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -126,6 +126,8 @@ class Project < ActiveRecord::Base has_many :hooks, dependent: :destroy, class_name: 'ProjectHook' has_many :protected_branches, dependent: :destroy + has_many :project_authorizations, dependent: :destroy + has_many :authorized_users, through: :project_authorizations, source: :user, class_name: 'User' has_many :project_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source alias_method :members, :project_members has_many :users, through: :project_members @@ -1293,14 +1295,6 @@ class Project < ActiveRecord::Base end end - # Checks if `user` is authorized for this project, with at least the - # `min_access_level` (if given). - def authorized_for_user?(user, min_access_level = nil) - return false unless user - - user.authorized_project?(self, min_access_level) - end - def append_or_update_attribute(name, value) old_values = public_send(name.to_s) diff --git a/app/models/project_team.rb b/app/models/project_team.rb index 65549d8cd08..8a53e974b6f 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -80,19 +80,19 @@ class ProjectTeam alias_method :users, :members def guests - @guests ||= fetch_members(:guests) + @guests ||= fetch_members(Gitlab::Access::GUEST) end def reporters - @reporters ||= fetch_members(:reporters) + @reporters ||= fetch_members(Gitlab::Access::REPORTER) end def developers - @developers ||= fetch_members(:developers) + @developers ||= fetch_members(Gitlab::Access::DEVELOPER) end def masters - @masters ||= fetch_members(:masters) + @masters ||= fetch_members(Gitlab::Access::MASTER) end def import(source_project, current_user = nil) @@ -141,8 +141,12 @@ class ProjectTeam max_member_access(user.id) == Gitlab::Access::MASTER end - def member?(user, min_member_access = Gitlab::Access::GUEST) - max_member_access(user.id) >= min_member_access + # Checks if `user` is authorized for this project, with at least the + # `min_access_level` (if given). + def member?(user, min_access_level = Gitlab::Access::GUEST) + return false unless user + + user.authorized_project?(project, min_access_level) end def human_max_access(user_id) @@ -165,112 +169,29 @@ class ProjectTeam # Lookup only the IDs we need user_ids = user_ids - access.keys + users_access = project.project_authorizations. + where(user: user_ids). + group(:user_id). + maximum(:access_level) - if user_ids.present? - user_ids.each { |id| access[id] = Gitlab::Access::NO_ACCESS } - - member_access = project.members.access_for_user_ids(user_ids) - merge_max!(access, member_access) - - if group - group_access = group.members.access_for_user_ids(user_ids) - merge_max!(access, group_access) - end - - # Each group produces a list of maximum access level per user. We take the - # max of the values produced by each group. - if project_shared_with_group? - project.project_group_links.each do |group_link| - invited_access = max_invited_level_for_users(group_link, user_ids) - merge_max!(access, invited_access) - end - end - end - + access.merge!(users_access) access end def max_member_access(user_id) - max_member_access_for_user_ids([user_id])[user_id] + max_member_access_for_user_ids([user_id])[user_id] || Gitlab::Access::NO_ACCESS end private - # For a given group, return the maximum access level for the user. This is the min of - # the invited access level of the group and the access level of the user within the group. - # For example, if the group has been given DEVELOPER access but the member has MASTER access, - # the user should receive only DEVELOPER access. - def max_invited_level_for_users(group_link, user_ids) - invited_group = group_link.group - capped_access_level = group_link.group_access - access = invited_group.group_members.access_for_user_ids(user_ids) - - # If the user is not in the list, assume he/she does not have access - missing_users = user_ids - access.keys - missing_users.each { |id| access[id] = Gitlab::Access::NO_ACCESS } - - # Cap the maximum access by the invited level access - access.each { |key, value| access[key] = [value, capped_access_level].min } - end - def fetch_members(level = nil) - project_members = project.members - group_members = group ? group.members : [] - - if level - project_members = project_members.public_send(level) - group_members = group_members.public_send(level) if group - end - - user_ids = project_members.pluck(:user_id) - - invited_members = fetch_invited_members(level) - user_ids.push(*invited_members.map(&:user_id)) if invited_members.any? - - user_ids.push(*group_members.pluck(:user_id)) if group + members = project.authorized_users + members = members.where(project_authorizations: { access_level: level }) if level - User.where(id: user_ids) + members end def group project.group end - - def merge_max!(first_hash, second_hash) - first_hash.merge!(second_hash) { |_key, old, new| old > new ? old : new } - end - - def project_shared_with_group? - project.invited_groups.any? && project.allowed_to_share_with_group? - end - - def fetch_invited_members(level = nil) - invited_members = [] - - return invited_members unless project_shared_with_group? - - project.project_group_links.includes(group: [:group_members]).each do |link| - invited_group_members = link.group.members - - if level - numeric_level = GroupMember.access_level_roles[level.to_s.singularize.titleize] - - # If we're asked for a level that's higher than the group's access, - # there's nothing left to do - next if numeric_level > link.group_access - - # Make sure we include everyone _above_ the requested level as well - invited_group_members = - if numeric_level == link.group_access - invited_group_members.where("access_level >= ?", link.group_access) - else - invited_group_members.public_send(level) - end - end - - invited_members << invited_group_members - end - - invited_members.flatten.compact - end end diff --git a/app/models/user.rb b/app/models/user.rb index 29fb849940a..223d84ba916 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -926,7 +926,7 @@ class User < ActiveRecord::Base # Returns a union query of projects that the user is authorized to access def project_authorizations_union relations = [ - personal_projects.select("#{id} AS user_id, projects.id AS project_id, #{Gitlab::Access::OWNER} AS access_level"), + personal_projects.select("#{id} AS user_id, projects.id AS project_id, #{Gitlab::Access::MASTER} AS access_level"), groups_projects.select_for_project_authorization, projects.select_for_project_authorization, groups.joins(:shared_projects).select_for_project_authorization |