Merge branch 'feature/include-custom-attributes-in-api' into 'master'

Allow including custom attributes in API responses

See merge request gitlab-org/gitlab-ce!16526

changelogs/unreleased/feature-include-custom-attributes-in-api.yml

+---
+title: Allow including custom attributes in API responses
+merge_request: 16526
+author: Markus Koller
+type: changed

doc/api/groups.md

@@ -15,6 +15,7 @@ Parameters:
 | `order_by` | string | no | Order groups by `name` or `path`. Default is `name` |
 | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` |
 | `statistics` | boolean | no | Include group statistics (admins only) |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 | `owned` | boolean | no | Limit to groups owned by the current user |
 
 ```
@@ -98,6 +99,7 @@ Parameters:
 | `order_by` | string | no | Order groups by `name` or `path`. Default is `name` |
 | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` |
 | `statistics` | boolean | no | Include group statistics (admins only) |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 | `owned` | boolean | no | Limit to groups owned by the current user |
 
 ```
@@ -145,6 +147,7 @@ Parameters:
 | `simple` | boolean | no | Return only the ID, URL, name, and path of each project |
 | `owned` | boolean | no | Limit by projects owned by the current user |
 | `starred` | boolean | no | Limit by projects starred by the current user |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 
 Example response:
 
@@ -204,6 +207,7 @@ Parameters:
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
 | `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 
 ```bash
 curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/4

doc/api/projects.md

@@ -37,6 +37,7 @@ GET /projects
 | `membership` | boolean | no | Limit by projects that the current user is a member of |
 | `starred` | boolean | no | Limit by projects starred by the current user |
 | `statistics` | boolean | no | Include project statistics |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 | `with_issues_enabled` | boolean | no | Limit by enabled issues feature |
 | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature |
 
@@ -220,6 +221,7 @@ GET /users/:user_id/projects
 | `membership` | boolean | no | Limit by projects that the current user is a member of |
 | `starred` | boolean | no | Limit by projects starred by the current user |
 | `statistics` | boolean | no | Include project statistics |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 | `with_issues_enabled` | boolean | no | Limit by enabled issues feature |
 | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature |
 
@@ -388,6 +390,7 @@ GET /projects/:id
 | --------- | ---- | -------- | ----------- |
 | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
 | `statistics` | boolean | no | Include project statistics |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 
 ```json
 {
@@ -664,6 +667,7 @@ GET /projects/:id/forks
 | `membership` | boolean | no | Limit by projects that the current user is a member of |
 | `starred` | boolean | no | Limit by projects starred by the current user |
 | `statistics` | boolean | no | Include project statistics |
+| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) |
 | `with_issues_enabled` | boolean | no | Limit by enabled issues feature |
 | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature |
 

doc/api/users.md

@@ -165,6 +165,12 @@ You can filter by [custom attributes](custom_attributes.md) with:
 GET /users?custom_attributes[key]=value&custom_attributes[other_key]=other_value
 ```
 
+You can include the users' [custom attributes](custom_attributes.md) in the response with:
+
+```
+GET /users?with_custom_attributes=true
+```
+
 ## Single user
 
 Get a single user.
