Merge branch 'fldubois/gitlab-ce-fix-api-group-createdat' into 'master'

Allow date parameters on Issues, Notes, and Discussions API for group owners

Closes #40059

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

12 files changed, 154 insertions, 38 deletions

diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 333643865e9..73c93b22c95 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -72,6 +72,8 @@ class GroupPolicy < BasePolicy
     enable :admin_namespace
     enable :admin_group_member
     enable :change_visibility_level
+
+    enable :set_note_created_at
   end
 
   rule { can?(:read_nested_project_resources) }.policy do
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 00c58f15013..fd6cc504a3b 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -143,6 +143,10 @@ class ProjectPolicy < BasePolicy
     enable :destroy_merge_request
     enable :destroy_issue
     enable :remove_pages
+
+    enable :set_issue_iid
+    enable :set_issue_created_at
+    enable :set_note_created_at
   end
 
   rule { can?(:guest_access) }.policy do
diff --git a/changelogs/unreleased/fix-api-group-createdat.yml b/changelogs/unreleased/fix-api-group-createdat.yml
new file mode 100644
index 00000000000..e628facf1bf
--- /dev/null
+++ b/changelogs/unreleased/fix-api-group-createdat.yml
@@ -0,0 +1,5 @@
+---
+title: Allow date parameters on Issues, Notes, and Discussions API for group owners
+merge_request: 21342
+author: Florent Dubois
+type: fixed
diff --git a/doc/api/discussions.md b/doc/api/discussions.md
index 65e2f9d6cd9..a1e1ff1419d 100644
--- a/doc/api/discussions.md
+++ b/doc/api/discussions.md
@@ -136,7 +136,7 @@ Parameters:
 | `id`            | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
 | `issue_iid`     | integer        | yes      | The IID of an issue |
 | `body`          | string         | yes      | The content of a discussion |
-| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions?body=comment
@@ -159,7 +159,7 @@ Parameters:
 | `discussion_id` | integer        | yes      | The ID of a discussion |
 | `note_id`       | integer        | yes      | The ID of a discussion note |
 | `body`          | string         | yes      | The content of a discussion |
-| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment
@@ -342,7 +342,7 @@ Parameters:
 | `id`            | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
 | `snippet_id`    | integer        | yes      | The ID of an snippet |
 | `body`          | string         | yes      | The content of a discussion |
-| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions?body=comment
@@ -365,7 +365,7 @@ Parameters:
 | `discussion_id` | integer        | yes      | The ID of a discussion |
 | `note_id`       | integer        | yes      | The ID of a discussion note |
 | `body`          | string         | yes      | The content of a discussion |
-| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`    | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment
@@ -601,7 +601,7 @@ Parameters:
 | `id`                      | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
 | `merge_request_iid`       | integer        | yes      | The IID of a merge request |
 | `body`                    | string         | yes      | The content of a discussion |
-| `created_at`              | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`              | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 | `position`                | hash           | no       | Position when creating a diff note |
 | `position[base_sha]`      | string         | yes      | Base commit SHA in the source branch |
 | `position[start_sha]`     | string         | yes      | SHA referencing commit in target branch |
@@ -659,7 +659,7 @@ Parameters:
 | `discussion_id`     | integer        | yes      | The ID of a discussion |
 | `note_id`           | integer        | yes      | The ID of a discussion note |
 | `body`              | string         | yes      | The content of a discussion |
-| `created_at`        | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`        | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment
@@ -894,7 +894,7 @@ Parameters:
 | `id`                      | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
 | `commit_id`               | integer        | yes      | The ID of a commit |
 | `body`                    | string         | yes      | The content of a discussion |
-| `created_at`              | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`              | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 | `position`                | hash           | no       | Position when creating a diff note |
 | `position[base_sha]`      | string         | yes      | Base commit SHA in the source branch |
 | `position[start_sha]`     | string         | yes      | SHA referencing commit in target branch |
@@ -930,7 +930,7 @@ Parameters:
 | `discussion_id`     | integer        | yes      | The ID of a discussion |
 | `note_id`           | integer        | yes      | The ID of a discussion note |
 | `body`              | string         | yes      | The content of a discussion |
