diff --git a/doc/api/groups.md b/doc/api/groups.md index c9ec64e83dbc421680a112418996c5fcfaacb2a7..f5c91e20b1a6d9cad5648a8217bf9f28f743fd31 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1285,6 +1285,87 @@ Example response: ] ``` +## List group users **(PREMIUM ALL EXPERIMENT)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/424505) in GitLab 16.6. This feature is an [Experiment](../policy/experiment-beta-support.md). + +Get a list of users for a group. This endpoint returns users that are related to a top-level group regardless +of their current membership. For example, users that have a SAML identity connected to the group, or service accounts created +by the group or subgroups. + +This endpoint is an [Experiment](../policy/experiment-beta-support.md) and might be changed or removed without notice. + +Requires at least the Maintainer role in the group. + +```plaintext +GET /groups/:id/users +``` + +```shell +curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups/345/users?include_saml_users=true&include_service_accounts=true" +``` + +Parameters: + +| Attribute | Type | Required | Description | +|:---------------------------|:---------------|:--------------------------|:-------------------------------------------------------------------------------| +| `id` | integer/string | yes | ID or [URL-encoded path of the group](rest/index.md#namespaced-path-encoding). | +| `include_saml_users` | boolean | yes (see description) | Include users with a SAML identity. Either this value or `include_service_accounts` must be `true`. | +| `include_service_accounts` | boolean | yes (see description) | Include service account users. Either this value or `include_saml_users` must be `true`. | +| `search` | string | no | Search users by name, email, username. | + +If successful, returns [`200 OK`](../api/rest/index.md#status-codes) and the +following response attributes: + +Example response: + +```json +[ + { + "id": 66, + "username": "user22", + "name": "John Doe22", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/xxx?s=80&d=identicon", + "web_url": "http://my.gitlab.com/user22", + "created_at": "2021-09-10T12:48:22.381Z", + "bio": "", + "location": null, + "public_email": "", + "skype": "", + "linkedin": "", + "twitter": "", + "website_url": "", + "organization": null, + "job_title": "", + "pronouns": null, + "bot": false, + "work_information": null, + "followers": 0, + "following": 0, + "local_time": null, + "last_sign_in_at": null, + "confirmed_at": "2021-09-10T12:48:22.330Z", + "last_activity_on": null, + "email": "user22@example.org", + "theme_id": 1, + "color_scheme_id": 1, + "projects_limit": 100000, + "current_sign_in_at": null, + "identities": [ ], + "can_create_group": true, + "can_create_project": true, + "two_factor_enabled": false, + "external": false, + "private_profile": false, + "commit_email": "user22@example.org", + "shared_runners_minutes_limit": null, + "extra_shared_runners_minutes_limit": null + }, + ... +] +``` + ## Service Accounts **(PREMIUM ALL)** ### Create Service Account User diff --git a/ee/app/finders/groups/users_finder.rb b/ee/app/finders/groups/users_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..67285f2310cbdc414605a7aa6f8ba74ae51bbacb --- /dev/null +++ b/ee/app/finders/groups/users_finder.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module Groups + class UsersFinder < UsersFinder + extend ::Gitlab::Utils::Override + + attr_accessor :group + + ALLOWED_FILTERS = [:include_saml_users, :include_service_accounts].freeze + + def initialize(current_user, group, params) + @current_user = current_user + @group = group.root_ancestor + @params = params || {} + + raise_params_error unless at_least_one_param_true? + end + + private + + override :base_scope + def base_scope + users = super + relations = relations_to_join(users) + + return relations.first if relations.size == 1 + + ::User.from_union(relations) # rubocop:disable CodeReuse/ActiveRecord -- Simple union of existing relations/scopes + end + + def relations_to_join(users) + relations = [] + + relations << saml_users(users) + relations << service_accounts(users) + + relations.compact + end + + def saml_users(users) + saml_provider_id = group.saml_provider + return unless params[:include_saml_users] && saml_provider_id + + users.with_saml_provider(saml_provider_id) + end + + def service_accounts(users) + return unless params[:include_service_accounts] + + users.service_account.with_provisioning_group(group) + end + + def at_least_one_param_true? + ALLOWED_FILTERS.any? { |param| params[param] } + end + + def raise_params_error + raise(ArgumentError, format(_("At least one of %{params} must be true"), params: ALLOWED_FILTERS.join(', '))) + end + end +end diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index 61b006cb35ca1b37eeb82652eb004120bf263b5e..817dcd3cb6ba3f9933eea0fd80dd077a358b9aa3 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -145,6 +145,12 @@ module User scope :with_provider, ->(provider) do joins(:identities).where(identities: { provider: provider }) end + scope :with_saml_provider, ->(saml_provider) do + joins(:identities).where(identities: { saml_provider: saml_provider }) + end + scope :with_provisioning_group, ->(group) do + joins(:user_detail).where(user_detail: { provisioned_by_group: group }) + end scope :with_invalid_expires_at_tokens, ->(expiration_date) do where(id: ::PersonalAccessToken.with_invalid_expires_at(expiration_date).select(:user_id)) diff --git a/ee/lib/ee/api/groups.rb b/ee/lib/ee/api/groups.rb index 3dfe983872db5b2a55cd9023a5bccac151589d0e..29731fc7b6ca4f100785a97e5c674f893dbda046 100644 --- a/ee/lib/ee/api/groups.rb +++ b/ee/lib/ee/api/groups.rb @@ -91,6 +91,16 @@ def filter_by_author(params) can?(current_user, :admin_group, user_group) ? params : params.merge(author_id: current_user.id) end + def users_params_error + format( + _("At least one of %{params} must be true"), params: ::Groups::UsersFinder::ALLOWED_FILTERS.join(', ') + ) + end + + def any_allowed_filters_present?(params) + ::Groups::UsersFinder::ALLOWED_FILTERS.any? { |param| params[param].presence } + end + def immediately_delete_subgroup_error(group) if !group.subgroup? '`permanently_remove` option is only available for subgroups.' @@ -251,6 +261,40 @@ def delete_group(group) end # rubocop: enable CodeReuse/ActiveRecord + desc 'Get a list of users for the group' do + success code: 200, model: ::API::Entities::UserPublic + failure [ + { code: 400, message: 'Bad request' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: '404 Not Found' } + ] + end + params do + optional :include_saml_users, + type: Grape::API::Boolean, + desc: 'Return users with a SAML identity in this group' + optional :include_service_accounts, + type: Grape::API::Boolean, + desc: 'Return service accounts owned by this group' + at_least_one_of :include_saml_users, :include_service_accounts + + optional :search, type: String, desc: 'Search users by name, email or username' + + use :pagination + end + # rubocop: disable CodeReuse/ActiveRecord + get ':id/users', feature_category: :user_management do + authorize! :read_group_member, user_group + params = declared_params(include_missing: false).except!(:id) + bad_request!(users_params_error) unless any_allowed_filters_present?(params) + + finder = ::Groups::UsersFinder.new(current_user, user_group, params) + users = finder.execute.preload(:identities, :scim_identities) + + present paginate(users), with: ::API::Entities::UserPublic + end + # rubocop: enable CodeReuse/ActiveRecord + desc 'Get a list of ssh certificates created for a group.' do summary 'Get a list of Groups::SshCertificate for a Group.' success code: 200, model: EE::API::Entities::SshCertificate diff --git a/ee/spec/finders/groups/users_finder_spec.rb b/ee/spec/finders/groups/users_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6e36df500d7ae461493f4f8d867ebcdfb0752902 --- /dev/null +++ b/ee/spec/finders/groups/users_finder_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::UsersFinder, feature_category: :user_management do + describe '#execute' do + let_it_be(:group) { create(:group) } + let_it_be(:other_group) { create(:group) } + let_it_be(:saml_provider) { create(:saml_provider, group: group) } + let_it_be(:other_saml_provider) { create(:saml_provider, group: other_group) } + + let_it_be(:saml_user1) { create(:user) } + let_it_be(:saml_user2) { create(:user) } + let_it_be(:service_account1) { create(:service_account, provisioned_by_group: group) } + let_it_be(:service_account2) { create(:service_account, provisioned_by_group: group) } + + let_it_be(:saml_identity1) do + create(:group_saml_identity, saml_provider: saml_provider, user: saml_user1) + end + + let_it_be(:saml_identity2) do + create(:group_saml_identity, saml_provider: saml_provider, user: saml_user2) + end + + let_it_be(:other_saml_user) { create(:user) } + + let_it_be(:other_saml_identity) do + create(:group_saml_identity, saml_provider: other_saml_provider, user: other_saml_user) + end + + let_it_be(:other_service_account) { create(:service_account, provisioned_by_group: other_group) } + + subject(:finder) { described_class.new(saml_user1, group, params) } + + context 'when params includes both saml users and service accounts' do + let(:params) { { include_saml_users: true, include_service_accounts: true } } + + it 'finds both saml users and service accounts' do + expect(finder.execute).to match_array([saml_user1, saml_user2, service_account1, service_account2]) + end + end + + context 'when params includes only saml users' do + let(:params) { { include_saml_users: true } } + + it 'finds only saml users' do + expect(finder.execute).to match_array([saml_user1, saml_user2]) + end + end + + context 'when params includes only service accounts' do + let(:params) { { include_service_accounts: true } } + + it 'finds only service accounts' do + expect(finder.execute).to match_array([service_account1, service_account2]) + end + end + + context 'when params do not include either saml users or service accounts' do + let(:params) { nil } + + it 'raises an argument error' do + expect { finder.execute } + .to raise_error(ArgumentError, + format( + _("At least one of %{params} must be true"), + params: described_class::ALLOWED_FILTERS.join(', ') + ) + ) + end + end + + context 'when params are all false' do + let(:params) { { include_saml_users: false, include_service_accounts: false } } + + it 'raises an argument error' do + expect { finder.execute } + .to raise_error(ArgumentError, + format( + _("At least one of %{params} must be true"), + params: described_class::ALLOWED_FILTERS.join(', ') + ) + ) + end + end + + context 'when a group has no SAML providers, SAML users, or Service Accounts' do + it 'returns no users' do + group = create(:group) + finder = described_class.new(saml_user1, group, { include_saml_users: true, include_service_accounts: true }) + + expect(finder.execute).to match_array([]) + end + end + end +end diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index 1d20cb0723539ab8d3ca9b205683c7be5f1c47a6..9bb4ce7bf80da10a4fc4a26204a5ac7dac3c4049 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -309,6 +309,59 @@ ) end end + + describe '.with_saml_provider' do + let_it_be(:user) { create(:user) } + let_it_be(:saml_provider) { create(:saml_provider) } + + subject(:with_saml_provider) { described_class.with_saml_provider(saml_provider) } + + it 'does not find users without a SAML identity' do + expect(with_saml_provider).to match_array([]) + end + + context 'when users have a SAML identity tied to the provider' do + let_it_be(:saml_identity) { create(:group_saml_identity, user: user, saml_provider: saml_provider) } + + it 'finds the matching users' do + expect(with_saml_provider).to match_array([user]) + end + + it 'does not find users with a different SAML provider' do + provider = create(:saml_provider) + + expect(described_class.with_saml_provider(provider)).to match_array([]) + end + end + end + + describe '.with_provisioning_group' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + + subject(:with_provisioning_group) { described_class.with_provisioning_group(group) } + + it 'does not find users without a provisioning group' do + expect(with_provisioning_group).to match_array([]) + end + + context 'when users have a provisioning group' do + before do + user.provisioned_by_group = group + user.save! + end + + it 'finds the matching users' do + expect(with_provisioning_group).to match_array([user]) + end + + it 'does not find users with a different SAML provider' do + group = create(:group) + + expect(described_class.with_provisioning_group(group)).to match_array([]) + end + end + end end describe 'after_create' do diff --git a/ee/spec/requests/api/groups_spec.rb b/ee/spec/requests/api/groups_spec.rb index 86bc1c62ea60de5ed00508347220336421e15832..091019f76b81a0e64ac3b602212422f755ae5996 100644 --- a/ee/spec/requests/api/groups_spec.rb +++ b/ee/spec/requests/api/groups_spec.rb @@ -1518,6 +1518,113 @@ end end + describe 'GET /groups/:id/users' do + let_it_be(:group) { create(:group, :private) } + let_it_be(:regular_user) { create(:user) } + let_it_be(:saml_provider) { create(:saml_provider, group: group) } + let_it_be(:group_member_user) { create(:user) { |u| group.add_developer(u) } } + + let_it_be(:saml_user) { create(:user, :public_email) } + let_it_be(:non_saml_user) { create(:user) { |u| group.add_maintainer(u) } } + let_it_be(:service_account) { create(:service_account, provisioned_by_group: group) } + + subject(:get_users) { get api("/groups/#{group.to_param}/users", current_user), params: params } + + before do + create(:saml_provider, group: group) + create(:group_saml_identity, user: group_member_user, saml_provider: saml_provider) + create(:group_saml_identity, user: saml_user, saml_provider: saml_provider) + end + + context 'when current_user is not a group member' do + let(:params) { { include_saml_users: true, include_service_accounts: true } } + + let_it_be(:current_user) { regular_user } + + it 'returns 404' do + get_users + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + shared_examples 'authorized current_user responses' do + context 'when no include params are present' do + let(:params) { {} } + + it 'returns 400' do + get_users + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when no include params are true' do + let(:params) { { include_saml_users: false } } + + it 'returns 404' do + get_users + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when all include params are present' do + let(:params) { { include_saml_users: true, include_service_accounts: true } } + + it 'returns a list of matching users' do + get_users + + expect(json_response.pluck('id')).to match_array([group_member_user.id, saml_user.id, service_account.id]) + end + end + + context 'when only include_saml_users is true' do + let(:params) { { include_saml_users: true } } + + it 'returns only SAML users' do + get_users + + expect(json_response.pluck('id')).to match_array([group_member_user.id, saml_user.id]) + end + end + + context 'when only include_service_accounts is true' do + let(:params) { { include_service_accounts: true } } + + it 'returns only service accounts' do + get_users + + expect(json_response.pluck('id')).to match_array([service_account.id]) + end + end + + context 'optional params' do + context 'search param' do + let(:params) { { include_saml_users: true, include_service_accounts: true, search: saml_user.email } } + + it 'filters by search' do + get_users + + expect(json_response.pluck('id')).to match_array([saml_user.id]) + end + end + end + end + + context 'when current_user is an admin', :enable_admin_mode do + let_it_be(:current_user) { create(:admin) } + + it_behaves_like 'authorized current_user responses' + end + + context 'when current_user is a group member' do + let_it_be(:current_user) { group_member_user } + + it_behaves_like 'authorized current_user responses' + end + end + shared_examples_for 'when unauthenticated' do it_behaves_like '401 response' do let(:message) { '401 Unauthorized' } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ab7cca65467c60ffe6670ba1a4b89533ca097af7..93b8216c031128d10113104761faf509c409b739 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6811,6 +6811,9 @@ msgstr "" msgid "At least one field of %{one_of_required_fields} must be present" msgstr "" +msgid "At least one of %{params} must be true" +msgstr "" + msgid "At least one of group_id or project_id must be specified" msgstr ""