Let only the owner delete a project, not an administrator (#272)
#179 decided that nobody deletes a project on someone else's behalf. The API still let an administrator do it, because the admin store treated every project as its own. Option A from the issue is now in place: opening, editing, snapshots and release stay open to the administrator, deleting stays with the owner. - Store::delete checks the owner under the save lock and throws NotOwnerException for anyone else. It follows the pattern of StaleRevisionException. - The route answers that with 403 "Only its owner may delete a project". A file without an owner is covered the same way. Non-admins still get 404 for a project that is not theirs. - The editor has no way to delete a foreign project, the project picker lists only one's own. Nothing changes there. - An account that owns projects can no longer be deleted by an administrator at all. Its owner has to delete them first. - e2e 49 deleted the throwaway projects as administrator, both in its sweep and in 49i. It now signs in as the owner for that. 49i checks both answers. Tests: - tests/php/accounts.test.php: 56 checks passed (was 52, one check turned around, five new) - e2e 49: 22 checks passed (was 21), deleteByAdmin 403, deleteByOwner 204 - Gate: 197 files, 2855 tests passed - Counter-checks: without the owner check 4 PHP checks and 49i fail - The e2e total of 1603 is computed (1602 + 1), not measured Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
f14c4ecb77
commit
cbda6d64a7
5 files changed
+69
-17
No files matched your search
+26
-4
@@ -10,6 +10,15 @@ final class StaleRevisionException extends RuntimeException
|
||||
}
|
||||
}
|
||||
|
||||
/** Thrown by Store::delete when an administrator reaches a project that is not their own (#272). */
|
||||
final class NotOwnerException extends RuntimeException
|
||||
{
|
||||
public function __construct()
|
||||
{
|
||||
parent::__construct('Only its owner may delete a project');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* File-based project store: one JSON document per project in `data/projects/{id}.json`.
|
||||
* Each file wraps the project with metadata: { id, name, owner, revision, updatedAt, project }.
|
||||
@@ -20,9 +29,9 @@ final class StaleRevisionException extends RuntimeException
|
||||
* `owner` is that user. Someone else's project is answered as missing rather than as forbidden,
|
||||
* because "this id exists" is already more than a stranger should learn.
|
||||
*
|
||||
* An administrator's store (`$seesAll`, #179) reads, writes and deletes every project as if it
|
||||
* were their own, and a save keeps the owner it found. The list stays their own: the others are
|
||||
* reached through the user management, one account at a time.
|
||||
* An administrator's store (`$seesAll`, #179) reads and writes every project as if it were their
|
||||
* own, and a save keeps the owner it found. Deleting stays with the owner (#272). The list stays
|
||||
* their own: the others are reached through the user management, one account at a time.
|
||||
*/
|
||||
final class Store
|
||||
{
|
||||
@@ -121,11 +130,24 @@ final class Store
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes a project of this store's user. False when there is none to reach; throws
|
||||
* NotOwnerException when an administrator reaches one of somebody else's.
|
||||
*/
|
||||
public function delete(string $id): bool
|
||||
{
|
||||
$path = $this->path($id);
|
||||
// Under the save lock, or a save running at the same time writes the project back.
|
||||
$deleted = $this->locked($path, fn(): bool => $this->load($id) !== null && unlink($path));
|
||||
$deleted = $this->locked($path, function () use ($id, $path): bool {
|
||||
$doc = $this->load($id);
|
||||
if ($doc === null) {
|
||||
return false;
|
||||
}
|
||||
if ($doc['owner'] !== $this->owner) {
|
||||
throw new NotOwnerException();
|
||||
}
|
||||
return unlink($path);
|
||||
});
|
||||
if ($deleted) {
|
||||
// Whoever still waits for the lock finds no project and writes nothing.
|
||||
@unlink($path . '.lock');
|
||||
|
||||
Reference in new issue
Block a user