Commit 5351f7f3 authored by Blair Lunceford's avatar Blair Lunceford Committed by Markus Koller

Add API for "Share groups with groups"

parent 3e32b86e
......@@ -27,7 +27,7 @@ class Groups::GroupLinksController < Groups::ApplicationController
end
def destroy
Groups::GroupLinks::DestroyService.new(nil, nil).execute(@group_link)
Groups::GroupLinks::DestroyService.new(group, current_user).execute(@group_link)
respond_to do |format|
format.html do
......
......@@ -496,6 +496,11 @@ class Group < Namespace
# TODO: group hooks https://gitlab.com/gitlab-org/gitlab/-/issues/216904
end
def preload_shared_group_links
preloader = ActiveRecord::Associations::Preloader.new
preloader.preload(self, shared_with_group_links: [shared_with_group: :route])
end
private
def update_two_factor_requirement
......
......@@ -14,6 +14,7 @@ class GroupGroupLink < ApplicationRecord
presence: true
scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) }
scope :public_or_visible_to_user, ->(group, user) { where(shared_group: group, shared_with_group: Group.public_or_visible_to_user(user)) } # rubocop:disable Cop/GroupPublicOrVisibleToUser
def self.access_options
Gitlab::Access.options_with_owner
......
......@@ -5,7 +5,7 @@ module Groups
class CreateService < BaseService
def execute(shared_group)
unless group && shared_group &&
can?(current_user, :admin_group, shared_group) &&
can?(current_user, :admin_group_member, shared_group) &&
can?(current_user, :read_group, group)
return error('Not Found', 404)
end
......
......@@ -3,7 +3,11 @@
module Groups
module GroupLinks
class DestroyService < BaseService
def execute(one_or_more_links)
def execute(one_or_more_links, skip_authorization: false)
unless skip_authorization || group && can?(current_user, :admin_group_member, group)
return error('Not Found', 404)
end
links = Array(one_or_more_links)
if GroupGroupLink.delete(links)
......
......@@ -12,7 +12,7 @@ class RemoveExpiredGroupLinksWorker # rubocop:disable Scalability/IdempotentWork
end
GroupGroupLink.expired.find_in_batches do |link_batch|
Groups::GroupLinks::DestroyService.new(nil, nil).execute(link_batch)
Groups::GroupLinks::DestroyService.new(nil, nil).execute(link_batch, skip_authorization: true)
end
end
end
---
title: Add API support for sharing groups with groups
merge_request: 32008
author:
type: added
......@@ -428,6 +428,15 @@ Example response:
"file_template_project_id": 1,
"parent_id": null,
"created_at": "2020-01-15T12:36:29.590Z",
"shared_with_groups": [
{
"group_id": 28,
"group_name": "H5bp",
"group_full_path": "h5bp",
"group_access_level": 20,
"expires_at": null
}
],
"projects": [ // Deprecated and will be removed in API v5
{
"id": 7,
......@@ -1101,3 +1110,35 @@ Read more in the [Group Badges](group_badges.md) documentation.
## Group Import/Export
Read more in the [Group Import/Export](group_import_export.md) documentation.
## Share Groups with Groups
These endpoints create and delete links for sharing a group with another group. For more information, see the related discussion in the [GitLab Groups](../user/group/index.md#sharing-a-group-with-another-group) page.
### Create a link to share a group with another group
Share group with another group. Returns `200` and the [group details](#details-of-a-group) on success.
```plaintext
POST /groups/:id/share
```
| Attribute | Type | Required | Description |
| --------- | -------------- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) |
| `group_id` | integer | yes | The ID of the group to share with |
| `group_access` | integer | yes | The [permissions level](members.md) to grant the group |
| `expires_at` | string | no | Share expiration date in ISO 8601 format: 2016-09-26 |
### Delete link sharing group with another group
Unshare the group from another group. Returns `204` and no content on success.
```plaintext
DELETE /groups/:id/share/:group_id
```
| Attribute | Type | Required | Description |
| --------- | -------------- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) |
| `group_id` | integer | yes | The ID of the group to share with |
......@@ -134,6 +134,7 @@ module EE
break not_found! unless user_group.feature_available?(:adjourned_deletion_for_projects_and_groups)
result = ::Groups::RestoreService.new(user_group, current_user).execute
user_group.preload_shared_group_links
if result[:status] == :success
present user_group, with: ::API::Entities::GroupDetail, current_user: current_user
......
......@@ -3,6 +3,9 @@
module API
module Entities
class GroupDetail < Group
expose :shared_with_groups do |group, options|
SharedGroupWithGroup.represent(group.shared_with_group_links.public_or_visible_to_user(group, options[:current_user]))
end
expose :runners_token, if: lambda { |group, options| options[:user_can_admin_group] }
expose :projects, using: Entities::Project do |group, options|
projects = GroupProjectsFinder.new(
......
......@@ -90,7 +90,7 @@ module API
expose :build_coverage_regex
expose :ci_config_path, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) }
expose :shared_with_groups do |project, options|
SharedGroup.represent(project.project_group_links, options)
SharedGroupWithProject.represent(project.project_group_links, options)
end
expose :only_allow_merge_if_pipeline_succeeds
expose :request_access_enabled
......
# frozen_string_literal: true
module API
module Entities
class SharedGroupWithGroup < Grape::Entity
expose :shared_with_group_id, as: :group_id
expose :group_name do |group_link|
group_link.shared_with_group.name
end
expose :group_full_path do |group_link|
group_link.shared_with_group.full_path
end
expose :group_access, as: :group_access_level
expose :expires_at
end
end
end
......@@ -2,7 +2,7 @@
module API
module Entities
class SharedGroup < Grape::Entity
class SharedGroupWithProject < Grape::Entity
expose :group_id
expose :group_name do |group_link, options|
group_link.group.name
......
......@@ -151,6 +151,7 @@ module API
end
group = create_group
group.preload_shared_group_links
if group.persisted?
present group, with: Entities::GroupDetail, current_user: current_user
......@@ -175,6 +176,8 @@ module API
end
put ':id' do
group = find_group!(params[:id])
group.preload_shared_group_links
authorize! :admin_group, group
if update_group(group)
......@@ -193,6 +196,7 @@ module API
end
get ":id" do
group = find_group!(params[:id])
group.preload_shared_group_links
options = {
with: params[:with_projects] ? Entities::GroupDetail : Entities::Group,
......@@ -299,6 +303,7 @@ module API
post ":id/projects/:project_id", requirements: { project_id: /.+/ } do
authenticated_as_admin!
group = find_group!(params[:id])
group.preload_shared_group_links
project = find_project!(params[:project_id])
result = ::Projects::TransferService.new(project, current_user).execute(group)
......@@ -308,6 +313,49 @@ module API
render_api_error!("Failed to transfer project #{project.errors.messages}", 400)
end
end
desc 'Share a group with a group' do
success Entities::GroupDetail
end
params do
requires :group_id, type: Integer, desc: 'The ID of the group to share'
requires :group_access, type: Integer, values: Gitlab::Access.all_values, desc: 'The group access level'
optional :expires_at, type: Date, desc: 'Share expiration date'
end
post ":id/share" do
shared_group = find_group!(params[:id])
shared_with_group = find_group!(params[:group_id])
group_link_create_params = {
shared_group_access: params[:group_access],
expires_at: params[:expires_at]
}
result = ::Groups::GroupLinks::CreateService.new(shared_with_group, current_user, group_link_create_params).execute(shared_group)
shared_group.preload_shared_group_links
if result[:status] == :success
present shared_group, with: Entities::GroupDetail, current_user: current_user
else
render_api_error!(result[:message], result[:http_status])
end
end
params do
requires :group_id, type: Integer, desc: 'The ID of the shared group'
end
# rubocop: disable CodeReuse/ActiveRecord
delete ":id/share/:group_id" do
shared_group = find_group!(params[:id])
link = shared_group.shared_with_group_links.find_by(shared_with_group_id: params[:group_id])
not_found!('Group Link') unless link
::Groups::GroupLinks::DestroyService.new(shared_group, current_user).execute(link)
no_content!
end
# rubocop: enable CodeReuse/ActiveRecord
end
end
end
......
......@@ -29,6 +29,32 @@ describe GroupGroupLink do
])
end
end
describe '.public_or_visible_to_user' do
let!(:user_with_access) { create :user }
let!(:user_without_access) { create :user }
let!(:shared_with_group) { create :group, :private }
let!(:shared_group) { create :group }
let!(:private_group_group_link) { create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group) }
before do
shared_group.add_owner(user_with_access)
shared_group.add_owner(user_without_access)
shared_with_group.add_developer(user_with_access)
end
context 'when user can access shared group' do
it 'returns the private group' do
expect(described_class.public_or_visible_to_user(shared_group, user_with_access)).to include(private_group_group_link)
end
end
context 'when user does not have access to shared group' do
it 'does not return private group' do
expect(described_class.public_or_visible_to_user(shared_group, user_without_access)).not_to include(private_group_group_link)
end
end
end
end
describe 'validation' do
......
......@@ -436,6 +436,8 @@ describe API::Groups do
it "returns one of user1's groups" do
project = create(:project, namespace: group2, path: 'Foo')
create(:project_group_link, project: project, group: group1)
group = create(:group)
link = create(:group_group_link, shared_group: group1, shared_with_group: group)
get api("/groups/#{group1.id}", user1)
......@@ -460,6 +462,13 @@ describe API::Groups do
expect(json_response['full_path']).to eq(group1.full_path)
expect(json_response['parent_id']).to eq(group1.parent_id)
expect(json_response['created_at']).to be_present
expect(json_response['shared_with_groups']).to be_an Array
expect(json_response['shared_with_groups'].length).to eq(1)
expect(json_response['shared_with_groups'][0]['group_id']).to eq(group.id)
expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name)
expect(json_response['shared_with_groups'][0]['group_full_path']).to eq(group.full_path)
expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
expect(json_response['shared_with_groups'][0]).to have_key('expires_at')
expect(json_response['projects']).to be_an Array
expect(json_response['projects'].length).to eq(2)
expect(json_response['shared_projects']).to be_an Array
......@@ -526,7 +535,7 @@ describe API::Groups do
.to contain_exactly(projects[:public].id, projects[:internal].id)
end
it 'avoids N+1 queries' do
it 'avoids N+1 queries with project links' do
get api("/groups/#{group1.id}", admin)
control_count = ActiveRecord::QueryRecorder.new do
......@@ -539,6 +548,24 @@ describe API::Groups do
get api("/groups/#{group1.id}", admin)
end.not_to exceed_query_limit(control_count)
end
it 'avoids N+1 queries with shared group links' do
# setup at least 1 shared group, so that we record the queries that preload the nested associations too.
create(:group_group_link, shared_group: group1, shared_with_group: create(:group))
control_count = ActiveRecord::QueryRecorder.new do
get api("/groups/#{group1.id}", admin)
end.count
# setup "n" more shared groups
create(:group_group_link, shared_group: group1, shared_with_group: create(:group))
create(:group_group_link, shared_group: group1, shared_with_group: create(:group))
# test that no of queries for 1 shared group is same as for n shared groups
expect do
get api("/groups/#{group1.id}", admin)
end.not_to exceed_query_limit(control_count)
end
end
context "when authenticated as admin" do
......@@ -1528,4 +1555,173 @@ describe API::Groups do
group2.add_owner(user1)
end
end
describe "POST /groups/:id/share" do
shared_examples 'shares group with group' do
it "shares group with group" do
expires_at = 10.days.from_now.to_date
expect do
post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id, group_access: Gitlab::Access::DEVELOPER, expires_at: expires_at }
end.to change { group.shared_with_group_links.count }.by(1)
expect(response).to have_gitlab_http_status(:created)
expect(json_response['shared_with_groups']).to be_an Array
expect(json_response['shared_with_groups'].length).to eq(1)
expect(json_response['shared_with_groups'][0]['group_id']).to eq(shared_with_group.id)
expect(json_response['shared_with_groups'][0]['group_name']).to eq(shared_with_group.name)
expect(json_response['shared_with_groups'][0]['group_full_path']).to eq(shared_with_group.full_path)
expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(Gitlab::Access::DEVELOPER)
expect(json_response['shared_with_groups'][0]['expires_at']).to eq(expires_at.to_s)
end
it "returns a 400 error when group id is not given" do
post api("/groups/#{group.id}/share", user), params: { group_access: Gitlab::Access::DEVELOPER }
expect(response).to have_gitlab_http_status(:bad_request)
end
it "returns a 400 error when access level is not given" do
post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id }
expect(response).to have_gitlab_http_status(:bad_request)
end
it 'returns a 404 error when group does not exist' do
post api("/groups/#{group.id}/share", user), params: { group_id: non_existing_record_id, group_access: Gitlab::Access::DEVELOPER }
expect(response).to have_gitlab_http_status(:not_found)
end
it "returns a 400 error when wrong params passed" do
post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id, group_access: non_existing_record_access_level }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['error']).to eq 'group_access does not have a valid value'
end
it "returns a 409 error when link is not saved" do
allow(::Groups::GroupLinks::CreateService).to receive_message_chain(:new, :execute)
.and_return({ status: :error, http_status: 409, message: 'error' })
post api("/groups/#{group.id}/share", user), params: { group_id: shared_with_group.id, group_access: Gitlab::Access::DEVELOPER }
expect(response).to have_gitlab_http_status(:conflict)
end
end
context 'when authenticated as owner' do
let(:owner_group) { create(:group) }
let(:owner_user) { create(:user) }
before do
owner_group.add_owner(owner_user)
end
it_behaves_like 'shares group with group' do
let(:user) { owner_user }
let(:group) { owner_group }
let(:shared_with_group) { create(:group) }
end
end
context 'when the user is not the owner of the group' do
let(:group) { create(:group) }
let(:user4) { create(:user) }
let(:expires_at) { 10.days.from_now.to_date }
before do
group1.add_maintainer(user4)
end
it 'does not create group share' do
post api("/groups/#{group1.id}/share", user4), params: { group_id: group.id, group_access: Gitlab::Access::DEVELOPER, expires_at: expires_at }
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when authenticated as admin' do
it_behaves_like 'shares group with group' do
let(:user) { admin }
let(:group) { create(:group) }
let(:shared_with_group) { create(:group) }
end
end
end
describe 'DELETE /groups/:id/share/:group_id' do
shared_examples 'deletes group share' do
it 'deletes a group share' do
expect do
delete api("/groups/#{shared_group.id}/share/#{shared_with_group.id}", user)
expect(response).to have_gitlab_http_status(:no_content)
expect(shared_group.shared_with_group_links).to be_empty
end.to change { shared_group.shared_with_group_links.count }.by(-1)
end
it 'requires the group id to be an integer' do
delete api("/groups/#{shared_group.id}/share/foo", user)
expect(response).to have_gitlab_http_status(:bad_request)
end
it 'returns a 404 error when group link does not exist' do
delete api("/groups/#{shared_group.id}/share/#{non_existing_record_id}", user)
expect(response).to have_gitlab_http_status(:not_found)
end
it 'returns a 404 error when group does not exist' do
delete api("/groups/123/share/#{non_existing_record_id}", user)
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when authenticated as owner' do
let(:group_a) { create(:group) }
before do
create(:group_group_link, shared_group: group1, shared_with_group: group_a)
end
it_behaves_like 'deletes group share' do
let(:user) { user1 }
let(:shared_group) { group1 }
let(:shared_with_group) { group_a }
end
end
context 'when the user is not the owner of the group' do
let(:group_a) { create(:group) }
let(:user4) { create(:user) }
before do
group1.add_maintainer(user4)
create(:group_group_link, shared_group: group1, shared_with_group: group_a)
end
it 'does not remove group share' do
expect do
delete api("/groups/#{group1.id}/share/#{group_a.id}", user4)
expect(response).to have_gitlab_http_status(:no_content)
end.not_to change { group1.shared_with_group_links }
end
end
context 'when authenticated as admin' do
let(:group_b) { create(:group) }
before do
create(:group_group_link, shared_group: group2, shared_with_group: group_b)
end
it_behaves_like 'deletes group share' do
let(:user) { admin }
let(:shared_group) { group2 }
let(:shared_with_group) { group_b }
end
end
end
end
......@@ -8,14 +8,20 @@ describe Groups::GroupLinks::DestroyService, '#execute' do
let_it_be(:group) { create(:group, :private) }
let_it_be(:shared_group) { create(:group, :private) }
let_it_be(:project) { create(:project, group: shared_group) }
let_it_be(:owner) { create(:user) }
subject { described_class.new(nil, nil) }
before do
group.add_developer(owner)
shared_group.add_owner(owner)
end
subject { described_class.new(shared_group, owner) }
context 'single link' do
let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) }
it 'destroys link' do
expect { subject.execute(link) }.to change { GroupGroupLink.count }.from(1).to(0)
expect { subject.execute(link) }.to change { shared_group.shared_with_group_links.count }.from(1).to(0)
end
it 'revokes project authorization' do
......
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