-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Restrict search results based on user permissions [FC-0040] #34471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
bradenmacdonald
merged 17 commits into
openedx:master
from
open-craft:jill/content-search-perms
Apr 17, 2024
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
6abe377
feat: adds SearchAccess model
pomegranited 80699f1
test: can't import content.search in lms tests
pomegranited c729b35
feat: use SearchAccess in documents and views
pomegranited 702abb6
Merge branch 'master' into jill/content-search-perms
pomegranited 0b32048
test: can't import content.search or content_staging in lms tests
pomegranited b864e6c
fix: make access_id field filterable
rpenido 6f78c0f
fix: use SearchAccess.get_or_create in signal handlers
pomegranited 902e75c
refactor: refactors the view tests to make them clearer
pomegranited 3b07501
feat: adds org filters to meilisearch filter
pomegranited 4edcebe
Merge branch 'master' into jill/content-search-perms
pomegranited ef89591
refactor: removes data migration
pomegranited a917c64
refactor: adds functions to common.djangoapps.student.role_helpers
pomegranited 6a3c30a
fix: get_access_ids_for_request only returns individual access
pomegranited c7215ac
fix: use org-level permissions when generating search filter
pomegranited a28f3a9
refactor: remove SearchAccess creation signal handlers
pomegranited ef24a88
Merge branch 'master' into jill/content-search-perms
pomegranited 811687d
feat: omit access_ids from the search filter that are covered by the …
pomegranited File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| """ | ||
| Signal/event handlers for content search | ||
| """ | ||
| from django.db.models.signals import post_delete | ||
| from django.dispatch import receiver | ||
| from openedx_events.content_authoring.data import ContentLibraryData | ||
| from openedx_events.content_authoring.signals import CONTENT_LIBRARY_DELETED | ||
|
|
||
| from openedx.core.djangoapps.content.course_overviews.models import CourseOverview | ||
| from openedx.core.djangoapps.content.search.models import SearchAccess | ||
|
|
||
|
|
||
| # Using post_delete here because there is no COURSE_DELETED event defined. | ||
| @receiver(post_delete, sender=CourseOverview) | ||
| def delete_course_search_access(sender, instance, **kwargs): # pylint: disable=unused-argument | ||
| """Deletes the SearchAccess instance for deleted CourseOverview""" | ||
| SearchAccess.objects.filter(context_key=instance.id).delete() | ||
|
|
||
|
|
||
| @receiver(CONTENT_LIBRARY_DELETED) | ||
| def delete_library_search_access(content_library: ContentLibraryData, **kwargs): | ||
| """Deletes the SearchAccess instance for deleted content libraries""" | ||
| SearchAccess.objects.filter(context_key=content_library.library_key).delete() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
openedx/core/djangoapps/content/search/migrations/0001_initial.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # Generated by Django 4.2.10 on 2024-04-02 04:55 | ||
|
|
||
| from django.db import migrations, models | ||
| from opaque_keys.edx.django.models import LearningContextKeyField | ||
| from opaque_keys.edx.locator import LibraryLocatorV2 | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| initial = True | ||
|
|
||
| dependencies = [ | ||
| ('course_overviews', '0001_initial'), | ||
| ('content_libraries', '0001_initial'), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.CreateModel( | ||
| name='SearchAccess', | ||
| fields=[ | ||
| ('id', models.BigAutoField(help_text='Numeric ID for each Course / Library context. This ID will generally require fewer bits than the full LearningContextKey, allowing more courses and libraries to be represented in content search filters.', primary_key=True, serialize=False)), | ||
| ('context_key', LearningContextKeyField(max_length=255, unique=True)), | ||
| ], | ||
| ), | ||
| ] |
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| """Database models for content search""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from django.db import models | ||
| from django.utils.translation import gettext_lazy as _ | ||
| from opaque_keys.edx.django.models import LearningContextKeyField | ||
| from rest_framework.request import Request | ||
|
|
||
| from common.djangoapps.student.role_helpers import get_course_roles | ||
| from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole | ||
| from openedx.core.djangoapps.content_libraries.api import get_libraries_for_user | ||
|
|
||
|
|
||
| class SearchAccess(models.Model): | ||
| """ | ||
| Stores a numeric ID for each ContextKey. | ||
|
|
||
| We use this shorter ID instead of the full ContextKey when determining a user's access to search-indexed course and | ||
| library content because: | ||
|
|
||
| a) in some deployments, users may be granted access to more than 1_000 individual courses, and | ||
| b) the search filter request is stored in the JWT, which is limited to 8Kib. | ||
| """ | ||
| id = models.BigAutoField( | ||
| primary_key=True, | ||
| help_text=_( | ||
| "Numeric ID for each Course / Library context. This ID will generally require fewer bits than the full " | ||
| "LearningContextKey, allowing more courses and libraries to be represented in content search filters." | ||
| ), | ||
| ) | ||
| context_key = LearningContextKeyField( | ||
| max_length=255, unique=True, null=False, | ||
| ) | ||
|
|
||
|
|
||
| def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> list[int]: | ||
| """ | ||
| Returns a list of SearchAccess.id values for courses and content libraries that the requesting user has been | ||
| individually grated access to. | ||
|
|
||
| Omits any courses/libraries with orgs in the `omit_orgs` list. | ||
| """ | ||
| omit_orgs = omit_orgs or [] | ||
|
|
||
| course_roles = get_course_roles(request.user) | ||
| course_clause = models.Q(context_key__in=[ | ||
| role.course_id | ||
| for role in course_roles | ||
| if ( | ||
| role.role in [CourseInstructorRole.ROLE, CourseStaffRole.ROLE] | ||
| and role.org not in omit_orgs | ||
| ) | ||
| ]) | ||
|
|
||
| libraries = get_libraries_for_user(user=request.user) | ||
| library_clause = models.Q(context_key__in=[ | ||
| lib.library_key for lib in libraries | ||
| if lib.library_key.org not in omit_orgs | ||
| ]) | ||
|
|
||
| # Sort by descending access ID to simulate prioritizing the "most recently created context keys". | ||
| return list( | ||
| SearchAccess.objects.filter( | ||
| course_clause | library_clause | ||
| ).order_by('-id').values_list("id", flat=True) | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.