Commit Graph

129 Commits

Author SHA1 Message Date
denkfabrik-li eade83a576 Translate the finalising-refusal message into all sixteen languages 2026-08-25 22:41:43 +02:00
denkfabrik-li 329aef98e0 Finalise each chunked upload once, under a per-session lock
complete() assembled the received parts into the one target file and
created the File row with no guard against a second complete() for the
same session running at the same time -- an Uppy retry, a double submit,
a resend after a lost connection. Two of them would interleave writes
into the session's single `assembled` file (the stored bytes then no
longer match the checksum computed from the in-memory buffers) and could
each create a File row.

Take a per-session lock around the finalisation and fail a second caller
fast; the lock's TTL releases the claim if a completion dies mid-flight,
so a genuine retry still works. The body moves to a finalise() helper so
complete() reads as auth + lock + finalise.
2026-08-25 22:13:46 +02:00
denkfabrik-li 71d6b8937e Enforce the max file size against the bytes a chunked upload assembles
store() checks Setting::MaxFileSizeMb against the size the client declares
when it opens the session, and complete() re-checks the storage quota
against the real assembled byte count -- but nothing re-checked the size
limit itself. A client that declared a one-byte upload and then streamed
gigabytes of parts passed store()'s check and was never stopped, so the
configured limit (which store() applies to everyone, staff included) did
not hold for the resumable path that real uploads use.

Re-check the assembled byte count against MaxFileSizeMb in complete(),
cleaning up the assembled bytes and the session exactly as the quota
branch already does.
2026-08-25 20:32:13 +02:00
ignacionelson 6ab90aee79 Say plainly that deleting an account is two calls, not one
StaffAccounts::delete() soft-deletes the row. What happens to the files
and folders that account owns is a separate collaborator, and a caller
that stops at the first one leaves them pointing at an account that no
longer exists.

The docblock mentioned "the content-reassignment step" in passing, as
context for the return value, which is not the same as saying it is
required. Worth stating outright because the mistake hides: validate()
returns an empty array when the account owns nothing, so an account with
no files deletes perfectly through delete() alone, and keeps doing so
until somebody deletes a colleague who had actually done some work.

Found while reviewing a design that was about to call delete() on its
own.
2026-08-25 14:22:25 -03:00
ignacionelson 91d34b204c Let something other than a browser session identify itself to the audit log
An actor with no personal access token has always meant a browser, and
for as long as a session and a Sanctum token were the only two ways to
authenticate, that was true. It stops being true the moment anything else
can, and the failure is silent: the action gets recorded as a person
clicking, in the one table whose whole purpose is answering "did I do
that, or did something acting for me?"

Nothing misreports today — every call site that passes an explicit actor
is a browser request, an API request whose actor carries the token, or a
console command with no actor at all. This closes the trap before the AI
connector in cloud-modules walks into it.

ActivityOrigin is a closed enum, so core has to publish both the case and
the hook before a package can use either. ResolvingActivityOrigin is
asked only in the ambiguous case: a request carrying a token is the API
and a request with nobody signed in is public or system, and neither is
in any doubt, so neither is offered — one package must not be able to
quietly relabel how every integration's actions are attributed.

The person stays the actor. They authorised it, and a log naming the
assistant instead would lose the only fact that matters. What the
connector was called goes in api_token_name, beside a null token id,
because that column means a row in personal_access_tokens and this is not
one.

The new origin is kept out of the activity filter unless the edition can
actually produce it. A filter option that can only ever return nothing is
a feature dangled at an edition that does not have it, which is the one
thing the edition boundary exists not to do.
2026-08-25 14:19:32 -03:00
ignacionelson 73533910b0 Move the API documentation tabs to the top, endpoints last
Having the endpoint table always visible with the tab strip halfway down
the page made the two prose documents look like a footnote to the table,
and it was not obvious there was anything to switch between.

One tab strip, directly under the heading, three views: the guide first
because it is what someone arriving here usually wants, then Zapier, then
the endpoint table. Nothing else changed.
2026-08-25 01:21:26 -03:00
ignacionelson 7059ee293a Add a Zapier guide to the API documentation page
The API has had everything Zapier needs since it shipped — a bearer
token, an auth-test endpoint at /me, and list endpoints that return
newest-first with a stable id, which is exactly the shape a polling
trigger wants. What was missing was anyone saying so.

This is written for somebody wiring up a Zap, not for somebody writing
code, which is why it is a second document rather than a section of the
guide. Same reason it renders in-app rather than linking to GitHub: the
installations most likely to need it are the ones least likely to have
outbound internet access.

