mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
docs/readme-projectsend-cloud
18 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 denkfabrik-li
|
19ee9d9833 |
Narrow the membership an API member write hands back
Adding or removing a group member answered with the group, and loaded the
relation whole:
return new GroupResource($group->loadCount('members')->load('members'));
GroupResource gives each member an id, a name and an email. So a
client-scoped staff member who added one of their own clients to a group
was handed, in the same response, the name and address of every other
client in it -- people they may not read anywhere else in the application,
and whom the group edit screen refuses to name for exactly that reason.
syncWithoutDetaching() makes the call idempotent, so the same request
returns the same list as often as it is sent.
The boundary is already written down. GroupResource's docblock:
both narrow the list to the clients the viewer may act on, and the
controller loading this relation is where that narrowing is applied
and Api\GroupsController::show() does it for the read of the same group,
noting that "it hands back the membership with addresses". Changing the
membership is not a reason to be told more than reading it is, so both
halves now narrow by the same query, through one private helper rather
than a third copy of it.
members_count is deliberately left whole, matching show(): a size is not
an identity, and it is the number the group listing already reports.
Nothing about who may perform the write changes -- StaffLibraryScope
::allowsGroupMembership() already decided that, and still does. This is
only what the answer is allowed to say.
Tests: added beside the existing "the API twin narrows the membership it
hands back", which covered the read half only. Both write tests fail
against the unfixed controller; the third pins that an unscoped token
still gets every member.
|
||
|
ignacionelson
|
5117511946 |
Merge pull request #1732 from denkfabrik-li/fix/public-preview-log-debounce
FileThumbnailController::preview() writes at most one FilePreviewed row per viewer per file per five minutes, because watching a video is a single deliberate act that the browser turns into dozens of Range requests. Its docblock ended by naming the route where the same act happens without an account -- PublicGroupsController::preview -- and that route logged unconditionally. Five requests for the same public file wrote five rows where the signed-in twin wrote one, so one visitor watching one clip buried the public half of the activity log, which is the half an operator reads to see what the outside world is doing. The window moves into a shared PreviewLog, next to PreviewKind, which those two routes already share for the same reason. Keying is unchanged for a signed-in viewer. An anonymous visitor has no account to key on, so the request IP stands in -- the same substitute ApiServiceProvider's rate limiter makes for an unauthenticated caller. It is a cache key with a five-minute life and never reaches the log, which keeps its own decision about recording an IP. Downloads are deliberately untouched and stay one row per download: each is a transfer, and DownloadAllowance::used() counts those rows to enforce a per-file cap, so swallowing one would hand out free downloads. The limit this leaves open, since the IP is a stand-in and not an identity: two anonymous visitors behind one address share a key, so within five minutes the second one's view of the same file is not recorded. That is the same trade the signed-in side has always made per account, and the alternative is the row-per-Range-request this fixes. Verified before merging: 24 passed across the public-preview and thumbnail suites on the trial-merge, which also confirms this co-exists with #1725 -- the two share both controllers and change different methods in each. With app/ reset and PreviewLog deleted, 1 failed / 9 passed. The signed-in route's existing debounce tests pass unchanged, which is what says the shared class did not move that side. request()->ip() honours the trusted-proxy configuration, so a forged X-Forwarded-For cannot defeat the window from outside. Reported and fixed by @denkfabrik-li. |
||
|
ignacionelson
|
1644d634d5 |
Merge pull request #1720 from denkfabrik-li/fix/group-reach-expired-file
groupReachesNoFurther() decides whether a client-scoped staff member may edit a group, by asking whether anything shared with it sits outside their library. |
||
|
denkfabrik-li
|
a1773cad5e |
Count a shared folder's contents as reach, not just the folder
groupReachesNoFurther() asks whether anything shared with a group sits outside the viewer's library. Its docblock says the folder half covers "the folders whose subtrees it can browse". It compares the folder ids the assignment names and stops there. A folder shared with a group hands its members the whole subtree -- File::scopeVisibleToClient matches on folder placement, and a folder is visible to a client when it or an ancestor is shared with them. So the guard passed on a subtree it had never looked into. Measured on main: a scoped rep's own folder, a subfolder somebody else created inside it, and that person's file in the subfolder. parent in the rep's library true subfolder in it false the file in it false add their own client to a group holding the parent 302, allowed the client can then reach the file true And because files() is "own uploads plus everything my clients can see", the file lands in the rep's own library on the next request. That is the widening this guard exists to refuse -- the first test in the file is called "a scoped staff member cannot widen their own library through a group". The folder half now walks each assigned folder's subtree, and the files inside it are checked too: a folder can be in the library while a file in it is not, since somebody else's upload into a folder this rep owns is neither their own nor their clients'. Expired files are skipped for the reason the deleted ones are -- membership grants nobody access to one. Three tests: the subfolder case, the stranger-file case, and a subtree wholly inside the library, which stays manageable. The first two go red without the fix. |
||
|
denkfabrik-li
|
c2dd2c758a |
Debounce the public preview log the way the signed-in one already is
FileThumbnailController::preview() writes at most one FilePreviewed row per viewer per file per five minutes, because a browser turns one video into a long tail of Range requests against the same URL. Its docblock names the anonymous route as the place the same act happens without an account -- and that route logs unconditionally. Measured: five requests for the same public file, five PublicFilePreviewed rows, against one for the signed-in twin. One visitor watching one clip buries the public half of the activity log, which is also the half an operator reads to see what the outside world is doing. The window is now a shared PreviewLog, next to PreviewKind, which the two preview routes already share for the same reason. Keying is unchanged for a signed-in viewer; an anonymous one has no account to key on, so the request IP stands in -- the same substitute the API's rate limiter makes for an unauthenticated caller. It is a cache key with a five-minute life and never reaches the log, which keeps its own decision about recording an IP (ActivityLogger::shouldRecordIp, Setting::DownloadIpLogging). Three tests: the replay is one row, two visitors are two rows, and the window is per file. Without the fix the first goes red. |
||
|
denkfabrik-li
|
db1dd71f3c |
Stop an expired file locking a group shut for a scoped staff member
groupReachesNoFurther() asks whether anything shared with a group sits
outside the viewer's library. `f1b35cc9` established the shape of the
answer for deleted files: start from the live row, because "a deleted file
is not reach, because nobody can reach it".
An expired file is the same case. Membership grants nobody access to it --
File::scopeVisibleToClient ends in notExpired(), so it has left every
member's /my-files and the download answers 403 -- but it is equally gone
from files(), where its absence reads as "outside my library". The group
then locks for a scoped staff member: they cannot add a member, cannot
rename it, and cannot remove their own client again.
So the reach query skips expired files as it already skips deleted ones.
Expiry is reversible where deletion is not, and that needs no special
handling: the guard asks what is reachable at the moment somebody is added
or removed, and the file counts again the moment it stops being expired.
Not changed: File::scopeVisibleToClient, whose treatment of expiry was
settled deliberately in
|
||
|
denkfabrik-li
|
eade690f73 |
Hold the group edit screen to the same library boundary as the rest
Every other group route asks StaffLibraryScope whether this viewer may
act on this group. GroupsController::update() and ::destroy() do, and so
do their API twins -- all four with abort_unless(allowsGroupChange, 404).
The two that read do not: edit() and Api\GroupsController::show() had no
boundary at all.
What they hand over is the membership, name and email per member, plus
the whole client roster of the installation as available_clients. So a
client-scoped staff member could open a group whose contents they cannot
see, read off every client on the installation, and only be refused when
they pressed save.
Two halves, because the leak has two shapes:
- The group itself. Reading it now asks the same reach question the write
half asks, one step earlier, with the same 404 -- a group that reaches
past the viewer's library is not theirs to open either.
- The lists inside it. Both narrow through StaffLibraryScope::clients(),
the listing half of the rule this screen's buttons are already guarded
with: allowsGroupMembership refuses removing a member outside the
roster, and refuses adding a client outside it. Naming them anyway,
with their address, is the mistake ClientsController made before
clients() existed -- that method's own docblock says so.
The reach guard alone would not have been enough. A group nobody has
shared anything with reaches nowhere, so it stays open to everybody --
and it can still hold a stranger's client. That case is why the lists
narrow separately, and there is a test for it.
members_count is left whole on purpose: a size is not an identity, and it
is the same number the group listing already reports.
GroupResource's docblock claimed members are safe to expose because "the
group edit screen already shows [them] to anyone holding edit_groups".
That was a claim about a screen, and it stopped being true the moment the
screen narrowed. Reworded to say what now holds it up, and where.
Not changed: the group listing. It reports names and member counts, not
identities, and every button on it is guarded. Nor Api\GroupsController::
index(), for the same reason. Nor the API document -- scramble:export is
byte-identical, because GET /groups/{group} already documented a 404.
Four tests. Three measured red against the unguarded controllers (3
failed / 21 passed): the group cannot be opened at all, the edit screen
stops naming strangers, and the API twin narrows what it hands back. The
fourth -- an unscoped viewer keeps the whole roster and every member -- is
green either way and guards against the fix over-refusing.
Full suite passes (2052 passed / 2 skipped), PHPStan level 8 clean.
|
||
|
Ignacio Nelson
|
dacf2b3eda |
Merge pull request #1693 from denkfabrik-li/fix/public-download-external-disk
Hand over a public download from the disk the file is on |
||
|
ignacionelson
|
eb2917f5ff |
Hold a group object to the same boundary its membership already has
This overturns something #1701 decided, so it should say so. That PR closed the membership hole and left GroupsController::update and destroy installation-wide on purpose, on the grounds that managing the group object is a different question from managing who is in it. What decides it is a measurement that was not in front of that decision. An assignment to a group is how its members reach a file, so deleting a group revokes that access for every member. Measured before this guard, with a client-scoped role holding the group permissions: stranger client can read the shared file true PATCH /groups/{stranger group} 302, renamed DELETE /groups/{stranger group} 302, group gone stranger client can read the shared file false So a staff member who may not add somebody to a group out of their reach could delete it out from under the people already in it. That is not a gentler version of the membership rule, it is a harder one, and the two sitting on opposite sides of the same boundary was the odd part. StaffLibraryScope::allowsGroupChange is the reach half of allowsGroupMembership on its own, since no client appears in this question -- one predicate, two callers, rather than a second statement of it. Both surfaces take it, at 404, matching the membership guards. A group that shares nothing beyond the actor's library still passes, so a group they created or one holding their own clients stays theirs, and unscoped staff are unaffected by construction. The API document moves a 404 above a 422 on two paths. Both already documented the 404 -- route model binding produced one -- and Scramble orders responses by where they appear in the method, so the guard landing before the validate() call is the whole of the change. |
||
|
Ignacio Nelson
|
350a7b3073 |
Merge pull request #1705 from denkfabrik-li/fix/deny-membership-request-once
Deny a membership request once, as approve() already does |
||
|
ignacionelson
|
f1b35cc9f6 |
Stop a deleted file locking a scoped staff member out of a group for good
#1701 closed a real hole: group membership decides what a client reaches, and through File::scopeVisibleToClient it decides what the staff member holding that client reaches, so `edit_groups` alone was never a boundary. The predicate it added asks whether everything shared with a group is already inside the actor's library. It asked by counting: pluck the group's assignment rows, count how many of those ids the library query returns, and require the two to match. An assignment row outlives the thing it points at — nothing clears them when a file or folder is deleted — while files() and folders() exclude trashed rows by construction. So one deleted file left a count that could never balance again, and the group closed permanently: the scoped staff member could no longer add their own client to it, or remove anybody from it, with a 403 and nothing to explain it. Every group accumulates dead assignments over time, so groups would have gone quiet one at a time. Asked the other way round — is there anything live, shared with this group, that is outside my library — the dead rows drop out by construction, because the query starts from File/Folder rather than from the assignment. That is also the truer question: a deleted file is not reach, since nobody can reach it. Three tests. A group stays usable after a file shared with it is deleted, including removing a member; the same for a deleted folder assignment; and the half that must not soften — a live file still out of reach is still refused, deleted siblings or not. |
||
|
denkfabrik-li
|
8a6543073b |
Group membership is a library boundary, not just a list
The four routes that edit a group's membership -- add and remove, web
and API -- contain no authorization call of any kind. `can:edit_groups`
in front of them is the whole of it, and a permission is not a boundary.
The authorization sweep looked at these and let them stand, on the
grounds that groups are installation-wide by design: GroupsController
::index lists every group unfiltered, so list and single-object access
agree, and there is no listing/direct-access mismatch to fix. That is
true, and it is the answer to the question of who may *see* a group.
This is a different question: what a write to one *does*.
Joining a group hands the new member everything shared with it. When
that member is one of a client-scoped staff member's own clients,
File::scopeVisibleToClient hands the same content straight back to them
-- that scope is what StaffLibraryScope::files() is built out of. So the
one write turns a file they get a 403 on into a file in their library,
and the download that follows is a 200. ResolvesShareTargets draws that
line on the sharing path through canAssignGroup(); nobody drew it on the
membership path, and canAssignGroup() is *derived from membership*, so
whoever may edit the list also decides what the list entitles them to.
StaffLibraryScope::allowsGroupMembership answers it directly instead of
through the derived predicate, which is the wrong tool here twice over.
Membership asks about reach, so it checks reach: the client must be one
this staff member holds, and the group must not already reach past their
library -- no file assigned to it, and no folder shared with it, outside
StaffLibraryScope. A group nothing has been shared with passes trivially,
which matters, because canAssignGroup() would have said no to a group
that has no members yet and left a scoped staff member unable to put the
first client into one they had just created.
The same write has a second door. MembershipRequestsController::approve
joins a client to a group with identical consequences, under
`approve_groups_memberships_requests`, and deny() decides about somebody
else's client and emails them about it. Both go through the same
boundary, answering 404 to match the guard already above approve().
The queue and its sidebar badge are narrowed to the clients the viewer
holds, through one scope on the model that both read -- the rule the
comment badge in HandleInertiaRequests already states two branches down
("a client-scoped staff member is not shown a number they cannot act
on"), and the reason VisibleCommentScope owns its own pendingTotal()
rather than leaving the middleware to count for itself. Each row carries
the client's name and email, so an unnarrowed queue was also handing
those over for clients outside the roster. Unscoped staff still see every
pending request.
That narrowing is on the client, not on the group: whether a group is
reachable depends on what is shared with it, which is not a question to
ask row by row in a listing. A scoped viewer may therefore still be
shown a request they would be refused on -- one of their own clients
asking to join a group out of their reach. The names were the part that
leaked.
Unscoped staff are unaffected throughout -- both halves of the predicate
are true for them by construction. No seeded role reaches this: Client
Manager is the only client-scoped role that ships, and it holds no group
permissions, so a custom role is needed to get here at all.
The published API document gains a 403 on both member routes.
Regenerated with php artisan scramble:export; Scramble reads abort_unless
out of the method body but not out of a private helper, which is why the
guard is written out at each of the four call sites rather than shared.
|
||
|
denkfabrik-li
|
5242169bb0 |
Deny a membership request once, as approve() already does
approve() refuses a request that is not pending:
abort_unless($group !== null && $client !== null
&& $membershipRequest->status === MembershipRequest::STATUS_PENDING, 404);
deny(), one method below, checks nothing. Denying is not idempotent, so
repeating it is not a no-op:
- denied_at is stamped again, and that is what the client's re-request
cooldown counts from (MyGroupsController::inDenyCooldown). Repeating
the request keeps one client out of one group for as long as somebody
cares to keep asking, without a single new decision being made.
- a second GroupMembershipDenied entry goes into the activity log, for
a denial that did not happen.
- a second "your request was declined" mail goes to the client.
The queue lists only pending requests, so nothing on the screen offers
this; it takes asking for the route directly. It needs
approve_groups_memberships_requests, so it is not a stranger's move.
The guard is the same one, answering the same 404, placed where deny()
can reach it. deny() keeps tolerating a vanished group or client -- that
tolerance is deliberate and separate: the denied row persists for the
cooldown even when the group it named is gone, and index() already
filters those rows out with whereHas.
Not in this change: deny() writes the status, the log entry and the
notification without a shared transaction. approve() has exactly the same
shape, so fixing one alone would replace a symmetry with a difference,
and doing both means also deciding where the mail sits relative to the
commit -- which is the question #1691 answers for file bytes, and worth
answering on its own rather than inside a state-machine fix.
|
||
|
denkfabrik-li
|
a8b1987e2c |
Hand over a public download from the disk the file is on
|
||
|
ignacionelson
|
57540164fa |
Read a file from the disk it is actually on, everywhere
Two routes still assumed every file sits on local disk, which stopped
being true the moment external storage was switched on. A share link
answered with X-Accel-Redirect whatever the file's disk said, pointing
nginx at a path it has nothing behind; a public listing built a
thumbnail from Storage::disk('files')->path(), which for an externally
stored file is a path nobody ever wrote. Both fail only for installs
using S3, and only on those two routes, so the same file downloading
correctly from the file manager made the share link look like the
broken thing rather than where the file lives.
Neither is a new rule. FileDownloadController and
FileThumbnailController already did it right, which is the actual
finding: the knowledge was sitting in a private method on one class and
inline in another, so the next caller could not inherit it and did not.
Both are now objects with one job.
StoredFileResponse replaces InlineFileResponse and grows an
attachment() alongside inline(), since the two differ only by
disposition. LocalSourceFile takes a closure rather than returning a
path: the version that returned one also left the caller to unlink it,
and both of those are exactly the mistakes made here.
The regression tests fail against the previous controllers — checked in
both directions rather than assumed.
|
||
|
elibrachas
|
2a82335e07 |
Move the shared public-listing helpers to tests/Helpers.php
publicListingFile() and publicListingImageFile() were defined in PublicGroupsTest.php and used from PublicFilePreviewTest.php too. Pest declares a test file's functions as ordinary globals, so that works only once the defining file has been loaded — which under --parallel depends on how the runner happens to distribute files across processes. Adding any unrelated test file anywhere in the suite reshuffles that and takes PublicFilePreviewTest.php down with "Call to undefined function", and running it on its own with --filter never worked at all. tests/Helpers.php exists for exactly this and its docblock describes this failure; these two had just been missed. publicPageProps() stays where it is, since only one file uses it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
ignacionelson
|
88c182cf3b |
Preview video, audio and PDF, not only images
v1 could preview four kinds of file in a modal — images, video, audio and PDF. v2 previewed only images, and not by decision: preview shipped as part of the image *thumbnail* work (1c68aa1), so "previewable" quietly became a synonym for "GD can decode it". FileThumbnailController::preview() gated on ThumbnailGenerator::SUPPORTED_MIME_TYPES, the frontend mirrored the same four types, and the dialog was a hardcoded <img>. Rather than widen that list — it drives pathFor(), extensionFor(), generate() and FileDiskCleanup, and a video reaching getimagesize() is a 500 — this separates the two questions. PreviewKind now answers "may these bytes be served inline, and what element renders them?", while ThumbnailGenerator keeps answering the narrower "can this app decode it itself?", which is what renditions, the cache and the watermark hook actually depend on. Image delegates to it so the two cannot drift. The allowlist stays a security boundary: mime_type is sniffed from the bytes, so text/html and image/svg+xml remain excluded, and PreviewKind is deliberately narrower than "formats a browser might cope with" — no quicktime, avi or matroska, because an embedded player for those shows a black rectangle. Those still download exactly as before. docs/security-audit-2026-08-05.md finding 1 recorded that adding application/pdf "should be a conscious decision". This is that decision, and three things were measured rather than assumed: - An <iframe sandbox> cannot be used. Chrome refuses to run its PDF viewer in a sandboxed frame at all (ERR_BLOCKED_BY_CLIENT, with or without allow-same-origin) — the attribute removes the feature, it does not harden it. - nginx's `Content-Security-Policy: sandbox; default-src 'none'` on /protected-files/ does work (a <video> frame lands in an opaque origin), but Chrome exempts its PDF viewer from it, so it is not what protects the PDF case. - What does is the allowlist plus the browser's own PDF sandbox, where PDF JavaScript has no DOM and no cookies. Range requests were verified end to end: 206 with a correct Content-Range, a byte-perfect file reassembled from three ranges, and a real browser seeking to 10s of a 20s clip. nginx drops the upstream Content-Length on the X-Accel path, so there is no collision. Two settings, both defaulting on so no installation loses what it has: clients_can_preview_files and public_listing_preview_enabled. Staff are never gated. The anonymous side needed a route of its own — there was no public preview endpoint — with its own throttle bucket, since a bare throttle: shares one counter across that whole block. A preview now logs at most one FilePreviewed per viewer per file per five minutes: a <video> turns one deliberate act into a long tail of Range requests, and a row each would bury the log. Also fixes a layout bug the tests could never catch. A portal file row was flex justify-between with three children — name, comment trigger, download — so the middle one settled wherever the name happened to end and the comment icon sat at a different place on every row. The name block now takes the slack and every action lives in one trailing group, with the comment trigger in a fixed-width slot so the icons form a column. And because half the previewable files have no thumbnail to click — a PDF, an mp3 and an mp4 all render as a generic icon — every row gains an explicit PreviewAction beside DownloadAction, matching whatever style that theme gives its download control. |
||
|
ignacionelson
|
6e47d76ba6 |
ProjectSend 2.0.0
Client file sharing, rebuilt from the ground up: a private area per client, resumable uploads, folders, groups and categories, sharing with expiry dates and download limits, comments, file versions, an activity log, a REST API, and sixteen languages. This repository begins here. ProjectSend 2 was developed privately, and that development history is not published — the previous generation remains available, with its own history, at projectsend/legacy. Free software under the GNU General Public License v2, or (at your option) any later version. |