diff --git a/app/graphql/types/notes/note_type.rb b/app/graphql/types/notes/note_type.rb index 0f2a01d73901bca74c75031ee3abe06cecb9ced2..d1e917b73c79b3821ff0ce01c92bffe9609721c1 100644 --- a/app/graphql/types/notes/note_type.rb +++ b/app/graphql/types/notes/note_type.rb @@ -13,6 +13,8 @@ class NoteType < BaseObject implements Types::ResolvableInterface + present_using NotePresenter + field :max_access_level_of_author, GraphQL::Types::String, null: true, description: "Max access level of the note author in the project.", @@ -28,11 +30,11 @@ class NoteType < BaseObject field :author, Types::UserType, null: true, - description: 'User who wrote this note.' + description: 'User who wrote the note.' field :system, GraphQL::Types::Boolean, null: false, - description: 'Indicates whether this note was created by the system or by a user.' + description: 'Indicates whether the note was created by the system or by a user.' field :system_note_icon_name, GraphQL::Types::String, null: true, @@ -49,7 +51,7 @@ class NoteType < BaseObject field :confidential, GraphQL::Types::Boolean, null: true, - description: 'Indicates if this note is confidential.', + description: 'Indicates if the note is confidential.', method: :confidential?, deprecated: { reason: :renamed, @@ -59,7 +61,7 @@ class NoteType < BaseObject field :internal, GraphQL::Types::Boolean, null: true, - description: 'Indicates if this note is internal.', + description: 'Indicates if the note is internal.', method: :confidential? field :created_at, Types::TimeType, @@ -67,16 +69,16 @@ class NoteType < BaseObject description: 'Timestamp of the note creation.' field :discussion, Types::Notes::DiscussionType, null: true, - description: 'Discussion this note is a part of.' + description: 'Discussion the note is a part of.' field :position, Types::Notes::DiffPositionType, null: true, - description: 'Position of this note on a diff.' + description: 'Position of the note on a diff.' field :updated_at, Types::TimeType, null: false, description: "Timestamp of the note's last activity." field :url, GraphQL::Types::String, null: true, - description: 'URL to view this Note in the Web UI.' + description: 'URL to view the note in the Web UI.' field :last_edited_at, Types::TimeType, null: true, @@ -95,7 +97,10 @@ class NoteType < BaseObject null: true, description: 'Metadata for the given note if it is a system note.' - markdown_field :body_html, null: true, method: :note + field :body_html, GraphQL::Types::String, + method: :note_html, + null: true, + description: "GitLab Flavored Markdown rendering of the content of the note." def url ::Gitlab::UrlBuilder.build(object) @@ -118,7 +123,8 @@ def author def id return super unless object.is_a?(SyntheticNote) - ::Gitlab::GlobalId.build(object, model_name: object.class.to_s, id: object.discussion_id) + # object is a presenter, so object.object returns the concrete note object. + ::Gitlab::GlobalId.build(object, model_name: object.object.class.to_s, id: object.discussion_id) end end end diff --git a/app/models/service_desk_setting.rb b/app/models/service_desk_setting.rb index 095eb0b67f38a2d1607b948dc435f28d0d1ccb2f..eba39cd5f2b7abfaff4e6b6ab36b5fb4e5c5b7cb 100644 --- a/app/models/service_desk_setting.rb +++ b/app/models/service_desk_setting.rb @@ -23,7 +23,7 @@ class ServiceDeskSetting < ApplicationRecord length: { maximum: 255 }, uniqueness: true, allow_nil: true, - format: /\A[\w\-._]+@[\w\-.]+\.{1}[a-zA-Z]{2,}\z/ + format: Gitlab::Utils::Email::EMAIL_REGEXP_WITH_ANCHORS validates :custom_email_credential, presence: true, diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 58b46a2038688086748fefb98338e331a9e7ca05..16a19de0f9dcfe7c962f945b077b548bdea43c09 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -25,6 +25,7 @@ class SystemNoteMetadata < ApplicationRecord tag due_date start_date_or_due_date pinned_embed cherry_pick health_status approved unapproved status alert_issue_added relate unrelate new_alert_added severity contact timeline_event issue_type relate_to_child unrelate_from_child relate_to_parent unrelate_from_parent override + issue_email_participants ].freeze validates :note, presence: true, unless: :importing? diff --git a/app/presenters/note_presenter.rb b/app/presenters/note_presenter.rb new file mode 100644 index 0000000000000000000000000000000000000000..00eeded05b73908bae53723133624b0dbf151e40 --- /dev/null +++ b/app/presenters/note_presenter.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class NotePresenter < Gitlab::View::Presenter::Delegated # rubocop:disable Gitlab/NamespacedClass -- Note is not namespaced + presents ::Note, as: :object # because we also have a #note method + + delegator_override :note + def note + obfuscate_participants_emails_in_system_note(object.note) + end + + delegator_override :note_html + def note_html + # Always use `redacted_note_html` because it removes references + # based on the current user context. + # But fall back to `note_html` if redacted is not available (same behavior as `markdown_field`) + obfuscate_participants_emails_in_system_note(object.redacted_note_html || object.note_html) + end + + private + + def obfuscate_participants_emails_in_system_note(text) + return text unless object.system? + return text if can?(current_user, :read_external_emails, object.project) + return text if object.system_note_metadata&.action != 'issue_email_participants' + + Gitlab::Utils::Email.obfuscate_emails_in_text(text) + end +end diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb index a50d893d24459efdeff7062215f65d2e1a9ba575..919d8f08a150c605daad27811fc0b501d7790e2b 100644 --- a/app/serializers/note_entity.rb +++ b/app/serializers/note_entity.rb @@ -17,9 +17,13 @@ class NoteEntity < API::Entities::Note expose :author, using: NoteUserEntity unexpose :note, as: :body - expose :note + expose :note do |note| + note_presenter(note).note + end - expose :redacted_note_html, as: :note_html + expose :note_html do |note| + note_presenter(note).note_html + end expose :last_edited_at, if: -> (note, _) { note.edited? } expose :last_edited_by, using: NoteUserEntity, if: -> (note, _) { note.edited? } @@ -118,6 +122,10 @@ def external_author Gitlab::Utils::Email.obfuscated_email(object.note_metadata.external_author, deform: true) end end + + def note_presenter(note) + NotePresenter.new(note, current_user: current_user) # rubocop: disable CodeReuse/Presenter -- Directly instantiate NotePresenter because we don't have presenters for all subclasses of Note + end end NoteEntity.prepend_mod_with('NoteEntity') diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index a5aae83c32a3fbf866225c740faf27db249859e7..876e694dabf042905ec406604dbb11c008a18b92 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -440,7 +440,7 @@ def mark_duplicate_issue(canonical_issue) end def email_participants(body) - create_note(NoteSummary.new(noteable, project, author, body)) + create_note(NoteSummary.new(noteable, project, author, body, action: 'issue_email_participants')) end def discussion_lock diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index a0a8eb480a9570b6f2dd3b4dd0a791522643ab62..3b2a6a6fe44131bc99e7d870494a43f49cd47a1a 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2918,7 +2918,7 @@ Input type: `CreateDiffNoteInput` | `confidential` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** This was renamed. Please use `internal`. Deprecated in GitLab 15.3. | | `internal` | [`Boolean`](#boolean) | Internal flag for a note. Default is false. | | `noteableId` | [`NoteableID!`](#noteableid) | Global ID of the resource to add a note to. | -| `position` | [`DiffPositionInput!`](#diffpositioninput) | Position of this note on a diff. | +| `position` | [`DiffPositionInput!`](#diffpositioninput) | Position of the note on a diff. | #### Fields @@ -2993,7 +2993,7 @@ Input type: `CreateImageDiffNoteInput` | `confidential` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated:** This was renamed. Please use `internal`. Deprecated in GitLab 15.3. | | `internal` | [`Boolean`](#boolean) | Internal flag for a note. Default is false. | | `noteableId` | [`NoteableID!`](#noteableid) | Global ID of the resource to add a note to. | -| `position` | [`DiffImagePositionInput!`](#diffimagepositioninput) | Position of this note on a diff. | +| `position` | [`DiffImagePositionInput!`](#diffimagepositioninput) | Position of the note on a diff. | #### Fields @@ -7185,7 +7185,7 @@ Input type: `RepositionImageDiffNoteInput` | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `id` | [`DiffNoteID!`](#diffnoteid) | Global ID of the DiffNote to update. | -| `position` | [`UpdateDiffImagePositionInput!`](#updatediffimagepositioninput) | Position of this note on a diff. | +| `position` | [`UpdateDiffImagePositionInput!`](#updatediffimagepositioninput) | Position of the note on a diff. | #### Fields @@ -8209,7 +8209,7 @@ Input type: `UpdateImageDiffNoteInput` | `body` | [`String`](#string) | Content of the note. | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `id` | [`NoteID!`](#noteid) | Global ID of the note to update. | -| `position` | [`UpdateDiffImagePositionInput`](#updatediffimagepositioninput) | Position of this note on a diff. | +| `position` | [`UpdateDiffImagePositionInput`](#updatediffimagepositioninput) | Position of the note on a diff. | #### Fields @@ -24537,30 +24537,30 @@ Represents the network policy. | Name | Type | Description | | ---- | ---- | ----------- | -| `author` | [`UserCore`](#usercore) | User who wrote this note. | +| `author` | [`UserCore`](#usercore) | User who wrote the note. | | `authorIsContributor` | [`Boolean`](#boolean) | Indicates whether the note author is a contributor. | | `awardEmoji` | [`AwardEmojiConnection`](#awardemojiconnection) | List of emoji reactions associated with the note. (see [Connections](#connections)) | | `body` | [`String!`](#string) | Content of the note. | -| `bodyHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of `note`. | +| `bodyHtml` | [`String`](#string) | GitLab Flavored Markdown rendering of the content of the note. | | `confidential` **{warning-solid}** | [`Boolean`](#boolean) | **Deprecated** in GitLab 15.5. This was renamed. Use: `internal`. | | `createdAt` | [`Time!`](#time) | Timestamp of the note creation. | -| `discussion` | [`Discussion`](#discussion) | Discussion this note is a part of. | +| `discussion` | [`Discussion`](#discussion) | Discussion the note is a part of. | | `id` | [`NoteID!`](#noteid) | ID of the note. | -| `internal` | [`Boolean`](#boolean) | Indicates if this note is internal. | +| `internal` | [`Boolean`](#boolean) | Indicates if the note is internal. | | `lastEditedAt` | [`Time`](#time) | Timestamp when note was last edited. | | `lastEditedBy` | [`UserCore`](#usercore) | User who last edited the note. | | `maxAccessLevelOfAuthor` | [`String`](#string) | Max access level of the note author in the project. | -| `position` | [`DiffPosition`](#diffposition) | Position of this note on a diff. | +| `position` | [`DiffPosition`](#diffposition) | Position of the note on a diff. | | `project` | [`Project`](#project) | Project associated with the note. | | `resolvable` | [`Boolean!`](#boolean) | Indicates if the object can be resolved. | | `resolved` | [`Boolean!`](#boolean) | Indicates if the object is resolved. | | `resolvedAt` | [`Time`](#time) | Timestamp of when the object was resolved. | | `resolvedBy` | [`UserCore`](#usercore) | User who resolved the object. | -| `system` | [`Boolean!`](#boolean) | Indicates whether this note was created by the system or by a user. | +| `system` | [`Boolean!`](#boolean) | Indicates whether the note was created by the system or by a user. | | `systemNoteIconName` | [`String`](#string) | Name of the icon corresponding to a system note. | | `systemNoteMetadata` | [`SystemNoteMetadata`](#systemnotemetadata) | Metadata for the given note if it is a system note. | | `updatedAt` | [`Time!`](#time) | Timestamp of the note's last activity. | -| `url` | [`String`](#string) | URL to view this Note in the Web UI. | +| `url` | [`String`](#string) | URL to view the note in the Web UI. | | `userPermissions` | [`NotePermissions!`](#notepermissions) | Permissions for the current user on the resource. | ### `NotePermissions` diff --git a/lib/api/entities/note.rb b/lib/api/entities/note.rb index c693edc611bbaf0150330d90fb259f3247c8e943..97f4810866215f0b3595e16629dd908e4024781d 100644 --- a/lib/api/entities/note.rb +++ b/lib/api/entities/note.rb @@ -8,7 +8,11 @@ class Note < Grape::Entity expose :id expose :type - expose :note, as: :body + + expose :body do |note| + NotePresenter.new(note, current_user: options[:current_user]).note + end + expose :attachment_identifier, as: :attachment expose :author, using: Entities::UserBasic expose :created_at, :updated_at diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 70b4a3735e3cc718567471fca64b598a81fff8d2..138b2fddc8363fbcd752b124b9f0593817d27f49 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -51,7 +51,7 @@ class Notes < ::API::Base notes = paginate(raw_notes) notes = prepare_notes_for_rendering(notes) notes = notes.select { |note| note.readable_by?(current_user) } - present notes, with: Entities::Note + present notes, with: Entities::Note, current_user: current_user end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/gitlab/email/service_desk/custom_email.rb b/lib/gitlab/email/service_desk/custom_email.rb index 1828f71984b8df935843b915f262891f1b0f3b06..2d2ca20aa115e88c5f1f4c207e95046aa5bf8717 100644 --- a/lib/gitlab/email/service_desk/custom_email.rb +++ b/lib/gitlab/email/service_desk/custom_email.rb @@ -8,7 +8,6 @@ module ServiceDesk # incoming_email and service_desk_email. module CustomEmail REPLY_ADDRESS_KEY_REGEXP = /\+([0-9a-f]{32})@/ - EMAIL_REGEXP = /\A[\w\-._]+@[\w\-.]+\.{1}[a-zA-Z]{2,}\z/ class << self def reply_address(issue, reply_key) @@ -40,7 +39,7 @@ def key_from_reply_address(email) def find_service_desk_setting_from_reply_address(email, key) potential_custom_email = email.sub("+#{key}", '') - return unless EMAIL_REGEXP.match?(potential_custom_email) + return unless Gitlab::Utils::Email::EMAIL_REGEXP_WITH_ANCHORS.match?(potential_custom_email) ServiceDeskSetting.find_by_custom_email(potential_custom_email) end diff --git a/lib/gitlab/utils/email.rb b/lib/gitlab/utils/email.rb index 5eb57a66c63d9ff2804b21b1a140104c337eaedd..6decb3da8f87f8dfa5b2459166524b84abd2ca63 100644 --- a/lib/gitlab/utils/email.rb +++ b/lib/gitlab/utils/email.rb @@ -5,6 +5,13 @@ module Utils module Email extend self + # Don't use Devise.email_regexp or URI::MailTo::EMAIL_REGEXP to be a bit more restrictive + # on the format of an email. Especially for custom email addresses which cannot contain a `+` + # in app/models/service_desk_setting.rb + EMAIL_REGEXP = /[\w\-._]+@[\w\-.]+\.{1}[a-zA-Z]{2,}/ + EMAIL_REGEXP_WITH_CAPTURING_GROUP = /(#{EMAIL_REGEXP})/ + EMAIL_REGEXP_WITH_ANCHORS = /\A#{EMAIL_REGEXP.source}\z/ + # Replaces most visible characters with * to obfuscate an email address # deform adds a fix number of * to ensure the address cannot be guessed. Also obfuscates TLD with ** def obfuscated_email(email, deform: false) @@ -14,6 +21,17 @@ def obfuscated_email(email, deform: false) masker_class.new(email).masked end + # Runs email address obfuscation on the given text. + def obfuscate_emails_in_text(text) + return text unless text.present? + + matches = text.scan(EMAIL_REGEXP_WITH_CAPTURING_GROUP).flatten.index_with do |match| + obfuscated_email(match, deform: true) + end + + text.gsub(Regexp.union(matches.keys), matches) + end + class Masker attr_reader :local_part, :sub_domain, :toplevel_domain, :at, :dot diff --git a/spec/graphql/types/notes/note_type_spec.rb b/spec/graphql/types/notes/note_type_spec.rb index 8aabdb78562cfc3b087b16b0b0f48d015bc6cbfe..02f311785db2bfdf18fa3d4ba52d0bb2fbaa2dbb 100644 --- a/spec/graphql/types/notes/note_type_spec.rb +++ b/spec/graphql/types/notes/note_type_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['Note'], feature_category: :team_planning do + include GraphqlHelpers + it 'exposes the expected fields' do expected_fields = %i[ author @@ -37,4 +39,35 @@ specify { expect(described_class).to expose_permissions_using(Types::PermissionTypes::Note) } specify { expect(described_class).to require_graphql_authorizations(:read_note) } + + context 'when system note with issue_email_participants action', feature_category: :service_desk do + let_it_be(:user) { build_stubbed(:user) } + let_it_be(:email) { 'user@example.com' } + let_it_be(:note_text) { "added #{email}" } + # Create project and issue separately because we need to public project. + # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Notes::RenderService updates #note and #cached_markdown_version + let_it_be(:project) { create(:project, :public) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:note) do + create(:note, :system, project: project, noteable: issue, author: Users::Internal.support_bot, note: note_text) + end + + let_it_be(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) } + # rubocop:enable RSpec/FactoryBot/AvoidCreate + + let(:prepared_note) { Notes::RenderService.new(user).execute([note]).first } + let(:obfuscated_email) { 'us*****@e*****.c**' } + + describe '#body' do + subject { resolve_field(:body, prepared_note, current_user: user) } + + it_behaves_like 'a note content field with obfuscated email address' + end + + describe '#body_html' do + subject { resolve_field(:body_html, prepared_note, current_user: user) } + + it_behaves_like 'a note content field with obfuscated email address' + end + end end diff --git a/spec/lib/gitlab/utils/email_spec.rb b/spec/lib/gitlab/utils/email_spec.rb index c81c2558f706b00ea45380e12ac1482d445202a1..430002eb520fbfed8546c2a8239332145c7da784 100644 --- a/spec/lib/gitlab/utils/email_spec.rb +++ b/spec/lib/gitlab/utils/email_spec.rb @@ -51,4 +51,20 @@ end end end + + describe '.obfuscate_emails_in_text' do + where(:input, :output) do + nil | nil + '' | '' + 'added no email address' | 'added no email address' + 'added user@example.com' | 'added us*****@e*****.c**' + 'added user@example.com and hello@example.com' | 'added us*****@e*****.c** and he*****@e*****.c**' + 'removed user@example.com, hello@example.com and bye@example.com' | + 'removed us*****@e*****.c**, he*****@e*****.c** and by*****@e*****.c**' + end + + with_them do + it { expect(described_class.obfuscate_emails_in_text(input)).to eq(output) } + end + end end diff --git a/spec/presenters/note_presenter_spec.rb b/spec/presenters/note_presenter_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2a8ae38f08274ce4e92112af859644116eee9c32 --- /dev/null +++ b/spec/presenters/note_presenter_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe NotePresenter, feature_category: :team_planning do + let_it_be(:user) { build_stubbed(:user) } + let_it_be(:email) { 'user@example.com' } + let_it_be(:note_text) { "added #{email}" } + # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Notes::RenderService updates #note and #cached_markdown_version + let_it_be_with_reload(:note) { create(:note, :system, author: Users::Internal.support_bot, note: note_text) } + let_it_be(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) } + # rubocop:enable RSpec/FactoryBot/AvoidCreate + + let(:obfuscated_email) { 'us*****@e*****.c**' } + + subject(:presenter) do + # The RenderService calls Banzai::ObjectRenderer which + # populates `redacted_note_html`. We also do this in + # API controllers. + prepared_notes = Notes::RenderService.new(user).execute([note]) + described_class.new(prepared_notes.first, current_user: user) + end + + describe '#note' do + subject { presenter.note } + + it_behaves_like 'a note content field with obfuscated email address' + end + + describe '#note_html' do + subject { presenter.note_html } + + it_behaves_like 'a note content field with obfuscated email address' + + context 'when redacted_note_html is not present' do + subject(:presenter) do + described_class.new(note, current_user: user).note_html + end + + it 'falls back to note_html and obfuscates emails' do + is_expected.to include(obfuscated_email) + end + end + end +end diff --git a/spec/requests/api/graphql/notes/note_spec.rb b/spec/requests/api/graphql/notes/note_spec.rb index daceaec0b94cd6002e622d980284043df4a52486..2a2ea8ce49b1dba32745d214cdaa7be75face07a 100644 --- a/spec/requests/api/graphql/notes/note_spec.rb +++ b/spec/requests/api/graphql/notes/note_spec.rb @@ -6,6 +6,7 @@ include GraphqlHelpers let_it_be(:current_user) { create(:user) } + let_it_be(:reporter_user) { create(:user) } let_it_be(:project) { create(:project, :private) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:note) { create(:note, noteable: issue, project: project) } @@ -19,6 +20,10 @@ graphql_query_for('note', note_params, note_fields) end + before_all do + project.add_reporter(reporter_user) + end + it_behaves_like 'a working graphql query' do before do post_graphql(query, current_user: current_user) @@ -44,7 +49,7 @@ end context 'when the user has access to read the note' do - before do + before_all do project.add_guest(current_user) end @@ -62,6 +67,40 @@ expect(note_data['id']).to eq(global_id_of(system_note).to_s) end + + context 'with issue_email_participants action' do + let_it_be(:email) { 'user@example.com' } + let_it_be(:note_text) { "added #{email}" } + let_it_be(:issue_email_participants_system_note) do + create(:note, :system, + project: project, noteable: issue, author: Users::Internal.support_bot, note: note_text) + end + + let_it_be(:system_note_metadata) do + create(:system_note_metadata, note: issue_email_participants_system_note, action: :issue_email_participants) + end + + let(:obfuscated_email) { 'us*****@e*****.c**' } + let(:note_params) { { 'id' => global_id_of(issue_email_participants_system_note) } } + + it 'returns obfuscated email' do + post_graphql(query, current_user: current_user) + + expect(note_data['id']).to eq(global_id_of(issue_email_participants_system_note).to_s) + expect(note_data['body']).to include(obfuscated_email) + expect(note_data['bodyHtml']).to include(obfuscated_email) + end + + context 'when user has at least the reporter role in project' do + it 'returns email' do + post_graphql(query, current_user: reporter_user) + + expect(note_data['id']).to eq(global_id_of(issue_email_participants_system_note).to_s) + expect(note_data['body']).to include(email) + expect(note_data['bodyHtml']).to include(email) + end + end + end end context 'and notes widget is not available' do diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 497bbeced38b4a1fff1c8af4a84abbfad87702b4..e7ff7baaa6ce09def70ab9baa591dd9b1d265f34 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -49,6 +49,37 @@ end end + context 'when system note with issue_email_participants action' do + let!(:email) { 'user@example.com' } + let!(:note_text) { "added #{email}" } + let!(:note) do + create(:note, :system, project: project, noteable: issue, author: Users::Internal.support_bot, note: note_text) + end + + let!(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) } + let!(:another_user) { create(:user) } + + let(:obfuscated_email) { 'us*****@e*****.c**' } + + it 'returns obfuscated email' do + get api("/projects/#{project.id}/issues/#{issue.iid}/notes", another_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.first['body']).to include(obfuscated_email) + end + + context 'when user has at least the reporter role in project' do + it 'returns email' do + get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.first['body']).to include(email) + end + end + end + context "when referencing other project" do # For testing the cross-reference of a private issue in a public project let(:private_project) do diff --git a/spec/serializers/note_entity_spec.rb b/spec/serializers/note_entity_spec.rb index d5c4a31a93785c4fd0250ecceb6c5547f8ff55e3..51335c5f73204b027bf93f77e25c66f09d3a0278 100644 --- a/spec/serializers/note_entity_spec.rb +++ b/spec/serializers/note_entity_spec.rb @@ -2,14 +2,17 @@ require 'spec_helper' -RSpec.describe NoteEntity do +RSpec.describe NoteEntity, feature_category: :team_planning do include Gitlab::Routing + # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Persisted records required + let_it_be(:note) { create(:note) } + let_it_be(:user) { create(:user) } + # rubocop:enable RSpec/FactoryBot/AvoidCreate + let(:request) { double('request', current_user: user, noteable: note.noteable) } let(:entity) { described_class.new(note, request: request) } - let(:note) { create(:note) } - let(:user) { create(:user) } subject { entity.as_json } @@ -53,9 +56,8 @@ end end - describe 'with email participant' do - let_it_be(:note) { create(:note) } - let_it_be(:note_metadata) { create(:note_metadata, note: note) } + describe 'with email participant', feature_category: :service_desk do + let_it_be(:note_metadata) { create(:note_metadata, note: note) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Persisted records required subject { entity.as_json[:external_author] } @@ -66,4 +68,36 @@ it_behaves_like 'external author' end end + + context 'when system note with issue_email_participants action', feature_category: :service_desk do + let_it_be(:email) { 'user@example.com' } + let_it_be(:note_text) { "added #{email}" } + # rubocop:disable RSpec/FactoryBot/AvoidCreate -- Notes::RenderService updates #note and #cached_markdown_version + let_it_be(:note) { create(:note, :system, author: Users::Internal.support_bot, note: note_text) } + let_it_be(:system_note_metadata) { create(:system_note_metadata, note: note, action: :issue_email_participants) } + # rubocop:enable RSpec/FactoryBot/AvoidCreate + + let(:obfuscated_email) { 'us*****@e*****.c**' } + let(:expected_text) { obfuscated_email } + + subject(:entity_hash) do + # The RenderService calls Banzai::ObjectRenderer which + # populates `redacted_note_html`. We also do this in + # API controllers. + prepared_notes = Notes::RenderService.new(user).execute([note]) + described_class.new(prepared_notes.first, request: request).as_json + end + + describe 'note' do + subject { entity_hash[:note] } + + it_behaves_like 'a note content field with obfuscated email address' + end + + describe 'note_html' do + subject { entity_hash[:note_html] } + + it_behaves_like 'a note content field with obfuscated email address' + end + end end diff --git a/spec/services/issue_email_participants/create_service_spec.rb b/spec/services/issue_email_participants/create_service_spec.rb index dc8d5a6ea74ec42b81b47121319fc8d224f253d1..05c9162dde14872c3f64972f6f83a82889adc177 100644 --- a/spec/services/issue_email_participants/create_service_spec.rb +++ b/spec/services/issue_email_participants/create_service_spec.rb @@ -12,6 +12,7 @@ note = issue.notes.last expect(note.system?).to be true expect(note.author).to eq(user) + expect(note.system_note_metadata.action).to eq('issue_email_participants') participants_emails = issue.email_participants_emails_downcase diff --git a/spec/support/shared_examples/graphql/notes_content_obfuscation_shared_examples.rb b/spec/support/shared_examples/graphql/notes_content_obfuscation_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..0f8f24a034f2e8825fb676bd42e83fe340c7395c --- /dev/null +++ b/spec/support/shared_examples/graphql/notes_content_obfuscation_shared_examples.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a note content field with obfuscated email address' do + context 'when anonymous' do + let(:user) { nil } + + it { is_expected.to include(obfuscated_email) } + end + + context 'with signed in user' do + before do + stub_member_access_level(note.project, access_level => user) if access_level + end + + context 'when user has no role in project' do + let(:access_level) { nil } + + it { is_expected.to include(obfuscated_email) } + end + + context 'when user has guest role in project' do + let(:access_level) { :guest } + + it { is_expected.to include(obfuscated_email) } + end + + context 'when user has reporter role in project' do + let(:access_level) { :reporter } + + it { is_expected.to include(email) } + end + + context 'when user has developer role in project' do + let(:access_level) { :developer } + + it { is_expected.to include(email) } + end + end +end