It says out loud the two things that will otherwise be discovered the
hard way — a token expires within a year and nothing renews it, and a
deletion cannot start a Zap because polling cannot see one.
2026-08-25 01:08:09 -03:00
ignacionelson 5e3ea5a48b Translate the OAuth mail strings into all sixteen languages
The 21 strings the Microsoft 365 and Gmail transports added in #1679.
Appended, so the diff is only what is new.

The two provider names stay in English on purpose — they are product
nouns, and will read as untranslated in the scanner the same way API and
OK do. The tenant-ID placeholder sits in a fixed-width input, so each
translation of it is kept to roughly the length of the English rather
than the length the sentence wants to be; a fuller Spanish rendering was
already truncating on screen.
2026-08-25 00:32:42 -03:00
ignacionelson eda8091aef Serialise OAuth token refreshes, and don't offer Connect for an unsaved provider
Both fixes are follow-ups to the mail providers @denkfabrik-li added in #1679.

A refresh token is good for exactly one use — Microsoft and Google both
retire it as they issue the next one. Two queue workers finding the same
expired access token would therefore both spend it, and the loser gets
invalid_grant back. That is the same answer a revoked grant gives, so a
healthy connection would be marked broken, painted red on the settings
page and mailed to every admin. Refreshes now hold a per-connection lock
and whoever waits re-reads the row, which normally means finding a token
the winner already stored and not refreshing at all.

The Connect button read the provider dropdown, but the flow it starts
uses the saved provider. On an installation with both vendors registered,
switching without saving would open the wrong consent screen. The
dropdown now counts as an unsaved change like any other field, which also
gives it the right "save first" hint for free.
2026-08-25 00:24:48 -03:00
Ignacio Nelson 6147b2721e Merge pull request #1679 from denkfabrik-li/feature/oauth-mail-providers
Send email through Microsoft 365 or Gmail as an admin-connected mailbox
2026-08-25 00:22:26 -03:00
ignacionelson 4f4fb92b85 Group the Settings menu by subject instead of by permission
The conditional entries were pushed onto the end of the list after the
unconditional ones, so where an item appeared depended on whether it
needed a capability rather than on what it was about: Storage sat under
Languages, Email templates sat nowhere near Email, and Scheduler landed
between Branding and About.

Now there is one ordered list and each entry carries its own condition,
so the order survives whatever the edition and permissions turn on.
2026-08-25 00:12:34 -03:00
ignacionelson 1b3abd28b7 Merge branch 'main' into feature/oauth-mail-providers
Both sides added a .gitignore rule in the same place: this branch's
exception for docs/email-oauth.md, and main's block for the local dev
TLS material. Keep both.
2026-08-25 00:07:40 -03:00
ignacionelson a14ff0c837 Tell people the proxy fix happened
#1674 changed the code, the test and both install guides, and stopped
there — so the release notes would have shipped without the one fix that
closed #1672, and the person who reported it would have read them and
found nothing.

Written from the symptom rather than the cause: nobody searching the
changelog is looking for "reads TRUSTED_PROXIES at the wrong point in
the boot sequence", they are looking for why signing in behind Traefik
gave them an error. The upgrade note names config:cache, because a
proxy install that runs it puts the bug straight back.
2026-08-24 20:48:12 -03:00
ignacionelson 6086821d6c Close the other three doors that answered a write with a 302
#1680 fixed the redirect rendered from an exception and said plainly
what it did not cover: EnsureSetupIsComplete, EnsureAccountIsActive and
EnforceTwoFactor answer before HandleInertiaRequests is ever entered, so
a response they return never unwinds through Inertia's 302 to 303
upgrade either. Same 405, reached a different way — an account
deactivated while its owner was part-way through a form, or one being
made to enrol in two-factor.

The rule now lives in one place rather than four. Three copies of "if
the method is PUT, PATCH or DELETE" is how the fourth caller gets it
wrong, and WriteSafeRedirect can carry the explanation of why 303 —
which is worth more than the three lines it replaces, because nothing
about a bare setStatusCode call says what a browser does with a 302.

PUT /timezone is the route the setup test uses: it is one of only two
writes a guest can reach and the only one that middleware does not
exempt, so the case is real rather than defensive. All three new tests
were run against the unfixed middleware and fail there.

Extends the work of @denkfabrik-li, who found the gap and wrote it down.
2026-08-24 20:31:50 -03:00
Ignacio Nelson 30f08fdd3b A write that meets an expired session lands on the login page
A write that meets an expired session lands on the login page

A browser follows a 302 by replaying the request method, POST aside, so
a widget save whose session had expired replayed as PUT /login — and
/login takes GET and POST only. The 405 that came back hid the one
thing the person needed to be told, which was to sign in again.

Inertia already upgrades 302 to 303 for writes, but a redirect rendered
during exception handling never travels back through the middleware
stack, so the guest redirect after AuthenticationException was never
reached. bootstrap/app.php now does it for those.