@@ -245,6 +251,12 @@ Parameters:
 }
 ```
 
+You can include the user's [custom attributes](custom_attributes.md) in the response with:
+
+```
+GET /users/:id?with_custom_attributes=true
+```
+
 ## User creation
 
 Creates a new user. Note only administrators can create new users. Either `password` or `reset_password` should be specified (`reset_password` takes priority).

lib/api/entities.rb

@@ -22,6 +22,7 @@ module API
       end
 
       expose :avatar_path, if: ->(user, options) { options.fetch(:only_path, false) && user.avatar_path }
+      expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes
 
       expose :web_url do |user, options|
         Gitlab::Routing.url_helpers.user_url(user)
@@ -109,6 +110,8 @@ module API
       expose :star_count, :forks_count
       expose :last_activity_at
 
+      expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes
+
       def self.preload_relation(projects_relation, options =  {})
         projects_relation.preload(:project_feature, :route)
                          .preload(namespace: [:route, :owner],
@@ -230,6 +233,8 @@ module API
         expose :parent_id
       end
 
+      expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes
+
       expose :statistics, if: :statistics do
         with_options format_with: -> (value) { value.to_i } do
           expose :storage_size

lib/api/groups.rb

 module API
   class Groups < Grape::API
     include PaginationParams
+    include Helpers::CustomAttributes
 
     before { authenticate_non_get! }
 
@@ -67,6 +68,8 @@ module API
         }
 
         groups = groups.with_statistics if options[:statistics]
+        groups, options = with_custom_attributes(groups, options)
+
         present paginate(groups), options
       end
     end
@@ -79,6 +82,7 @@ module API
       end
       params do
         use :group_list_params
+        use :with_custom_attributes
       end
       get do
         groups = find_groups(params)
@@ -142,9 +146,20 @@ module API
       desc 'Get a single group, with containing projects.' do
         success Entities::GroupDetail
       end
+      params do
+        use :with_custom_attributes
+      end
       get ":id" do
         group = find_group!(params[:id])
-        present group, with: Entities::GroupDetail, current_user: current_user
+
+        options = {
+          with: Entities::GroupDetail,
+          current_user: current_user
+        }
+
+        group, options = with_custom_attributes(group, options)
+
+        present group, options
       end
 
       desc 'Remove a group.'
@@ -175,12 +190,19 @@ module API
         optional :starred, type: Boolean, default: false, desc: 'Limit by starred status'
 
         use :pagination
+        use :with_custom_attributes
       end
       get ":id/projects" do
         projects = find_group_projects(params)
-        entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project
 
-        present entity.prepare_relation(projects), with: entity, current_user: current_user
+        options = {
+          with: params[:simple] ? Entities::BasicProjectDetails : Entities::Project,
+          current_user: current_user
+        }
+
+        projects, options = with_custom_attributes(projects, options)
+
+        present options[:with].prepare_relation(projects), options
       end
 
       desc 'Get a list of subgroups in this group.' do
@@ -188,6 +210,7 @@ module API
       end
       params do
         use :group_list_params
+        use :with_custom_attributes
       end
       get ":id/subgroups" do
         groups = find_groups(params)

lib/api/helpers/custom_attributes.rb

+module API
+  module Helpers
+    module CustomAttributes
+      extend ActiveSupport::Concern
+
+      included do
+        helpers do
+          params :with_custom_attributes do
+            optional :with_custom_attributes, type: Boolean, default: false, desc: 'Include custom attributes in the response'
+          end
+
+          def with_custom_attributes(collection_or_resource, options = {})
+            options = options.merge(
+              with_custom_attributes: params[:with_custom_attributes] &&
+                can?(current_user, :read_custom_attribute)
+            )
+
+            if options[:with_custom_attributes] && collection_or_resource.is_a?(ActiveRecord::Relation)
+              collection_or_resource = collection_or_resource.includes(:custom_attributes)
+            end
+
+            [collection_or_resource, options]
+          end
+        end
+      end
+    end
+  end
+end

lib/api/projects.rb

@@ -3,6 +3,7 @@ require_dependency 'declarative_policy'
 module API
   class Projects < Grape::API
     include PaginationParams
+    include Helpers::CustomAttributes
 
     before { authenticate_non_get! }
 
@@ -80,6 +81,7 @@ module API
         projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled]
         projects = projects.with_statistics if params[:statistics]
         projects = paginate(projects)
+        projects, options = with_custom_attributes(projects, options)
 
         if current_user
           project_members = current_user.project_members.preload(:source, user: [notification_settings: :source])
@@ -107,6 +109,7 @@ module API
         requires :user_id, type: String, desc: 'The ID or username of the user'
         use :collection_params
         use :statistics_params
+        use :with_custom_attributes
       end
       get ":user_id/projects" do
         user = find_user(params[:user_id])
@@ -127,6 +130,7 @@ module API
       params do
         use :collection_params
         use :statistics_params
+        use :with_custom_attributes
       end
       get do
         present_projects load_projects
@@ -196,11 +200,19 @@ module API
       end
       params do
         use :statistics_params
+        use :with_custom_attributes
       end
       get ":id" do
-        entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails
-        present user_project, with: entity, current_user: current_user,
-                              user_can_admin_project: can?(current_user, :admin_project, user_project), statistics: params[:statistics]
+        options = {
+          with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails,
+          current_user: current_user,
+          user_can_admin_project: can?(current_user, :admin_project, user_project),
+          statistics: params[:statistics]
+        }
+
+        project, options = with_custom_attributes(user_project, options)
+
+        present project, options
       end
 
       desc 'Fork new project for the current user or provided namespace.' do
@@ -242,6 +254,7 @@ module API
       end
       params do
         use :collection_params
+        use :with_custom_attributes
       end
       get ':id/forks' do
         forks = ForkProjectsFinder.new(user_project, params: project_finder_params, current_user: current_user).execute

lib/api/users.rb

@@ -2,6 +2,7 @@ module API
   class Users < Grape::API
     include PaginationParams
     include APIGuard
+    include Helpers::CustomAttributes
 
     allow_access_with_scope :read_user, if: -> (request) { request.get? }
 
@@ -70,6 +71,7 @@ module API
 
         use :sort_params
         use :pagination
+        use :with_custom_attributes
       end
       get do
         authenticated_as_admin! if params[:external].present? || (params[:extern_uid].present? && params[:provider].present?)
@@ -94,8 +96,9 @@ module API
 
         entity = current_user&.admin? ? Entities::UserWithAdmin : Entities::UserBasic
         users = users.preload(:identities, :u2f_registrations) if entity == Entities::UserWithAdmin
+        users, options = with_custom_attributes(users, with: entity)
 
-        present paginate(users), with: entity
+        present paginate(users), options
       end
 
       desc 'Get a single user' do
@@ -103,12 +106,16 @@ module API
       end
       params do
         requires :id, type: Integer, desc: 'The ID of the user'
+
+        use :with_custom_attributes
       end
       get ":id" do
         user = User.find_by(id: params[:id])
         not_found!('User') unless user && can?(current_user, :read_user, user)
 
         opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : { with: Entities::User }
+        user, opts = with_custom_attributes(user, opts)
+
         present user, opts
       end
 

spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb

@@ -17,12 +17,88 @@ shared_examples 'custom attributes endpoints' do |attributable_name|
       end
     end
 
-    it 'filters by custom attributes' do
-      get api("/#{attributable_name}", admin), custom_attributes: { foo: 'foo', bar: 'bar' }
+    context 'with an authorized user' do
+      it 'filters by custom attributes' do
+        get api("/#{attributable_name}", admin), custom_attributes: { foo: 'foo', bar: 'bar' }
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(json_response.size).to be 1
-      expect(json_response.first['id']).to eq attributable.id
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response.size).to be 1
+        expect(json_response.first['id']).to eq attributable.id
+      end
+    end
+  end
+
+  describe "GET /#{attributable_name} with custom attributes" do
+    before do
+      other_attributable
+    end
+
+    context 'with an unauthorized user' do
+      it 'does not include custom attributes' do
+        get api("/#{attributable_name}", user), with_custom_attributes: true
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response.size).to be 2
+        expect(json_response.first).not_to include 'custom_attributes'
+      end
+    end
+
+    context 'with an authorized user' do
+      it 'does not include custom attributes by default' do
+        get api("/#{attributable_name}", admin)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response.size).to be 2
+        expect(json_response.first).not_to include 'custom_attributes'
+        expect(json_response.second).not_to include 'custom_attributes'
+      end
+
+      it 'includes custom attributes if requested' do
+        get api("/#{attributable_name}", admin), with_custom_attributes: true
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response.size).to be 2
+
+        attributable_response = json_response.find { |r| r['id'] == attributable.id }
+        other_attributable_response = json_response.find { |r| r['id'] == other_attributable.id }
+
+        expect(attributable_response['custom_attributes']).to contain_exactly(
+          { 'key' => 'foo', 'value' => 'foo' },
+          { 'key' => 'bar', 'value' => 'bar' }
+        )
+
+        expect(other_attributable_response['custom_attributes']).to eq []
+      end
+    end
+  end
+
+  describe "GET /#{attributable_name}/:id with custom attributes" do
+    context 'with an unauthorized user' do
+      it 'does not include custom attributes' do
+        get api("/#{attributable_name}/#{attributable.id}", user), with_custom_attributes: true
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).not_to include 'custom_attributes'
+      end
+    end
+
+    context 'with an authorized user' do
+      it 'does not include custom attributes by default' do
+        get api("/#{attributable_name}/#{attributable.id}", admin)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).not_to include 'custom_attributes'
+      end
+
+      it 'includes custom attributes if requested' do
+        get api("/#{attributable_name}/#{attributable.id}", admin), with_custom_attributes: true
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['custom_attributes']).to contain_exactly(
+          { 'key' => 'foo', 'value' => 'foo' },
+          { 'key' => 'bar', 'value' => 'bar' }
+        )
+      end
     end
   end
 
@@ -33,14 +109,16 @@ shared_examples 'custom attributes endpoints' do |attributable_name|
       it_behaves_like 'an unauthorized API user'
     end
 
-    it 'returns all custom attributes' do
-      get api("/#{attributable_name}/#{attributable.id}/custom_attributes", admin)
+    context 'with an authorized user' do
+      it 'returns all custom attributes' do
+        get api("/#{attributable_name}/#{attributable.id}/custom_attributes", admin)
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(json_response).to contain_exactly(
-        { 'key' => 'foo', 'value' => 'foo' },
-        { 'key' => 'bar', 'value' => 'bar' }
-      )
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to contain_exactly(
+          { 'key' => 'foo', 'value' => 'foo' },
+          { 'key' => 'bar', 'value' => 'bar' }
+        )
+      end
     end
   end
 
@@ -51,11 +129,13 @@ shared_examples 'custom attributes endpoints' do |attributable_name|
       it_behaves_like 'an unauthorized API user'
     end
 
-    it 'returns a single custom attribute' do
-      get api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin)
+    context 'with an authorized user' do
+      it'returns a single custom attribute' do
+        get api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin)
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(json_response).to eq({ 'key' => 'foo', 'value' => 'foo' })
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to eq({ 'key' => 'foo', 'value' => 'foo' })
+      end
     end
   end
 
@@ -66,24 +146,26 @@ shared_examples 'custom attributes endpoints' do |attributable_name|
       it_behaves_like 'an unauthorized API user'
     end
 
-    it 'creates a new custom attribute' do
-      expect do
-        put api("/#{attributable_name}/#{attributable.id}/custom_attributes/new", admin), value: 'new'
-      end.to change { attributable.custom_attributes.count }.by(1)
+    context 'with an authorized user' do
+      it 'creates a new custom attribute' do
+        expect do
+          put api("/#{attributable_name}/#{attributable.id}/custom_attributes/new", admin), value: 'new'
+        end.to change { attributable.custom_attributes.count }.by(1)
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(json_response).to eq({ 'key' => 'new', 'value' => 'new' })
-      expect(attributable.custom_attributes.find_by(key: 'new').value).to eq 'new'
-    end
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to eq({ 'key' => 'new', 'value' => 'new' })
+        expect(attributable.custom_attributes.find_by(key: 'new').value).to eq 'new'
+      end
 
-    it 'updates an existing custom attribute' do
-      expect do
-        put api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin), value: 'new'
-      end.not_to change { attributable.custom_attributes.count }
+      it 'updates an existing custom attribute' do
+        expect do
+          put api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin), value: 'new'
+        end.not_to change { attributable.custom_attributes.count }
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(json_response).to eq({ 'key' => 'foo', 'value' => 'new' })
-      expect(custom_attribute1.reload.value).to eq 'new'
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to eq({ 'key' => 'foo', 'value' => 'new' })
+        expect(custom_attribute1.reload.value).to eq 'new'
+      end
     end
   end
 
@@ -94,13 +176,15 @@ shared_examples 'custom attributes endpoints' do |attributable_name|
       it_behaves_like 'an unauthorized API user'
     end
 
-    it 'deletes an existing custom attribute' do
-      expect do
-        delete api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin)
-      end.to change { attributable.custom_attributes.count }.by(-1)
+    context 'with an authorized user' do
+      it 'deletes an existing custom attribute' do
+        expect do
+          delete api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin)
+        end.to change { attributable.custom_attributes.count }.by(-1)
 
-      expect(response).to have_gitlab_http_status(204)
-      expect(attributable.custom_attributes.find_by(key: 'foo')).to be_nil
+        expect(response).to have_gitlab_http_status(204)
+        expect(attributable.custom_attributes.find_by(key: 'foo')).to be_nil
+      end
     end
   end
 end