Merge branch '351581-follow-up-fix-with-full-path' into 'master'

Fix provisioned_users API endpoint with full_path

See merge request gitlab-org/gitlab!80061

doc/api/groups.md

@@ -1090,6 +1090,79 @@ GET /groups?search=foobar
 ]
 ```
 
+## List provisioned users **(PREMIUM)**
+
+> Introduced in GitLab 14.8.
+
+Get a list of users provisioned by a given group. Does not include users provisioned by subgroups.
+
+Requires at least the Maintainer role on the group.
+
+```plaintext
+GET /groups/:id/provisioned_users
+```
+
+Parameters:
+
+| Attribute        | Type           | Required | Description                                                              |
+|:-----------------|:---------------|:---------|:-------------------------------------------------------------------------|
+| `id`             | integer/string | yes      | ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) |
+| `username`       | string         | no       | Return single user with a specific username                              |
+| `search`         | string         | no       | Search users by name, email, username                                    |
+| `active`         | boolean        | no       | Return only active users                                                 |
+| `blocked`        | boolean        | no       | Return only blocked users                                                |
+| `created_after`  | datetime       | no       | Return users created after the specified time                            |
+| `created_before` | datetime       | no       | Return users created before the specified time                           |
+
+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
+  },
+  ...
+]
+```
+
 ## Hooks **(PREMIUM)**
 
 Also called Group Hooks and Webhooks.
@@ -1524,75 +1597,3 @@ DELETE /groups/:id/push_rule
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
 | `id` | integer/string | yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) |
-
-## Provisioned users API **(PREMIUM)**
-
-> Introduced in GitLab 14.8.
-
-### List provisioned users
-
-Get a list of users provisioned by a given group. Does not include users provisioned by subgroups.
-Requires at least the Maintainer role on the group.
-
-```plaintext
-GET /groups/:provisioned_group_id/provisioned_users
-```
-
-Parameters:
-
-| Attribute              | Type     | Required | Description                                   |
-|------------------------|----------|----------|-----------------------------------------------|
-| `provisioned_group_id` | integer  | yes      | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding)                               |
-| `username`             | string   | no       | Get a single user with a specific username    |
-| `search`               | string   | no       | Filter list by name, email or username        |
-| `active`               | boolean  | no       | Filters only active users |
-| `blocked`              | boolean  | no       | Filters only blocked users |
-| `created_after`               | datetime | no       | Return users created after the specified time |
-| `created_before`               | datetime | no       | Return users created before the specified time |
-
-```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
-  },
-  ...
-]
-```

ee/app/finders/auth/provisioned_users_finder.rb

@@ -8,10 +8,10 @@ module Auth
 
     override :base_scope
     def base_scope
-      group_id = params[:provisioning_group_id]
-      raise "Provisioning group is required for ProvisionedUsersFinder" unless group_id
+      group = params[:provisioning_group]
+      raise(ArgumentError, "Provisioning group is required for ProvisionedUsersFinder") unless group
 
-      User.provisioned_by_group(group_id).order_id_desc
+      group.provisioned_users.order_id_desc
     end
 
     override :by_search

ee/app/models/ee/user.rb

@@ -108,10 +108,6 @@ module EE
         where(id: ::PersonalAccessToken.with_invalid_expires_at(expiration_date).select(:user_id))
       end
 
-      scope :provisioned_by_group, -> (group_id) do
-        left_joins(:user_detail).where(user_detail: { provisioned_by_group_id: group_id })
-      end
-
       accepts_nested_attributes_for :namespace
 
       enum roadmap_layout: { weeks: 1, months: 4, quarters: 12 }

ee/lib/ee/api/groups.rb

@@ -161,26 +161,25 @@ module EE
           desc 'Get a list of users provisioned by the group' do
             success Entities::UserPublic
           end
+          params do
+            optional :username, type: String, desc: 'Return a single user with a specific username'
+            optional :search, type: String, desc: 'Search users by name, email or username'
+            optional :active, type: Grape::API::Boolean, default: false, desc: 'Return only active users'
+            optional :blocked, type: Grape::API::Boolean, default: false, desc: 'Return only blocked users'
+            optional :created_after, type: DateTime, desc: 'Return users created after the specified time'
+            optional :created_before, type: DateTime, desc: 'Return users created before the specified time'
+
+            use :pagination
+          end
           # rubocop: disable CodeReuse/ActiveRecord
-          get ':provisioning_group_id/provisioned_users' do
-            params do
-              requires :provisioning_group_id, type: String, desc: 'The ID of the group'
-
-              optional :username, type: String, desc: 'Get a single user with a specific username'
-              optional :search, type: String, desc: 'Search for a user name, email or username'
-              optional :active, type: Boolean, default: false, desc: 'Filters only active users'
-              optional :blocked, type: Boolean, default: false, desc: 'Filters only blocked users'
-              optional :created_after, type: DateTime, desc: 'Return users created after the specified time'
-              optional :created_before, type: DateTime, desc: 'Return users created before the specified time'
-
-              use :sort_params
-              use :pagination
-            end
+          get ':id/provisioned_users' do
+            authorize! :maintainer_access, user_group
 
-            group = find_group!(params[:provisioning_group_id])
-            authorize! :maintainer_access, group
+            finder = ::Auth::ProvisionedUsersFinder.new(
+              current_user,
+              declared_params.merge(provisioning_group: user_group))
 
-            users = ::Auth::ProvisionedUsersFinder.new(current_user, params).execute.preload(:identities)
+            users = finder.execute.preload(:identities)
 
             present paginate(users), with: ::API::Entities::UserPublic
           end

ee/spec/finders/auth/provisioned_users_finder_spec.rb

