edx-platform icon indicating copy to clipboard operation
edx-platform copied to clipboard

Restrict search results based on user permissions [FC-0040]

Open pomegranited opened this issue 10 months ago • 2 comments

Description

Meilisearch allows us to create JWTs with embedded "filters", which guarantee that users searching with these tokens will only be permitted to see the documents that match these filters.

In Open edX, we grant permissions to block content based on its learning context, i.e. the course or content library that contains the block. Access to courses can be done in two ways, either individually on the course, or using org level rights. Access to content libraries is managed individually on the library (only library creation uses org level rights).

Because of these individual access rights, the list of courses and libraries a user has access to can be very long, >1000 in some instances (see comment). But since we need to encode that list into a JWT, we're limited to a total token size of 8Kib, but course keys can be very long (255 char). So to facilitate packing as many course/library identifiers into this token, this PR:

  • Adds a SearchAccess model with a numeric ID, mapped to all CourseOverview and ContentLibrary context_keys.
  • Adds an access_id field to the search-indexed Document, storing the SearchAccess.id relevant to that block's learning context.
  • Adds an access_id filter to the JWT token generation, containing the SearchAccess.id's for the top 1,000 learning contexts that the user has access to. (We omit access_ids covered by the org filter described below.)
  • Adds an org filter to the JWT token generation, containing max 1,000 org names that the user has been granted access to.

Supporting information

Content search is not enabled by default, so this change should have no impact on edX/2U or other sites running from master.

closes https://github.com/openedx/modular-learning/issues/198

Private-ref: FAL-3692

Testing instructions

Setup:

  1. Ensure that you are running the latest https://github.com/open-craft/tutor-contrib-meilisearch -- this will ensure that MEILISEARCH_ENABLED = True is set in the CMS and the MFE.
  2. Enable course-authoring MFE, running from the branch in https://github.com/openedx/frontend-app-course-authoring/pull/928
  3. Load https://github.com/open-craft/taxonomy-sample-data/
  4. Using this branch, run migrations: python manage.py cms migrate
  5. Also using this branch, reindex studio content: python manage.py cms reindex_studio --experimental

Testing:

  1. Login to Studio and the Course Authoring MFE as a global staff user.
  2. Click the 'search' button in the header bar to start searching studio content. Ensure that you can see content from all your courses.
  3. Repeat these steps with a non-global staff user who only has access to a single course or library. Ensure that user can only see that course/library when searching Studio content.

Deadline

None

Other information

  1. Anyone using content.search will need to run a full reindex to add the new document access_id field:
    python manage.py cms reindex_studio --experimental
    

pomegranited avatar Apr 04 '24 07:04 pomegranited

Thanks for the pull request, @pomegranited! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

openedx-webhooks avatar Apr 04 '24 07:04 openedx-webhooks

I put some tests (that are falling) here (https://github.com/open-craft/edx-platform/pull/650) to show what I'm expecting. Could you check if my understanding is right? 😄

Thank you for those added tests! I didn't use them directly, but please see 6a3c30aa71b83e385e6f5e698187b33c7cbff1e3 and c7215acf86d90a08ce0c62f95e5fe5d74c85e2aa for the relevant changes and tests.

pomegranited avatar Apr 09 '24 07:04 pomegranited

@pomegranited 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks avatar Apr 17 '24 18:04 openedx-webhooks

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

edx-pipeline-bot avatar Apr 17 '24 19:04 edx-pipeline-bot

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot avatar Apr 17 '24 20:04 edx-pipeline-bot

2U Release Notice: This PR has been deployed to the edX production environment.

edx-pipeline-bot avatar Apr 17 '24 20:04 edx-pipeline-bot