fix: let authenticated users connect Google/GitHub from Settings#17
Merged
Conversation
The Connect button on /settings/authentication pointed at the
auth.{provider}.redirect routes that live behind `guest` middleware,
so authenticated users were bounced to /app/home before reaching
Socialite. The OAuth callback also needed to handle two flows
(signup/login vs link to current user) but had no branch for the
second case — meaning a different-email GitHub account would have
been registered as a new user, logging the original session out.
Splits the flows by intent:
- New `app.authentication.connect-provider` route in the auth group,
handled by the settings controller (where it sits next to
disconnect-provider). Replaces the OAuth signup link as the
Connect button's target.
- Auth callbacks moved out of the guest group (still one URL per
provider, since OAuth apps only register one) and gain a single
Auth::check() branch that calls connectToCurrentUser().
- connectToCurrentUser() rejects if the provider id already belongs
to a different user; otherwise sets it on the current user and
redirects back to settings with a flash message.
Splits the OAuth connect flow off entirely from signup/login so each
route has a single responsibility.
- routes/auth.php returns to its original state — redirect + callback
both back inside the `guest` group.
- routes/app.php gains a paired callback route at
/settings/authentication/providers/{provider}/callback.
- AuthenticationController::connectProvider /
connectProviderCallback override Socialite's redirectUrl so the
round-trip stays on the connect-flow URL. The Auth::check() branch
in the auth controllers is gone.
The OAuth apps in Google Cloud and GitHub Developer Settings need the
new callback URL registered alongside the existing one — documented in
.env.example.
Drops the dedicated /settings/authentication/providers/{provider}/callback
route added in the previous refactor — registering a second callback URL
in each OAuth app is more ops cost than the trade is worth.
Back to one callback URL per provider, with a small `Auth::check()`
branch in the auth controllers' callbacks. The check is safe because
the redirects that initiate the round-trip enforce the right
middleware (signup/login is `guest`-only, connect is `auth`-only),
so the auth state at callback time matches the flow's intent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a bug in the Settings → Authentication "Connect" button: clicking it while logged in was bouncing the user to /app/home (because the OAuth redirect lived behind
guestmiddleware) and would have corrupted the session even if it had reached the callback (different-email GitHub account → new user registered, original user logged out).Root cause
Two compounding issues:
auth.{provider}.redirectandauth.{provider}.callbackwere both insideroutes/auth.php'sguestgroup. Authenticated users couldn't reach either.The fix
Splits signup/login from connect-from-settings by intent, not by guessing in the callback:
app.authentication.connect-providerinroutes/app.php'sauthgroup, handled byApp\Settings\AuthenticationController::connectProvider. Sits next todisconnectProviderfor symmetry.auth.{provider}.redirectstays in theguestgroup (original behavior: signup/login only).auth.{provider}.callbackmoves out ofguestsince OAuth providers only allow one registered callback URL — it has to accept both flows. It gains a singleAuth::check()branch that delegates toconnectToCurrentUser().connectToCurrentUser()refuses to rebind a provider id that's already tied to a different user (flash error → settings); otherwise updates the current user's{provider}_idand flashes success.Authentication.vuenow usesconnectProvider(account.provider)instead of the OAuth signup link.Test plan
tests/Feature/Auth/ConnectProviderTest.php— 9 tests covering route accessibility (auth required, unknown provider 404, both providers redirect to OAuth) and callback behavior (link to current user including different-email case, reject when already linked elsewhere).Why not separate callback URLs
OAuth providers (Google, GitHub) only let you register a fixed list of callback URLs per app. Splitting signup vs connect into separate URLs would mean a second URL registered in each provider's console plus a second env var per provider — overhead with no real win, since the callback's branch is one
if (Auth::check())line. The middleware separation on the redirect routes is what guarantees the branch is safe (a guest can never reach the connect-provider route, and an authenticated user is bounced fromauth.{provider}.redirectby theguestmiddleware).