Fixes #1673, reported by @mstewart14. Found, diagnosed and fixed by
@denkfabrik-li, who also wrote down the part this does not cover: the
direct redirects from EnsureSetupIsComplete, EnsureAccountIsActive and
EnforceTwoFactor sit outside HandleInertiaRequests and can still produce
the same 405 on a write.
2026-08-24 20:23:49 -03:00
ignacionelson a8f5fa5e10 Update .gitignore 2026-08-24 20:21:31 -03:00
ignacionelson ce487da19a Keep other people's warnings out of our users' consoles
React strips its own development warnings from a production build;
several of our dependencies do not. Radix emits an accessibility warning
for every dialog it considers underdescribed, and it was reaching
anyone who opened devtools on a real installation — advice aimed at
whoever builds the software, shown to whoever uses it.

console.error is deliberately left in. Something has genuinely gone
wrong when it fires, and a support conversation that opens with a real
stack trace is worth more than a tidy console. Only the advisory levels
are dropped, and only from production: npm run dev still shows
everything, which is where those warnings are useful.

The bundle goes from 12 console.log and 18 console.warn to none and two
— the survivors being a Recharts truthiness guard and Uppy's logger
object, neither of which speaks unless asked. What this does not do is
fix what Radix was complaining about: sixteen of twenty-three dialogs
have no description, which is a real accessibility gap and its own
piece of work.
2026-08-24 20:11:48 -03:00
ignacionelson a86017f2c2 Merge: Google Cloud Storage as a storage backend
External storage stops meaning S3 and nothing else. The Storage settings
screen asks which provider first, and Google Cloud Storage sits beside
the S3-compatible option for every self-hosted installation; the hosted
edition is handed a bucket by its environment instead, through a
capability declared here and behaviour that lives in cloud-modules.

Three of the bugs fixed here predate the feature and affect S3 users
today: share links and public group thumbnails both assumed the local
disk, and an upload the storage backend refused was recorded as if it
had been stored.

Verified against a live bucket, not only against tests — which is how
the last two were found.
2026-08-24 20:06:02 -03:00
ignacionelson 55e17498a2 Log which bucket an upload could not be written to
The failure message names the disk, which reads as a credentials problem
even when the real cause is a bucket name that was never changed — the
exact confusion produced by switching an existing S3 configuration over
to Google and leaving the old bucket in the field.

Logged rather than shown, because 'throw' => false means the reason is
already gone by the time this code runs, and because the message goes to
whoever was uploading. That can be a client, and a bucket name is not
theirs to see.
2026-08-24 19:56:10 -03:00
ignacionelson a459d45c87 Store the bytes, or say you did not
Two bugs a green suite could not find, both from pointing the
application at a real Google Cloud Storage bucket.

The adapter attaches a legacy per-object ACL to every write, and a
bucket with uniform bucket-level access — which our own setup
instructions require, and which Google recommends — refuses it:
"Cannot insert legacy ACL for an object when uniform bucket-level access
is enabled". So the default configuration could not write to the
recommended bucket. The library ships
UniformBucketLevelAccessVisibility for exactly this, and nothing is
lost by never setting an ACL: every object here is private and every
read is a signed URL.

The second is worse and was never about Google. Both file disks are
configured 'throw' => false, so a refused write returns false rather
than raising, and LocalPartStore ignored the return. The upload reported
success, the File row was written, and the bytes were nowhere — the
listing showed a file whose download could never work. An expired S3
credential did the same thing. It now checks, and the controller already
turns that into a validation error rather than a 500, so the person
uploading is told.

Verified against a live bucket with a key scoped to
roles/storage.objectAdmin: the probe lists, writes land, reads
round-trip byte for byte, and a signed URL comes back 200 carrying
"Informe año.pdf" intact through both the ASCII and RFC 8187 forms of
Content-Disposition.
2026-08-24 19:52:05 -03:00
ignacionelson b22d3cf33c Translate the storage provider strings into all sixteen locales
Eight new strings from the Google Cloud Storage work, and nothing else:
the scan reported the same eight missing everywhere, so this is a
translation pass rather than a backlog.

Each locale keeps the word for a bucket it was already using — kova in
Turkish, бакет in Russian, 存储桶 in Chinese — and its own level of
formality, Sie in German and vous in French against tú in Spanish and
Italian. "Google Cloud Storage" is a product name and stays as it is in
all sixteen, the way API and OK already do. The :field placeholder
survives verbatim, which is asserted rather than assumed.