-| `created_at`        | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z |
+| `created_at`        | string         | no       | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights) |
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/commits/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment
diff --git a/doc/api/issues.md b/doc/api/issues.md
index 103eaa5655f..f4c0f4ea65b 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -470,7 +470,7 @@ POST /projects/:id/issues
 | `assignee_ids`                            | Array[integer] | no       | The ID of the users to assign issue |
 | `milestone_id`                            | integer | no       | The global ID of a milestone to assign issue  |
 | `labels`                                  | string  | no       | Comma-separated label names for an issue  |
-| `created_at`                              | string  | no       | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project owner rights) |
+| `created_at`                              | string  | no       | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project/group owner rights) |
 | `due_date`                                | string  | no       | Date time string in the format YEAR-MONTH-DAY, e.g. `2016-03-11` |
 | `merge_request_to_resolve_discussions_of` | integer | no       | The IID of a merge request in which to resolve all issues. This will fill the issue with a default description and mark all discussions as resolved. When passing a description or title, these values will take precedence over the default values.|
 | `discussion_to_resolve`                   | string  | no       | The ID of a discussion to resolve. This will fill in the issue with a default description and mark the discussion as resolved. Use in combination with `merge_request_to_resolve_discussions_of`. |
diff --git a/doc/api/notes.md b/doc/api/notes.md
index c271d46688f..44940bdd9e5 100644
--- a/doc/api/notes.md
+++ b/doc/api/notes.md
@@ -100,7 +100,7 @@ Parameters:
 - `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding)
 - `issue_id` (required) - The IID of an issue
 - `body` (required) - The content of a note
-- `created_at` (optional) - Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z
+- `created_at` (optional) - Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z (requires admin or project/group owner rights)
 
 ```bash
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes?body=note
diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb
index e2984b08eca..7b1f5c2584b 100644
--- a/lib/api/helpers/notes_helpers.rb
+++ b/lib/api/helpers/notes_helpers.rb
@@ -92,10 +92,7 @@ module API
 
         parent = noteable_parent(noteable)
 
-        if opts[:created_at]
-          opts.delete(:created_at) unless
-            current_user.admin? || parent.owned_by?(current_user)
-        end
+        opts.delete(:created_at) unless current_user.can?(:set_note_created_at, policy_object)
 
         opts[:updated_at] = opts[:created_at] if opts[:created_at]
 
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index bda05d1795b..cedfd2fbaa0 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -172,11 +172,8 @@ module API
 
         authorize! :create_issue, user_project
 
-        # Setting created_at time or iid only allowed for admins and project owners
-        unless current_user.admin? || user_project.owner == current_user
-          params.delete(:created_at)
-          params.delete(:iid)
-        end
+        params.delete(:created_at) unless current_user.can?(:set_issue_created_at, user_project)
+        params.delete(:iid) unless current_user.can?(:set_issue_iid, user_project)
 
         issue_params = declared_params(include_missing: false)
 
@@ -216,8 +213,8 @@ module API
         issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
         authorize! :update_issue, issue
 
-        # Setting created_at time only allowed for admins and project owners
-        unless current_user.admin? || user_project.owner == current_user
+        # Setting created_at time only allowed for admins and project/group owners
+        unless current_user.admin? || user_project.owner == current_user || current_user.owned_groups.include?(user_project.owner)
           params.delete(:updated_at)
         end
 
diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb
index 652d54f3a0d..5e583be457e 100644
--- a/spec/policies/group_policy_spec.rb
+++ b/spec/policies/group_policy_spec.rb
@@ -31,6 +31,7 @@ describe GroupPolicy do
       :admin_namespace,
       :admin_group_member,
       :change_visibility_level,
+      :set_note_created_at,
       (Gitlab::Database.postgresql? ? :create_subgroup : nil)
     ].compact
   end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index dd3fa4e6a51..b7ec35d6ec5 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -64,6 +64,7 @@ describe ProjectPolicy do
     %i[
       change_namespace change_visibility_level rename_project remove_project
       archive_project remove_fork_project destroy_merge_request destroy_issue
+      set_issue_iid set_issue_created_at set_note_created_at
     ]
   end
 
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 28ba00c7293..f64815feffa 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -1023,6 +1023,20 @@ describe API::Issues do
         end
       end
 
+      context 'by a group owner' do
+        let(:group) { create(:group) }
+        let(:group_project) { create(:project, :public, namespace: group) }
+
+        it 'sets the internal ID on the new issue' do
+          group.add_owner(user2)
+          post api("/projects/#{group_project.id}/issues", user2),
+            title: 'new issue', iid: 9001
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(json_response['iid']).to eq 9001
+        end
+      end
+
       context 'by another user' do
         it 'ignores the given internal ID' do
           post api("/projects/#{project.id}/issues", user2),
