Three middleware answer before HandleInertiaRequests and so repeat its 302-to-303 upgrade themselves: EnsureSetupIsComplete, EnsureUserIsActive and EnforceTwoFactor. This file has a write case for each. The rule has a second half -- a read still gets a plain 302, because a 303 there is an upgrade nobody asked for -- and that half was checked once, on the deactivation door, under the name "leaves a read alone in every one of those cases". So a change that upgraded reads at the setup door or the two-factor door would have gone through with the suite green and this test still claiming it would not.
One case per door now, as a dataset. The setup case reads a guest-reachable GET for the same reason the write case posts to /timezone: anything behind auth is answered by the guest redirect before EnsureSetupIsComplete ever sees it. No production code changes -- all three doors answer a read with 302 today, which is what the new cases assert.
Verified before merging: 9 passed on the trial-merge, and the mutation counter-check was run here rather than taken from the PR. With EnsureSetupIsComplete answering 303 to everything, this branch's file goes 1 failed / 8 passed and main's version goes 7 passed. The write case for that door stays green under the mutation, which is right: 303 is what a write should get. The mutation itself was confirmed live first, by making the middleware throw and watching the response become a 500 -- a first attempt at it bound no argument and was a silent no-op, which would have looked exactly like the new test failing to notice.
Reported and fixed by @denkfabrik-li.
Three middleware answer before HandleInertiaRequests and so have to
repeat its 302→303 upgrade themselves: EnsureSetupIsComplete,
EnsureUserIsActive and EnforceTwoFactor. This file has a write case for
each, and the rule has a second half -- a read still gets a plain 302,
because a 303 there would be an upgrade nobody asked for.
That half was checked once, on the deactivation door, under a name that
said otherwise: "leaves a read alone in every one of those cases". The
setup door and the two-factor door were not covered at all, so a change
that upgraded reads at either of them would have gone through with the
suite green and this test's name still claiming it would not.
Both are covered now, as a dataset with one case per door. The setup case
reads a guest-reachable GET for the same reason the write case posts to
/timezone: anything behind `auth` is answered by the guest redirect before
EnsureSetupIsComplete sees it.
No production code changes; today all three doors answer a read with 302,
which is what the new cases assert. Demonstrated by mutation rather than
reversion: making EnsureSetupIsComplete upgrade every redirect to 303
fails this file (1 failed / 8 passed) and passes the old one (7 passed).
The unique index on `email` spans soft-deleted rows -- AvailableEmailRule
is built on exactly that, so a deleted account keeps its address until
erasure removes the row. The registration form learns this from
validation. The machine paths have no form: a directory or an identity
provider hands over an address and provision() inserts it.
Measured on main, a client deleted last week signing in through a
provider that may auto-provision:
GET /auth/google/callback → 500 (QueryException, unique constraint)
Same shape through LDAP at POST /login. Nothing is created, nothing is
signed in, and what the person meets is a server error.
Both provisioners now ask ClientProvisioning::addressIsFree() first and
refuse. The social flow already has a refusal for an identity it cannot
provision -- "There is no account here for that address." -- which is also
all a stranger should learn: whether an address was once an account here
is not the provider's to publish. The LDAP flow falls through to the
ordinary failed sign-in.
Deliberately not resurrecting the deleted account. Restoring one because
a directory says the address exists is a decision for a person, not a
side effect of somebody logging in.
Two tests, one per path: the sign-in is refused, nothing is created, and
the trashed row is still trashed. Both go red without the fix.
An installation brought over from v1 before the migration tool learned to
relabel carries $2a$ or $2b$ digests in users.password. All three bcrypt
labels name the same algorithm and password_verify() reads any of them,
but Laravel's hasher asks password_get_info() first, gets "unknown", and
throws before it looks at the password -- so the login form answers 500
for every migrated account while accounts created in v2 sign in fine.
Relabelling the stored digest is the whole fix. Four bytes change; salt
and digest are the same, so nobody resets anything and there is no mail to
send. Guarded on password_get_info() reading bcrypt afterwards, so a
truncated row is left visibly broken rather than quietly rewritten to no
effect.
$2x$ is left alone on purpose -- it asks for the pre-2011 handling of
bytes above 127, so relabelling it would lock out anybody whose password
is not plain ASCII.
The 500 itself is asserted, not just the repair, so nobody removes the
migration later on the grounds that bcrypt is bcrypt.
Reported by @pabloalvarez44 in #1706.
#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.
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.
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.
ignacionelson