Commit a0b3eb3d authored by Rémy Coutable's avatar Rémy Coutable Committed by Rémy Coutable

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: default avatarRémy Coutable <remy@rymai.me>
parent c158f6e1
...@@ -6,6 +6,7 @@ v 8.12.5 ...@@ -6,6 +6,7 @@ v 8.12.5
- Improve issue load time performance by avoiding ORDER BY in find_by call. !6724 - 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 - Add a new gitlab:users:clear_all_authentication_tokens task. !6745
- Don't send Private-Token (API authentication) headers to Sentry - 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 v 8.12.4
- Fix "Copy to clipboard" tooltip to say "Copied!" when clipboard button is clicked. !6294 (lukehowell) - Fix "Copy to clipboard" tooltip to say "Copied!" when clipboard button is clicked. !6294 (lukehowell)
......
...@@ -10,7 +10,7 @@ class ProjectGroupLink < ActiveRecord::Base ...@@ -10,7 +10,7 @@ class ProjectGroupLink < ActiveRecord::Base
belongs_to :group belongs_to :group
validates :project_id, presence: true 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_id, uniqueness: { scope: [:project_id], message: "already shared with this group" }
validates :group_access, presence: true validates :group_access, presence: true
validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true
......
...@@ -403,6 +403,12 @@ module API ...@@ -403,6 +403,12 @@ module API
authorize! :admin_project, user_project authorize! :admin_project, user_project
required_attributes! [:group_id, :group_access] 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? unless user_project.allowed_to_share_with_group?
return render_api_error!("The project sharing with group is disabled", 400) return render_api_error!("The project sharing with group is disabled", 400)
end end
......
...@@ -11,7 +11,7 @@ describe ProjectGroupLink do ...@@ -11,7 +11,7 @@ describe ProjectGroupLink do
it { should validate_presence_of(:project_id) } 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_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) } it { should validate_presence_of(:group_access) }
end end
end end
...@@ -786,6 +786,20 @@ describe API::API, api: true do ...@@ -786,6 +786,20 @@ describe API::API, api: true do
expect(response.status).to eq 400 expect(response.status).to eq 400
end 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 it "returns a 409 error when wrong params passed" do
post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: 1234 post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: 1234
expect(response.status).to eq 409 expect(response.status).to eq 409
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment