mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 17:15:08 +00:00
denkfabrik-li
c8de16101f
FilePolicy::view() has two halves for staff: one of the three file keys
(upload / edit_files / edit_others_files), AND StaffLibraryScope. Every
comment surface that spans files narrowed by the library half alone.
A role holding moderate_comments and no file key therefore got a 403 on
every file in the installation while reading every comment written about
them on /comments: the text, staff-only notes, the client name a
Clients-visibility comment carries, and a visitor's IP address. The API
queue answered the same way, and approving through it hands the body back
in the response, so it was a reading door as well as a writing one.
The class says this is not supposed to happen -- across()'s own docblock
("a moderation screen is not a way around the visibility model"), the
route comment on /comments ("the list itself is still narrowed by
VisibleCommentScope, so holding the permission does not widen what a
viewer may read"), and routes/api.php ("reading and writing a comment is
gated by 'may see this file', the same three keys the file endpoints
use"). FileCommentPolicy::view() enforces it for a single comment, by
running the file's own gate first. Only the cross-file queries did not.
So they now take their files from ViewableFileScope, which is
FilePolicy::view() expressed as a query, instead of from StaffLibraryScope,
which is only its second half: across(), pendingTotal() and the API's
pending list. The permission half moves into a named method on that class,
since three modules now ask the same question.
FileCommentPolicy::moderate() gets it too, in both forms. Its row form is
otherwise unchanged -- the library check still runs by file id, so a
comment on a soft-deleted file behaves exactly as before.
No system role changes behaviour: Account Manager and System Administrator
are the two that ship with moderate_comments, and both hold upload. What
changes is a hand-built role that holds moderation and nothing else.
Seven tests. Without the fix, five go red; the other two are the premise
(that the viewer really is refused the file itself) and the guard that a
moderator who may read files still moderates the whole installation.
docs/api/openapi.json regenerated for the one changed description.
83 lines
3.0 KiB
PHP
83 lines
3.0 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Comments\Http\Controllers\Api;
|
|
|
|
use App\Http\Controllers\Controller;
|
|
use App\Modules\Comments\FileComments;
|
|
use App\Modules\Comments\Http\Resources\Api\FileCommentResource;
|
|
use App\Modules\Comments\Models\FileComment;
|
|
use App\Modules\Files\Access\ViewableFileScope;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Http\Resources\Json\AnonymousResourceCollection;
|
|
use Illuminate\Support\Facades\Gate;
|
|
|
|
/**
|
|
* Comments from visitors, waiting for a decision.
|
|
*
|
|
* Only anonymous comments ever land here — an account's comment is
|
|
* published the moment it is written.
|
|
*
|
|
* This exists so the two halves of moderation match. Deleting somebody
|
|
* else's comment was already reachable through `DELETE /comments/{id}`,
|
|
* which runs the same policy the web does; approving one was not, so a
|
|
* token carried the destructive half and none of the constructive one, and
|
|
* `moderate_comments` was not selectable as an ability at all because no
|
|
* route named it. Both are fixed by these two endpoints existing.
|
|
*/
|
|
class CommentModerationController extends Controller
|
|
{
|
|
public function __construct(
|
|
private readonly FileComments $comments,
|
|
private readonly ViewableFileScope $viewable,
|
|
) {}
|
|
|
|
/**
|
|
* List comments awaiting approval.
|
|
*
|
|
* Scoped by the same file boundary as everything else — the whole of
|
|
* it, not just its library half: a client-scoped token sees pending
|
|
* comments only on files its owner could already open, and a token
|
|
* whose owner holds no file key at all sees none. Oldest first, so
|
|
* working through the list means working through the backlog.
|
|
*/
|
|
public function index(Request $request): AnonymousResourceCollection
|
|
{
|
|
$viewer = $request->user();
|
|
assert($viewer !== null);
|
|
Gate::forUser($viewer)->authorize('moderate', FileComment::class);
|
|
|
|
$pending = FileComment::query()
|
|
->whereNull('approved_at')
|
|
->whereIn('file_id', $this->viewable->for($viewer)->select('id'))
|
|
->with(['author', 'clientContext'])
|
|
->orderBy('created_at')
|
|
->orderBy('id')
|
|
->get();
|
|
|
|
return FileCommentResource::collection($pending);
|
|
}
|
|
|
|
/**
|
|
* Approve a comment left by a visitor.
|
|
*
|
|
* Nobody can see it until this happens. Approving an already-approved
|
|
* comment changes nothing and announces nothing, so a retried request
|
|
* is safe — which matters more here than on the web, where a human does
|
|
* not retry automatically.
|
|
*/
|
|
public function approve(Request $request, FileComment $comment): FileCommentResource
|
|
{
|
|
$viewer = $request->user();
|
|
assert($viewer !== null);
|
|
// Moderation rights are not a way around the library boundary; the
|
|
// policy weighs the comment's file, so name the comment.
|
|
Gate::authorize('moderate', $comment);
|
|
|
|
$this->comments->approve($comment, $viewer);
|
|
|
|
return new FileCommentResource($comment->fresh()?->load(['author', 'clientContext']) ?? $comment);
|
|
}
|
|
}
|