feat: Add server option fileDownload to restrict file download#10394
feat: Add server option fileDownload to restrict file download#10394mtrezza merged 3 commits intoparse-community:alphafrom
fileDownload to restrict file download#10394Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10394 +/- ##
==========================================
+ Coverage 92.51% 92.52% +0.01%
==========================================
Files 192 192
Lines 16624 16663 +39
Branches 229 229
==========================================
+ Hits 15379 15417 +38
- Misses 1223 1224 +1
Partials 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Config.js (1)
596-622: Optional DRY refactor: unify file access-flag validation.
validateFileDownloadOptionsandvalidateFileUploadOptionsshare the same boolean-flag normalization pattern. A small shared helper could reduce drift in future updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Config.js` around lines 596 - 622, Both validateFileDownloadOptions and validateFileUploadOptions duplicate the same boolean normalization logic; extract a small helper (e.g., normalizeBooleanFlag(obj, key, defaultValue)) and use it from both methods. The helper should accept the options object, the property name (like "enableForAnonymousUser", "enableForPublic", "enableForAuthenticatedUser"), and the default from FileDownloadOptions/FileUploadOptions, set obj[key] = default when undefined, and throw the same type error when the value exists but is not a boolean; keep the initial object-type check in validateFileDownloadOptions/validateFileUploadOptions and then call normalizeBooleanFlag for each of the three flags to preserve identical behavior and error messages.spec/FileDownload.spec.js (1)
48-239: Add coverage for the compatibility edge cases this router change touches.This suite covers the new allow/deny matrix, but it still doesn't pin two easy regressions: a stale/invalid
X-Parse-Session-Tokenshould fall back to anonymous/public handling instead of surfacingINVALID_SESSION_TOKEN, andreadOnlyMasterKeyshould bypass read-only download restrictions just like the regular master key. Based on learnings,src/Routers/FilesRouter.jsintentionally catchesauth.getAuthForSessionTokenfailures and falls back to anonymous so stale client tokens do not break file downloads.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/FileDownload.spec.js` around lines 48 - 239, Add two tests to FileDownload.spec.js: one that uploads a file, then attempts to GET it with an invalid/stale X-Parse-Session-Token and asserts the request falls back to anonymous/public handling (i.e., does not return Parse.Error.INVALID_SESSION_TOKEN but obeys fileDownload flags), and another that configures fileDownload restrictions then performs a GET with the readOnlyMasterKey header and asserts it is allowed (status 200) just like using X-Parse-Master-Key. Reference the existing uploadTestFile helper and the request usage in the suite; these tests should exercise the FilesRouter behavior around auth.getAuthForSessionToken fallbacks and readOnlyMasterKey bypass logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Routers/FilesRouter.js`:
- Around line 183-205: The _validateFileDownload guard currently only exempts
requests with req.auth.isMaster; include read-only master key requests as well
by checking req.auth.readOnlyMasterKey (or a combined flag like
isMasterOrReadOnly = req.auth?.isMaster || req.auth?.readOnlyMasterKey) and use
that combined flag in each conditional instead of isMaster so
read-only-master-key requests bypass the fileDownload restrictions; update
references in _validateFileDownload and keep all checks against
config.fileDownload.enableForAnonymousUser, enableForAuthenticatedUser, and
enableForPublic unchanged.
- Around line 94-111: The GET file routes currently call initInfo then
Middlewares.handleParseSession which bypasses handleParseHeaders and so never
populates req.config or key-based auth; revert to the original file-download
auth flow by replacing Middlewares.handleParseSession on the routes used for
metadataHandler and getHandler with the file-specific auth middleware (or call
Middlewares.handleParseHeaders before session handling) so req.config and
master/read-only keys are available, and ensure the file-download auth
resolution (the code around auth.getAuthForSessionToken) is preserved to catch
session-token errors and fall back to anonymous/public access instead of
surfacing auth errors for stale tokens.
---
Nitpick comments:
In `@spec/FileDownload.spec.js`:
- Around line 48-239: Add two tests to FileDownload.spec.js: one that uploads a
file, then attempts to GET it with an invalid/stale X-Parse-Session-Token and
asserts the request falls back to anonymous/public handling (i.e., does not
return Parse.Error.INVALID_SESSION_TOKEN but obeys fileDownload flags), and
another that configures fileDownload restrictions then performs a GET with the
readOnlyMasterKey header and asserts it is allowed (status 200) just like using
X-Parse-Master-Key. Reference the existing uploadTestFile helper and the request
usage in the suite; these tests should exercise the FilesRouter behavior around
auth.getAuthForSessionToken fallbacks and readOnlyMasterKey bypass logic.
In `@src/Config.js`:
- Around line 596-622: Both validateFileDownloadOptions and
validateFileUploadOptions duplicate the same boolean normalization logic;
extract a small helper (e.g., normalizeBooleanFlag(obj, key, defaultValue)) and
use it from both methods. The helper should accept the options object, the
property name (like "enableForAnonymousUser", "enableForPublic",
"enableForAuthenticatedUser"), and the default from
FileDownloadOptions/FileUploadOptions, set obj[key] = default when undefined,
and throw the same type error when the value exists but is not a boolean; keep
the initial object-type check in
validateFileDownloadOptions/validateFileUploadOptions and then call
normalizeBooleanFlag for each of the three flags to preserve identical behavior
and error messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 047e67d3-d7d6-430a-9e02-160446340e98
📒 Files selected for processing (8)
README.mdresources/buildConfigDefinitions.jsspec/FileDownload.spec.jssrc/Config.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.jssrc/Routers/FilesRouter.js
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/FileDownload.spec.js (1)
48-281: Add a regression for invalid or empty session tokens on GET.The suite covers public/authenticated/anonymous/master/maintenance, but it does not pin the deliberate change that a bad
X-Parse-Session-Tokenmust be rejected instead of silently falling back to public access. One GET case here would catch that behavior, including the empty-header edge path.Based on learnings, invalid/stale session tokens on file downloads are now intentionally rejected rather than silently falling back to public access.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/FileDownload.spec.js` around lines 48 - 281, Add a regression test in the permissions suite to ensure an invalid or empty X-Parse-Session-Token does not fall back to public access: use the existing uploadTestFile() helper to get file.url, then call the same request(...) GET to file.url but include headers with 'X-Parse-Session-Token' set to an empty string and another case with an explicit invalid token; assert the request throws and that e.data.code equals Parse.Error.OPERATION_FORBIDDEN (similar to other negative tests), placing this new it(...) alongside the other permission cases so it exercises the same request handling paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Routers/FilesRouter.js`:
- Around line 96-105: The code treats an empty string X-Parse-Session-Token as
"no token" because it uses a falsy check on sessionToken; update the conditional
so only null/undefined (not empty string) triggers the public path. In
FilesRouter.js, change the check around req.info/sessionToken and req.auth to
explicitly test for undefined/null (e.g., sessionToken === undefined ||
sessionToken === null) so an empty string still counts as a provided token and
will be passed to handleParseSession for validation instead of falling back to
public access.
---
Nitpick comments:
In `@spec/FileDownload.spec.js`:
- Around line 48-281: Add a regression test in the permissions suite to ensure
an invalid or empty X-Parse-Session-Token does not fall back to public access:
use the existing uploadTestFile() helper to get file.url, then call the same
request(...) GET to file.url but include headers with 'X-Parse-Session-Token'
set to an empty string and another case with an explicit invalid token; assert
the request throws and that e.data.code equals Parse.Error.OPERATION_FORBIDDEN
(similar to other negative tests), placing this new it(...) alongside the other
permission cases so it exercises the same request handling paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 838e52fe-c208-4812-af18-3feac3ccb881
📒 Files selected for processing (2)
spec/FileDownload.spec.jssrc/Routers/FilesRouter.js
# [9.8.0-alpha.4](9.8.0-alpha.3...9.8.0-alpha.4) (2026-04-03) ### Features * Add server option `fileDownload` to restrict file download ([#10394](#10394)) ([fc117ef](fc117ef))
|
🎉 This change has been released in version 9.8.0-alpha.4 |
2 participants
Pull Request
Issue
File download and metadata endpoints are fully public with no access control. While
fileUploadprovides granular permission flags, no equivalent exists for downloads. Developers serving files via CDNs have no way to disable direct downloads through Parse Server.Approach
Adds a new
fileDownloadoption mirroring thefileUploadpermission model with three boolean flags:enableForAnonymousUser,enableForAuthenticatedUser,enableForPublic. All default totrueto preserve current behavior (non-breaking). Master key always bypasses. Applies to both file download and metadata endpoints.Tasks
Summary by CodeRabbit
New Features
Tests
Documentation
Bug Fixes