From 74532158e0599e918e911d1039a92972a7902911 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Thu, 4 Oct 2018 16:52:35 +0000
Subject: Merge branch 'security-osw-user-info-leak-discussions-11-3' into
 'security-11-3'

[11.3] Filter user sensitive data from discussions JSON

See merge request gitlab/gitlabhq!2537
---
 app/serializers/discussion_entity.rb               |  2 +-
 ...ecurity-osw-user-info-leak-discussions-11-3.yml |  5 +++++
 .../api/schemas/entities/note_user_entity.json     | 26 ++++++++++++++++++++++
 spec/serializers/discussion_entity_spec.rb         |  7 ++++++
 4 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/security-osw-user-info-leak-discussions-11-3.yml
 create mode 100644 spec/fixtures/api/schemas/entities/note_user_entity.json

diff --git a/app/serializers/discussion_entity.rb b/app/serializers/discussion_entity.rb
index ebe76c9fcda..b6786a0d597 100644
--- a/app/serializers/discussion_entity.rb
+++ b/app/serializers/discussion_entity.rb
@@ -27,7 +27,7 @@ class DiscussionEntity < Grape::Entity
 
   expose :resolved?, as: :resolved
   expose :resolved_by_push?, as: :resolved_by_push
-  expose :resolved_by
+  expose :resolved_by, using: NoteUserEntity
   expose :resolved_at
   expose :resolve_path, if: -> (d, _) { d.resolvable? } do |discussion|
     resolve_project_merge_request_discussion_path(discussion.project, discussion.noteable, discussion.id)
diff --git a/changelogs/unreleased/security-osw-user-info-leak-discussions-11-3.yml b/changelogs/unreleased/security-osw-user-info-leak-discussions-11-3.yml
new file mode 100644
index 00000000000..0276ffe032f
--- /dev/null
+++ b/changelogs/unreleased/security-osw-user-info-leak-discussions-11-3.yml
@@ -0,0 +1,5 @@
+---
+title: Filter user sensitive data from discussions JSON
+merge_request: 2537
+author:
+type: security
diff --git a/spec/fixtures/api/schemas/entities/note_user_entity.json b/spec/fixtures/api/schemas/entities/note_user_entity.json
new file mode 100644
index 00000000000..aab98981dd9
--- /dev/null
+++ b/spec/fixtures/api/schemas/entities/note_user_entity.json
@@ -0,0 +1,26 @@
+{
+  "type": "object",
+  "required": [
+    "id",
+    "state",
+    "avatar_url",
+    "path",
+    "name",
+    "username"
+  ],
+  "properties": {
+    "id": { "type": "integer" },
+    "state": { "type": "string" },
+    "avatar_url": { "type": "string" },
+    "path": { "type": "string" },
+    "name": { "type": "string" },
+    "username": { "type": "string" },
+    "status_tooltip_html": { 
+      "oneOf": [
+        { "type": "null" },
+        { "type": "string" }
+      ]
+    }
+  },
+  "additionalProperties": false
+}
diff --git a/spec/serializers/discussion_entity_spec.rb b/spec/serializers/discussion_entity_spec.rb
index 378540a35b6..0590304e832 100644
--- a/spec/serializers/discussion_entity_spec.rb
+++ b/spec/serializers/discussion_entity_spec.rb
@@ -36,6 +36,13 @@ describe DiscussionEntity do
     )
   end
 
+  it 'resolved_by matches note_user_entity schema' do
+    Notes::ResolveService.new(note.project, user).execute(note)
+
+    expect(subject[:resolved_by].with_indifferent_access)
+      .to match_schema('entities/note_user_entity')
+  end
+
   context 'when is LegacyDiffDiscussion' do
     let(:project) { create(:project) }
     let(:merge_request) { create(:merge_request, source_project: project) }
-- 
cgit v1.2.1