diff options
author | Rémy Coutable <remy@gitlab.com> | 2016-10-11 10:20:35 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-10-11 13:51:22 +0200 |
commit | a0b3eb3db5a2d1146ddd44bf7475f19788b1dae9 (patch) | |
tree | ceff67c646d84e715880139ebf86fdbe96e36e8d | |
parent | c158f6e186cbbe77d2a790b9de2df3678371f30e (diff) | |
download | gitlab-ce-a0b3eb3db5a2d1146ddd44bf7475f19788b1dae9.tar.gz |
Merge branch 'api-fix-project-group-sharing' into 'security'
API: Share projects only with groups current_user can access
Aims to address the issues here: https://gitlab.com/gitlab-org/gitlab-ce/issues/23004
* Projects can be shared with non-existent groups
* Projects can be shared with groups that the current user does not have access to read
Concerns:
The new implementation of the API endpoint allows projects to be shared with a larger range of groups than can be done via the web UI.
The form for sharing a project with a group uses the following API endpoint to index the available groups: https://gitlab.com/gitlab-org/gitlab-ce/blob/494269fc92f61098ee6bd635a0426129ce2c5456/lib/api/groups.rb#L17. The groups indexed in the web form will only be those groups that the user is currently a member of.
The new implementation allows projects to be shared with any group that the authenticated user has access to view. This widens the range of groups to those that are public and internal.
See merge request !2005
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/models/project_group_link.rb | 2 | ||||
-rw-r--r-- | lib/api/projects.rb | 6 | ||||
-rw-r--r-- | spec/models/project_group_link_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/projects_spec.rb | 14 |
5 files changed, 23 insertions, 2 deletions
diff --git a/CHANGELOG b/CHANGELOG index ffab3092a84..ca557b63a4e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 8.12.5 - Improve issue load time performance by avoiding ORDER BY in find_by call. !6724 - Add a new gitlab:users:clear_all_authentication_tokens task. !6745 - Don't send Private-Token (API authentication) headers to Sentry + - Share projects via the API only with groups the authenticated user can access v 8.12.4 - Fix "Copy to clipboard" tooltip to say "Copied!" when clipboard button is clicked. !6294 (lukehowell) diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb index 7613cbdea93..db46def11eb 100644 --- a/app/models/project_group_link.rb +++ b/app/models/project_group_link.rb @@ -10,7 +10,7 @@ class ProjectGroupLink < ActiveRecord::Base belongs_to :group validates :project_id, presence: true - validates :group_id, presence: true + validates :group, presence: true validates :group_id, uniqueness: { scope: [:project_id], message: "already shared with this group" } validates :group_access, presence: true validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 6d99617b56f..8452320cdd3 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -403,6 +403,12 @@ module API authorize! :admin_project, user_project required_attributes! [:group_id, :group_access] + group = Group.find_by_id(attrs[:group_id]) + + unless group && can?(current_user, :read_group, group) + not_found!('Group') + end + unless user_project.allowed_to_share_with_group? return render_api_error!("The project sharing with group is disabled", 400) end diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index 2fa6715fcaf..c5ff1941378 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -11,7 +11,7 @@ describe ProjectGroupLink do it { should validate_presence_of(:project_id) } it { should validate_uniqueness_of(:group_id).scoped_to(:project_id).with_message(/already shared/) } - it { should validate_presence_of(:group_id) } + it { should validate_presence_of(:group) } it { should validate_presence_of(:group_access) } end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 192c7d14c13..5f7040d884b 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -786,6 +786,20 @@ describe API::API, api: true do expect(response.status).to eq 400 end + it 'returns a 404 error when user cannot read group' do + private_group = create(:group, :private) + + post api("/projects/#{project.id}/share", user), group_id: private_group.id, group_access: Gitlab::Access::DEVELOPER + + expect(response.status).to eq 404 + end + + it 'returns a 404 error when group does not exist' do + post api("/projects/#{project.id}/share", user), group_id: 1234, group_access: Gitlab::Access::DEVELOPER + + expect(response.status).to eq 404 + end + it "returns a 409 error when wrong params passed" do post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: 1234 expect(response.status).to eq 409 |