Entries are inserted in place rather than appended, so each file shows
eight added lines and nothing else moved. Verified by re-running the
scan to zero missing, the Locale suite, and reading the settings screen
in Spanish in a browser — a file that parses is not evidence that a
sentence fits its button.
2026-08-24 19:40:58 -03:00
ignacionelson f7db586c7e Say that files can live in Google Cloud Storage too
Three lines still told readers S3 was the only option, which stopped
being true and is the sort of thing somebody chooses a different product
over. The install guide's storage section now says what each backend is
for, that Test connection exists and is worth using before switching
uploads over, and — the part people actually get wrong — that choosing a
backend applies to new uploads and moves nothing that is already stored.
2026-08-24 19:40:58 -03:00
ignacionelson 23b7dc0d11 Declare the capability a managed installation's storage hangs off
Cloud instances are given a bucket rather than configuring one, which is
the counterpart of StorageConfigure above it rather than a contradiction
of it: one edition points itself at storage, the other is pointed.

Only the declaration lives here. The behaviour is in the private
cloud-modules package, the same division Branding already uses, and
without that package the capability is inert and files stay on local
disk — so a self-hosted installation that somehow holds it is unchanged.
2026-08-24 18:35:44 -03:00
denkfabrik-li 4737849ec5 Answer redirected writes with 303 so browsers follow with GET
A redirect born in exception handling - the guest redirect after an
expired login, above all - never travels back through the middleware
stack, so Inertia's usual 302-to-303 upgrade cannot reach it. Browsers
follow a 302 by replaying the request method on the redirect target
(only POST is downgraded to GET), so a widget save whose session just
died replays as PUT /login and fails with a 405 that hides the real
"please sign in again" (#1673).

Repeat the upgrade in the exception pipeline: any 302 answered to a
PUT, PATCH or DELETE becomes a 303. Reads keep their 302, POST needs
nothing - browsers already downgrade it.
2026-08-24 23:33:55 +02:00
ignacionelson daec0a877e Offer Google Cloud Storage as a storage backend
External storage meant S3 and nothing else, which is an odd hole for a
product whose users are as likely to be standing on Google Cloud as on
AWS — and paying to move bytes between two clouds to use this. The
Storage screen now asks which provider first, and the answer decides
which fields it shows, which it validates, and which driver the
files_external disk resolves to.

One disk, not two. files.disk is a stored column, so a third disk name
would fragment the data model and make every $file->disk consumer know
three names instead of two; the driver is swapped instead. A service
account key gets its own encrypted column rather than sharing `secret`,
because the two are validated, labelled and displayed differently and
one column meaning two things is how that goes wrong later.

Three things do not work by simply adding the adapter, and all three
fail quietly:

Laravel's temporaryUrl() looks for getTemporaryUrl() on the adapter,
while League's GCS adapter names it temporaryUrl(), so without the
registered callback every download and preview is a 500.

The two SDKs spell the signing options differently, and an unrecognised
one is dropped in silence — the symptom is a download named after the
storage key, not an exception. GoogleCloudStorageDriver translates, so
callers keep speaking one vocabulary, and the test asserts on the URL's
contents rather than on "a redirect happened", which is what would let
it regress.

That callback is also re-bound to the FilesystemAdapter before it runs,
so the translation is captured before registering rather than called as
$this->

`provider` is validated with 'sometimes', not 'required': absent means
S3, which is what every payload written before this choice meant, and
stops a browser holding a stale bundle from failing to save on a field
it cannot see.

Verified in a browser as well as in tests — which is how the null
provider on an unmigrated row was found, since the suite migrates and
never sees that state.
2026-08-24 16:38:13 -03:00
ignacionelson 57540164fa Read a file from the disk it is actually on, everywhere
Two routes still assumed every file sits on local disk, which stopped
being true the moment external storage was switched on. A share link
answered with X-Accel-Redirect whatever the file's disk said, pointing
nginx at a path it has nothing behind; a public listing built a
thumbnail from Storage::disk('files')->path(), which for an externally
stored file is a path nobody ever wrote. Both fail only for installs
using S3, and only on those two routes, so the same file downloading
correctly from the file manager made the share link look like the
broken thing rather than where the file lives.

Neither is a new rule. FileDownloadController and
FileThumbnailController already did it right, which is the actual
finding: the knowledge was sitting in a private method on one class and
inline in another, so the next caller could not inherit it and did not.
Both are now objects with one job.

StoredFileResponse replaces InlineFileResponse and grows an
attachment() alongside inline(), since the two differ only by
disposition. LocalSourceFile takes a closure rather than returning a
path: the version that returned one also left the caller to unlink it,
and both of those are exactly the mistakes made here.

The regression tests fail against the previous controllers — checked in
both directions rather than assumed.
2026-08-24 16:24:39 -03:00
denkfabrik-li 8b59bb2a5a Move fakeIdToken() to the shared test helpers
It is used by both OAuth mail test files, which --parallel runs in
separate processes — exactly the situation tests/Helpers.php exists
for, as its own header explains. CI caught what a whole-suite serial
run hides.
2026-08-24 09:56:04 +02:00
denkfabrik-li 6a8a287984 Name the tab Outgoing email, not Sending
Discussion feedback: in ProjectSend "sending" can just as well mean
files. Outgoing email is unambiguous and the established name for this
screen elsewhere (GitLab, Jira, Moodle all call it that).
2026-08-24 06:58:28 +02:00
ignacionelson 457fed0c86 Stop spending CI time on checks that check nothing
The tests job spent 191 of its 270 seconds running the suite one process
at a time; --parallel runs the same 1763 tests across the runner's cores
with nothing skipped. paratest is already a dev dependency.

The linter job was worse: 150 of its 195 seconds went to `pint` with no
--test and its auto-commit step commented out, so it reformatted the
runner's checkout, exited 0 and threw the result away. `npm run format`
is `prettier --write` and did the same. Both are gone, with a note on
what reinstating them as real gates would take -- a formatting sweep
first, then the flag. What remains is eslint, now read-only so it can
actually fail, and the job no longer needs PHP at all.

Both workflows now cancel superseded runs, and neither runs for a change
that only touches prose nobody's code reads. CHANGELOG.md and docs/ are
deliberately absent from that list: ReleaseNotes parses one and two
controllers serve the other.
2026-08-23 23:59:53 -03:00
ignacionelson 4f38c9adee Show one confirmation toast, not two
Every page wraps itself in AppLayout, so a flashed redirect that lands on
a different page component tears the layout down and builds it again --
Toaster with it. The fresh Toaster then reads the flash at mount *and*
catches the router success event for the same visit, and every "Client
created." arrived twice. Saves that stay on the same component never
remount, which is why this survived unnoticed.

Deduping on the flash object's identity rather than its text is what
keeps the success listener doing its job: two genuine identical messages
in a row are separate objects and still both toast.

Verified in a real browser rather than by types: create a client, two
toasts before, one after, and two consecutive creates over SPA
navigation still toast once each.

Reported and diagnosed by @denkfabrik-li in #1675.
2026-08-23 23:44:07 -03:00
Ignacio Nelson f7d6fe929e Merge pull request #1676 from denkfabrik-li/fix/social-connect-inertia-location
Send the browser to the provider, not the XHR
2026-08-23 23:39:44 -03:00
ignacionelson 1030f719fc Record the connect-a-provider fix in the changelog
The Connect button on Settings → Connected accounts did nothing at all,
which is the kind of thing somebody upgrading needs to see written down.

Found and fixed by @denkfabrik-li in #1676.
2026-08-23 23:28:42 -03:00
denkfabrik-li 19d34ef38b Document the OAuth mail providers, linked from the settings screen
A step-by-step guide (docs/email-oauth.md, same shape as the API
guide, whitelisted alongside it) through both vendor consoles: the
Entra app registration with its three account-type choices and what
each means for the tenant field, and the Google Cloud client with its
consent screen, test users and the testing-status 7-day refresh-token
expiry. The troubleshooting entries are errors actually hit while
building this — including Graph's ErrorQuotaExceeded, whose message
text hides that the mailbox may simply be full.

The Sending tab links to the guide right where an admin picks an
OAuth provider, via the shared links.source origin.
2026-08-23 22:46:25 +02:00
denkfabrik-li 4eb8cf915a Add Google / Gmail as the second OAuth mail provider
Same delegated shape as the Microsoft 365 provider, through the same
broker interface: the admin registers an OAuth client in Google Cloud
Console, connects the Google account the installation should send as,
and outgoing email goes through the Gmail API's messages.send as that
account.

The shared authorization-code machinery (exchange, refresh, token
storage, id_token account detection, RFC 6749 failure telling a dead
grant from a transient one) moves into an abstract OAuthCodeFlowBroker;
the two vendor brokers keep only their endpoints, scopes and consent
URL parameters. Google's quirks live where they belong: offline access
with a forced consent screen (the only way Google issues a refresh
token), and a refresh response that never re-sends one — the store
keeps what it has.

The settings screen needed no changes: the dropdown, the credential
form and the connect flow all derive from the provider enum.
2026-08-23 22:46:24 +02:00
denkfabrik-li 933eaa2ba4 Send mail through Microsoft Graph as an admin-connected mailbox
Adds "Microsoft 365 (OAuth)" to the Email settings provider dropdown.
Selecting it swaps the SMTP form for an app registration (client id,
secret, optional tenant) and a "Connect mailbox" flow: the admin signs
into the mailbox the installation should send as, and outgoing email
goes through Graph sendMail as that mailbox — no password, no app
password, no SMTP AUTH, which Microsoft is winding down.

Delegated flow on purpose: it needs no admin consent and works for
work/school and personal accounts alike. Its one weakness — a grant
can die silently behind a password reset or a Conditional Access
change — is answered by a daily scheduled refresh that keeps the
token alive and, on a dead grant, warns the settings admins once
in-app and on the settings page instead of letting mail stop quietly.

Tokens and the client secret live encrypted in their own row and are
read fresh at send time, never through the boot-config cache. The
stored SMTP transport survives a provider switch untouched.
2026-08-23 22:46:24 +02:00
denkfabrik-li fbd6c3603d Add tests for the connect redirect navigation 2026-08-23 22:15:36 +02:00
denkfabrik-li 5bfc5a0883 Send the browser to the provider, not the XHR 2026-08-23 22:13:36 +02:00
Eliana Bracciaforte 94e4aa36e4 Merge pull request #1674 from projectsend/fix/trusted-proxies-read-from-env
Read TRUSTED_PROXIES late enough for it to be seen
2026-08-22 15:56:31 -03:00
elibrachas 027e8532d2 Name the 419 as a symptom of an untrusted proxy
DOCKER.md said getting TRUSTED_PROXIES wrong gives you wrong client IPs
or wrong links but never affects whether a request succeeds. It does: it
is what makes the create-your-admin form come back as a 419, which is the
first thing a new install behind a proxy hits and gives no hint about the
cause. Said so, and kept the point that a 502 is a different problem.

INSTALL.md told operators never to run config:cache, and the only reason
it gave was that doing so disabled TRUSTED_PROXIES. That read now goes
through the config layer, so the reason is gone and the section claimed
something untrue. Replaced with the caveat that does apply to a cached
config: re-run it after editing .env.

Also a troubleshooting entry under the symptom people search for — 419 on
login, or being returned to the login screen at random — since the
existing proxy entry only covered rate limiting and the download log.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 15:41:49 -03:00
elibrachas 2a82335e07 Move the shared public-listing helpers to tests/Helpers.php
publicListingFile() and publicListingImageFile() were defined in
PublicGroupsTest.php and used from PublicFilePreviewTest.php too. Pest
declares a test file's functions as ordinary globals, so that works only
once the defining file has been loaded — which under --parallel depends
on how the runner happens to distribute files across processes. Adding
any unrelated test file anywhere in the suite reshuffles that and takes
PublicFilePreviewTest.php down with "Call to undefined function", and
running it on its own with --filter never worked at all.

tests/Helpers.php exists for exactly this and its docblock describes this
failure; these two had just been missed. publicPageProps() stays where it
is, since only one file uses it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 15:41:49 -03:00
elibrachas 1aaab1bf66 Read TRUSTED_PROXIES late enough for it to be seen
The value was read with env() inside the withMiddleware closure in
bootstrap/app.php. That closure runs when the HTTP kernel is resolved,
which is before the dotenv bootstrapper reads .env — so on every web
request env() returned null for anything set in .env, and the proxy was
never trusted. It worked when the value came from a real environment
variable, which is why the Docker compose path was fine and the manual
install described in INSTALL.md, where we tell people to put it in .env,
was not. Artisan bootstraps in the other order, so a check from the
command line reported the setting as working the whole time.

Behind a TLS-terminating proxy the consequence is not subtle. Laravel
falls back to the connecting address and the plain scheme, builds every
link and redirect with http:// while the browser is on https://, and
marks the session cookie non-secure. The browser then declines to send
that cookie to what it reads as a different, less secure origin, the
session arrives empty, and the first write fails with a 419 that reads as
"your session expired" — most often on the create-your-admin form, which
is the first thing a new install submits. Afterwards each redirect leaves
and re-enters over the wrong scheme, which is the random bounce back to
the login screen people report as flakiness.

Moved to config/trustedproxy.php, the key the framework's TrustProxies
middleware already falls back to on its own. Config files load after
dotenv, so the value is there whether it comes from .env or from the
environment.

This was also the only env() read outside config/, which means
config:cache is no longer dangerous on this application.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-22 15:41:31 -03:00
ignacionelson 503676647f Let a split-user host serve downloads
A download is not served by PHP. PHP authorizes it and hands the web
server the path with X-Accel-Redirect, so the web server has to open a
file PHP wrote. Where those are different users — cPanel and Plesk
commonly arrange it that way — it cannot: uploads land 0600 inside a 0700
directory, and traversing 0700 means being its owner. Nothing else on the
site shows a symptom. Uploading works, the library lists everything, and
only downloads fail, as ERR_INVALID_RESPONSE in the browser and
`open() ... failed (13: Permission denied)` in the web server's log.

FILES_WEB_SERVER_READABLE writes uploads 0644/0755 instead. Opt-in and
spread into the disk configuration rather than switched by a ternary, so
an install that does not set it keeps byte-for-byte the configuration it
had: the relaxed modes are readable by every account on the machine,
which is the wrong trade wherever the web server and PHP are one user, as
in the image and on most self-administered servers.

The two halves are not enforced alike, which is the part worth knowing.
`visibility` has Flysystem chmod each file after writing it, so 0644
holds under any umask. A directory is created by mkdir(), which masks its
mode argument, so 0755 is a ceiling: a pool at umask 0077 still produces
0700 and still cannot be traversed. That cannot be fixed from config, so
INSTALL.md carries it — how to tell the two users apart, the one-time
chmod for files already on disk, and the pool setting for the umask.
FilePermissionsTest asserts all three modes, umask cases included, since
the asymmetry is invisible from the configuration.

Reported by @denkfabrik-li (#1668), who diagnosed it and verified the
remedy on the affected host.
2026-08-21 16:34:14 -03:00
ignacionelson 5a7c9938dd Work properly behind a reverse proxy
Three findings from one report of intermittent 502s behind Nginx Proxy
Manager, all of them ours.

Stop sending the Link: preload header. AddLinkHeadersForPreloadedAssets
copied every Vite preload into a response header, duplicating tags the
document already carried in its head — twenty on the login page. nginx
buffers a response's headers into a single block defaulting to 4 KB, so
/files, at 6060 bytes of headers, was refused with "upstream sent too big
header" and the proxy answered 502. Which pages went over depended on how
many assets they loaded, which is why it read as intermittent rather than
as a header that is always too big: the login screen fitted, the
application did not. Removing it takes /files to 1247 bytes and
/dashboard from 4544 to 1247. Nothing is lost — the browser reads the
tags in the document, and we send no 103 Early Hints.

Send nginx's logs to the container's streams. supervisord captures what
each program writes to its own stdout, but nginx opens the files named in
the package's nginx.conf as soon as it reads its config, so access and
error logs went to /var/log/nginx/ inside the container. That is where
the reason for every 502 and every 403 was written, and docker logs never
showed it — so a proxy problem presented as no logs on either side, which
is exactly how it was reported.

Document the thing neither guide covered. DOCKER.md had no reverse-proxy
section at all: no mention of proxies, of 502s, or of TRUSTED_PROXIES,
which until now was explained only in a comment in the compose example.
It gains one, including that TRUSTED_PROXIES cannot cause a 502 and is
the wrong place to dig. INSTALL.md's nginx-in-front-of-Apache path gains
the proxy_* buffer settings its fastcgi_* equivalents already had.

Reported by @denkfabrik-li (#1664), who traced it to the middleware
independently, and separately by a user running Nginx Proxy Manager who
found the too-big-header line in the proxy's own log.
2026-08-21 15:19:44 -03:00
Ignacio Nelson d1d1999216 Merge pull request #1671 from projectsend/feature/file-downloads-previews-tab
A Downloads & previews tab on the file page
2026-08-21 15:10:13 -03:00
ignacionelson 51eea30dda Answer "did they ever actually get it?" from the file itself
The two things staff most often want to know about a file — who
downloaded it, who looked at it — were answerable only by reading the
whole activity log past everything else that had happened to it, or by
going back to the library list for the details panel.

The file's own page now has a Downloads & previews tab: the twenty most
recent times it was taken or looked at, each with who did it and the
address it went to, over a running count of both. Below them, two
buttons open the file's full history already filtered — one to every
download, one to every preview — so the narrow question is one click
and the whole log is still one click further.

Which filter value stands for "every download" is decided server-side
and travels with the payload, because it is a fact about the log's
vocabulary: downloads are three actions and share a group, previews are
one action and are filtered by name. The history page now also keeps
whatever filter it was sent with visible in its dropdown even at a
count of zero, so a button cannot land somebody on an empty table above
a select that has gone blank.
2026-08-21 15:04:11 -03:00
Ignacio Nelson c18f2f0f73 Merge pull request #1670 from projectsend/fix/1661-update-instructions-for-source-builds
Tell a clone-and-build install to rebuild, not to pull
2026-08-21 14:55:19 -03:00
ignacionelson 3d6089a501 Docs: clear up two contradictions in the migration and Docker guides
Step 2 of the v1 migration guide said Direct hardlinks your files instead of
copying them. It does not: copy is the default in both the command and the
screen, and hardlink is one of the four strategies you choose in step 3a. Say
that where the choice is first mentioned.

DOCKER.md's "Move the data you already have" reads like it is about the data in
a Legacy install. It is about relocating an already-running install's named
volumes onto the host paths chosen a step earlier, which is why it opens by
telling a new installation to skip it. Retitle it and spell out that a new
install waiting for a v1 migration skips it too — that data arrives later,
through the migration tool, and the install has to be empty when it does.
2026-08-21 14:39:24 -03:00
ignacionelson 8f12c83d21 Tell a clone-and-build install to rebuild, not to pull
ProjectSend prints the update instructions for the way this server was
installed, and it knew two answers where it needed three: anything inside
a container was handed `docker compose pull && docker compose up -d`. On
the Compose stack that builds from a checkout there is no image behind
those containers, so `pull` skips every ProjectSend service and `up -d`
then finds them all current — the update reports success, changes
nothing, and the dashboard goes on offering the same release. Reported by
@mueller7382, who stayed on 2.0.0 that way while 2.1.0 was out (#1661).

Those installations are now their own kind, told to `git pull` and
rebuild, with the two steps a checkout needs that an image does not: its
dependencies and its compiled frontend live outside git, so a release
that moved either leaves them stale.

Two signals decide it, in that order. The published image now declares
itself with PROJECTSEND_IMAGE, which is the only evidence an operator
bind-mounting over /var/www/html can neither hide nor forge; failing that
— images published before this — a working tree in the install directory,
which the image never has and the repository's own stack always does.
getenv() rather than env(), because a cached configuration makes env()
outside a config file return null, and the answer would flip silently on
exactly the installs most likely to have cached it.

The stale-code banner keeps treating both container kinds alike: what
clears it is recreating the container, whichever way its image was built.

The changelog also credits the reporter of #1663, which was missed when
that entry was written.
2026-08-21 14:35:49 -03:00
Ignacio Nelson 11e6876826 Merge pull request #1669 from projectsend/feature/preview-video-audio-pdf
Preview video, audio and PDF, not only images
2026-08-21 14:21:31 -03:00
ignacionelson 88c182cf3b Preview video, audio and PDF, not only images
v1 could preview four kinds of file in a modal — images, video, audio and
PDF. v2 previewed only images, and not by decision: preview shipped as part
of the image *thumbnail* work (1c68aa1), so "previewable" quietly became a
synonym for "GD can decode it". FileThumbnailController::preview() gated on
ThumbnailGenerator::SUPPORTED_MIME_TYPES, the frontend mirrored the same
four types, and the dialog was a hardcoded <img>.

Rather than widen that list — it drives pathFor(), extensionFor(),
generate() and FileDiskCleanup, and a video reaching getimagesize() is a
500 — this separates the two questions. PreviewKind now answers "may these
bytes be served inline, and what element renders them?", while
ThumbnailGenerator keeps answering the narrower "can this app decode it
itself?", which is what renditions, the cache and the watermark hook
actually depend on. Image delegates to it so the two cannot drift.

The allowlist stays a security boundary: mime_type is sniffed from the
bytes, so text/html and image/svg+xml remain excluded, and PreviewKind is
deliberately narrower than "formats a browser might cope with" — no
quicktime, avi or matroska, because an embedded player for those shows a
black rectangle. Those still download exactly as before.

docs/security-audit-2026-08-05.md finding 1 recorded that adding
application/pdf "should be a conscious decision". This is that decision,
and three things were measured rather than assumed:

- An <iframe sandbox> cannot be used. Chrome refuses to run its PDF viewer
  in a sandboxed frame at all (ERR_BLOCKED_BY_CLIENT, with or without
  allow-same-origin) — the attribute removes the feature, it does not
  harden it.
- nginx's `Content-Security-Policy: sandbox; default-src 'none'` on
  /protected-files/ does work (a <video> frame lands in an opaque origin),
  but Chrome exempts its PDF viewer from it, so it is not what protects
  the PDF case.
- What does is the allowlist plus the browser's own PDF sandbox, where PDF
  JavaScript has no DOM and no cookies.

Range requests were verified end to end: 206 with a correct Content-Range,
a byte-perfect file reassembled from three ranges, and a real browser
seeking to 10s of a 20s clip. nginx drops the upstream Content-Length on
the X-Accel path, so there is no collision.

Two settings, both defaulting on so no installation loses what it has:
clients_can_preview_files and public_listing_preview_enabled. Staff are
never gated. The anonymous side needed a route of its own — there was no
public preview endpoint — with its own throttle bucket, since a bare
throttle: shares one counter across that whole block.

A preview now logs at most one FilePreviewed per viewer per file per five
minutes: a <video> turns one deliberate act into a long tail of Range
requests, and a row each would bury the log.

Also fixes a layout bug the tests could never catch. A portal file row was
flex justify-between with three children — name, comment trigger, download
— so the middle one settled wherever the name happened to end and the
comment icon sat at a different place on every row. The name block now
takes the slack and every action lives in one trailing group, with the
comment trigger in a fixed-width slot so the icons form a column. And
because half the previewable files have no thumbnail to click — a PDF, an
mp3 and an mp4 all render as a generic icon — every row gains an explicit
PreviewAction beside DownloadAction, matching whatever style that theme
gives its download control.
2026-08-21 14:14:23 -03:00