mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 00:55:07 +00:00
main
22 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 ignacionelson
|
5f414c7a4a |
Give the API the same file filters the library screen has
The staff library grew filters for uploader, role, public/private, download count and version. Two of those already existed on /api/v1/files (`uploaded_by`, `public`); the other four did not, so an integration could not ask what the screen asks. Adds `role_id`, `downloads=none|any`, `version=current|outdated` and `visibility=public|private`. `visibility` rather than changing `public`, and that is the decision worth explaining. `public` has always tested the file's own column, and callers depend on that answer; changing what an existing filter means is breaking for everyone already sending it, however much better the new meaning is. So `public` is untouched and `visibility` is added beside it with the application's own definition -- File::isEffectivelyPublic(), the flag or a public folder anywhere above the file -- which is what the badge on a staff row means. The guide says in a sentence which to reach for. Point the visibility filter at the column instead and the test that separates them fails, which is the whole point of having both. That predicate now lives once, as File::scopeEffectivelyPublic(), beside the isEffectivelyPublic() it has to agree with. It was a private helper on FoldersController until a second surface wanted it. `role_id` deliberately carries no identity guard, unlike `uploaded_by` beside it. A role names nobody: the files in the result are ones the caller may already read, and learning one came from somebody holding the Client role narrows to a set they could have guessed. `uploaded_by` is different in kind -- a non-empty answer confirms exactly the identity the response is redacting -- which is why only it is guarded. The reasoning is in the code, because an absent guard sitting next to a present one is the kind of thing a reader should not have to re-derive. Tests cover each filter, the public/visibility split, and the client-scoped negative: every new filter still returns nothing outside the token's own library, because a filter narrows a library and never widens one. |
||
|
ignacionelson
|
7c7ba7cd53 |
Stop a typed-in storage quota from 500ing when a client is created
Filling the "Storage quota (MB)" field on the new-client form raised a TypeError and the request died with a 500. Leaving it blank worked, which is why it reached a release: that path goes through `null ?? 0`, and the 0 is an int. The `integer` validation rule checks that a value looks like an integer. It does not convert it. `$request->validate()` returns the raw input, so the form field arrives as the string "2048" -- and the create form types that field as a string in React, so it is a string even over JSON. Both controllers declare strict_types, so handing it to `ClientAccounts::create()`'s `int $storageQuotaMb` is a TypeError. Fixed on both surfaces that call create(): the staff screen and /api/v1/clients. The API twin had the same defect, reachable by sending the quota as a quoted JSON value or a form-encoded body -- its own create test only ever sent a JSON number. Two more call sites had the same shape and are cast too, though nothing sends them a string today: the share-link download cap and a comment's reply_to. Both are safe only because a frontend file happens to call Number() first, which is a fact about that file rather than anything the signature guarantees. The null in each is preserved rather than collapsed to 0 -- "no cap" is not a cap of zero. `storage_quota_mb` is also cast on User and Invitation. The column is an unsignedInteger and both docblocks already promise int; it is read straight into provision()'s typed parameter when an invitation is redeemed, and which type a driver hands back is not something that call site should depend on. Found on the new files-test rehearsal instance, on its first real use, against the same build the whole fleet is running. |
||
|
ignacionelson
|
922be7226c |
Let a client edit and delete the files they uploaded
A client could upload a file and then never touch it again. No rename, no description, no expiry, no categories, no delete — the portal has three file routes and all three are GET. Meanwhile the Roles screen happily grants the Client role edit_files, delete_files, set_file_categories, set_file_expiration_date and upload_public, and every one of them was inert, because the routes that honour them are `staff`-gated rather than permission-gated. That is what #1771 hit: a permission granted, saved, and silently doing nothing. A client owns what they uploaded. Ownership is now what lets them edit and delete it, subject to the same per-field keys staff are subject to. The obvious implementation is a trap, and it is worth writing down. Both policy methods began `if (! $user->isStaff()) return false;` and both end in StaffLibraryScope, whose allowsFile() reads `if (! isClientScoped()) return true` — and isClientScoped() is `isStaff() && role->client_scoped`, so it is false for every client. Delete the early return and a client falls into the branch meaning "this staff member is unrestricted" and is handed the whole library. Same for folders(), which returns an unfiltered query: a client could move their file into any folder on the installation. So clients get their own branch, reaching neither. The portal asks Folder::uploadableBy() instead — a file cannot be moved somewhere it could not have been uploaded. edit_others_files and delete_others_files stay inert for clients by construction. A client has no others' files, only files somebody showed them, and being shown a file is not being given it. Which fields an editor may write moved into ApplyFileEdits, shared by the staff editor, /api/v1 and the portal. There were two copies of the same eight permission checks and this would have been the third; the checks are easy, which is exactly why the drift would have been invisible. Callers normalise their own request shape, this gates and writes and logs. Expiry reading and writing came along too, as FileExpiry — three copies, of which only the API's could read a timestamp. Clients do not choose the public slug. It is derived from the name they already picked, because an installation-wide unique slug a client sets is a name to squat and an existence oracle to probe with. One consequence for later, written up in docs/api-todo.md: the policy now says yes to a client for file writes, so `staff-token` is the only thing holding the API boundary where there used to be two independent refusals. ActorBoundaryTest pins it, and asserts the policy passes first so the test cannot quietly stop testing the middleware. Also corrects a stale comment that claimed a deleted file's bytes stay on disk. They have not since File::booted() grew a `deleted` hook; nothing ever forceDelete()s a File row, so "until a purge lands" would have meant never — which is why a client's delete frees their quota by exactly what it frees on disk. The UI comes next; this is the authorization, the routes and the tests. Fixes #1771 |
||
|
ignacionelson
|
9c6f4df5bc |
Merge pull request #1736 from denkfabrik-li/fix/scoped-creator-keeps-client
A client-scoped staff member with create_clients created a client and lost it in the same request. guardTarget() answers 404 for anything off their roster, and StaffLibraryScope::clients() leaves it out of their list -- so the record existed, was logged, was welcomed by email, and was invisible to the person who made it. store() redirects to the edit page, which is exactly where they landed on a 404. The API twin had the same shape: a scoped token got a 404 from every route that binds the client it had just created. The new client is now attached to the creator's roster when the creator is client-scoped, on both sides. That is where a client they created belongs -- the roster is the same list assignedClients already uses for everything else they may reach. Unscoped creators gain nothing: they see every client already, and a roster entry would change what assignedClients means for them. Nothing is attached retroactively. The widening this involves is self-limited: the only thing added is an account the creator just made, which starts with no files, no folders and no group memberships, so assignableClientIds gains nothing to reach. Seats do not move either, since they are counted from active and account_requested. Verified before merging: 34 passed across both suites on the trial-merge, 2 failed / 32 passed with app/ reset. This is the busiest file set of the series -- it shares ClientsController with #1718 and Api/ClientsController plus the API test file with #1723 -- so the merged result was read rather than trusted: #1718's reassign_candidates gating and #1723's patchCustomFieldValues are both intact alongside it. scramble:export reproduces the committed docs/api/openapi.json byte for byte. Reported and fixed by @denkfabrik-li. |
||
|
ignacionelson
|
6b99e37d01 |
Merge pull request #1729 from denkfabrik-li/fix/api-surface-by-route
Two places asked "is this the API?", and each got it wrong in the opposite direction.
EnsureCapability asked $request->expectsJson(). Whether a feature exists in this installation's edition is a property of the installation, not of what the caller is willing to parse: the same capability-gated API route answered 403 capability_unavailable to Accept: application/json and a bare 404 to Accept: */*, which is curl's default, while routes/api.php promises the 403 in as many words. The mirror image was worse -- an Inertia visit to a capability-gated web screen accepts JSON, so it took the API branch and announced the feature by name, where the whole point of the 404 is that an unavailable feature is absent rather than teased.
ProblemDetails asked $request->is('api/*'). Two staff pages live under that prefix -- the API dashboard at /api and the OpenAPI reference at /api/docs, both from routes/web.php -- so a signed-out visitor to /api/docs got a 401 problem+json telling them to send a Bearer token instead of the login redirect every other page gives.
One question now, asked once, in App\Support\ApiSurface: under the API prefix, and not part of the web middleware group. The group is what actually separates the two surfaces -- sessions, cookies and CSRF on one side, tokens on the other -- and it keeps answering correctly for a future /api/v2 without being edited. An unmatched path has no route to ask and stays the API's answer, which is what the existing "a missing API route is a problem+json 404" test pins. EnsureStaff keeps its expectsJson() check: there the question really is about the caller.
Verified before merging: the discriminator was checked in the running application rather than assumed -- /api/docs and /api resolve to [web, auth, staff], /api/v1/files to [api, auth:sanctum, api-active, staff-token, token-can:...]. 12 passed on the trial-merge; with app/ reset and ApiSurface deleted, 3 failed / 9 passed, every new test and no old one. Wider suites green: tests/Feature/Api 239 passed, tests/Feature/Platform 485 passed. scramble:export reproduces main's docs/api/openapi.json byte for byte. Worth recording that the blast radius here is the shape of a refusal and never whether one happens: both call sites run after authentication and authorization.
Reported and fixed by @denkfabrik-li.
|
||
|
ignacionelson
|
eb3d6e321d |
Merge pull request #1727 from denkfabrik-li/fix/api-expiry-end-of-day
FilesController::expiryInstant() exists because a calendar day ends where the person naming it lives: the web form posts a bare YYYY-MM-DD, which Eloquent would otherwise store as midnight UTC, so "expires on the 12th" would cut the file off partway through the 11th for anyone in the Americas. PATCH /api/v1/files/{id} took the same field, validated it as a date, and stored it exactly as it arrived -- so the same value that meant end-of-the-12th on the web meant start-of-the-12th over the API, and earlier still for a caller west of Greenwich.
A bare YYYY-MM-DD now means the end of that day in the caller's timezone, through the same LocalDay::end() the web path uses. A value carrying a time is unchanged: that is an instant the caller named on purpose, the API can express one where a date input cannot, and it is stored as it arrives. null still clears the expiry, and the validation rule and permission gate are untouched.
Note for the release notes: this lengthens the life of a file whose expiry an existing integration sets with a bare date, by up to a day. That is the correct meaning and the one the web has always had, but it is a behaviour change for callers who were relying on the old one.
Verified before merging: 19 passed on the trial-merge, 1 failed / 18 passed with app/ reset. The timestamp and clearing tests are green either way. The bare-date branch is gated on a strict ^\d{4}-\d{2}-\d{2}$ match, so nothing else takes it. scramble:export on the merged tree reproduces the committed docs/api/openapi.json byte for byte.
Reported and fixed by @denkfabrik-li.
|
||
|
ignacionelson
|
262cb2457a |
Merge pull request #1723 from denkfabrik-li/fix/api-patch-custom-fields
Api\ClientsController::update() states the rule eighteen lines above the bug: "PATCH semantics, unlike the web form which always submits every field: an absent key means 'leave alone', not 'clear'." Every column obeyed it. The custom fields did not -- they went through saveCustomFieldValues(), which is create()'s pass: it walks every field there is and writes null for the ones the request did not carry. A PATCH naming one field emptied all the others, with nothing in the response to say so and no second copy of the value anywhere. The write pass is still shared but is now entered two ways: create() keeps writing every field, and update() writes only the fields the request named. Creating a client is deliberately unchanged -- it is not a partial update, and a checkbox nobody ticked is a recorded "no" rather than an absent row. Clearing a field by naming it with an empty value still clears it, and the validation rules are untouched. Verified before merging: 23 passed on the trial-merge, 1 failed / 22 passed with app/ reset. The two guard tests -- a named empty value still clears, create still records every field -- are green either way, so the write path was not simply switched off. The keys reaching whereIn() are stripped to real field ids by validateCustomFieldValues() before they get there. scramble:export re-run on the merged tree produces a docs/api/openapi.json identical to main's, so the published spec does not move. Reported and fixed by @denkfabrik-li. |
||
|
ignacionelson
|
bc68a24ef5 |
Merge pull request #1721 from denkfabrik-li/fix/api-dashboard-activity-log-scope
ApiUsage::recentActions() read the activity log without ActivityLogScope::apply(). It was the only ActivityLog::query() outside ActivityLogger and AccountEraser that skipped it. Its only boundary was view_actions_log -- the permission ActivityLogScope's own docblock says "is not the whole answer for a client-scoped staff member", because a log row carries the subject's name. The Client Manager system role is client_scoped and ships with that permission, so this was the default configuration and not an exotic one: the same person who gets a 403 on a file and an empty /activity read that file's name off /api?all=1. ApiUsage now takes ActivityLogScope and applies it to the recent-actions query, on both sides of the install-wide branch rather than only in the install-wide arm -- the own-actor filter already stays inside what the scope allows, and a boundary that exists in only one arm of an if is one refactor away from not existing. The token inventory, request counts and endpoint table keep ApiUsageScope alone: those rows are about the viewer's own credentials rather than library content. Verified before merging: 17 passed on the trial-merge and 1 failed / 16 passed with app/ reset. The two tests guarding against narrowing further than /activity does -- a viewer's own actions stay whole, an unscoped viewer's feed is unchanged -- are green either way. ActivityLogScope::apply() wraps its conditions in a single where(Closure), so it composes with the origin and actor_id filters around it without a precedence trap, and ApiUsage is never constructed with new, so the added dependency is wired by the container everywhere. Reported and fixed by @denkfabrik-li. |
||
|
ignacionelson
|
479dc61d2d |
Move branding into core, and leave white-labelling behind
Logo and watermark belonged in the private package for one reason: that is where they were written. Nothing about them needs a hosted platform, and an installation wanting its own mark on the pages it serves is the ordinary case rather than the exotic one. They are core's now, and every installation has them. Hiding "Powered by ProjectSend" did not come. That is what a hosted customer pays for, and its gate is not a capability key but the absence of the code: cloud-modules keeps the listener, so an installation without that package holds the column and has nothing able to read it. Flipping an edition variable buys nothing, which was true before and stays true. Core renders the switch where Capability::AttributionHide is held and has no route that can save it -- there is a test asserting exactly that, which fails the day white-labelling quietly becomes free. The migrations move with their original filenames on purpose. A Cloud tenant already ran them under those names, so Laravel skips them there and the table and its data are untouched; a fresh install or a community one runs them from here for the first time. What got better on the way rather than merely moving: The watermark listeners take core's real RenderingImage and ResolvingImageRendering instead of duck-typed `object` payloads, and the tests construct the genuine events rather than anonymous stand-ins that imitated their shape. The package had to do it that way -- it builds with no host present -- so three PHPStan ignore entries existed to describe what the type system could not see. They are gone. ModuleBoundaryTest asserted "branding is cloud-only, and the suite runs as community", which was never what it was testing. It now reads the capability off the route and subtracts it, so the invariant holds for whichever module is installed. The 43 branding strings arrived in all sixteen locales from the package's own catalogues rather than being retranslated, and the package's are pruned to the one string it still uses. A hosted plan without branding subtracts branding.customize and attribution.hide from the instance's environment. The row is never deleted by that: a downgrade is usually an expired card rather than a decision, and wiping somebody's artwork over a billing event is a loss they would find weeks later with no way to know what it used to be. Hiding reverses; deleting does not. |
||
|
denkfabrik-li
|
776d3d99f4 |
Put a client on the roster of the scoped staff member who created them
A client-scoped staff member with create_clients creates a client and loses it immediately. guardTarget() answers 404 for anything off their roster, and StaffLibraryScope::clients() leaves it out of their list -- so the record exists, is logged, is welcomed by email, and is invisible to the person who made it. store() redirects to the edit page, which is where they land: POST /clients → 302 → /clients/4 on_creator_roster → false GET /clients/4/edit → 404 clients listed → ["Mine"] the new client is not there Their own roster is where a client they created belongs, so it is attached there. An unscoped creator gains nothing: they see every client already, and a roster entry would change what assignedClients means for them. The API twin does the same, for the same reason -- a scoped token gets a 404 from every route that binds the client it just created. Three tests: the scoped creator can open and list the client, an unscoped creator gains no roster entry, and the API twin behaves like the web. The first and third go red without the fix. |
||
|
denkfabrik-li
|
250e8664d3 |
Stop a client PATCH clearing custom fields it never mentioned
update() states the rule eighteen lines above the bug: "PATCH semantics, unlike the web form which always submits every field: an absent key means 'leave alone', not 'clear'." Every column obeys it. The custom fields did not, because they went through create()'s pass, which walks every field there is and writes null for the ones the request did not carry. Two fields filled, a PATCH naming one: status → 200 named field → "Robin" the other one → null (was "ATU12345678") Nothing says so in the response, and there is no other copy of the value. The write pass is now shared but entered two ways: create() keeps writing every field, since a new client has no values and a checkbox nobody ticked is a recorded "no"; update() writes only the fields the request named. Three tests: the untouched field survives, a named empty value still clears, and create still records every field. Without the fix the first goes red. |
||
|
denkfabrik-li
|
e1cd010f9d |
Give an API expiry date the same meaning the web gives it
FilesController::expiryInstant exists because a calendar day ends where the person naming it lives: the web form posts a bare YYYY-MM-DD, and storing that as it arrives would cut a file off at midnight UTC -- "expires on the 12th" ending partway through the 11th for anyone in the Americas. The API takes the same field, validates it as a date, and stores it raw: web → 2026-09-12T23:59:59+00:00 (end of the day, as the docblock means) API → 2026-09-12T00:00:00+00:00 (raw) Same value, same field, same file, two meanings -- and the earlier of the two is a file that dies at the start of the day it was promised. A bare date now means the end of that day in the caller's timezone, as it does on the web. A value carrying a time is unchanged: it is an instant the caller named on purpose, the API can express one and a date input cannot. The endpoint's docblock says both, so the OpenAPI document does too. Three tests: the day, the timestamp, and clearing. Without the fix the first goes red. |
||
|
denkfabrik-li
|
f424fe5365 |
Decide what is an API request from the route, not from the caller's headers
Two places asked "is this the API?" and got it wrong in opposite ways.
EnsureCapability asked $request->expectsJson(). Whether a feature exists
in this installation's edition is a property of the installation, not of
what the caller is willing to parse, so the same route answered
differently per header: `Accept: application/json` got the 403
`capability_unavailable` routes/api.php promises, `Accept: */*` -- curl's
default -- got a bare 404 `not_found`. The mirror image is worse: an
Inertia visit to a capability-gated *web* screen accepts JSON, so it got
403 with Laravel's default error body, naming the exception class, where
the point of the 404 is that an unavailable feature is absent rather than
teased.
ProblemDetails asked $request->is('api/*'). Two staff pages live under
that prefix -- the API dashboard at /api and the OpenAPI reference at
/api/docs, both registered in routes/web.php -- so a signed-out visitor to
either got 401 problem+json, "Send a valid API token in the Authorization
header as \"Bearer <token>\"", instead of the login redirect every other
page gives them.
Both now ask App\Support\ApiSurface: under the API prefix, and not part of
the `web` middleware group. The group is what actually separates the two
-- sessions and CSRF on one side, tokens on the other -- and it keeps
answering correctly for a future /api/v2 without being edited. An
unmatched path has no route to ask, which is the API's answer anyway: a
404 under its prefix is one it should describe in its own format, and the
existing test for that stays green.
Three tests, in the two files that already own these rules. Without the
fix all three go red.
|
||
|
denkfabrik-li
|
84e9f6e2fe |
Scope the API dashboard's recent actions to what the viewer may read
ApiUsage::recentActions() is the only ActivityLog query outside ActivityLogger and AccountEraser that does not run through ActivityLogScope::apply(). Its whole boundary is view_actions_log -- the permission whose own scope class says, in as many words, that it "is not the whole answer for a client-scoped staff member". The Client Manager system role is client_scoped and ships with that permission, so this is the default configuration. Such a viewer opening /api?all=1 reads the fifteen most recent API log rows for the entire installation, each with its subject_name: the names of files and clients they get a 403 on. /activity, the download history and the dashboard's recent-activity widget all narrow the same rows; the API dashboard was missed. The scope is applied on both sides of the install-wide branch. The own-actor filter for the narrow view already stays inside what the scope allows, and a boundary that exists in only one arm of an `if` is one refactor away from not existing. Three tests: the scoped viewer sees only the entry about a file in their library, their own actions stay whole even when the subject is outside it, and an unscoped viewer's feed is unchanged. Without the fix the first goes red; the other two are green either way and guard against narrowing too far. |
||
|
denkfabrik-li
|
3e15237f90 |
Refuse self-deactivation over the API however the boolean is written
Api\UsersController::update() compares the validated value strictly:
if ($user->is($actor) && ($validated['active'] ?? true) === false) {
The `boolean` rule accepts 0 and "0" as well as false, and it does not
cast. `0 === false` is false, so the refusal never fires -- and the
model's own `boolean` cast then stores as false exactly the value the
guard had just decided was not a deactivation.
Measured against main, with a second administrator present so that
guardLastAdministrator is not what answers:
{"active": false} -> 422, still active
{"active": 0} -> 200, active is now false
{"active": "0"} -> 200, active is now false
The method's own docblock says it is "Refused with a 422 if the change
would leave the installation with no active administrator, or if you
would be deactivating yourself", and the web screen does refuse. This is
the API half of that sentence.
RolesController::guardScopeRemoval documents the rule this breaks, in the
same words: callers resolve the flag with Request::boolean() and hand the
same value to the guard and to the write, deliberately, because reading
the validated array and comparing it strictly "would let a request
through here that the model's `boolean` cast then stores as false anyway
-- the guard and the write disagreeing about one value is exactly the
shape this guard exists to prevent".
So read it once, with Request::boolean(), and give that one value to both.
Not changed: the validation rule. It stays `boolean`, so the accepted
inputs are the same as before -- what changes is that one of them stops
meaning two different things on its way through. Nor anything about
deactivating somebody else: all three forms still work, and there are
tests saying so.
Six cases from two datasets. Two measured red against the unfixed
controller (2 failed / 4 passed): 0 and "0" on yourself. `false` was
already refused, and the three "somebody else" cases are green either way
-- they guard against the fix over-refusing, not against the bug.
Full suite passes (2054 passed / 2 skipped), PHPStan level 8 clean.
|
||
|
ignacionelson
|
623ad686da |
Open user management on the cloud edition
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. |
||
|
ignacionelson
|
553f5fd2bf |
Declare the capability a managed installation's staff seats hang off
Cloud instances are sold seats rather than administering them, so the tenant's own /users screens stay closed — capability:users.manage is already Community-only — and a control plane creates, deactivates and password-resets staff from outside. This is the key that plane gates on. Only the declaration lives here, the same division StorageManaged and Branding already use. Everything behind it is a module in the private cloud-modules package. Declared before that module exists, deliberately. A capability added after a release is invisible to every image built from one, and that is not hypothetical: StorageManaged landed 36 commits after v2.1.0 and has never shipped, so a fleet with buckets provisioned, credentials scoped and eight environment variables in place still writes every upload to local disk — because the gate is here and the gate never left. Declaring this one now is refusing to make the same mistake twice. The seat *number* deliberately does not live here. There are no billing or plan tiers in this application to key off, which is the reason config/api.php gives for not inventing an installation-level rate limit, and it holds for the same reason: the number lives where the plans do. This capability says only who is in charge. ModuleBoundaryTest grows the other half of its own rule. It filtered on `api/v1/`, so a package claiming a route anywhere else passed — not because that was sanctioned, but because nothing was looking, and /platform/v1 is about to be somewhere else. What it polices now is machine surfaces, the roots something other than a browser authenticates to, with api/v1/modules and platform/v1 as the two sanctioned prefixes. Written twice, because the first version was wrong in a useful way: it policed every route and immediately caught community-modules' Custom Assets screens. Those are a module doing exactly what a module is for, through the host's session and capability middleware in plain sight, and listing them would be the hardcoded URI list the test above it explains it is avoiding. Web screens are not the boundary; trusted perimeters are. Verified by making it fail: a package controller on platform/v2 is caught and named. |
||
|
ignacionelson
|
9fc5042f4e |
Merge pull request #1688 from denkfabrik-li/fix/atomic-account-deletion
Delete an account and dispose of its content in one transaction Resolved the conflict with #1678 the way that PR's merge note predicted: the erasure stamp goes inside the new transaction, so a deletion that rolls back cannot leave a live account carrying a date on which it would be erased. |
||
|
ignacionelson
|
5c00d189e4 |
Deletions can start a Zap after all
I said they could not, in the guide, the Zapier page and the changelog. That was carried over from the limitation of polling a list, where a deleted row stops being returned and nothing marks the moment it went. It was never true of the activity endpoint: file.deleted is recorded like any other action, so ?action[]=file.deleted works today. Checked with a test rather than the enum, which turned up the shape a caller needs: a deletion entry has no subject, because the row is gone by the time the entry is written, so the name is snapshotted into context.name instead. Reading subject.name there gets you null. |
||
|
ignacionelson
|
7646e99f33 |
Add an activity endpoint, so an integration can react rather than poll for shape
Every list in /api/v1 answers "what is there now". Nothing answered "what happened", and for the two events people most want to act on there was nowhere to look at all. Sharing a file writes an assignment row and never touches the file, so no amount of polling /files?updated_since= will ever show a share. A download is recorded only in the activity log. So the most requested automations for a file-sharing product — tell me when a client gets a file, tell me when they open it — were not possible to build. GET /api/v1/activity is one feed rather than one endpoint per event, because the log already records every one of them and a caller filtering by action gets whatever the application grows later without waiting for us to expose it. It reuses what already exists: view_actions_log is the permission the activity screen uses, and ActivityLogScope narrows the rows the same way, so a staff member limited to their assigned clients cannot read the whole installation's log through a token when the screen would not show it. Two deliberate limits. Class names never reach the wire — subject.type is a stable public string, or moving a model between namespaces would be a breaking change to a frozen contract. And no ip_address, though the column exists and the screen shows it: a person looking at a log has decided to look, where an integration streams every row to somebody else's servers by default. PollingQuery grew an optional column so it can walk a table that is appended to rather than edited. The parameter stays updated_since everywhere, because on an append-only log the two timestamps are the same thing and one shape learned once is worth more than a second name. |
||
|
denkfabrik-li
|
61c385e423 |
Delete an account and dispose of its content in one transaction
Deleting a staff or client account is two writes: soft-delete the account, then cascade or reassign the files and folders it owns. All four destroy() paths (Users + Clients, web + API) ran them one after the other with nothing tying them together. If the second write throws, the account is already gone but its content is not handled. The concrete way in is the reassign branch: validate() checks reassign_to_id with exists(active), but apply() re-resolves it with findOrFail() a moment later (AccountContentDeletion:108), so a target deactivated or deleted in between throws — leaving a soft-deleted account whose files still point at it, and a UserDeleted log for a deletion that did not finish. Wrap the delete()+apply() pair in a single DB::transaction() in each of the four destroy() methods. validate() and the authorization guards stay outside it: they are read-only and must be able to reject before anything is written. cascadeDelete()/reassignTo() already open their own transaction, which nests as a savepoint under this one, so the account soft-delete, its activity log, and the content work now commit or roll back together. Tests: a DeletedAccountContent double that reports content to handle and then throws while handling it (tests/Helpers.php) drives one test per destroy() endpoint asserting the account survives the failure and no UserDeleted entry is written; each goes red against the un-wrapped controller. |
||
|
ignacionelson
|
6e47d76ba6 |
ProjectSend 2.0.0
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. |