@@ -23,16 +23,16 @@ RSpec.describe Auth::ProvisionedUsersFinder do
     subject(:finder) { described_class.new(user, params).execute }
 
     describe '#base_scope' do
-      context 'when provisioning_group_id param is not passed' do
-        let(:params) { { provisioning_group_id: nil } }
+      context 'when provisioning_group param is not passed' do
+        let(:params) { { provisioning_group: nil } }
 
         it 'raises provisioning group error' do
-          expect { finder }.to raise_error RuntimeError, 'Provisioning group is required for ProvisionedUsersFinder'
+          expect { finder }.to raise_error ArgumentError, 'Provisioning group is required for ProvisionedUsersFinder'
         end
       end
 
-      context 'when provisioning_group_id param is passed' do
-        let(:params) { { provisioning_group_id: group.id } }
+      context 'when provisioning_group param is passed' do
+        let(:params) { { provisioning_group: group } }
 
         it 'returns provisioned_user' do
           users = finder
@@ -42,7 +42,7 @@ RSpec.describe Auth::ProvisionedUsersFinder do
     end
 
     describe '#by_search' do
-      let(:params) { { provisioning_group_id: group.id, search: provisioned_user.email } }
+      let(:params) { { provisioning_group: group, search: provisioned_user.email } }
 
       it 'filters by search' do
         users = finder

ee/spec/models/ee/user_spec.rb

@@ -104,18 +104,6 @@ RSpec.describe User do
         expect(described_class.managed_by(group)).to match_array(managed_users)
       end
     end
-
-    describe '.provisioned_by_group' do
-      let_it_be(:group) { create(:group) }
-      let_it_be(:scim_identity) { create(:scim_identity, group: group) }
-      let_it_be(:provisioned_user) { create(:user, provisioned_by_group_id: scim_identity.group.id) }
-      let_it_be(:non_provisioned_user) { create(:user).tap { |u| group.add_developer(u) } }
-      let_it_be(:user) { create(:user) }
-
-      it 'returns users provisioned or managed by the specified group' do
-        expect(described_class.provisioned_by_group(group.id)).to match_array([provisioned_user])
-      end
-    end
   end
 
   describe 'after_create' do

ee/spec/requests/api/groups_spec.rb

@@ -1056,7 +1056,7 @@ RSpec.describe API::Groups do
     end
   end
 
-  describe 'GET /groups/:provisioning_group_id/provisioned_users' do
+  describe 'GET /groups/:id/provisioned_users' do
     let_it_be(:group) { create(:group) }
     let_it_be(:regular_user) { create(:user) }
     let_it_be(:saml_provider) { create(:saml_provider, group: group) }
@@ -1068,9 +1068,9 @@ RSpec.describe API::Groups do
     let_it_be(:blocked_provisioned_user) { create(:user, :blocked, provisioned_by_group_id: group.id) }
     let_it_be(:non_provisioned_user) { create(:user) { |u| group.add_maintainer(u) } }
 
-    let(:params) { { provisioning_group_id: group.id } }
+    let(:params) { {} }
 
-    subject(:get_provisioned_users) { get api("/groups/#{params[:provisioning_group_id]}/provisioned_users", current_user), params: params }
+    subject(:get_provisioned_users) { get api("/groups/#{group.to_param}/provisioned_users", current_user), params: params }
 
     context 'when current_user is not a group maintainer' do
       let_it_be(:current_user) { developer }
@@ -1085,16 +1085,6 @@ RSpec.describe API::Groups do
     context 'when current_user is a group maintainer' do
       let_it_be(:current_user) { maintainer }
 
-      context 'requires group id' do
-        let(:params) { { provisioning_group_id: nil } }
-
-        it 'group not found' do
-          get_provisioned_users
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-
       it 'returns a list of users provisioned by the group' do
         get_provisioned_users
 
@@ -1103,7 +1093,7 @@ RSpec.describe API::Groups do
 
       context 'optional params' do
         context 'search param' do
-          let(:params) { { provisioning_group_id: group.id, search: provisioned_user.email } }
+          let(:params) { { search: provisioned_user.email } }
 
           it 'filters by search' do
             get_provisioned_users
@@ -1113,7 +1103,7 @@ RSpec.describe API::Groups do
         end
 
         context 'username param' do
-          let(:params) { { provisioning_group_id: group.id, username: provisioned_user.username } }
+          let(:params) { { username: provisioned_user.username } }
 
           it 'filters by username' do
             get_provisioned_users
@@ -1123,7 +1113,7 @@ RSpec.describe API::Groups do
         end
 
         context 'blocked param' do
-          let(:params) { { provisioning_group_id: group.id, blocked: true } }
+          let(:params) { { blocked: true } }
 
           it 'filters by blocked' do
             get_provisioned_users
@@ -1133,7 +1123,7 @@ RSpec.describe API::Groups do
         end
 
         context 'active param' do
-          let(:params) { { provisioning_group_id: group.id, active: true } }
+          let(:params) { { active: true } }
 
           it 'filters by active status' do
             get_provisioned_users
@@ -1143,7 +1133,7 @@ RSpec.describe API::Groups do
         end
 
         context 'created_after' do
-          let(:params) { { provisioning_group_id: group.id, created_after: 1.year.ago } }
+          let(:params) { { created_after: 1.year.ago } }
 
           it 'filters by created_at' do
             get_provisioned_users
@@ -1153,7 +1143,7 @@ RSpec.describe API::Groups do
         end
 
         context 'created_before' do
-          let(:params) { { provisioning_group_id: group.id, created_before: 1.year.ago } }
+          let(:params) { { created_before: 1.year.ago } }
 
           it 'filters by created_at' do
             get_provisioned_users