Merge branch '12800-read-and-write-user-admin-notes-via-api' into 'master'

Resolve "Read and write User "Admin notes" via API"

See merge request gitlab-org/gitlab-ee!14662

doc/api/users.md

@@ -147,6 +147,21 @@ GET /users
 ]
 ```
 
+Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see the `shared_runners_minutes_limit`, `extra_shared_runners_minutes_limit`, and `note` parameters.
+
+```json
+[
+  {
+    "id": 1,
+    ...
+    "shared_runners_minutes_limit": 133,
+    "extra_shared_runners_minutes_limit": 133,
+    "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123"
+    ...
+  }
+]
+```
+
 Users on GitLab [Silver or higher](https://about.gitlab.com/pricing/) will also see
 the `group_saml` provider option:
 
@@ -284,14 +299,15 @@ Example Responses:
 ```
 
 Users on GitLab [Starter, Bronze, or higher](https://about.gitlab.com/pricing/) will also see
-the `shared_runners_minutes_limit` and `extra_shared_runners_minutes_limit` parameters.
+the `shared_runners_minutes_limit`, `extra_shared_runners_minutes_limit`, and `note` parameters.
 
 ```json
 {
   "id": 1,
   "username": "john_smith",
   "shared_runners_minutes_limit": 133,
-  "extra_shared_runners_minutes_limit": 133
+  "extra_shared_runners_minutes_limit": 133,
+  "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123"
   ...
 }
 ```
@@ -304,7 +320,8 @@ see the `group_saml` option:
   "id": 1,
   "username": "john_smith",
   "shared_runners_minutes_limit": 133,
-  "extra_shared_runners_minutes_limit": 133
+  "extra_shared_runners_minutes_limit": 133,
+  "note": "DMCA Request: 2018-11-05 | DMCA Violation | Abuse | https://gitlab.zendesk.com/agent/tickets/123"
   "identities": [
     {"provider": "github", "extern_uid": "2435223452345"},
     {"provider": "bitbucket", "extern_uid": "john.smith"},
@@ -399,6 +416,7 @@ Parameters:
 - `private_profile` (optional)     - User's profile is private - true or false (default)
 - `shared_runners_minutes_limit` (optional)       - Pipeline minutes quota for this user **(STARTER)**
 - `extra_shared_runners_minutes_limit` (optional) - Extra pipeline minutes quota for this user **(STARTER)**
+- `note` (optional) - Admin notes for this user **(STARTER)**
 
 On password update, user will be forced to change it upon next login.
 Note, at the moment this method does only return a `404` error,

ee/changelogs/unreleased/12800-read-and-write-user-admin-notes-via-api.yml

+---
+title: Read and write User Admin notes via API
+merge_request: 14662
+author:
+type: added

ee/lib/ee/api/entities.rb

@@ -30,6 +30,14 @@ module EE
         end
       end
 
+      module UserWithAdmin
+        extend ActiveSupport::Concern
+
+        prepended do
+          expose :note
+        end
+      end
+
       module Project
         extend ActiveSupport::Concern
 

ee/lib/ee/api/helpers/users_helpers.rb

@@ -12,6 +12,7 @@ module EE
             optional :shared_runners_minutes_limit, type: Integer, desc: 'Pipeline minutes quota for this user'
             optional :extra_shared_runners_minutes_limit, type: Integer, desc: '(admin-only) Extra pipeline minutes quota for this user'
             optional :group_id_for_saml, type: Integer, desc: 'ID for group where SAML has been configured'
+            optional :note, type: String, desc: 'Admin note for this user'
           end
 
           params :optional_index_params_ee do

ee/spec/requests/api/users_spec.rb

@@ -86,4 +86,143 @@ describe API::Users do
       expect(json_response['message']).to eq({ "identities.provider" => ["can't be blank"] })
     end
   end
+
+  context 'admin notes' do
+    let(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') }
+    let(:user) { create(:user, note: '2018-11-05 | 2FA removed | user requested | www.gitlab.com') }
+
+    describe 'GET /users/:id' do
+      context 'when unauthenticated' do
+        it 'does not contain the note of the user' do
+          get api("/users/#{user.id}")
+
+          expect(json_response).not_to have_key('note')
+        end
+      end
+
+      context 'when authenticated' do
+        context 'as an admin' do
+          it 'contains the note of the user' do
+            get api("/users/#{user.id}", admin)
+
+            expect(json_response).to have_key('note')
+            expect(json_response['note']).to eq(user.note)
+          end
+        end
+
+        context 'as a regular user' do
+          it 'does not contain the note of the user' do
+            get api("/users/#{user.id}", user)
+
+            expect(json_response).not_to have_key('note')
+          end
+        end
+      end
+    end
+
+    describe "PUT /users/:id" do
+      context 'when user is an admin' do
+        it "updates note of the user" do
+          new_note = '2019-07-07 | Email changed | user requested | www.gitlab.com'
+
+          expect do
+            put api("/users/#{user.id}", admin), params: { note: new_note }
+          end.to change { user.reload.note }
+                   .from('2018-11-05 | 2FA removed | user requested | www.gitlab.com')
+                   .to(new_note)
+
+          expect(response).to have_gitlab_http_status(:success)
+          expect(json_response['note']).to eq(new_note)
+        end
+      end
+
+      context 'when user is not an admin' do
+        it "cannot update their own note" do
+          expect do
+            put api("/users/#{user.id}", user), params: { note: 'new note' }
+          end.not_to change { user.reload.note }
+
+          expect(response).to have_gitlab_http_status(403)
+        end
+      end
+    end
+
+    describe 'GET /users/' do
+      context 'when unauthenticated' do
+        it "does not contain the note of users" do
+          get api("/users"), params: { username: user.username }
+
+          expect(json_response.first).not_to have_key('note')
+        end
+      end
+
+      context 'when authenticated' do
+        context 'as a regular user' do
+          it 'does not contain the note of users' do
+            get api("/users", user), params: { username: user.username }
+
+            expect(json_response.first).not_to have_key('note')
+          end
+        end
+
+        context 'as an admin' do
+          it 'contains the note of users' do
+            get api("/users", admin), params: { username: user.username }
+
+            expect(response).to have_gitlab_http_status(:success)
+            expect(json_response.first).to have_key('note')
+            expect(json_response.first['note']).to eq '2018-11-05 | 2FA removed | user requested | www.gitlab.com'
+          end
+        end
+      end
+    end
+
+    describe 'GET /user' do
+      context 'when authenticated' do
+        context 'as an admin' do
+          context 'accesses their own profile' do
+            it 'contains the note of the user' do
+              get api("/user", admin)
+
+              expect(json_response).to have_key('note')
+              expect(json_response['note']).to eq(admin.note)
+            end
+          end
+
+          context 'sudo' do
+            let(:admin_personal_access_token) { create(:personal_access_token, user: admin, scopes: %w[api sudo]).token }
+
+            context 'accesses the profile of another regular user' do
+              it 'does not contain the note of the user' do
+                get api("/user?private_token=#{admin_personal_access_token}&sudo=#{user.id}")
+
+                expect(json_response['id']).to eq(user.id)
+                expect(json_response).not_to have_key('note')
+              end
+            end
+
+            context 'accesses the profile of another admin' do
+              let(:admin_2) {create(:admin, note: '2010-10-10 | 2FA added | admin requested | www.gitlab.com')}
+
+              it 'contains the note of the user' do
+                get api("/user?private_token=#{admin_personal_access_token}&sudo=#{admin_2.id}")
+
+                expect(json_response['id']).to eq(admin_2.id)
+                expect(json_response).to have_key('note')
+                expect(json_response['note']).to eq(admin_2.note)
+              end
+            end
+          end
+        end
+
+        context 'as a regular user' do
+          it 'does not contain the note of the user' do
+            get api("/user", user)
+
+            expect(json_response).not_to have_key('note')
+          end
+        end
+      end
+    end
+  end
 end

lib/api/entities.rb

@@ -1709,3 +1709,4 @@ API::Entities.prepend_entity(::API::Entities::Variable, with: EE::API::Entities:
 API::Entities.prepend_entity(::API::Entities::Todo, with: EE::API::Entities::Todo)
 API::Entities.prepend_entity(::API::Entities::ProtectedBranch, with: EE::API::Entities::ProtectedBranch)
 API::Entities.prepend_entity(::API::Entities::Identity, with: EE::API::Entities::Identity)
+API::Entities.prepend_entity(::API::Entities::UserWithAdmin, with: EE::API::Entities::UserWithAdmin)

lib/api/users.rb

@@ -148,7 +148,7 @@ module API
       end
 
       desc 'Create a user. Available only for admins.' do
-        success Entities::UserPublic
+        success Entities::UserWithAdmin
       end
       params do
         requires :email, type: String, desc: 'The email of the user'
@@ -168,7 +168,7 @@ module API
         user = ::Users::CreateService.new(current_user, params).execute(skip_authorization: true)
 
         if user.persisted?
-          present user, with: Entities::UserPublic, current_user: current_user
+          present user, with: Entities::UserWithAdmin, current_user: current_user
         else
           conflict!('Email has already been taken') if User
             .by_any_email(user.email.downcase)
@@ -183,7 +183,7 @@ module API
       end
 
       desc 'Update a user. Available only for admins.' do
-        success Entities::UserPublic
+        success Entities::UserWithAdmin
       end
       params do
         requires :id, type: Integer, desc: 'The ID of the user'
@@ -215,7 +215,7 @@ module API
         result = ::Users::UpdateService.new(current_user, user_params.merge(user: user)).execute
 
         if result[:status] == :success
-          present user, with: Entities::UserPublic, current_user: current_user
+          present user, with: Entities::UserWithAdmin, current_user: current_user
         else
           render_validation_error!(user)
         end

spec/requests/api/users_spec.rb

@@ -448,6 +448,7 @@ describe API::Users do
 
     it "returns 201 Created on success" do
       post api("/users", admin), params: attributes_for(:user, projects_limit: 3)
+      expect(response).to match_response_schema('public_api/v4/user/admin')
       expect(response).to have_gitlab_http_status(201)
     end
 
@@ -643,6 +644,13 @@ describe API::Users do
   describe "PUT /users/:id" do
     let!(:admin_user) { create(:admin) }
 
+    it "returns 200 OK on success" do
+      put api("/users/#{user.id}", admin), params: { bio: 'new test bio' }
+
+      expect(response).to match_response_schema('public_api/v4/user/admin')
+      expect(response).to have_gitlab_http_status(200)
+    end
+
     it "updates user with new bio" do
       put api("/users/#{user.id}", admin), params: { bio: 'new test bio' }