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:
- We need the user, so the user is passed in to the search request
- Collection class now has a
redact_unauthorized_results!method which does the checking- 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.
- This requires each collection class to define a way to convert ids returned from the response to objects. The redaction checks
- The
QueryResultclass handles calling the redaction: it takes in user, result and collection and callscollection. redact_unauthorized_results! - Each client is updated to:
- Take in a
user - Return only non-redacted results
- Take in a
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)