Skip to content

Add permission for indexed notes & update when they change (2nd attempt)

Terri Chu requested to merge 300351-remove-joins-from-the-elasticsearch-query-for-project-group-scoped-notes-comments-search-3 into master

gitlab-com/gl-infra/production#3665 (closed) must be completed before merging this: It is completed!

What does this MR do?

Related to #300351 (closed)

This change is the 2nd attempt to add new permissions fields to note documents in Elasticsearch

Original MR: !53415 (merged)

Revert MR: !54417 (merged)

The original MR did not account for all note types (specs only accounted for Noteable types: Issue, MergeRequest, Commit, and Snippet). However only those types are searchable (even though other types are indexed). We will handle the permissions for additional types in future issues (already created) and only add permissions for searchable types in this MR.

This MR:

  • only adds permissions for searchable note types (issue, commit, merge request, and snippet)
  • adds testing for ALL noteable types that support include Noteable or include ::Noteable to make sure the json generated does not throw an error

How to test

You can test the json documents created by running the following commands on each note type. I would create notes of each type and verify they get the correct permissions fields when indexed. You will need notes of each type in the DB.

Test can include:

  • Issue, Commit, Snippet, MergeRequest (these should have permission data added, visibility level & specific project feature level)
  • Epic, AlertManagement::Alert, DesignManagement::Design, Vulnerability (visibility level is present but specific project feature level should not be present, but the method should not throw any errors)

Example Note.where(noteable_type: Issue).first.__elasticsearch__.as_indexed_json

Output

{"id"=>1,
 "note"=>"Quod nostrum sint dolore sed laudantium.",
 "project_id"=>1,
 "noteable_type"=>"Issue",
 "noteable_id"=>1,
 "created_at"=>Wed, 30 Dec 2020 20:10:45 UTC +00:00,
 "updated_at"=>Wed, 30 Dec 2020 20:10:45 UTC +00:00,
 "confidential"=>nil,
 "issue"=>{"assignee_id"=>[1], "author_id"=>6, "confidential"=>false},
 "visibility_level"=>10,
 "issues_access_level"=>20,
 "type"=>"note",
 "join_field"=>{"name"=>"note", "parent"=>"project_1"}}

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Terri Chu

Merge request reports