diff --git a/app/models/principals/scopes/visible.rb b/app/models/principals/scopes/visible.rb index cc03492307..38263d0fe1 100644 --- a/app/models/principals/scopes/visible.rb +++ b/app/models/principals/scopes/visible.rb @@ -38,7 +38,7 @@ module Principals::Scopes class_methods do def visible(user = ::User.current) - if user.allowed_to_globally?(:manage_members) + if user.allowed_to_globally?(:manage_members) || user.allowed_to_globally?(:manage_user) all else in_visible_project_or_me(user) diff --git a/app/models/queries/members/filters/principal_filter.rb b/app/models/queries/members/filters/principal_filter.rb index 6065757888..e9a75e666a 100644 --- a/app/models/queries/members/filters/principal_filter.rb +++ b/app/models/queries/members/filters/principal_filter.rb @@ -35,7 +35,7 @@ class Queries::Members::Filters::PrincipalFilter < Queries::Members::Filters::Me @allowed_values ||= begin values = Principal .not_locked - .in_visible_project_or_me + .visible .map { |s| [s.name, s.id.to_s] } .sort diff --git a/spec/models/principals/scopes/visible_spec.rb b/spec/models/principals/scopes/visible_spec.rb index e53431b533..d22b42f092 100644 --- a/spec/models/principals/scopes/visible_spec.rb +++ b/spec/models/principals/scopes/visible_spec.rb @@ -57,6 +57,14 @@ describe Principals::Scopes::Visible, type: :model do end end + context 'when user has no manage_members permission, but has manage_user global permission' do + current_user { FactoryBot.create :user, global_permissions: %i[manage_user] } + + it 'sees all users' do + expect(subject).to match_array [current_user, other_project_user, global_user] + end + end + context 'when user has no permission' do current_user { create :user } diff --git a/spec/models/queries/members/filters/principal_filter_spec.rb b/spec/models/queries/members/filters/principal_filter_spec.rb index 0b1c5ca782..42fa98d0e6 100644 --- a/spec/models/queries/members/filters/principal_filter_spec.rb +++ b/spec/models/queries/members/filters/principal_filter_spec.rb @@ -45,7 +45,7 @@ describe Queries::Members::Filters::PrincipalFilter, type: :model do .and_return(principal_scope) allow(principal_scope) - .to receive(:in_visible_project_or_me) + .to receive(:visible) .and_return([user, group, current_user]) end diff --git a/spec/requests/api/v3/membership_resources_spec.rb b/spec/requests/api/v3/membership_resources_spec.rb index c81ca7cb56..6df6b20de1 100644 --- a/spec/requests/api/v3/membership_resources_spec.rb +++ b/spec/requests/api/v3/membership_resources_spec.rb @@ -42,9 +42,9 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do end let(:own_member) do create(:member, - roles: [create(:role, permissions: permissions)], - project: project, - user: current_user) + roles: [create(:role, permissions: permissions)], + project: project, + user: current_user) end let(:permissions) { %i[view_members manage_members] } let(:project) { create(:project) } @@ -53,17 +53,17 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do let(:other_user) { create(:user) } let(:other_member) do create(:member, - roles: [other_role], - principal: other_user, - project: project) + roles: [other_role], + principal: other_user, + project: project) end let(:invisible_member) do create(:member, - roles: [create(:role)]) + roles: [create(:role)]) end let(:global_member) do create(:global_member, - roles: [global_role]) + roles: [global_role]) end subject(:response) { last_response } @@ -87,6 +87,8 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do describe 'GET api/v3/memberships' do let(:members) { [own_member, other_member, invisible_member, global_member] } + let(:filters) { nil } + let(:path) { api_v3_paths.path_for(:memberships, filters: filters, sort_by: [%i(id asc)]) } before do members @@ -96,9 +98,6 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do get path end - let(:filters) { nil } - let(:path) { api_v3_paths.path_for(:memberships, filters: filters, sort_by: [%i(id asc)]) } - context 'without params' do it 'responds 200 OK' do expect(subject.status).to eq(200) @@ -183,9 +182,9 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do let(:group) { create(:group) } let(:group_member) do create(:member, - roles: [create(:role)], - project: project, - principal: group) + roles: [create(:role)], + project: project, + principal: group) end let(:members) { [own_member, group_member] } @@ -214,9 +213,9 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do end let(:placeholder_member) do create(:member, - roles: [create(:role)], - project: project, - principal: placeholder_user) + roles: [create(:role)], + project: project, + principal: placeholder_user) end let(:members) { [own_member, placeholder_member] } @@ -239,7 +238,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do end end - context 'filtering by user name' do + context 'when filtering by user name' do let(:filters) do [{ 'any_name_attribute' => { 'operator' => '~', @@ -258,14 +257,14 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do end end - context 'filtering by project' do + context 'when filtering by project' do let(:members) { [own_member, other_member, invisible_member, own_other_member] } let(:own_other_member) do create(:member, - roles: [create(:role, permissions: permissions)], - project: other_project, - user: current_user) + roles: [create(:role, permissions: permissions)], + project: other_project, + user: current_user) end let(:other_project) { create(:project) } @@ -288,13 +287,13 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do end end - context 'filtering by principal' do + context 'when filtering by principal' do let(:group) { create(:group) } let(:group_member) do create(:member, - roles: [create(:role)], - principal: group, - project: project) + roles: [create(:role)], + principal: group, + project: project) end let(:members) { [own_member, other_member, group_member, invisible_member] } @@ -318,6 +317,24 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do .to be_json_eql(group_member.id.to_json) .at_path('_embedded/elements/1/id') end + + context 'when principal is a group without any memberships' do + let(:members) { [own_member, other_member, invisible_member] } + let(:filters) do + [{ 'principal' => { + 'operator' => '=', + 'values' => [group.id.to_s] + } }] + end + + it 'returns empty members' do + expect(subject.status).to eq(200) + + expect(subject.body) + .to be_json_eql([]) + .at_path('_embedded/elements') + end + end end context 'with the outdated created_on sort by (renamed to created_at)' do @@ -363,6 +380,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do context 'without permissions' do let(:permissions) { [] } + it 'is empty' do expect(subject.body) .to be_json_eql('0') @@ -416,7 +434,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do end it 'creates the member' do - expect(Member.find_by(user_id: principal.id, project: project)) + expect(Member.find_by(principal: principal, project: project)) .to be_present end @@ -551,6 +569,33 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do .to be_empty end end + + context 'when creating global role permission as admin' do + let(:current_user) { admin } + let(:project) { nil } + let(:expected_role) { global_role } + let(:body) do + { + _links: { + principal: { + href: principal_path + }, + roles: [ + { + href: api_v3_paths.role(global_role.id) + } + ] + }, + _meta: { + notificationMessage: { + raw: custom_message + } + } + }.to_json + end + + it_behaves_like 'successful member creation' + end end context 'for a placeholder user' do @@ -738,7 +783,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do it 'returns 200 OK' do expect(subject.status) - .to eql(200) + .to be(200) end it 'returns the member' do @@ -758,7 +803,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do it 'returns 404 NOT FOUND' do expect(subject.status) - .to eql(404) + .to be(404) end end @@ -767,7 +812,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do it 'returns 404 NOT FOUND' do expect(subject.status) - .to eql(404) + .to be(404) end end end @@ -869,13 +914,19 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do end context 'with a group' do + # first user has no direct roles + # second user has direct role `another_role` + # both users belong to a group which has `other_role`, so this role is inherited by users + # when updating `group` role from `other_role` to `another_role` + # expecting to have first user role changed from `other_role` to `another_role` + # and second user role extended from `[other_role]` to `[other_role, another_role]` because has direct role let(:group) do create(:group, member_in_project: project, member_through_role: other_role, members: users) end let(:principal) { group } let(:users) { [create(:user), create(:user)] } let(:other_member) do - Member.find_by(principal: group).tap do |m| + Member.find_by(principal: group).tap do # Behaves as if the user had that role before the role's membership was created. # Because the user had the role independent of the group, it is not to be removed. user_member = Member.find_by(principal: users.first) @@ -967,6 +1018,45 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do .to be_empty end end + + context 'when updating global role permission as admin' do + let(:group) do + create(:group, global_role: other_role, members: users) + end + let(:current_user) { admin } + let(:project) { nil } + let(:other_role) { create(:global_role) } + let(:another_role) { create(:global_role) } + + it 'responds with 200' do + expect(last_response.status).to eq(200) + end + + it 'updates the member and all inherited members but does not update memberships users have already had' do + # other member is the group member + expect(other_member.reload.roles) + .to match_array [another_role] + + expect(other_member.updated_at > other_member_updated_at) + .to be_truthy + + last_user_member = Member.find_by(principal: users.last) + + expect(last_user_member.roles) + .to match_array [another_role] + + expect(last_user_member.updated_at > last_user_member_updated_at) + .to be_truthy + + first_user_member = Member.find_by(principal: users.first) + + expect(first_user_member.roles.uniq) + .to match_array [other_role, another_role] + + expect(first_user_member.updated_at) + .to eql first_user_member_updated_at + end + end end context 'if attempting to empty the roles' do @@ -980,7 +1070,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do it 'returns 422' do expect(last_response.status) - .to eql(422) + .to be(422) expect(last_response.body) .to be_json_eql("Roles need to be assigned.".to_json) @@ -1004,7 +1094,7 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do it 'returns 422' do expect(last_response.status) - .to eql(422) + .to be(422) expect(last_response.body) .to be_json_eql("Roles has an unassignable role.".to_json) @@ -1016,9 +1106,9 @@ describe 'API v3 memberships resource', type: :request, content_type: :json do let(:other_project) do create(:project).tap do |p| create(:member, - project: p, - roles: [create(:role, permissions: [:manage_members])], - user: current_user) + project: p, + roles: [create(:role, permissions: [:manage_members])], + user: current_user) end end