mirror of
https://github.com/projectsend/projectsend.git
synced 2026-10-04 05:25:51 +00:00
Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 51477cbd02 | |||
| 7c9847981a | |||
| 7da4635f13 | |||
| ddf09677f0 | |||
| 9b2aea4812 |
@@ -39,6 +39,7 @@ yarn-error.log
|
||||
/database/seeders/DevDataSeeder.php
|
||||
/docs/*.md
|
||||
!/docs/api-guide.md
|
||||
!/docs/api-modules.md
|
||||
!/docs/email-oauth.md
|
||||
!/docs/api-zapier.md
|
||||
|
||||
|
||||
@@ -13,6 +13,28 @@ Anything under **Upgrade notes** is something you have to do, not something we d
|
||||
This section collects changes as they land; the release process turns it into a numbered entry
|
||||
when a version is cut.
|
||||
|
||||
**Closed holes in who can see what**
|
||||
|
||||
- A staff member limited to their own assigned clients could read the names of other clients out of
|
||||
file details. Sharing means a file can reach somebody through one client while it was uploaded by
|
||||
another, or while it is also shared with another. That is normal and the file is theirs to open —
|
||||
but the uploader's name, the other recipient's name, and both their ID numbers were being sent
|
||||
along with it, on the library list, the file's edit page, the details panel, the per-client file
|
||||
list, and the matching API responses. A group holding none of their clients was named the same
|
||||
way. The file list could also be filtered by uploader, which answered "does this client of yours
|
||||
share files with that client of mine" without naming anybody.
|
||||
|
||||
Those names are now left out for a limited staff member, and the uploader filter no longer answers
|
||||
for a client they are not assigned to. Administrators and any unrestricted role see exactly what
|
||||
they saw before.
|
||||
|
||||
**Who this affected.** Only installations using the Client Manager role, or a custom role with
|
||||
"Limit to assigned clients" switched on, and only where files are shared with more than one client
|
||||
or through groups. No files, downloads or credentials were reachable this way — a file belonging
|
||||
to a client outside the roster was refused before, and still is.
|
||||
|
||||
Reported by [@Noorkhalel](https://github.com/Noorkhalel) (GHSA-whmp-p9hv-r7j7).
|
||||
|
||||
## 2.3.0 — 1 September 2026
|
||||
|
||||
If you run ProjectSend on Apache or LiteSpeed, this is the release to take. It installed fine on
|
||||
|
||||
@@ -23,6 +23,11 @@ page to download it.
|
||||
No public link passed around by email, no third-party service holding your clients' documents, no
|
||||
per-seat pricing. It runs on your server, and the files stay there.
|
||||
|
||||
Prefer not to run the server yourself? [ProjectSend Cloud](https://projectsend.cloud) is the
|
||||
official hosted version of ProjectSend, run by the same team — every subscription funds this free
|
||||
software. The line between the free core and Cloud, and the commitments that go with it, are set
|
||||
out in [LICENSING.md](LICENSING.md).
|
||||
|
||||
## What it does
|
||||
|
||||
**For the people you send to**
|
||||
|
||||
@@ -0,0 +1,227 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Modules\Files\Access;
|
||||
|
||||
use App\Models\User;
|
||||
use App\Modules\Groups\Models\Group;
|
||||
|
||||
/**
|
||||
* Whether a viewer may be told who a client is.
|
||||
*
|
||||
* A different question from whether they may read a file, and the gap
|
||||
* between the two is the whole reason this exists. A stranger client's
|
||||
* upload can sit legitimately inside a client-scoped staff member's
|
||||
* library — shared with a group one of their own clients belongs to, or
|
||||
* assigned to one of their clients alongside somebody else's. The file is
|
||||
* theirs to read. The other client's name is not theirs to see.
|
||||
*
|
||||
* Commit 12a8ebe3 said exactly that while fixing one dashboard widget, and
|
||||
* then the rule stayed in that widget. Every other place that serialises a
|
||||
* file went on publishing the uploader and each recipient by name, so a
|
||||
* manager assigned to one client could read the names and ids of clients
|
||||
* on nobody's roster but their own out of ordinary file metadata. That is
|
||||
* what this class ends: one statement of the rule, asked by every surface
|
||||
* that names a client.
|
||||
*
|
||||
* Two things it deliberately is not:
|
||||
*
|
||||
* - It is not a download check. The file boundary is StaffLibraryScope's
|
||||
* and FilePolicy's, and it is already correct — a file belonging only
|
||||
* to a client off the roster is a 403 today. This narrows what a
|
||||
* permitted response is allowed to say, nothing more.
|
||||
* - It is not applied to staff. A colleague's name is not a client
|
||||
* identity, and hiding it would hide who uploaded most of the library
|
||||
* from the people who work in it.
|
||||
*
|
||||
* Unscoped staff are unaffected: they may identify everyone, which is what
|
||||
* `null` means everywhere StaffLibraryScope answers this shape of question.
|
||||
*/
|
||||
class ClientIdentityScope
|
||||
{
|
||||
/**
|
||||
* Memoised per viewer, since the listings ask once per row and each
|
||||
* miss is a roster query. Registered as `scoped`, so this lasts a
|
||||
* request and is dropped between queue jobs — the same lifetime, and
|
||||
* for the same reason, as StaffLibraryScope's own memo.
|
||||
*
|
||||
* @var array<int, list<int>|null>
|
||||
*/
|
||||
private array $clientIds = [];
|
||||
|
||||
/** @var array<int, list<int>|null> */
|
||||
private array $groupIds = [];
|
||||
|
||||
public function __construct(private readonly StaffLibraryScope $scope) {}
|
||||
|
||||
/**
|
||||
* Whether $viewer may be told that $subject exists, and what they are
|
||||
* called.
|
||||
*
|
||||
* A null subject is permitted: there is no identity to leak, and every
|
||||
* caller here is reading an optional relation.
|
||||
*/
|
||||
public function permits(?User $viewer, ?User $subject): bool
|
||||
{
|
||||
if ($subject === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (! $subject->isClient()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if ($viewer === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($viewer->is($subject)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
$ids = $this->identifiableClientIds($viewer);
|
||||
|
||||
return $ids === null || in_array($subject->id, $ids, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* The same question about a client known only by id — used where a
|
||||
* caller has a foreign key rather than a loaded model.
|
||||
*
|
||||
* An id that belongs to nobody, or to a staff member, is permitted:
|
||||
* there is no client identity behind it to protect.
|
||||
*/
|
||||
public function permitsClientId(?User $viewer, ?int $id): bool
|
||||
{
|
||||
if ($id === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return $this->permits($viewer, User::query()->find($id));
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether $viewer may be told a group exists.
|
||||
*
|
||||
* A group is a list of clients wearing one name, so naming one to
|
||||
* somebody who may reach none of its members says the same thing
|
||||
* naming a client would. The set is StaffLibraryScope's
|
||||
* assignableGroupIds — every group holding at least one of the
|
||||
* viewer's own clients.
|
||||
*/
|
||||
public function permitsGroupId(?User $viewer, ?int $id): bool
|
||||
{
|
||||
if ($id === null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if ($viewer === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$ids = $this->identifiableGroupIds($viewer);
|
||||
|
||||
return $ids === null || in_array($id, $ids, true);
|
||||
}
|
||||
|
||||
/**
|
||||
* A client's name, or null when this viewer may not be told it.
|
||||
*
|
||||
* Null rather than a placeholder on purpose: every consumer of these
|
||||
* fields already renders "no uploader recorded" for a null, because a
|
||||
* deleted account leaves one behind. Inventing a "Hidden" string would
|
||||
* be a new thing for sixteen locales to translate and would itself
|
||||
* announce that there is somebody there to hide.
|
||||
*/
|
||||
public function nameOf(?User $viewer, ?User $subject): ?string
|
||||
{
|
||||
return $this->permits($viewer, $subject) ? $subject?->name : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop the entries this viewer may not be told about from a list of
|
||||
* id/name pairs describing clients.
|
||||
*
|
||||
* @param list<array{id: int, name: string}> $pairs
|
||||
* @return list<array{id: int, name: string}>
|
||||
*/
|
||||
public function filterClientPairs(?User $viewer, array $pairs): array
|
||||
{
|
||||
if ($this->identifiableClientIds($viewer) === null) {
|
||||
return $pairs;
|
||||
}
|
||||
|
||||
return array_values(array_filter(
|
||||
$pairs,
|
||||
fn (array $pair): bool => $this->permitsClientId($viewer, $pair['id']),
|
||||
));
|
||||
}
|
||||
|
||||
/**
|
||||
* @param list<array{id: int, name: string}> $pairs
|
||||
* @return list<array{id: int, name: string}>
|
||||
*/
|
||||
public function filterGroupPairs(?User $viewer, array $pairs): array
|
||||
{
|
||||
if ($this->identifiableGroupIds($viewer) === null) {
|
||||
return $pairs;
|
||||
}
|
||||
|
||||
return array_values(array_filter(
|
||||
$pairs,
|
||||
fn (array $pair): bool => $this->permitsGroupId($viewer, $pair['id']),
|
||||
));
|
||||
}
|
||||
|
||||
/**
|
||||
* Both halves of a `shares` payload at once, since the two lists are
|
||||
* always filtered together.
|
||||
*
|
||||
* @param array{clients: list<array{id: int, name: string}>, groups: list<array{id: int, name: string}>} $shares
|
||||
* @return array{clients: list<array{id: int, name: string}>, groups: list<array{id: int, name: string}>}
|
||||
*/
|
||||
public function filterShares(?User $viewer, array $shares): array
|
||||
{
|
||||
return [
|
||||
'clients' => $this->filterClientPairs($viewer, $shares['clients']),
|
||||
'groups' => $this->filterGroupPairs($viewer, $shares['groups']),
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this viewer is narrowed at all. Callers use it to skip
|
||||
* per-row work for the common unscoped case.
|
||||
*/
|
||||
public function isNarrowed(?User $viewer): bool
|
||||
{
|
||||
return $viewer === null || $this->identifiableClientIds($viewer) !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<int>|null
|
||||
*/
|
||||
private function identifiableClientIds(?User $viewer): ?array
|
||||
{
|
||||
if ($viewer === null) {
|
||||
return [];
|
||||
}
|
||||
|
||||
// Deliberately the same set as "who may I share with". A client on
|
||||
// the roster is one this viewer already works with by name; a
|
||||
// client off it is one they have no business knowing exists.
|
||||
return $this->clientIds[$viewer->id] ??= $this->scope->assignableClientIds($viewer);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return list<int>|null
|
||||
*/
|
||||
private function identifiableGroupIds(?User $viewer): ?array
|
||||
{
|
||||
if ($viewer === null) {
|
||||
return [];
|
||||
}
|
||||
|
||||
return $this->groupIds[$viewer->id] ??= $this->scope->assignableGroupIds($viewer);
|
||||
}
|
||||
}
|
||||
@@ -28,13 +28,25 @@ use Illuminate\Support\Collection;
|
||||
*/
|
||||
class ShareTargets
|
||||
{
|
||||
public function __construct(private readonly StaffLibraryScope $scope) {}
|
||||
public function __construct(
|
||||
private readonly StaffLibraryScope $scope,
|
||||
private readonly ClientIdentityScope $identity,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* The clients and groups a subject is already shared with, as id/name
|
||||
* pairs. Neutral keys, so callers can nest it ('shares' on the details
|
||||
* panel) or flatten it (the edit pages' assigned_* props).
|
||||
*
|
||||
* **This is the unfiltered truth, and it is not what a screen should
|
||||
* show.** Everyone a file is really in front of is the right answer for
|
||||
* deciding something — VisibleCommentScope resolves notification
|
||||
* recipients from it, and a recipient left out of that list is one who
|
||||
* never hears about a message addressed to them. It is the wrong answer
|
||||
* for telling somebody, because a client-scoped viewer may hold a file
|
||||
* that is also shared with a client they have no business knowing
|
||||
* exists. Anything rendering these names wants assignedFor() below.
|
||||
*
|
||||
* @return array{clients: list<array{id: int, name: string}>, groups: list<array{id: int, name: string}>}
|
||||
*/
|
||||
public function assigned(File|Folder $subject): array
|
||||
@@ -47,6 +59,17 @@ class ShareTargets
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* assigned(), narrowed to the recipients this viewer may be told
|
||||
* about. The display half of the pair — see the warning above.
|
||||
*
|
||||
* @return array{clients: list<array{id: int, name: string}>, groups: list<array{id: int, name: string}>}
|
||||
*/
|
||||
public function assignedFor(File|Folder $subject, ?User $viewer): array
|
||||
{
|
||||
return $this->identity->filterShares($viewer, $this->assigned($subject));
|
||||
}
|
||||
|
||||
/**
|
||||
* The assigned lists plus everything still available to share with,
|
||||
* narrowed to what this viewer is allowed to reach.
|
||||
@@ -76,7 +99,12 @@ class ShareTargets
|
||||
->orderBy('name')
|
||||
->get();
|
||||
|
||||
$assigned = $this->assigned($subject);
|
||||
// assignedFor, not assigned: an edit page listing a recipient this
|
||||
// viewer may not identify would both name them and offer a control
|
||||
// for a share the viewer cannot otherwise reach. available_* below
|
||||
// was already narrowed this way; assigned_* was not, which is the
|
||||
// asymmetry that made the whole panel a roster listing.
|
||||
$assigned = $this->assignedFor($subject, $viewer);
|
||||
|
||||
return [
|
||||
'assigned_clients' => $assigned['clients'],
|
||||
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Modules\Files;
|
||||
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\Access\StaffLibraryScope;
|
||||
use App\Modules\Files\Models\File;
|
||||
use App\Modules\Files\Models\Folder;
|
||||
@@ -30,6 +31,10 @@ class FilesServiceProvider extends ServiceProvider
|
||||
// reached twice. Scoped rather than a singleton so a long-lived
|
||||
// queue worker starts each job with an empty memo.
|
||||
$this->app->scoped(StaffLibraryScope::class);
|
||||
|
||||
// Same lifetime, same reason: the identity rule memoises a roster
|
||||
// per viewer and the file listings ask it once per row.
|
||||
$this->app->scoped(ClientIdentityScope::class);
|
||||
}
|
||||
|
||||
public function boot(): void
|
||||
|
||||
@@ -12,6 +12,7 @@ use App\Modules\Audit\ActivityLogger;
|
||||
use App\Modules\Clients\ClientStorageUsage;
|
||||
use App\Modules\Comments\CommentingRules;
|
||||
use App\Modules\Comments\CommentScope;
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\Access\StaffLibraryScope;
|
||||
use App\Modules\Files\Access\ViewableFileScope;
|
||||
use App\Modules\Files\DownloadLimitScope;
|
||||
@@ -62,6 +63,7 @@ class FilesController extends Controller
|
||||
private readonly ActivityLogger $activity,
|
||||
private readonly CommentingRules $commenting,
|
||||
private readonly StaffLibraryScope $scope,
|
||||
private readonly ClientIdentityScope $identity,
|
||||
private readonly TimezoneRegistry $timezones,
|
||||
) {}
|
||||
|
||||
@@ -114,6 +116,17 @@ class FilesController extends Controller
|
||||
}
|
||||
|
||||
if (array_key_exists('uploaded_by', $filters) && $filters['uploaded_by'] !== null) {
|
||||
// A filter is a question, and this one asks "did client N put
|
||||
// anything into my library". Answered plainly it is an oracle:
|
||||
// a client-scoped caller could walk the id space and learn
|
||||
// which clients off their roster share files with clients on
|
||||
// it, without ever reading a name. So an id this caller may
|
||||
// not identify matches nothing — indistinguishable from a
|
||||
// client who has uploaded nothing, which is the point.
|
||||
if (! $this->identity->permitsClientId($user, (int) $filters['uploaded_by'])) {
|
||||
$query->whereRaw('1 = 0');
|
||||
}
|
||||
|
||||
$query->where('files.uploaded_by', $filters['uploaded_by']);
|
||||
}
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ namespace App\Modules\Files\Http\Controllers;
|
||||
|
||||
use App\Http\Controllers\Controller;
|
||||
use App\Models\User;
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\Access\StaffLibraryScope;
|
||||
use App\Modules\Files\Models\Category;
|
||||
use App\Modules\Files\Models\File;
|
||||
@@ -28,6 +29,7 @@ class ClientFilesController extends Controller
|
||||
{
|
||||
public function __construct(
|
||||
private readonly StaffLibraryScope $scope,
|
||||
private readonly ClientIdentityScope $identity,
|
||||
) {}
|
||||
|
||||
public function index(Request $request, User $client): Response
|
||||
@@ -66,7 +68,11 @@ class ClientFilesController extends Controller
|
||||
'size' => $file->size,
|
||||
'created_at' => $file->created_at?->toIso8601String(),
|
||||
'uploaded_by_client' => $file->uploaded_by === $client->id,
|
||||
'uploader' => $file->uploader?->name,
|
||||
// Being allowed to browse this client's files does not
|
||||
// extend to the other clients who shared files with them:
|
||||
// a file reaches this listing through the client in the
|
||||
// URL, and its uploader can be somebody else entirely.
|
||||
'uploader' => $this->identity->nameOf($viewer, $file->uploader),
|
||||
'downloads_count' => $file->downloads_count,
|
||||
'can_download' => Gate::forUser($viewer)->allows('view', $file),
|
||||
'categories' => $file->categories->map(fn (Category $category): array => [
|
||||
|
||||
@@ -11,6 +11,7 @@ use App\Modules\Audit\ActivityLog;
|
||||
use App\Modules\Audit\ActivityPresenter;
|
||||
use App\Modules\Audit\DownloadPresenter;
|
||||
use App\Modules\Comments\CommentingRules;
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\Access\DownloadAllowance;
|
||||
use App\Modules\Files\Access\ShareTargets;
|
||||
use App\Modules\Files\DownloadLimitScope;
|
||||
@@ -79,6 +80,7 @@ class FileDetailsController extends Controller
|
||||
private readonly ActivityPresenter $presenter,
|
||||
private readonly DownloadPresenter $downloadPresenter,
|
||||
private readonly ShareTargets $shareTargets,
|
||||
private readonly ClientIdentityScope $identity,
|
||||
private readonly CommentingRules $commenting,
|
||||
private readonly FileVersionLinks $versionLinks,
|
||||
private readonly DownloadAllowance $allowance,
|
||||
@@ -100,7 +102,10 @@ class FileDetailsController extends Controller
|
||||
'size' => $file->size,
|
||||
'mime_type' => $file->mime_type,
|
||||
'checksum' => $file->checksum,
|
||||
'uploader' => $file->uploader?->name,
|
||||
// Null when the uploader is a client this viewer may not
|
||||
// be told about, which reads the same as an uploader whose
|
||||
// account has since been deleted.
|
||||
'uploader' => $this->identity->nameOf($viewer, $file->uploader),
|
||||
'folder' => $file->folder?->only('id', 'name'),
|
||||
'categories' => $file->categories()->orderBy('name')->get()
|
||||
->map(fn (Category $category): array => ['id' => $category->id, 'name' => $category->name, 'color' => $category->color])
|
||||
@@ -140,7 +145,7 @@ class FileDetailsController extends Controller
|
||||
// Resolved from the chain root for a revision (ShareTargets
|
||||
// does that), so this names who really has the file. The panel
|
||||
// says where those recipients are set.
|
||||
'shares' => $this->shareTargets->assigned($file),
|
||||
'shares' => $this->shareTargets->assignedFor($file, $viewer),
|
||||
'sharing_root' => $file->isRevision()
|
||||
? File::query()->find($file->sharingOwnerId())?->only('id', 'name')
|
||||
: null,
|
||||
@@ -368,7 +373,7 @@ class FileDetailsController extends Controller
|
||||
'name' => $folder->name,
|
||||
'files_count' => $folder->files()->count(),
|
||||
'children_count' => $folder->children()->count(),
|
||||
'creator' => $folder->creator?->name,
|
||||
'creator' => $this->identity->nameOf($viewer, $folder->creator),
|
||||
'created_at' => $folder->created_at?->toIso8601String(),
|
||||
'open_url' => route('files.index', ['folder' => $folder->id], false),
|
||||
// Read-only here, same as a file's shares — sharing (and every
|
||||
@@ -377,7 +382,7 @@ class FileDetailsController extends Controller
|
||||
'edit_url' => route('folders.share', $folder, false),
|
||||
'can_update' => Gate::forUser($viewer)->allows('update', $folder),
|
||||
'can_view_activity' => $viewer->can('view_actions_log'),
|
||||
'shares' => $this->shareTargets->assigned($folder),
|
||||
'shares' => $this->shareTargets->assignedFor($folder, $viewer),
|
||||
]);
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ use App\Modules\Audit\Action;
|
||||
use App\Modules\Audit\ActivityLogger;
|
||||
use App\Modules\Comments\CommentingRules;
|
||||
use App\Modules\Comments\CommentScope;
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\Access\ShareTargets;
|
||||
use App\Modules\Files\Access\StaffLibraryScope;
|
||||
use App\Modules\Files\DownloadLimitScope;
|
||||
@@ -49,6 +50,7 @@ class FilesController extends Controller
|
||||
private readonly StaffLibraryScope $scope,
|
||||
private readonly PublicUrl $publicUrl,
|
||||
private readonly ShareTargets $shareTargets,
|
||||
private readonly ClientIdentityScope $identity,
|
||||
private readonly CommentingRules $commenting,
|
||||
private readonly FileVersions $versions,
|
||||
private readonly FileVersionLinks $versionLinks,
|
||||
@@ -164,7 +166,7 @@ class FilesController extends Controller
|
||||
'original_name' => $file->original_name,
|
||||
'size' => $file->size,
|
||||
'mime_type' => $file->mime_type,
|
||||
'uploader' => $file->uploader?->name,
|
||||
'uploader' => $this->identity->nameOf($viewer, $file->uploader),
|
||||
'folder_id' => $file->folder_id,
|
||||
'public' => $file->public,
|
||||
'commentable' => $file->commentable,
|
||||
|
||||
@@ -10,6 +10,7 @@ use App\Modules\Audit\Action;
|
||||
use App\Modules\Audit\ActivityLogger;
|
||||
use App\Modules\Comments\Access\VisibleCommentScope;
|
||||
use App\Modules\Comments\CommentingRules;
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\Access\DownloadAllowance;
|
||||
use App\Modules\Files\Access\ShareTargets;
|
||||
use App\Modules\Files\Access\StaffLibraryScope;
|
||||
@@ -54,6 +55,7 @@ class FoldersController extends Controller
|
||||
private readonly ActivityLogger $activity,
|
||||
private readonly PublicUrl $publicUrl,
|
||||
private readonly ShareTargets $shareTargets,
|
||||
private readonly ClientIdentityScope $identity,
|
||||
private readonly BreadcrumbBuilder $breadcrumbs,
|
||||
private readonly CommentingRules $commenting,
|
||||
private readonly VisibleCommentScope $comments,
|
||||
@@ -240,7 +242,11 @@ class FoldersController extends Controller
|
||||
'original_name' => $file->original_name,
|
||||
'mime_type' => $file->mime_type,
|
||||
'size' => $file->size,
|
||||
'uploader' => $file->uploader ? [
|
||||
// The whole block goes, not just the name: type and role
|
||||
// describe the same person, and "a client uploaded this" on a
|
||||
// row whose uploader is off this viewer's roster narrows who
|
||||
// it could be just as effectively as naming them.
|
||||
'uploader' => ($file->uploader !== null && $this->identity->permits($user, $file->uploader)) ? [
|
||||
'name' => $file->uploader->name,
|
||||
'type' => $file->uploader->type->value,
|
||||
'role' => $file->uploader->role?->name,
|
||||
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Modules\Files\Http\Resources\Api;
|
||||
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\DownloadLimitScope;
|
||||
use App\Modules\Files\Models\File;
|
||||
use App\Modules\Files\Models\FileAssignment;
|
||||
@@ -26,6 +27,23 @@ use Illuminate\Http\Resources\Json\JsonResource;
|
||||
* - `checksum` is included deliberately, since verifying an integration's
|
||||
* own download is a real use case, and it reveals nothing about
|
||||
* location.
|
||||
*
|
||||
* Two fields are narrowed to the caller: the uploader and the assignment
|
||||
* list both name clients, and a client-scoped account may hold a file whose
|
||||
* uploader or co-recipients are clients off their own roster — the file is
|
||||
* theirs to read, those names are not theirs to see. ClientIdentityScope is
|
||||
* the rule; a name dropped here is dropped to null or out of the list, and
|
||||
* an unscoped account is unaffected.
|
||||
*
|
||||
* That narrowing happens here rather than in the controllers, which is the opposite of how the version counterparts are
|
||||
* handled a few files over — and deliberately so. Whether a counterpart may
|
||||
* be named is a set-shaped question with a query to express it, so it is
|
||||
* asked once in the caller's eager load. Whether a client may be named is a
|
||||
* per-row check against the viewer's roster with no query to fold it into,
|
||||
* and this resource is built at eight call sites across four controllers,
|
||||
* two of them re-loading `assignments.assignable` after a write. Asking at
|
||||
* the point of serialisation is the only version of this rule that cannot
|
||||
* be forgotten by the ninth caller.
|
||||
*/
|
||||
class FileResource extends JsonResource
|
||||
{
|
||||
@@ -34,6 +52,15 @@ class FileResource extends JsonResource
|
||||
*/
|
||||
public function toArray(Request $request): array
|
||||
{
|
||||
$viewer = $request->user();
|
||||
$identity = app(ClientIdentityScope::class);
|
||||
|
||||
// The morph class rather than ::class, matching ShareTargets: with
|
||||
// a morph map registered the two disagree, and this line now
|
||||
// decides which roster an entry is checked against, so getting it
|
||||
// wrong would mean checking a group id against the client list.
|
||||
$groupMorph = (new Group)->getMorphClass();
|
||||
|
||||
return [
|
||||
'id' => $this->id,
|
||||
'name' => $this->name,
|
||||
@@ -96,11 +123,16 @@ class FileResource extends JsonResource
|
||||
]),
|
||||
|
||||
// Name only. The uploader is a user record; their email address
|
||||
// is not part of what "this file exists" needs to say.
|
||||
'uploaded_by' => $this->whenLoaded('uploader', fn (): ?array => $this->uploader === null ? null : [
|
||||
'id' => $this->uploader->id,
|
||||
'name' => $this->uploader->name,
|
||||
]),
|
||||
// is not part of what "this file exists" needs to say. Null
|
||||
// when the uploader is a client the token's owner is not
|
||||
// scoped to; an unscoped account always gets the name.
|
||||
'uploaded_by' => $this->whenLoaded(
|
||||
'uploader',
|
||||
fn (): ?array => $identity->permits($viewer, $this->uploader) && $this->uploader !== null ? [
|
||||
'id' => $this->uploader->id,
|
||||
'name' => $this->uploader->name,
|
||||
] : null,
|
||||
),
|
||||
|
||||
'categories' => $this->whenLoaded('categories', fn (): array => $this->categories
|
||||
->map(fn ($category): array => [
|
||||
@@ -109,15 +141,22 @@ class FileResource extends JsonResource
|
||||
])
|
||||
->all()),
|
||||
|
||||
// Who the file is shared with, as far as this caller is
|
||||
// concerned: a recipient the token's owner is not scoped to is
|
||||
// left out rather than returned without a name.
|
||||
'assignments' => $this->whenLoaded('assignments', fn (): array => $this->assignments
|
||||
->filter(fn (FileAssignment $assignment): bool => $assignment->assignable_type === $groupMorph
|
||||
? $identity->permitsGroupId($viewer, (int) $assignment->assignable_id)
|
||||
: $identity->permitsClientId($viewer, (int) $assignment->assignable_id))
|
||||
->map(fn (FileAssignment $assignment): array => [
|
||||
'type' => $assignment->assignable_type === Group::class ? 'group' : 'client',
|
||||
'type' => $assignment->assignable_type === $groupMorph ? 'group' : 'client',
|
||||
'id' => $assignment->assignable_id,
|
||||
// getAttribute() rather than ->name: the relation is a
|
||||
// MorphTo over User|Group, so the property is only
|
||||
// knowable at runtime. Both targets carry a name.
|
||||
'name' => $assignment->assignable?->getAttribute('name'),
|
||||
])
|
||||
->values()
|
||||
->all()),
|
||||
|
||||
'links' => [
|
||||
|
||||
@@ -450,6 +450,16 @@ class StatusCommand extends Command
|
||||
// on the day it happens to be empty rather than on the day it
|
||||
// is written. It cannot be empty today, but the allowlist is
|
||||
// meant to be edited.
|
||||
//
|
||||
// What that costs, confirmed against the reader rather than
|
||||
// guessed at: the hosted platform's probe decodes this block
|
||||
// into a typed struct and discards a block it cannot read, and
|
||||
// Go refuses a JSON list into a map outright. So a `[]` here
|
||||
// would not lose `actions` — it would lose the whole `usage`
|
||||
// block, downloads and uploads with it, on the day a tenant
|
||||
// happened to have no counted activity. The quietest
|
||||
// installations would stop reporting and nothing would log a
|
||||
// fault. Both shapes are pinned by tests on that side too.
|
||||
'actions' => (object) $this->usageActions($since),
|
||||
];
|
||||
}
|
||||
|
||||
Generated
+12
-12
@@ -2811,16 +2811,16 @@
|
||||
},
|
||||
{
|
||||
"name": "league/commonmark",
|
||||
"version": "2.9.0",
|
||||
"version": "2.10.0",
|
||||
"source": {
|
||||
"type": "git",
|
||||
"url": "https://github.com/thephpleague/commonmark.git",
|
||||
"reference": "5703d83ba3da3b2e356a5fedc848ed6d8ffb6529"
|
||||
"reference": "d2d1aa8b35e072966c89bc0c66cf926e56767dc4"
|
||||
},
|
||||
"dist": {
|
||||
"type": "zip",
|
||||
"url": "https://api.github.com/repos/thephpleague/commonmark/zipball/5703d83ba3da3b2e356a5fedc848ed6d8ffb6529",
|
||||
"reference": "5703d83ba3da3b2e356a5fedc848ed6d8ffb6529",
|
||||
"url": "https://api.github.com/repos/thephpleague/commonmark/zipball/d2d1aa8b35e072966c89bc0c66cf926e56767dc4",
|
||||
"reference": "d2d1aa8b35e072966c89bc0c66cf926e56767dc4",
|
||||
"shasum": ""
|
||||
},
|
||||
"require": {
|
||||
@@ -2857,7 +2857,7 @@
|
||||
"type": "library",
|
||||
"extra": {
|
||||
"branch-alias": {
|
||||
"dev-main": "2.10-dev"
|
||||
"dev-main": "2.11-dev"
|
||||
}
|
||||
},
|
||||
"autoload": {
|
||||
@@ -2914,7 +2914,7 @@
|
||||
"type": "tidelift"
|
||||
}
|
||||
],
|
||||
"time": "2026-08-03T13:42:31+00:00"
|
||||
"time": "2026-08-11T16:06:25+00:00"
|
||||
},
|
||||
{
|
||||
"name": "league/config",
|
||||
@@ -3883,16 +3883,16 @@
|
||||
},
|
||||
{
|
||||
"name": "nette/schema",
|
||||
"version": "v1.3.5",
|
||||
"version": "v1.3.6",
|
||||
"source": {
|
||||
"type": "git",
|
||||
"url": "https://github.com/nette/schema.git",
|
||||
"reference": "f0ab1a3cda782dbc5da270d28545236aa80c4002"
|
||||
"reference": "c54350438cd6914616f790a49cb424605f421562"
|
||||
},
|
||||
"dist": {
|
||||
"type": "zip",
|
||||
"url": "https://api.github.com/repos/nette/schema/zipball/f0ab1a3cda782dbc5da270d28545236aa80c4002",
|
||||
"reference": "f0ab1a3cda782dbc5da270d28545236aa80c4002",
|
||||
"url": "https://api.github.com/repos/nette/schema/zipball/c54350438cd6914616f790a49cb424605f421562",
|
||||
"reference": "c54350438cd6914616f790a49cb424605f421562",
|
||||
"shasum": ""
|
||||
},
|
||||
"require": {
|
||||
@@ -3944,9 +3944,9 @@
|
||||
],
|
||||
"support": {
|
||||
"issues": "https://github.com/nette/schema/issues",
|
||||
"source": "https://github.com/nette/schema/tree/v1.3.5"
|
||||
"source": "https://github.com/nette/schema/tree/v1.3.6"
|
||||
},
|
||||
"time": "2026-02-23T03:47:12+00:00"
|
||||
"time": "2026-08-16T21:58:41+00:00"
|
||||
},
|
||||
{
|
||||
"name": "nette/utils",
|
||||
|
||||
+4
-2
@@ -449,8 +449,10 @@ the core vocabulary. A slug must be unique and lowercase; a clash throws at boot
|
||||
silently shadowing.
|
||||
|
||||
Module endpoints are deliberately absent from the document above — `OpenApiContractTest` skips
|
||||
`api/v1/modules/*` — so each package documents its own surface in its own repository. The
|
||||
`branding` module's endpoints are in `packages/cloud-modules/docs/api.md`.
|
||||
`api/v1/modules/*` — because that document is served unauthenticated and has to be identical on
|
||||
every installation. The modules that ship with the application document their endpoints in
|
||||
[`api-modules.md`](api-modules.md); a module living in its own package documents its surface in its
|
||||
own repository.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
# API — module endpoints
|
||||
|
||||
Optional modules add endpoints under `/api/v1/modules/{module}/…`. They are **not** in the committed
|
||||
[`api/openapi.json`](api/openapi.json): `OpenApiContractTest` skips `api/v1/modules/*`, because that
|
||||
document is served unauthenticated and has to be identical on every installation, while a module's
|
||||
paths exist only where the module does. This file is the documentation for the modules that ship
|
||||
with the application; a module living in its own package documents its surface in its own repository.
|
||||
|
||||
Everything [`api-guide.md`](api-guide.md) describes — bearer-token authentication, ability checks,
|
||||
RFC 7807 errors, rate limits — applies unchanged. The core supplies all of it; none of it is
|
||||
restated per module.
|
||||
|
||||
`GET /api/v1/me` lists the modules an installation actually carries, so an integration can check for
|
||||
`branding` before calling anything below rather than guessing from a 404.
|
||||
|
||||
---
|
||||
|
||||
## Branding
|
||||
|
||||
Mounted at `/api/v1/modules/branding`, behind `capability:branding.customize`. Every edition has
|
||||
that capability; a hosted plan can have it subtracted from its environment, in which case these
|
||||
paths answer 403 like any other gated route.
|
||||
|
||||
Both endpoints are read-only. Uploading either image is a multipart flow whose content-sniffing
|
||||
rules only make sense behind a file picker, and settings writes follow the rule that there is never
|
||||
a generic `PATCH /settings`.
|
||||
|
||||
Hiding the attribution line is not here. That switch is Cloud-only, has no API surface, and its
|
||||
column is written by the `cloud-modules` package.
|
||||
|
||||
### `GET /logo` — ability: `edit_settings`
|
||||
|
||||
The logo shown in place of the default sidebar icon.
|
||||
|
||||
```json
|
||||
{
|
||||
"data": {
|
||||
"logo_url": "https://example.test/storage/branding/9f3c….png",
|
||||
"updated_at": "2026-08-07T16:02:30+00:00"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`logo_url` is `null` when no logo has been uploaded, which is the normal state rather than an error.
|
||||
|
||||
### `GET /watermark` — ability: `edit_settings`
|
||||
|
||||
The mark stamped onto the thumbnails and previews clients and anonymous public visitors see. What
|
||||
this installation's own staff see goes unmarked, and the stored files — including every download —
|
||||
are never altered.
|
||||
|
||||
```json
|
||||
{
|
||||
"data": {
|
||||
"enabled": true,
|
||||
"image_url": "https://example.test/storage/branding/a68a….png",
|
||||
"position": "bottom-right",
|
||||
"size": 35,
|
||||
"opacity": 55
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
| Field | Meaning |
|
||||
|---|---|
|
||||
| `enabled` | Whether client- and public-facing images are *actually* being watermarked. False whenever nothing is drawn — including when the toggle is on but its image has since been removed. It answers "is this installation watermarking?", not "which way is the switch pointing?" What staff see is never marked regardless. |
|
||||
| `image_url` | The artwork, or `null` if none was ever chosen. |
|
||||
| `position` | One of `top-left`, `top-center`, `top-right`, `middle-left`, `center`, `middle-right`, `bottom-left`, `bottom-center`, `bottom-right`. |
|
||||
| `size` | Percentage of the image the mark is scaled to fit inside, keeping its proportions — so a thumbnail and a preview carry the same design at different scales. 5–100. |
|
||||
| `opacity` | Percentage. 1–100. |
|
||||
|
||||
An installation that has never opened the branding screen answers with the defaults it would start
|
||||
from (`enabled: false`, `bottom-right`, `30`, `60`) rather than a payload of nulls.
|
||||
|
||||
---
|
||||
|
||||
## Deferred, on purpose
|
||||
|
||||
- **Writes for either image.** See above.
|
||||
- **Rendered thumbnails themselves.** Already deferred by the host ([`api-todo.md`](api-todo.md));
|
||||
the watermark endpoint exists so an integration generating its own derivative images can reproduce
|
||||
the installation's mark, not as a step toward serving thumbnails over the API.
|
||||
@@ -4075,7 +4075,7 @@
|
||||
"object",
|
||||
"null"
|
||||
],
|
||||
"description": "Name only. The uploader is a user record; their email address\nis not part of what \"this file exists\" needs to say.",
|
||||
"description": "Name only. The uploader is a user record; their email address\nis not part of what \"this file exists\" needs to say. Null\nwhen the uploader is a client the token's owner is not\nscoped to; an unscoped account always gets the name.",
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": "integer"
|
||||
@@ -4109,6 +4109,7 @@
|
||||
},
|
||||
"assignments": {
|
||||
"type": "array",
|
||||
"description": "Who the file is shared with, as far as this caller is\nconcerned: a recipient the token's owner is not scoped to is\nleft out rather than returned without a name.",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
|
||||
Generated
+27
-26
@@ -4,6 +4,7 @@
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "cloud",
|
||||
"dependencies": {
|
||||
"@codemirror/lang-css": "^6.3.1",
|
||||
"@codemirror/lang-html": "^6.4.11",
|
||||
@@ -1190,41 +1191,41 @@
|
||||
}
|
||||
},
|
||||
"node_modules/@humanfs/core": {
|
||||
"version": "0.19.1",
|
||||
"resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.1.tgz",
|
||||
"integrity": "sha512-5DyQ4+1JEUzejeK1JGICcideyfUbGixgS9jNgex5nqkW+cY7WZhxBigmieN5Qnw9ZosSNVC9KQKyb+GUaGyKUA==",
|
||||
"version": "0.19.2",
|
||||
"resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.2.tgz",
|
||||
"integrity": "sha512-UhXNm+CFMWcbChXywFwkmhqjs3PRCmcSa/hfBgLIb7oQ5HNb1wS0icWsGtSAUNgefHeI+eBrA8I1fxmbHsGdvA==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@humanfs/types": "^0.15.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=18.18.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@humanfs/node": {
|
||||
"version": "0.16.6",
|
||||
"resolved": "https://registry.npmjs.org/@humanfs/node/-/node-0.16.6.tgz",
|
||||
"integrity": "sha512-YuI2ZHQL78Q5HbhDiBA1X4LmYdXCKCMQIfw0pw7piHJwyREFebJUvrQN4cMssyES6x+vfUbx1CIpaQUKYdQZOw==",
|
||||
"version": "0.16.8",
|
||||
"resolved": "https://registry.npmjs.org/@humanfs/node/-/node-0.16.8.tgz",
|
||||
"integrity": "sha512-gE1eQNZ3R++kTzFUpdGlpmy8kDZD/MLyHqDwqjkVQI0JMdI1D51sy1H958PNXYkM2rAac7e5/CnIKZrHtPh3BQ==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@humanfs/core": "^0.19.1",
|
||||
"@humanwhocodes/retry": "^0.3.0"
|
||||
"@humanfs/core": "^0.19.2",
|
||||
"@humanfs/types": "^0.15.0",
|
||||
"@humanwhocodes/retry": "^0.4.0"
|
||||
},
|
||||
"engines": {
|
||||
"node": ">=18.18.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@humanfs/node/node_modules/@humanwhocodes/retry": {
|
||||
"version": "0.3.1",
|
||||
"resolved": "https://registry.npmjs.org/@humanwhocodes/retry/-/retry-0.3.1.tgz",
|
||||
"integrity": "sha512-JBxkERygn7Bv/GbN5Rv8Ul6LVknS+5Bp6RgDC/O8gEBU/yeH5Ui5C/OlWrTb6qct7LjjfT6Re2NxB0ln0yYybA==",
|
||||
"node_modules/@humanfs/types": {
|
||||
"version": "0.15.0",
|
||||
"resolved": "https://registry.npmjs.org/@humanfs/types/-/types-0.15.0.tgz",
|
||||
"integrity": "sha512-ZZ1w0aoQkwuUuC7Yf+7sdeaNfqQiiLcSRbfI08oAxqLtpXQr9AIVX7Ay7HLDuiLYAaFPu8oBYNq/QIi9URHJ3Q==",
|
||||
"dev": true,
|
||||
"license": "Apache-2.0",
|
||||
"engines": {
|
||||
"node": ">=18.18"
|
||||
},
|
||||
"funding": {
|
||||
"type": "github",
|
||||
"url": "https://github.com/sponsors/nzakas"
|
||||
"node": ">=18.18.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@humanwhocodes/module-importer": {
|
||||
@@ -4057,9 +4058,9 @@
|
||||
}
|
||||
},
|
||||
"node_modules/brace-expansion": {
|
||||
"version": "1.1.11",
|
||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz",
|
||||
"integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==",
|
||||
"version": "1.1.18",
|
||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz",
|
||||
"integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
@@ -6688,9 +6689,9 @@
|
||||
"license": "MIT"
|
||||
},
|
||||
"node_modules/nanoid": {
|
||||
"version": "3.3.17",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.17.tgz",
|
||||
"integrity": "sha512-xQLf0A3HOMlgHq0n247/LRuAOYmB7dXJ/DvAxGvsSBij45XtBSmQycu+F8ODbHwns/XyFZagyL1+J0Offw1E0g==",
|
||||
"version": "3.3.18",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz",
|
||||
"integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==",
|
||||
"funding": [
|
||||
{
|
||||
"type": "github",
|
||||
@@ -7220,9 +7221,9 @@
|
||||
}
|
||||
},
|
||||
"node_modules/qs": {
|
||||
"version": "6.15.3",
|
||||
"resolved": "https://registry.npmjs.org/qs/-/qs-6.15.3.tgz",
|
||||
"integrity": "sha512-O9gl3zCl5h5blw1KGUzQKhA5oUXSl8rwUIM5o0S3nCXMliSvy5Dzx7/DJcI+SwgICv+IneSZwhBh1oSyEHA71A==",
|
||||
"version": "6.16.0",
|
||||
"resolved": "https://registry.npmjs.org/qs/-/qs-6.16.0.tgz",
|
||||
"integrity": "sha512-h6fhOIaRrID2CbEY2fqs+7t+UXZo+MLAnU5gRIq85uFtdiUPCdsApMlHhXogKVM4HM2DVbIjGNTTYH2OcmP1vA==",
|
||||
"license": "BSD-3-Clause",
|
||||
"dependencies": {
|
||||
"es-define-property": "^1.0.1",
|
||||
|
||||
@@ -0,0 +1,291 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Models\User;
|
||||
use App\Modules\Files\Access\ClientIdentityScope;
|
||||
use App\Modules\Files\Models\File;
|
||||
use App\Modules\Groups\Models\Group;
|
||||
use App\Modules\Identity\Models\Role;
|
||||
use App\Modules\Identity\Models\RolePermission;
|
||||
use App\Modules\Identity\Permissions\Permission;
|
||||
use App\Modules\Identity\Permissions\SystemRole;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
||||
/**
|
||||
* A client-scoped staff member may hold a file whose uploader, or whose
|
||||
* other recipients, are clients off their roster — that is a legitimate
|
||||
* consequence of group and folder sharing, and the file boundary is right
|
||||
* to allow it. What is not legitimate is learning those clients' names and
|
||||
* ids from the file's metadata.
|
||||
*
|
||||
* Every one of these asserts on the rendered response body rather than on a
|
||||
* particular key, because the leak was never in one field: the same client
|
||||
* name arrived through the uploader, through the recipient list, and
|
||||
* through four different screens. A body that does not contain the name
|
||||
* anywhere is the only assertion that would have caught all of them.
|
||||
*/
|
||||
beforeEach(function () {
|
||||
Storage::fake('files');
|
||||
|
||||
$this->admin = User::factory()->create();
|
||||
$this->onRoster = User::factory()->client()->create(['name' => 'Roster Client']);
|
||||
$this->offRoster = User::factory()->client()->create(['name' => 'Offroster Client']);
|
||||
|
||||
$this->manager = User::factory()->role(SystemRole::ClientManager)->create();
|
||||
$this->manager->assignedClients()->sync([$this->onRoster->id]);
|
||||
|
||||
$this->token = $this->manager->createToken('t', [Permission::Upload->value])->plainTextToken;
|
||||
});
|
||||
|
||||
/** A file the manager may read, uploaded by a client they may not identify. */
|
||||
function fileFromStranger(): File
|
||||
{
|
||||
$file = File::factory()->create([
|
||||
'uploaded_by' => test()->offRoster->id,
|
||||
'name' => 'shared-onward',
|
||||
]);
|
||||
shareFileWith($file, test()->onRoster);
|
||||
|
||||
return $file;
|
||||
}
|
||||
|
||||
/**
|
||||
* A client-scoped staff member holding wider permissions than the built-in
|
||||
* Client Manager — the roles that can open a colleague's file for editing
|
||||
* and browse a client's own library. Scoping and permissions are separate
|
||||
* axes, and the leak is a property of the scoping.
|
||||
*
|
||||
* @param list<string> $permissions
|
||||
*/
|
||||
function scopedStaffWith(array $permissions): User
|
||||
{
|
||||
$role = Role::query()->create(['name' => 'Scoped '.uniqid(), 'client_scoped' => true]);
|
||||
|
||||
foreach ($permissions as $permission) {
|
||||
RolePermission::query()->create(['role_id' => $role->id, 'permission' => $permission]);
|
||||
}
|
||||
|
||||
$staff = User::factory()->create(['role_id' => $role->id]);
|
||||
$staff->assignedClients()->sync([test()->onRoster->id]);
|
||||
|
||||
return $staff;
|
||||
}
|
||||
|
||||
/** A file the manager may read that is also shared with a stranger client. */
|
||||
function fileWithStrangerCoRecipient(): File
|
||||
{
|
||||
$file = File::factory()->create(['uploaded_by' => test()->admin->id, 'name' => 'shared-both']);
|
||||
shareFileWith($file, test()->onRoster);
|
||||
shareFileWith($file, test()->offRoster);
|
||||
|
||||
return $file;
|
||||
}
|
||||
|
||||
test('the file boundary itself is unchanged: a stranger-only file is still refused', function () {
|
||||
$file = File::factory()->create(['uploaded_by' => $this->offRoster->id]);
|
||||
shareFileWith($file, $this->offRoster);
|
||||
|
||||
$this->withToken($this->token)->getJson("/api/v1/files/{$file->id}")->assertForbidden();
|
||||
});
|
||||
|
||||
test('the file boundary itself is unchanged on the web', function () {
|
||||
$file = File::factory()->create(['uploaded_by' => $this->offRoster->id]);
|
||||
shareFileWith($file, $this->offRoster);
|
||||
|
||||
$this->actingAs($this->manager)->get("/files/{$file->id}/details")->assertForbidden();
|
||||
});
|
||||
|
||||
test('the API does not name a stranger uploader of a file the caller may read', function () {
|
||||
$file = fileFromStranger();
|
||||
|
||||
$show = $this->withToken($this->token)->getJson("/api/v1/files/{$file->id}")->assertOk();
|
||||
$index = $this->withToken($this->token)->getJson('/api/v1/files')->assertOk();
|
||||
|
||||
expect($show->getContent())->not->toContain('Offroster Client')
|
||||
->and($index->getContent())->not->toContain('Offroster Client')
|
||||
->and($show->json('data.uploaded_by'))->toBeNull()
|
||||
// The file is still readable — this narrows the answer, it does
|
||||
// not withdraw it.
|
||||
->and($show->json('data.id'))->toBe($file->id);
|
||||
});
|
||||
|
||||
test('the API does not list a stranger co-recipient', function () {
|
||||
$file = fileWithStrangerCoRecipient();
|
||||
|
||||
$show = $this->withToken($this->token)->getJson("/api/v1/files/{$file->id}")->assertOk();
|
||||
|
||||
expect($show->getContent())->not->toContain('Offroster Client')
|
||||
->and($show->json('data.assignments'))->toHaveCount(1)
|
||||
->and($show->json('data.assignments.0.name'))->toBe('Roster Client');
|
||||
});
|
||||
|
||||
test('a stranger co-recipient is not named in the reply to a write', function () {
|
||||
$file = fileWithStrangerCoRecipient();
|
||||
|
||||
// The assignment endpoints re-load assignments.assignable and hand the
|
||||
// result straight back, which is a second serialisation path — and one
|
||||
// that a fix applied only to the read controllers would have missed.
|
||||
// The 200 is asserted on purpose: without it this passes on a 403,
|
||||
// whose body names nobody either.
|
||||
$writer = scopedStaffWith([Permission::Upload->value, Permission::EditFiles->value, Permission::EditOthersFiles->value]);
|
||||
$token = $writer->createToken('w', [Permission::EditFiles->value, Permission::EditOthersFiles->value])->plainTextToken;
|
||||
|
||||
$response = $this->withToken($token)
|
||||
->postJson("/api/v1/files/{$file->id}/assignments", ['type' => 'client', 'id' => $this->onRoster->id])
|
||||
->assertOk();
|
||||
|
||||
expect($response->getContent())->not->toContain('Offroster Client')
|
||||
->and($response->json('data.assignments'))->toHaveCount(1);
|
||||
});
|
||||
|
||||
test('uploaded_by cannot be used to probe for a client the caller may not identify', function () {
|
||||
fileFromStranger();
|
||||
|
||||
$probe = $this->withToken($this->token)->getJson("/api/v1/files?uploaded_by={$this->offRoster->id}")->assertOk();
|
||||
|
||||
expect($probe->json('data'))->toBeEmpty();
|
||||
});
|
||||
|
||||
test('uploaded_by still filters by a client on the roster', function () {
|
||||
$own = File::factory()->create(['uploaded_by' => $this->onRoster->id, 'name' => 'theirs']);
|
||||
shareFileWith($own, $this->onRoster);
|
||||
fileFromStranger();
|
||||
|
||||
$hit = $this->withToken($this->token)->getJson("/api/v1/files?uploaded_by={$this->onRoster->id}")->assertOk();
|
||||
|
||||
expect($hit->json('data'))->toHaveCount(1)
|
||||
->and($hit->json('data.0.id'))->toBe($own->id);
|
||||
});
|
||||
|
||||
test('uploaded_by still filters by a staff member', function () {
|
||||
$file = fileWithStrangerCoRecipient();
|
||||
|
||||
$hit = $this->withToken($this->token)->getJson("/api/v1/files?uploaded_by={$this->admin->id}")->assertOk();
|
||||
|
||||
expect($hit->json('data'))->toHaveCount(1)
|
||||
->and($hit->json('data.0.id'))->toBe($file->id);
|
||||
});
|
||||
|
||||
test('the details panel names neither a stranger uploader nor a stranger recipient', function () {
|
||||
$stranger = fileFromStranger();
|
||||
$both = fileWithStrangerCoRecipient();
|
||||
|
||||
$one = $this->actingAs($this->manager)->get("/files/{$stranger->id}/details")->assertOk();
|
||||
$two = $this->actingAs($this->manager)->get("/files/{$both->id}/details")->assertOk();
|
||||
|
||||
expect($one->getContent())->not->toContain('Offroster Client')
|
||||
->and($one->json('uploader'))->toBeNull()
|
||||
->and($two->getContent())->not->toContain('Offroster Client')
|
||||
->and($two->json('shares.clients'))->toHaveCount(1);
|
||||
});
|
||||
|
||||
test('the library listing does not describe a stranger uploader', function () {
|
||||
fileFromStranger();
|
||||
|
||||
$body = $this->actingAs($this->manager)->get('/files')->assertOk()->getContent();
|
||||
|
||||
// Not the name, and not the "a client uploaded this" shape either.
|
||||
expect($body)->not->toContain('Offroster Client');
|
||||
});
|
||||
|
||||
test('the edit page does not name a stranger uploader or recipient', function () {
|
||||
$file = fileWithStrangerCoRecipient();
|
||||
$file->update(['uploaded_by' => $this->offRoster->id]);
|
||||
|
||||
$editor = scopedStaffWith([Permission::Upload->value, Permission::EditFiles->value, Permission::EditOthersFiles->value]);
|
||||
|
||||
// route() rather than a built path: File binds by slug, so an id in
|
||||
// the URL is a 404 rather than the page under test.
|
||||
$body = $this->actingAs($editor)->get(route('files.edit', $file))->assertOk()->getContent();
|
||||
|
||||
expect($body)->not->toContain('Offroster Client');
|
||||
});
|
||||
|
||||
test('the per-client file listing does not name a stranger uploader', function () {
|
||||
fileFromStranger();
|
||||
|
||||
$browser = scopedStaffWith([Permission::Upload->value, Permission::EditClients->value]);
|
||||
|
||||
$body = $this->actingAs($browser)
|
||||
->get("/clients/{$this->onRoster->id}/files")->assertOk()->getContent();
|
||||
|
||||
expect($body)->not->toContain('Offroster Client');
|
||||
});
|
||||
|
||||
test('a group holding none of the viewer clients is not named', function () {
|
||||
$group = Group::query()->create(['name' => 'Offroster Group']);
|
||||
$group->members()->sync([$this->offRoster->id]);
|
||||
|
||||
$file = File::factory()->create(['uploaded_by' => $this->admin->id]);
|
||||
shareFileWith($file, $this->onRoster);
|
||||
shareFileWithGroup($file, $group);
|
||||
|
||||
$api = $this->withToken($this->token)->getJson("/api/v1/files/{$file->id}")->assertOk();
|
||||
|
||||
expect($api->getContent())->not->toContain('Offroster Group')
|
||||
->and($api->json('data.assignments'))->toHaveCount(1);
|
||||
});
|
||||
|
||||
test('a group holding none of the viewer clients is not named on the web', function () {
|
||||
$group = Group::query()->create(['name' => 'Offroster Group']);
|
||||
$group->members()->sync([$this->offRoster->id]);
|
||||
|
||||
$file = File::factory()->create(['uploaded_by' => $this->admin->id]);
|
||||
shareFileWith($file, $this->onRoster);
|
||||
shareFileWithGroup($file, $group);
|
||||
|
||||
$details = $this->actingAs($this->manager)->get("/files/{$file->id}/details")->assertOk();
|
||||
|
||||
expect($details->getContent())->not->toContain('Offroster Group')
|
||||
->and($details->json('shares.groups'))->toBeEmpty();
|
||||
});
|
||||
|
||||
test('an unscoped administrator still sees every name', function () {
|
||||
$file = fileWithStrangerCoRecipient();
|
||||
$file->update(['uploaded_by' => $this->offRoster->id]);
|
||||
|
||||
$token = $this->admin->createToken('a', [Permission::Upload->value])->plainTextToken;
|
||||
|
||||
$api = $this->withToken($token)->getJson("/api/v1/files/{$file->id}")->assertOk();
|
||||
|
||||
expect($api->json('data.uploaded_by.name'))->toBe('Offroster Client')
|
||||
->and($api->json('data.assignments'))->toHaveCount(2);
|
||||
});
|
||||
|
||||
test('an unscoped administrator still sees every name on the web', function () {
|
||||
$file = fileWithStrangerCoRecipient();
|
||||
$file->update(['uploaded_by' => $this->offRoster->id]);
|
||||
|
||||
$details = $this->actingAs($this->admin)->get("/files/{$file->id}/details")->assertOk();
|
||||
|
||||
expect($details->json('uploader'))->toBe('Offroster Client')
|
||||
->and($details->json('shares.clients'))->toHaveCount(2);
|
||||
});
|
||||
|
||||
test('a scoped viewer is still told about their own roster and their own uploads', function () {
|
||||
$mine = File::factory()->create(['uploaded_by' => $this->manager->id, 'name' => 'mine']);
|
||||
shareFileWith($mine, $this->onRoster);
|
||||
|
||||
$api = $this->withToken($this->token)->getJson("/api/v1/files/{$mine->id}")->assertOk();
|
||||
|
||||
expect($api->json('data.uploaded_by.name'))->toBe($this->manager->name)
|
||||
->and($api->json('data.assignments.0.name'))->toBe('Roster Client');
|
||||
});
|
||||
|
||||
test('the rule itself: staff are never hidden, strangers always are', function () {
|
||||
$identity = app(ClientIdentityScope::class);
|
||||
|
||||
expect($identity->permits($this->manager, $this->admin))->toBeTrue()
|
||||
->and($identity->permits($this->manager, $this->onRoster))->toBeTrue()
|
||||
->and($identity->permits($this->manager, $this->offRoster))->toBeFalse()
|
||||
// A client may always be told who they themselves are.
|
||||
->and($identity->permits($this->offRoster, $this->offRoster))->toBeTrue()
|
||||
// An unscoped viewer is narrowed by nothing.
|
||||
->and($identity->permits($this->admin, $this->offRoster))->toBeTrue()
|
||||
->and($identity->isNarrowed($this->admin))->toBeFalse()
|
||||
->and($identity->isNarrowed($this->manager))->toBeTrue()
|
||||
// No viewer at all is the closed case, not the open one.
|
||||
->and($identity->permits(null, $this->offRoster))->toBeFalse()
|
||||
->and($identity->permits(null, $this->admin))->toBeTrue();
|
||||
});
|
||||
Reference in New Issue
Block a user