mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 09:05:08 +00:00
5fb17388cd
Reported by @Drescargot as GHSA-r3hg-3fxw-rcmr, in two halves. The groups listing never narrowed at all. Every other action in that controller is guarded with allowsGroupChange(), and index() — web and API alike — built a bare Group::query(), so a client-scoped staff member was shown every group on the installation with its name, description and member count. StaffLibraryScope::groups() is that narrowing, and assignableGroupIds() now reads from it rather than restating the same rule a second time, which is how the two drifted apart to begin with. The second half is the one that mattered. allowsGroupChange() asked only groupReachesNoFurther() — "is anything shared with this group outside my library" — which a group with nothing shared with it yet passes vacuously. So a scoped staff member could rename, delete or publish a group whose every member was somebody else's client. Publishing is the sharp end: whatever is shared with the group afterwards is reachable without signing in. The reporter suggested putting the membership check inside groupReachesNoFurther(). Tried, and it breaks two things. That predicate is shared with allowsGroupMembership(), where a group nobody has joined must stay usable so its creator can add the first member. And "every member must be mine" is the obvious reading of the rule and is wrong: it turns GHSA-whmp-p9hv-r7j7's narrowing — a mixed group's edit screen loads and simply does not name the stranger — back into a 404, undoing that fix. Four tests from it fail that way. So the check sits in allowsGroupChange() alone, and asks whether the group is wholly somebody else's rather than whether it is wholly theirs. A mixed group stays workable and is still covered by the reach check; an empty one stays nameable by whoever just made it; a group with members and none of them theirs is refused.
157 lines
5.8 KiB
PHP
157 lines
5.8 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Groups\Http\Controllers\Api;
|
|
|
|
use App\Http\Controllers\Controller;
|
|
use App\Modules\Api\Support\PollingQuery;
|
|
use App\Modules\Audit\Action;
|
|
use App\Modules\Audit\ActivityLogger;
|
|
use App\Modules\Groups\Http\Resources\Api\GroupResource;
|
|
use App\Modules\Files\Access\StaffLibraryScope;
|
|
use App\Modules\Groups\Models\Group;
|
|
use App\Support\Rules;
|
|
use Illuminate\Database\Eloquent\Builder;
|
|
use Illuminate\Database\Eloquent\Relations\BelongsToMany;
|
|
use Illuminate\Http\JsonResponse;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Http\Resources\Json\AnonymousResourceCollection;
|
|
use Illuminate\Validation\Rule;
|
|
|
|
/**
|
|
* Groups — collections of clients used as a sharing target.
|
|
*
|
|
* Distinct from a Category (a label on a file) and a Folder (hierarchy);
|
|
* the three are easy to conflate from outside, which is why the guide
|
|
* names them separately.
|
|
*/
|
|
class GroupsController extends Controller
|
|
{
|
|
public function __construct(
|
|
private readonly PollingQuery $polling,
|
|
private readonly ActivityLogger $activity,
|
|
private readonly StaffLibraryScope $scope,
|
|
) {}
|
|
|
|
public function index(Request $request): AnonymousResourceCollection
|
|
{
|
|
$filters = $request->validate($this->polling->rules() + [
|
|
'search' => ['nullable', 'string', 'max:255'],
|
|
'visibility' => ['nullable', Rule::in(['public', 'private'])],
|
|
]);
|
|
|
|
$viewer = $request->user();
|
|
assert($viewer !== null);
|
|
|
|
// The API twin of the web listing's narrowing, and it has to be
|
|
// here rather than only there: the same disclosure through a token
|
|
// is the same disclosure (GHSA-r3hg-3fxw-rcmr).
|
|
$query = $this->scope->groups($viewer)->withCount('members');
|
|
|
|
if (($filters['search'] ?? null) !== null) {
|
|
$search = $filters['search'];
|
|
$query->where(fn (Builder $inner) => $inner
|
|
->where('name', 'like', "%{$search}%")
|
|
->orWhere('description', 'like', "%{$search}%"));
|
|
}
|
|
|
|
if (($filters['visibility'] ?? null) !== null) {
|
|
$query->where('public', $filters['visibility'] === 'public');
|
|
}
|
|
|
|
return GroupResource::collection($this->polling->paginate($request, $query, 'groups'));
|
|
}
|
|
|
|
public function show(Request $request, Group $group): GroupResource
|
|
{
|
|
$viewer = $request->user();
|
|
assert($viewer !== null);
|
|
|
|
// The web edit screen's boundary, on its API twin: this is the read
|
|
// half of the group that update() and destroy() below already refuse
|
|
// to touch, and it hands back the membership with addresses.
|
|
abort_unless($this->scope->allowsGroupChange($viewer, $group), 404);
|
|
|
|
return new GroupResource($group->loadCount('members')->load([
|
|
'members' => fn (BelongsToMany $members) => $members
|
|
->whereIn('users.id', $this->scope->clients($viewer)->select('id')),
|
|
]));
|
|
}
|
|
|
|
public function store(Request $request): JsonResponse
|
|
{
|
|
$validated = $request->validate([
|
|
'name' => ['required', 'string', 'max:255'],
|
|
'slug' => Rules::slug('groups'),
|
|
'description' => ['nullable', 'string', 'max:2000'],
|
|
'public' => ['required', 'boolean'],
|
|
]);
|
|
|
|
$validated['slug'] = ($validated['slug'] ?? '') ?: Group::uniqueSlugFrom($validated['name']);
|
|
|
|
$group = Group::query()->create($validated);
|
|
|
|
$this->activity->log(Action::GroupCreated, subject: $group);
|
|
|
|
if ($group->public) {
|
|
$this->activity->log(Action::GroupMadePublic, subject: $group, context: ['slug' => $group->slug]);
|
|
}
|
|
|
|
return (new GroupResource($group->loadCount('members')))->response()->setStatusCode(201);
|
|
}
|
|
|
|
public function update(Request $request, Group $group): GroupResource
|
|
{
|
|
$viewer = $request->user();
|
|
assert($viewer !== null);
|
|
|
|
// Mirrors the web controller: a group reaching past this token
|
|
// owner's library is not theirs to change, and deleting one
|
|
// revokes its members' access to everything assigned to it.
|
|
abort_unless($this->scope->allowsGroupChange($viewer, $group), 404);
|
|
|
|
$validated = $request->validate([
|
|
'name' => ['sometimes', 'string', 'max:255'],
|
|
'slug' => Rules::slug('groups', $group->id),
|
|
'description' => ['sometimes', 'nullable', 'string', 'max:2000'],
|
|
'public' => ['sometimes', 'boolean'],
|
|
]);
|
|
|
|
// A slug must never change silently just because the name did.
|
|
$validated['slug'] = ($validated['slug'] ?? '') ?: ($group->slug ?: Group::uniqueSlugFrom($validated['name'] ?? $group->name, $group->id));
|
|
|
|
$wasPublic = $group->public;
|
|
|
|
$group->update($validated);
|
|
|
|
$this->activity->log(Action::GroupUpdated, subject: $group);
|
|
|
|
if (! $wasPublic && $group->public) {
|
|
$this->activity->log(Action::GroupMadePublic, subject: $group, context: ['slug' => $group->slug]);
|
|
} elseif ($wasPublic && ! $group->public) {
|
|
$this->activity->log(Action::GroupMadePrivate, subject: $group);
|
|
}
|
|
|
|
return new GroupResource($group->refresh()->loadCount('members'));
|
|
}
|
|
|
|
public function destroy(Request $request, Group $group): JsonResponse
|
|
{
|
|
$viewer = $request->user();
|
|
assert($viewer !== null);
|
|
|
|
// Mirrors the web controller: a group reaching past this token
|
|
// owner's library is not theirs to change, and deleting one
|
|
// revokes its members' access to everything assigned to it.
|
|
abort_unless($this->scope->allowsGroupChange($viewer, $group), 404);
|
|
|
|
$name = $group->name;
|
|
$group->delete();
|
|
|
|
$this->activity->log(Action::GroupDeleted, context: ['name' => $name]);
|
|
|
|
return response()->json(status: 204);
|
|
}
|
|
}
|