file_comments.client_context_id is cascadeOnDelete, but users are
soft-deleted, so the cascade never fires: the column keeps pointing at a
row that is still there while the Eloquent relation resolves to null.
resolveClientContext branched on the relation, and a null context on a
Clients comment is the branch every client on the file reads -- so a
staff reply into one client's private thread became a circular to all of
them, with the canAssignClient check skipped on the way.
VisibleCommentScope says so in its own docblock: "A Clients comment
carrying client_context_id = C is never returned to any non-staff viewer
other than C ... A Clients comment with a null context is a staff message
to everyone on the file, and every client with access reads it."
Measured on main, with one file shared with two clients and the first of
them deleted after commenting:
column client_context_id 3
relation clientContext null
POST reply into her thread 201, stored with client_context_id null
read by the other client yes
Ask the column, and refuse when the account behind it is gone. There is
nobody left to answer, and the one outcome that must not follow from a
filled column is the broadcast, so this throws rather than falling
through to it.
authorName() had the same root cause from the other column: its docblock
claimed author_id cascades so there is no deleted author, and a deleted
client's comment was going out as "Anonymous" -- which is what a guest
comment looks like, and a guest comment is read by different rules. Guest
is now decided by author_id alone, the same question isFromGuest() asks,
and a trashed author is read with withTrashed(). Nothing comes back only
once the grace-period erasure has removed the row for real.
That read costs one query per comment whose author is trashed. Measured
on a ten-comment thread: 11 queries before, 21 after, against 20 for the
same thread with every author alive. Left as a lazy read rather than
eager-loading with withTrashed() at every call site, because the callers
would each have to remember it and the cost only applies to comments
whose author is gone.
Five tests, two measured red against the unfixed code (2 failed / 3
passed) -- one per column. The three that stay green either way are the
branches that must not move: a staff message with no context still
reaches everybody, a reply into a live client's thread still lands in
that thread alone, and a genuine guest comment is still anonymous.
Full suite passes (2053 passed / 2 skipped), PHPStan level 8 clean.
A managed installation's staff accounts were expected to arrive from
outside it, so users.manage was Community-only and /users, /roles and
their API twins answered 404 there. The platform side spent a long
document designing its way around that gate; opening it is cheaper than
routing around it, and more honest about where the knowledge sits.
The division that settles it is the one managed storage already uses. We
do not manage a tenant's files from outside — a bucket is provisioned, a
scoped credential handed over, and what goes in it is the tenant's
business. Seats are the same kind of thing. A platform knows how many
staff accounts it sold; it does not know whether Alice should be an
Account Manager, and it certainly does not know where her files go when
she leaves. Capacity is the platform's, occupancy is the tenant's, and
the cap belongs in an environment variable rather than in a closed
screen.
The capability stays in front of the routes rather than being deleted.
It is currently true in both editions, but it is the seam an edition
difference has to travel through, and removing it would mean inventing
one again later.
Seven test files asserted the old rule, which is the tests doing their
job. Most flip. Two needed a different example instead: EnsureCapability
and AbilityCapability were both using users.manage to stand for
"Community-only", so they now use storage.configure and manage_updates —
keys that still are.
Two rationales half-expired and say so rather than being quietly
rewritten. CommentAuthors gave two reasons for being a setting rather
than a permission; the first was that roles are uneditable on cloud,
which stopped being true here, and the second — that `Everyone` includes
anonymous visitors, who have no role to hold a key — was always the
stronger and is now the whole of it.
The seat cap this makes necessary is the next commit, not this one. On
its own this change lets a managed tenant create staff accounts without
limit, which is why the two belong in the same release.
VisibleCommentScope says at the top of the class that its callers must
already have established that the viewer may see the file. The public
listing's comment endpoint establishes only the guest half of that — the
file is reachable without an account — and then hands $request->user()
straight to the authenticated reading.
For anyone the file's own gate would refuse, that reading is far too
wide. A staff account outside its library, or one holding no file
permission at all, read the file's staff-only notes; a client the file
was never shared with read the messages staff addressed to that file's
clients. Both get a 403 from GET /files/{file}/comments and needed only
to ask the public URL instead.
The endpoint now asks the file's own gate which reading applies. A reader
it admits sees no less than before. A reader it refuses gets what a
visitor gets, widened by their own comments — which is what this
controller has always promised them, and all it promised. The
held-comment rule moves into a method both readings share rather than
being restated.
The same root reaches PATCH and DELETE /comments/{comment}, which bind a
comment rather than a file and so never authorized `view` on it either.
They answer with the thread, and now with the one the file's gate allows.
Refusing them outright would be wrong: somebody who commented through
the public page is exactly the person entitled to edit their own words.
FileCommentPolicy::moderate() asked only whether somebody is staff and
holds moderate_comments. It never weighed the file the comment sits on,
and delete() returns true the moment moderate() does — so a client-scoped
moderator could delete any comment on the installation by naming its id.
Three call sites already knew this and wrote the boundary out by hand,
each with its own abort_unless($library->allowsFile(...), 403) after the
gate. The two that did not are FileCommentsController::destroy(), web and
API: both bind a comment directly, so nothing earlier in the request
establishes that the viewer may see its file.
The intent was documented in three places and enforced in none of them by
the policy — StaffLibraryScope says "the policies consult allowsFile() so
direct access respects the same boundary", VisibleCommentScope says "a
moderation screen is not a way around the visibility model". Put the rule
where those docblocks already say it lives.
moderate() now takes the comment when there is one. Named against the
class it still answers the coarser "does this user moderate at all",
which is what the queue's gate and the affordances ask. Membership is
tested by file id, so a file soft-deleted out from under its comments is
not in a scoped moderator's library either.
The author branch of delete() is deliberately untouched: deleting your
own words inside the edit window is not moderation, and a client is not
client-scoped in StaffLibraryScope's sense.
Approving through the API now derives its 403 from Gate::authorize rather
than the removed abort_unless, so the committed OpenAPI document gains
the shared AuthorizationException ref in place of an inline "An error"
schema — the shape nine of the other twelve documented 403s already use.
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.