ActiveContext: redact unauthorized results

What does this MR do and why?

Adds redaction logic for ActiveContext so that we're not showing data a user should not have access to. The redaction works by checking

Ability.allowed?(user, :"read_#{object.to_ability_name}", object)

For every returned result.

This requires a few changes:

  1. We need the user, so the user is passed in to the search request
  2. Collection class now has a redact_unauthorized_results! method which does the checking
    1. This requires each collection class to define a way to convert ids returned from the response to objects. The redaction checks object.respond_to?(:to_ability_name) so that remains flexible for non-AR collections.
  3. The QueryResult class handles calling the redaction: it takes in user, result and collection and calls collection. redact_unauthorized_results!
  4. Each client is updated to:
    1. Take in a user
    2. Return only non-redacted results

One major change in this is that we now return objects instead of the documents as they come from the vector store. I believe we were eventually going to do so to keep consistency between the stores.

Also adds a search method that's called from the Collection class for easier invocation:

Ai::Context::Collections::MergeRequest.search(user: current_user, query: query)

Demo

User 1 has access to everything:

Elasticsearch:

Ai::Context::Collections::MergeRequest.search(query: ActiveContext::Query.filter(namespace_id: [2,4]), user: User.find(1))
=> [#<MergeRequest id:10 gitlab-org/gitlab-test!5>, #<MergeRequest id:11 gitlab-org/gitlab-test!6>, #<MergeRequest id:20 gnuwget/Wget2!1>]

Postgres:

Ai::Context::Collections::MergeRequest.search(query: ActiveContext::Query.filter(namespace_id: [2,4]), user: User.find(1))
=> [#<MergeRequest id:10 gitlab-org/gitlab-test!5>, #<MergeRequest id:11 gitlab-org/gitlab-test!6>, #<MergeRequest id:20 gnuwget/Wget2!1>]

User 70 does not have access to everything:

Elasticsearch:

Ai::Context::Collections::MergeRequest.search(query: ActiveContext::Query.filter(namespace_id: [2,4]), user: User.find(70))
=> [#<MergeRequest id:10 gitlab-org/gitlab-test!5>, #<MergeRequest id:11 gitlab-org/gitlab-test!6>]

Postgres:

Ai::Context::Collections::MergeRequest.search(query: ActiveContext::Query.filter(namespace_id: [2,4]), user: User.find(70))
=> [#<MergeRequest id:10 gitlab-org/gitlab-test!5>, #<MergeRequest id:11 gitlab-org/gitlab-test!6>]

References

How to set up and validate locally

  • Follow instructions for setup
  • Connect ES/OS
  • Perform a search using an admin user (has access to everything), e.g.
Ai::Context::Collections::MergeRequest.search(query: ActiveContext::Query.filter(namespace_id: [2,4]), user: User.find(1))
  • Note how many results are returned
  • Perform a search using a user that doesn't have access to all of the results, e.g.
Ai::Context::Collections::MergeRequest.search(query: ActiveContext::Query.filter(namespace_id: [2,4]), user: User.find(70))
  • Note that some results are redacted
  • Connect postgres
  • Update the client to read from your populated table, i.e. change to conn.execute('SELECT * FROM gitlab_active_context_merge_requests') in gems/gitlab-active-context/lib/active_context/databases/postgresql/client.rb
  • Repeat the steps

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #524571 (closed)

Edited by Madelein van Niekerk

Merge request reports

Loading