Skip to content

Ensure that the user is able to see the photo they are setting as highlighted#4449

Merged
ildyria merged 2 commits into
masterfrom
fix-highlight-access-check
Jun 22, 2026
Merged

Ensure that the user is able to see the photo they are setting as highlighted#4449
ildyria merged 2 commits into
masterfrom
fix-highlight-access-check

Conversation

@ildyria

@ildyria ildyria commented Jun 22, 2026

Copy link
Copy Markdown
Member

This pull request updates the logic for determining if a photo can be highlighted, ensuring that only users who have permission to view a photo can highlight it. Additionally, it improves the loading of related data for photos by eager-loading access permissions for albums.

Authorization logic improvements:

  • Updated the canHighlight method in PhotoPolicy so that only users who can actually see a photo are allowed to highlight it, even for anonymous and authenticated users. This adds an extra layer of security by tying highlight permissions to view permissions.

Data loading enhancements:

  • Modified the photo query in SetPhotosHighlightedRequest.php to also eager-load albums.access_permissions, ensuring that access permissions are available when processing highlighted photos.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected photo highlighting authorization so anonymous and authenticated visibility modes are granted only when the user can actually view the photo.
    • Improved highlighted-photo handling to load the necessary album access details to enforce visibility consistently.
  • Tests
    • Updated image processing authorization tests to use the correct photo IDs for non-owning user scenarios.

@ildyria ildyria requested a review from a team as a code owner June 22, 2026 10:52
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ed201d28-92d4-4826-b6fd-15907b77921f

📥 Commits

Reviewing files that changed from the base of the PR and between 373569e and 618ca84.

📒 Files selected for processing (1)
  • tests/ImageProcessing/Photo/PhotoStarTest.php
✅ Files skipped from review due to trivial changes (1)
  • tests/ImageProcessing/Photo/PhotoStarTest.php

📝 Walkthrough

Walkthrough

SetPhotosHighlightedRequest adds albums.access_permissions to the photo query's eager-load list. PhotoPolicy::canHighlight updates its ANONYMOUS and AUTHENTICATED match branches to require canSee($user, $photo) (and non-null user for AUTHENTICATED) instead of returning unconditional true or checking only authentication status. Tests are updated to exercise the tightened logic by substituting photo4 for photo1 in highlight requests.

Changes

Highlight visibility permission fix

Layer / File(s) Summary
Eager-load access_permissions and tighten canHighlight
app/Http/Requests/Photo/SetPhotosHighlightedRequest.php, app/Policies/PhotoPolicy.php
Request expands the photo eager-load to include albums.access_permissions; policy's canHighlight replaces unconditional/auth-only branches for ANONYMOUS and AUTHENTICATED with canSee($user, $photo) calls that use those loaded permissions.
Test updates for tightened canHighlight logic
tests/ImageProcessing/Photo/PhotoStarTest.php
testSetStarPhotoAnonymous and testSetStarPhotoWithAuthenticatedVisibility now use photo4 instead of photo1 to exercise the permission check with specific photo visibility configurations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A highlight once shown to all eyes,
Now hides behind a permission disguise.
canSee must return true,
Before the photo breaks through —
No more free passes, no more surprise!

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.43%. Comparing base (7d55399) to head (618ca84).
⚠️ Report is 5 commits behind head on master.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ildyria ildyria merged commit fe07567 into master Jun 22, 2026
48 checks passed
@ildyria ildyria deleted the fix-highlight-access-check branch June 22, 2026 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant