Download as
Browse Code »

Commit 0771109bb8f7d8a1b8132439806675110b1c9fd3

Authored by Dmitriy Zaporozhets
1 parent 995d193d
Exists in spb-stable and in 2 other branches 7-0-stable, 7-0-stable-spb

Fix permission issue with highest access level for group 

If user was a member of both group and project and group access level
was higher it was not respected and user got lowest project access
level. Now it is fixed and user get highest access level

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing 2 changed files with 66 additions and 25 deletions   Show diff stats
app/models/project_team.rb
  Diff comments   View file @0771109
@@ -118,19 +118,30 @@ class ProjectTeam @@ -118,19 +118,30 @@ class ProjectTeam
118 end 118 end
119 119
120 def guest?(user) 120 def guest?(user)
121 - find_tm(user.id).try(:access_field) == Gitlab::Access::GUEST 121 + max_tm_access(user.id) == Gitlab::Access::GUEST
122 end 122 end
123 123
124 def reporter?(user) 124 def reporter?(user)
125 - find_tm(user.id).try(:access_field) == Gitlab::Access::REPORTER 125 + max_tm_access(user.id) == Gitlab::Access::REPORTER
126 end 126 end
127 127
128 def developer?(user) 128 def developer?(user)
129 - find_tm(user.id).try(:access_field) == Gitlab::Access::DEVELOPER 129 + max_tm_access(user.id) == Gitlab::Access::DEVELOPER
130 end 130 end
131 131
132 def master?(user) 132 def master?(user)
133 - find_tm(user.id).try(:access_field) == Gitlab::Access::MASTER 133 + max_tm_access(user.id) == Gitlab::Access::MASTER
  134 + end
  135 +
  136 + def max_tm_access(user_id)
  137 + access = []
  138 + access << project.users_projects.find_by(user_id: user_id).try(:access_field)
  139 +
  140 + if group
  141 + access << group.users_groups.find_by(user_id: user_id).try(:access_field)
  142 + end
  143 +
  144 + access.compact.max
134 end 145 end
135 146
136 private 147 private
spec/models/project_team_spec.rb
  Diff comments   View file @0771109
1 require "spec_helper" 1 require "spec_helper"
2 2
3 describe ProjectTeam do 3 describe ProjectTeam do
4 - let(:group) { create(:group) }  
5 - let(:project) { create(:empty_project, group: group) }  
6 -  
7 let(:master) { create(:user) } 4 let(:master) { create(:user) }
8 let(:reporter) { create(:user) } 5 let(:reporter) { create(:user) }
9 let(:guest) { create(:user) } 6 let(:guest) { create(:user) }
10 let(:nonmember) { create(:user) } 7 let(:nonmember) { create(:user) }
11 8
12 - before do  
13 - group.add_user(master, Gitlab::Access::MASTER)  
14 - group.add_user(reporter, Gitlab::Access::REPORTER)  
15 - group.add_user(guest, Gitlab::Access::GUEST) 9 + context 'personal project' do
  10 + let(:project) { create(:empty_project) }
16 11
17 - # Add group guest as master to this project  
18 - # to test project access priority over group members  
19 - project.team << [guest, :master]  
20 - end 12 + before do
  13 + project.team << [master, :master]
  14 + project.team << [reporter, :reporter]
  15 + project.team << [guest, :guest]
  16 + end
21 17
22 - describe 'members collection' do  
23 - it { project.team.masters.should include(master) }  
24 - it { project.team.masters.should include(guest) }  
25 - it { project.team.masters.should_not include(reporter) }  
26 - it { project.team.masters.should_not include(nonmember) } 18 + describe 'members collection' do
  19 + it { project.team.masters.should include(master) }
  20 + it { project.team.masters.should_not include(guest) }
  21 + it { project.team.masters.should_not include(reporter) }
  22 + it { project.team.masters.should_not include(nonmember) }
  23 + end
  24 +
  25 + describe 'access methods' do
  26 + it { project.team.master?(master).should be_true }
  27 + it { project.team.master?(guest).should be_false }
  28 + it { project.team.master?(reporter).should be_false }
  29 + it { project.team.master?(nonmember).should be_false }
  30 + end
27 end 31 end
28 32
29 - describe 'access methods' do  
30 - it { project.team.master?(master).should be_true }  
31 - it { project.team.master?(guest).should be_true }  
32 - it { project.team.master?(reporter).should be_false }  
33 - it { project.team.master?(nonmember).should be_false } 33 + context 'group project' do
  34 + let(:group) { create(:group) }
  35 + let(:project) { create(:empty_project, group: group) }
  36 +
  37 + before do
  38 + group.add_user(master, Gitlab::Access::MASTER)
  39 + group.add_user(reporter, Gitlab::Access::REPORTER)
  40 + group.add_user(guest, Gitlab::Access::GUEST)
  41 +
  42 + # If user is a group and a project member - GitLab uses highest permission
  43 + # So we add group guest as master and add group master as guest
  44 + # to this project to test highest access
  45 + project.team << [guest, :master]
  46 + project.team << [master, :guest]
  47 + end
  48 +
  49 + describe 'members collection' do
  50 + it { project.team.reporters.should include(reporter) }
  51 + it { project.team.masters.should include(master) }
  52 + it { project.team.masters.should include(guest) }
  53 + it { project.team.masters.should_not include(reporter) }
  54 + it { project.team.masters.should_not include(nonmember) }
  55 + end
  56 +
  57 + describe 'access methods' do
  58 + it { project.team.reporter?(reporter).should be_true }
  59 + it { project.team.master?(master).should be_true }
  60 + it { project.team.master?(guest).should be_true }
  61 + it { project.team.master?(reporter).should be_false }
  62 + it { project.team.master?(nonmember).should be_false }
  63 + end
34 end 64 end
35 end 65 end
36 66