From 22ba5d8a7f0920f39ba33bdc4af54531ffe40b1e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Tue, 5 Jul 2016 14:24:58 +0200
Subject: New :request_access ability to replace a ugly helper
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Group / project members cannot request access
- Group members cannot request access to a group's project

This addresses an issue where project owners could request access
to their own project, leading to UI inconsistency where their requester
status would replace their owner status.

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 app/models/ability.rb | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

(limited to 'app/models/ability.rb')

diff --git a/app/models/ability.rb b/app/models/ability.rb
index ba1f2ae4075..ec4ef287421 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -157,10 +157,11 @@ class Ability
         # Push abilities on the users team role
         rules.push(*project_team_rules(project.team, user))
 
-        if project.owner == user ||
-          (project.group && project.group.has_owner?(user)) ||
-          user.admin?
+        owner = project.owner == user ||
+                (project.group && project.group.has_owner?(user)) ||
+                user.admin?
 
+        if owner
           rules.push(*project_owner_rules)
         end
 
@@ -169,6 +170,15 @@ class Ability
 
           # Allow to read builds for internal projects
           rules << :read_build if project.public_builds?
+
+          group_member =
+            project.group &&
+            (
+              project.group.members.exists?(user_id: user.id) ||
+              project.group.requesters.exists?(user_id: user.id)
+            )
+
+          rules << :request_access unless owner || project.team.member?(user) || group_member
         end
 
         if project.archived?
@@ -345,8 +355,11 @@ class Ability
       rules = []
       rules << :read_group if can_read_group?(user, group)
 
+      owner = group.has_owner?(user) || user.admin?
+      master = owner || user.admin?
+
       # Only group masters and group owners can create new projects
-      if group.has_master?(user) || group.has_owner?(user) || user.admin?
+      if master
         rules += [
           :create_projects,
           :admin_milestones
@@ -354,7 +367,7 @@ class Ability
       end
 
       # Only group owner and administrators can admin group
-      if group.has_owner?(user) || user.admin?
+      if owner
         rules += [
           :admin_group,
           :admin_namespace,
@@ -363,6 +376,10 @@ class Ability
         ]
       end
 
+      if (group.public? || (group.internal? && !user.external?))
+        rules << :request_access unless group.users.include?(user)
+      end
+
       rules.flatten
     end
 
@@ -484,7 +501,8 @@ class Ability
       target_user = subject.user
       project = subject.project
 
-      unless target_user == project.owner
+      # Allow owners that requested access to their own project to destroy themselves
+      if target_user != project.owner || subject.request?
         can_manage = project_abilities(user, project).include?(:admin_project_member)
 
         if can_manage
-- 
cgit v1.2.3