Merge branch 'fix-permissions' into 'master'

Fix permissions

Fixes #1358

2 files changed, 66 insertions, 25 deletions

diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index afaca374130..0bbbd3d00e8 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -118,19 +118,30 @@ class ProjectTeam
   end
 
   def guest?(user)
-    find_tm(user.id).try(:access_field) == Gitlab::Access::GUEST
+    max_tm_access(user.id) == Gitlab::Access::GUEST
   end
 
   def reporter?(user)
-    find_tm(user.id).try(:access_field) == Gitlab::Access::REPORTER
+    max_tm_access(user.id) == Gitlab::Access::REPORTER
   end
 
   def developer?(user)
-    find_tm(user.id).try(:access_field) == Gitlab::Access::DEVELOPER
+    max_tm_access(user.id) == Gitlab::Access::DEVELOPER
   end
 
   def master?(user)
-    find_tm(user.id).try(:access_field) == Gitlab::Access::MASTER
+    max_tm_access(user.id) == Gitlab::Access::MASTER
+  end
+
+  def max_tm_access(user_id)
+    access = []
+    access << project.users_projects.find_by(user_id: user_id).try(:access_field)
+
+    if group
+      access << group.users_groups.find_by(user_id: user_id).try(:access_field)
+    end
+
+    access.compact.max
   end
 
   private
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index 1cc37719e7a..34c1a686c96 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -1,36 +1,66 @@
 require "spec_helper"
 
 describe ProjectTeam do
-  let(:group) { create(:group) }
-  let(:project) { create(:empty_project, group: group) }
-
   let(:master) { create(:user) }
   let(:reporter) { create(:user) }
   let(:guest) { create(:user) }
   let(:nonmember) { create(:user) }
 
-  before do
-    group.add_user(master, Gitlab::Access::MASTER)
-    group.add_user(reporter, Gitlab::Access::REPORTER)
-    group.add_user(guest, Gitlab::Access::GUEST)
+  context 'personal project' do
+    let(:project) { create(:empty_project) }
 
-    # Add group guest as master to this project
-    # to test project access priority over group members
-    project.team << [guest, :master]
-  end
+    before do
+      project.team << [master, :master]
+      project.team << [reporter, :reporter]
+      project.team << [guest, :guest]
+    end
 
-  describe 'members collection' do
-    it { project.team.masters.should include(master) }
-    it { project.team.masters.should include(guest) }
-    it { project.team.masters.should_not include(reporter) }
-    it { project.team.masters.should_not include(nonmember) }
+    describe 'members collection' do
+      it { project.team.masters.should include(master) }
+      it { project.team.masters.should_not include(guest) }
+      it { project.team.masters.should_not include(reporter) }
+      it { project.team.masters.should_not include(nonmember) }
+    end
+
+    describe 'access methods' do
+      it { project.team.master?(master).should be_true }
+      it { project.team.master?(guest).should be_false }
+      it { project.team.master?(reporter).should be_false }
+      it { project.team.master?(nonmember).should be_false }
+    end
   end
 
-  describe 'access methods' do
-    it { project.team.master?(master).should be_true }
-    it { project.team.master?(guest).should be_true }
-    it { project.team.master?(reporter).should be_false }
-    it { project.team.master?(nonmember).should be_false }
+  context 'group project' do
+    let(:group) { create(:group) }
+    let(:project) { create(:empty_project, group: group) }
+
+    before do
+      group.add_user(master, Gitlab::Access::MASTER)
+      group.add_user(reporter, Gitlab::Access::REPORTER)
+      group.add_user(guest, Gitlab::Access::GUEST)
+
+      # If user is a group and a project member - GitLab uses highest permission
+      # So we add group guest as master and add group master as guest
+      # to this project to test highest access
+      project.team << [guest, :master]
+      project.team << [master, :guest]
+    end
+
+    describe 'members collection' do
+      it { project.team.reporters.should include(reporter) }
+      it { project.team.masters.should include(master) }
+      it { project.team.masters.should include(guest) }
+      it { project.team.masters.should_not include(reporter) }
+      it { project.team.masters.should_not include(nonmember) }
+    end
+
+    describe 'access methods' do
+      it { project.team.reporter?(reporter).should be_true }
+      it { project.team.master?(master).should be_true }
+      it { project.team.master?(guest).should be_true }
+      it { project.team.master?(reporter).should be_false }
+      it { project.team.master?(nonmember).should be_false }
+    end
   end
 end