mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-18 09:35:07 +00:00
denkfabrik-li
bde86c10e4
Connecting a provider needed nothing but the session. Anyone holding one
could POST /settings/connected-accounts/google, follow the returned
Inertia::location(), sign in at the provider as *themselves*, and
completeLink() would bind their identity to the victim's account.
SocialAccount says what that row is:
This row *is* the authorization to sign in as that account.
So it is not a preference -- it is a credential, and one that outlives
every way the victim has of ending the session that created it. It
survives a password change, it survives Auth::logoutOtherDevices(), it
survives invalidating every session. Where a stolen session gives an
attacker access until it is noticed, this gives them an account.
routes/settings.php already makes exactly this argument, twenty lines
down, for the two-factor block and the API token routes:
a token outlives the session that minted it, so a stolen session must
not be enough to mint one
The link has that property too, and was the one thing on this screen
without the gate. Now it has it.
The gate goes on `connect`, not on the callback: starting the flow is what
writes the intent the callback completes, and the callback deliberately
sits outside every group so a provider sign-in works without a session.
Not changed, deliberately: `connected-accounts.destroy`. Disconnecting
removes a way in rather than adding one, and destroy() already refuses to
remove the last one ("This is the only way you can sign in. Set a password
first"). Putting it behind password.confirm would fall hardest on the
accounts a provider provisioned -- they hold a Str::password(64) nobody
has ever seen -- and leave them unable to disconnect anything at all.
There is a test pinning that it stays reachable.
Also not changed: the account owner still is not told. SocialLoginController
writes an activity log entry, and that sits behind `staff` +
can:view_actions_log, so a client never sees it. Notifying them is a real
gap and a separate change; this one closes the door rather than adding a
bell to it.
Tests: two that fail against the ungated route -- the redirect, and the
whole attack end to end with a stranger identity never binding. The
existing connect() helper now confirms the password, the way
enableTwoFactor() already did, so the rest of the file keeps exercising
the real gate rather than asserting around it.