@@ -1154,14 +1168,47 @@ describe API::Issues do
       end
     end
 
-    context 'when an admin or owner makes the request' do
-      it 'accepts the creation date to be set' do
-        creation_time = 2.weeks.ago
-        post api("/projects/#{project.id}/issues", user),
-          title: 'new issue', labels: 'label, label2', created_at: creation_time
+    context 'setting created_at' do
+      let(:creation_time) { 2.weeks.ago }
+      let(:params) { { title: 'new issue', labels: 'label, label2', created_at: creation_time } }
 
-        expect(response).to have_gitlab_http_status(201)
-        expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+      context 'by an admin' do
+        it 'sets the creation time on the new issue' do
+          post api("/projects/#{project.id}/issues", admin), params
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+        end
+      end
+
+      context 'by a project owner' do
+        it 'sets the creation time on the new issue' do
+          post api("/projects/#{project.id}/issues", user), params
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+        end
+      end
+
+      context 'by a group owner' do
+        it 'sets the creation time on the new issue' do
+          group = create(:group)
+          group_project = create(:project, :public, namespace: group)
+          group.add_owner(user2)
+          post api("/projects/#{group_project.id}/issues", user2), params
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+        end
+      end
+
+      context 'by another user' do
+        it 'ignores the given creation time' do
+          post api("/projects/#{project.id}/issues", user2), params
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(Time.parse(json_response['created_at'])).not_to be_like_time(creation_time)
+        end
       end
     end
 
diff --git a/spec/support/shared_examples/requests/api/notes.rb b/spec/support/shared_examples/requests/api/notes.rb
index 1b563021244..0e20dfe0725 100644
--- a/spec/support/shared_examples/requests/api/notes.rb
+++ b/spec/support/shared_examples/requests/api/notes.rb
@@ -111,17 +111,79 @@ shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
       post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: 'hi!'
     end
 
-    context 'when an admin or owner makes the request' do
-      it 'accepts the creation date to be set' do
-        creation_time = 2.weeks.ago
-        post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user),
-          body: 'hi!', created_at: creation_time
+    context 'setting created_at' do
+      let(:creation_time) { 2.weeks.ago }
+      let(:params) { { body: 'hi!', created_at: creation_time } }
+
+      context 'by an admin' do
+        it 'sets the creation time on the new note' do
+          admin = create(:admin)
+          post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", admin), params
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(json_response['body']).to eq('hi!')
+          expect(json_response['author']['username']).to eq(admin.username)
+          expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+          expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time)
+        end
+      end
 
-        expect(response).to have_gitlab_http_status(201)
-        expect(json_response['body']).to eq('hi!')
-        expect(json_response['author']['username']).to eq(user.username)
-        expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
-        expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time)
+      if parent_type == 'projects'
+        context 'by a project owner' do
+          it 'sets the creation time on the new note' do
+            post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), params
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response['body']).to eq('hi!')
+            expect(json_response['author']['username']).to eq(user.username)
+            expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+            expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time)
+          end
+        end
+
+        context 'by a group owner' do
+          it 'sets the creation time on the new note' do
+            user2 = create(:user)
+            group = create(:group)
+            group.add_owner(user2)
+            parent.update!(namespace: group)
+            user2.refresh_authorized_projects
+
+            post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user2), params
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response['body']).to eq('hi!')
+            expect(json_response['author']['username']).to eq(user2.username)
+            expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+            expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time)
+          end
+        end
+      elsif parent_type == 'groups'
+        context 'by a group owner' do
+          it 'sets the creation time on the new note' do
+            post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), params
+
+            expect(response).to have_gitlab_http_status(201)
+            expect(json_response['body']).to eq('hi!')
+            expect(json_response['author']['username']).to eq(user.username)
+            expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
+            expect(Time.parse(json_response['updated_at'])).to be_like_time(creation_time)
+          end
+        end
+      end
+
+      context 'by another user' do
+        it 'ignores the given creation time' do
+          user2 = create(:user)
+          parent.add_developer(user2)
+          post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user2), params
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(json_response['body']).to eq('hi!')
+          expect(json_response['author']['username']).to eq(user2.username)
+          expect(Time.parse(json_response['created_at'])).not_to be_like_time(creation_time)
+          expect(Time.parse(json_response['updated_at'])).not_to be_like_time(creation_time)
+        end
       end
     end