From 19b80e82521384284227b31003889c9ac41b7c8c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Tue, 5 Jul 2016 18:55:35 +0200
Subject: [PATCH] Add a migration to remove requesters that are owners of their
 project
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 app/models/ability.rb                         | 22 +++++-----
 ...63108_remove_requesters_that_are_owners.rb | 40 +++++++++++++++++++
 db/schema.rb                                  |  2 +-
 3 files changed, 53 insertions(+), 11 deletions(-)
 create mode 100644 db/migrate/20160705163108_remove_requesters_that_are_owners.rb

diff --git a/app/models/ability.rb b/app/models/ability.rb
index 2c0fd0338fd..eeb0ceba081 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -171,14 +171,9 @@ 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 || group_member || project.team.member?(user)
+          unless owner || project.team.member?(user) || project_group_member?(project, user)
+            rules << :request_access
+          end
         end
 
         if project.archived?
@@ -501,8 +496,7 @@ class Ability
       target_user = subject.user
       project = subject.project
 
-      # Allow owners that requested access to their own project to destroy themselves
-      if target_user != project.owner || subject.request?
+      unless target_user == project.owner
         can_manage = project_abilities(user, project).include?(:admin_project_member)
 
         if can_manage
@@ -582,5 +576,13 @@ class Ability
 
       rules
     end
+
+    def project_group_member?(project, user)
+      project.group &&
+      (
+        project.group.members.exists?(user_id: user.id) ||
+        project.group.requesters.exists?(user_id: user.id)
+      )
+    end
   end
 end
diff --git a/db/migrate/20160705163108_remove_requesters_that_are_owners.rb b/db/migrate/20160705163108_remove_requesters_that_are_owners.rb
new file mode 100644
index 00000000000..1fca230c019
--- /dev/null
+++ b/db/migrate/20160705163108_remove_requesters_that_are_owners.rb
@@ -0,0 +1,40 @@
+class RemoveRequestersThatAreOwners < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  def up
+    # Delete requesters that are owner of their projects and actually requested
+    # access to it
+    execute <<-SQL
+      DELETE FROM members
+      WHERE members.source_type = 'Project'
+      AND members.type = 'ProjectMember'
+      AND members.requested_at IS NOT NULL
+      AND members.user_id = (
+        SELECT namespaces.owner_id
+        FROM namespaces
+        JOIN projects ON namespaces.id = projects.namespace_id
+        WHERE namespaces.type IS NULL
+        AND projects.id = members.source_id
+        AND namespaces.owner_id = members.user_id);
+    SQL
+
+    # Delete requesters that are owner of their project's group and actually requested
+    # access to it
+    execute <<-SQL
+      DELETE FROM members
+      WHERE members.source_type = 'Project'
+      AND members.type = 'ProjectMember'
+      AND members.requested_at IS NOT NULL
+      AND members.user_id = (
+        SELECT namespaces.owner_id
+        FROM namespaces
+        JOIN projects ON namespaces.id = projects.namespace_id
+        WHERE namespaces.type = 'Group'
+        AND projects.id = members.source_id
+        AND namespaces.owner_id = members.user_id);
+    SQL
+  end
+
+  def down
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5b9ed985fac..c1e88c1ed7e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160703180340) do
+ActiveRecord::Schema.define(version: 20160705163108) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
-- 
GitLab