Skip to content

Fix download bypass on password protected albums#4459

Merged
ildyria merged 1 commit into
masterfrom
fix-download-bypass
Jun 24, 2026
Merged

Fix download bypass on password protected albums#4459
ildyria merged 1 commit into
masterfrom
fix-download-bypass

Conversation

@ildyria

@ildyria ildyria commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes
    • Download access now matches album visibility more consistently, blocking downloads when a user cannot access the album.
    • Password-protected albums now follow the intended access rules for browsing and downloads, preventing unexpected download bypasses.
    • Download behavior remains available where album permissions allow it, including for public albums with the proper download setting.

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

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

AlbumPolicy::canDownload() gains an early denial when canAccess() returns false, closing a bypass path where anonymous users could download password-protected albums via /api/v2/Zip. A new feature test class exercises the fixed behavior across three scenarios: anonymous download blocked, download denied without the grant flag, and direct policy-level assertion.

Changes

Password-protected album download bypass fix

Layer / File(s) Summary
canDownload() access gate
app/Policies/AlbumPolicy.php
Adds a 5-line early return in canDownload() that evaluates canAccess($user, $abstract_album) first and immediately returns false if it fails, gating all subsequent download logic behind album accessibility.
Feature tests: bypass, negative-control, policy asymmetry
tests/Feature_v2/Album/PasswordDownloadBypassTest.php
Adds PasswordDownloadBypassTest with a private setup helper, a PoC test asserting /api/v2/Zip is now denied for anonymous users on a password-protected album, a negative-control test disabling grants_download, and a direct AlbumPolicy resolution test asserting canAccess/canDownload outcomes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A password's a lock, not just a door,
The bunny patched the zip to check once more.
No sneaking through with streaming tricks,
canAccess guards the download mix.
The tests confirm the bypass is sealed — hooray!

🚥 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 57e2b3a5-9e8b-4c40-a672-a0be294c3292

📥 Commits

Reviewing files that changed from the base of the PR and between 4df83b4 and 48bc309.

📒 Files selected for processing (2)
  • app/Policies/AlbumPolicy.php
  • tests/Feature_v2/Album/PasswordDownloadBypassTest.php

Comment thread tests/Feature_v2/Album/PasswordDownloadBypassTest.php
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.45%. Comparing base (4df83b4) to head (48bc309).
⚠️ Report is 1 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 4366dd0 into master Jun 24, 2026
48 checks passed
@ildyria ildyria deleted the fix-download-bypass branch June 24, 2026 11:12
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