Store learner photos in private storage, not public#711
Merged
Conversation
Learner.photo (including photos imported during Google Workspace group enrollment) was saved via Django's default public media storage, so anyone with the URL could view a learner's photo without authentication. - Learner.photo now uses get_private_file_storage, matching the existing AssignmentSubmission.file_submission pattern. - GoogleGroupEnrollmentHandler._get_user() saves the downloaded profile photo via PRIVATE_FILE_STORAGE instead of default_storage. - Added Learner.private_photo_url and a shared build_private_file_url() helper, and switched LearnerResponse.photo to serve that signed, access-controlled URL instead of a raw public media URL. - Broadened PrivateFileView's role check from admin/instructor to admin/editor/instructor/viewer, matching who can already see the Learners page — this also affects its other current use (assignment submission files), intentionally. Existing photos already sitting in public storage from past imports are left as-is; only new photos go to private storage going forward. Closes #709 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Learner.photo now defaults to private storage, but existing photos saved before this change still physically live in public storage since they weren't migrated. serialize_photo/private_photo_url previously always built a private-storage URL regardless, which would 404 via PrivateFileView for every pre-existing photo. Add resolve_private_or_public_file_url(): checks PRIVATE_FILE_STORAGE first (where new photos land), falls back to public storage (where un-migrated existing photos still are), and returns None only if the file exists in neither. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
payamnj
force-pushed
the
feature/private-learner-photos
branch
from
01a9a5b to
9f8b1dc
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Fixes #709 —
Learner.photo(including photos imported during Google Workspace group enrollment) was saved via Django's default public media storage, so anyone with the URL could view a learner's photo without authentication.Learner.photonow usesstorage=get_private_file_storage(django_email_learning/models/enrollments.py), matching the existingAssignmentSubmission.file_submissionpattern — private storage ifPRIVATE_FILE_STORAGE_ALIAS/PRIVATE_FILE_STORAGE_LOCATIONis configured, otherwise a localprivate_files/folder.GoogleGroupEnrollmentHandler._get_user()now saves the downloaded Google Directory profile photo viaPRIVATE_FILE_STORAGEinstead ofdefault_storage.Learner.private_photo_url(mirrorsAssignmentSubmission.private_file_url) and a sharedbuild_private_file_url()helper inservices/utils.py.LearnerResponse.photo(platform/api/serializers/common.py) now serializes to this signed, access-controlled URL instead of calling.urldirectly on the (now-private)ImageField.PrivateFileView's role check (platform/views/misc.py) fromadmin/instructortoadmin/editor/instructor/viewer, matching who can already see the Learners page. This intentionally also affects its other current use — assignment submission files are now viewable by editor/viewer roles too, sincePrivateFileViewis a shared, generic private-file endpoint, not learner-photo-specific.Decisions made (see discussion on #709)
Learner.photoonly.OrganizationUser.photohas the identical gap but is left for a separate follow-up.PrivateFileViewrather than building a parallel endpoint, since duplicating the signed-URL + role-check logic for a second view seemed worse than accepting the (deliberate, and arguably correct) side effect of loosening submission-file access to match who already sees the Learners page.Test plan
pytest -q— 779 passed, 81.56% coveragemypy— no issues across 164 source filespython manage.py makemigrations --check— no driftruff check/ruff format— cleanLearner.photosaving toPRIVATE_FILE_STORAGE,Learner.private_photo_url,LearnerResponse.photoreturning a private (not/media/) URL from the learners list API,GoogleGroupEnrollmentHandler._get_user()saving the downloaded photo to private storage, andPrivateFileView's updated role matrix (editor/viewer now allowed; non-org-members still rejected)Closes #709
🤖 Generated with Claude Code