diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-01 07:28:22 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-06-01 07:28:28 +0000 |
commit | 37f194bbc19045abe013a58274494c1a6c8bbdd5 (patch) | |
tree | 99ae3d2a13d8d5592c8fabc7ed38d5117dbfe163 | |
parent | de222caa576cab3d0894c65531f5822f205877d5 (diff) | |
download | gitlab-ce-37f194bbc19045abe013a58274494c1a6c8bbdd5.tar.gz |
Add latest changes from gitlab-org/security/gitlab@15-0-stable-ee
-rw-r--r-- | app/assets/javascripts/gfm_auto_complete.js | 2 | ||||
-rw-r--r-- | app/controllers/groups/application_controller.rb | 6 | ||||
-rw-r--r-- | app/controllers/groups/group_members_controller.rb | 1 | ||||
-rw-r--r-- | app/policies/group_policy.rb | 9 | ||||
-rw-r--r-- | doc/user/group/subgroups/index.md | 3 | ||||
-rw-r--r-- | lib/api/helpers/members_helpers.rb | 4 | ||||
-rw-r--r-- | lib/api/members.rb | 8 | ||||
-rw-r--r-- | spec/features/security/group/private_access_spec.rb | 2 | ||||
-rw-r--r-- | spec/frontend/gfm_auto_complete_spec.js | 15 | ||||
-rw-r--r-- | spec/requests/api/members_spec.rb | 15 |
10 files changed, 63 insertions, 2 deletions
diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index b1af1ad797b..146255df31f 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -955,7 +955,7 @@ GfmAutoComplete.Milestones = { }; GfmAutoComplete.Contacts = { templateFunction({ email, firstName, lastName }) { - return `<li><small>${firstName} ${lastName}</small> ${escape(email)}</li>`; + return `<li><small>${escape(firstName)} ${escape(lastName)}</small> ${escape(email)}</li>`; }, }; diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index bf72ade32d0..aec3247f4b2 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -67,6 +67,12 @@ class Groups::ApplicationController < ApplicationController end end + def authorize_read_group_member! + unless can?(current_user, :read_group_member, group) + render_403 + end + end + def build_canonical_path(group) params[:group_id] = group.to_param diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index d325bb402e7..b95d8c87a4a 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -14,6 +14,7 @@ class Groups::GroupMembersController < Groups::ApplicationController # Authorize before_action :authorize_admin_group_member!, except: admin_not_required_endpoints + before_action :authorize_read_group_member!, only: :index skip_before_action :check_two_factor_requirement, only: :leave skip_cross_project_access_check :index, :update, :destroy, :request_access, diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index a4600c720a3..9aae295aea7 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -23,6 +23,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy condition(:parent_share_with_group_locked, scope: :subject) { @subject.parent&.share_with_group_lock? } condition(:can_change_parent_share_with_group_lock) { can?(:change_share_with_group_lock, @subject.parent) } condition(:migration_bot, scope: :user) { @user.migration_bot? } + condition(:can_read_group_member) { can_read_group_member? } desc "User is a project bot" condition(:project_bot) { user.project_bot? && access_level >= GroupMember::GUEST } @@ -128,6 +129,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy rule { ~public_group & ~has_access }.prevent :read_counts + rule { ~can_read_group_member }.policy do + prevent :read_group_member + end + rule { ~can?(:read_group) }.policy do prevent :read_design_activity end @@ -316,6 +321,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy true end + def can_read_group_member? + !(@subject.private? && access_level == GroupMember::NO_ACCESS) + end + def resource_access_token_creation_allowed? resource_access_token_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed? end diff --git a/doc/user/group/subgroups/index.md b/doc/user/group/subgroups/index.md index 2cddbaa9723..5f3c859d15a 100644 --- a/doc/user/group/subgroups/index.md +++ b/doc/user/group/subgroups/index.md @@ -93,6 +93,9 @@ For more information, view the [permissions table](../../permissions.md#group-me ## Subgroup membership +NOTE: +There is a bug that causes some pages in the parent group to be accessible by subgroup members. For more details, see [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/340421). + When you add a member to a group, that member is also added to all subgroups. The user's permissions are inherited from the group's parent. diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb index c91e153c7b9..6a3cf5c87ae 100644 --- a/lib/api/helpers/members_helpers.rb +++ b/lib/api/helpers/members_helpers.rb @@ -15,6 +15,10 @@ module API public_send("find_#{source_type}!", id) # rubocop:disable GitlabSecurity/PublicSend end + def authorize_read_source_member!(source_type, source) + authorize! :"read_#{source_type}_member", source + end + def authorize_admin_source!(source_type, source) authorize! :"admin_#{source_type}", source end diff --git a/lib/api/members.rb b/lib/api/members.rb index e2045c6def7..b94f68f60b5 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -32,6 +32,8 @@ module API get ":id/members", feature_category: feature_category do source = find_source(source_type, params[:id]) + authorize_read_source_member!(source_type, source) + members = paginate(retrieve_members(source, params: params)) present_members members @@ -51,6 +53,8 @@ module API get ":id/members/all", feature_category: feature_category do source = find_source(source_type, params[:id]) + authorize_read_source_member!(source_type, source) + members = paginate(retrieve_members(source, params: params, deep: true)) present_members members @@ -66,6 +70,8 @@ module API get ":id/members/:user_id", feature_category: feature_category do source = find_source(source_type, params[:id]) + authorize_read_source_member!(source_type, source) + members = source_members(source) member = members.find_by!(user_id: params[:user_id]) @@ -83,6 +89,8 @@ module API get ":id/members/all/:user_id", feature_category: feature_category do source = find_source(source_type, params[:id]) + authorize_read_source_member!(source_type, source) + members = find_all_members(source) member = members.find_by!(user_id: params[:user_id]) diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb index fc1fb3e3848..f733145b5e3 100644 --- a/spec/features/security/group/private_access_spec.rb +++ b/spec/features/security/group/private_access_spec.rb @@ -97,7 +97,7 @@ RSpec.describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_allowed_for(project_guest) } + it { is_expected.to be_denied_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } diff --git a/spec/frontend/gfm_auto_complete_spec.js b/spec/frontend/gfm_auto_complete_spec.js index aa98b2774ea..552377e3381 100644 --- a/spec/frontend/gfm_auto_complete_spec.js +++ b/spec/frontend/gfm_auto_complete_spec.js @@ -868,4 +868,19 @@ describe('GfmAutoComplete', () => { ); }); }); + + describe('Contacts', () => { + it('escapes name and email correct', () => { + const xssPayload = '<script>alert(1)</script>'; + const escapedPayload = '<script>alert(1)</script>'; + + expect( + GfmAutoComplete.Contacts.templateFunction({ + email: xssPayload, + firstName: xssPayload, + lastName: xssPayload, + }), + ).toBe(`<li><small>${escapedPayload} ${escapedPayload}</small> ${escapedPayload}</li>`); + }); + }); }); diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 0db42e7439c..63ef8643088 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -184,6 +184,21 @@ RSpec.describe API::Members do expect(json_response).to be_an Array expect(json_response.map { |u| u['id'] }).to match_array [maintainer.id, developer.id, nested_user.id] end + + context 'with a subgroup' do + let(:group) { create(:group, :private)} + let(:subgroup) { create(:group, :private, parent: group)} + let(:project) { create(:project, group: subgroup) } + + before do + subgroup.add_developer(developer) + end + + it 'subgroup member cannot get parent group members list' do + get api("/groups/#{group.id}/members/all", developer) + expect(response).to have_gitlab_http_status(:forbidden) + end + end end shared_examples 'GET /:source_type/:id/members/(all/):user_id' do |source_type, all| |