Skip to content

[2.x] feat(core): redirect-only OAuth flow — rewrite ResponseFactory#4461

Merged
imorland merged 15 commits into2.xfrom
im/oauth-overhaul
Mar 15, 2026
Merged

[2.x] feat(core): redirect-only OAuth flow — rewrite ResponseFactory#4461
imorland merged 15 commits into2.xfrom
im/oauth-overhaul

Conversation

@imorland
Copy link
Member

@imorland imorland commented Mar 15, 2026

Summary

This PR is one half of a two-part OAuth overhaul. The companion PR is FriendsOfFlarum/oauth#109 — both must be merged together.

Docs will follow shortly after review.

Changes

  • ResponseFactory::make() now accepts a $returnTo string and returns a RedirectResponse in all cases. Logged-in users are redirected with a remember-me cookie; new users are redirected with _flarum_auth=<token> appended so the frontend can open SignUpModal.
  • ResponseFactory stores suggested fields (username, email) in the RegistrationToken payload under a suggested key so they survive the redirect and can be returned to the frontend.
  • ResponseFactory::makeLoggedInResponse() and makeRegistrationResponse() are now protected — extensions can subclass and override either method to customise the login redirect or registration handoff. _flarum_linked URL-building is handled in make() before the call, keeping both method signatures clean and stable.
  • _flarum_linked={provider} param — appended to the redirect URL when a user's account is linked to an OAuth provider for the first time (email-match auto-link path). The companion fof/oauth PR does the same for the manual security-page link flow. The frontend detects the param and shows an AccountLinkedModal.
  • POST /api/registration-token — new endpoint (ResolveRegistrationTokenController) that accepts a token in the POST body and returns only username, email, and provided[]. Provider name, identifier, and payload internals are never exposed. Token in POST body keeps it out of server access logs, browser history, and Referer headers.
  • registration-token added to CSRF exempt paths — unauthenticated POST is allowed since token possession is the credential.
  • ForumApplication::authenticationComplete() removed — it existed solely for the popup JS callback path.
  • SignUpModal field labels — Username, Email, and Password fields now show a visible <label> above the input. Nicknames extension's Nickname field updated the same way. Fixes the OAuth pre-fill UX where placeholder text disappears once a value is present, making Username and Nickname look identical.
  • Unit and integration test coverage added for Registration, ResponseFactory, LoginProvider, and ResolveRegistrationTokenController.

⚠️ Breaking Changes

What changed Migration
ResponseFactory::make() signature adds a required string $returnTo fourth argument Pass the validated same-origin returnTo URL; defaults to '/'
ResponseFactory::make() returns RedirectResponse instead of HtmlResponse Controllers that rendered the old HTML/JS response must be updated
ForumApplication::authenticationComplete() removed Remove any calls to this method; the frontend now reads _flarum_auth from the URL on boot

Any extension with a custom OAuth controller that calls ResponseFactory::make() directly must be updated.

Test plan

  • New user OAuth signup redirects to returnTo with _flarum_auth param, SignUpModal opens pre-populated with username/email
  • Existing user OAuth login redirects to returnTo with remember cookie and no _flarum_linked param
  • Email-match auto-link redirects with _flarum_linked={provider}, AccountLinkedModal appears
  • POST /api/registration-token returns username/email/provided for valid token
  • POST /api/registration-token returns 404 for invalid/expired/missing token
  • Provider name, identifier, and payload internals not present in token resolution response
  • SignUpModal shows labels for all fields; Username and Nickname are clearly distinguishable when both pre-filled
  • Unit tests pass: php vendor/bin/phpunit framework/core/tests/unit/Forum
  • Integration tests pass: php vendor/bin/phpunit framework/core/tests/integration/forum/Auth
  • Token resolution tests pass: php vendor/bin/phpunit framework/core/tests/integration/api/registration_tokens

imorland and others added 2 commits March 15, 2026 18:11
Replace the HtmlResponse popup approach with a proper RedirectResponse
flow throughout Forum\Auth\ResponseFactory:

- Logged-in users: redirect to returnTo with remember-me cookie set.
- Email-match users: auto-link provider then same redirect.
- New users: redirect to returnTo with _flarum_auth=<token> appended so
  the frontend can open the SignUpModal without any JS window tricks.

Remove authenticationComplete() from ForumApplication — it existed only
to call window.opener callbacks from the popup flow.

The returnTo parameter must be validated by the caller (AbstractOAuth-
Controller::validateReturnTo) before being passed to make(); this class
trusts it is a safe same-origin path.

Add unit and integration test coverage for Registration, ResponseFactory
and LoginProvider.

BREAKING CHANGE: ResponseFactory::make() now requires a $returnTo string
as the fourth argument, and returns a RedirectResponse instead of an
HtmlResponse. Extensions or custom OAuth controllers that call make()
directly must be updated. authenticationComplete() is removed from
ForumApplication.
@imorland
Copy link
Member Author

Companion PR: FriendsOfFlarum/oauth#109 — both must be merged together.

@imorland imorland changed the title feat(core): redirect-only OAuth flow — rewrite ResponseFactory [2.x] feat(core): redirect-only OAuth flow — rewrite ResponseFactory Mar 15, 2026
@imorland imorland added this to the 2.0.0-beta.8 milestone Mar 15, 2026
@imorland imorland linked an issue Mar 15, 2026 that may be closed by this pull request
imorland and others added 3 commits March 15, 2026 18:18
- Registration::$payload initialised to null to prevent "must not be
  accessed before initialization" error on getPayload() when nothing
  has been provided.
- ResponseFactoryTest: remove makeLoggedInResponse unit tests that hit
  RememberAccessToken::generate() → Eloquent DB connection. The logged-
  in redirect path is already covered by the integration test suite;
  unit tests now only cover URL construction logic for the registration
  response (which needs no DB).
@imorland imorland force-pushed the im/oauth-overhaul branch from 8167041 to fae5776 Compare March 15, 2026 18:24
imorland and others added 9 commits March 15, 2026 18:47
…e-population

Redirect-based OAuth flows (e.g. fof/oauth) bounce the user back to the forum
with a _flarum_auth query param carrying a RegistrationToken. Previously there
was no way to resolve that token back to username/email/provided fields without
a round-trip through the popup authenticationComplete callback.

- Add GET /api/registration-tokens/{token} — returns username, email, and the
  provided[] field list. Provider name, identifier, and payload internals are
  NOT exposed. The token acts as its own credential; no auth required.
- Store suggested fields (suggestUsername, suggestEmail) in the token payload
  under a 'suggested' key so they survive the redirect.
- Add RegistrationTokenResource to ApiServiceProvider.
- Integration tests: 15 cases covering happy paths, all field combinations,
  security (sensitive fields absent), expiry, and rejected write methods.
- ResponseFactory integration test: assert suggested fields land in payload.
…up pre-population

Redirect-based OAuth flows return the user to the forum with a
_flarum_auth token in the URL. The frontend needs to resolve that token
to username/email/provided[] before opening the SignUpModal.

Using POST (body: {token}) rather than GET /{token} keeps the token out
of server access logs, browser history, and Referer headers.

- ResolveRegistrationTokenController: accepts {token} in POST body,
  returns {username, email, provided[]}. Provider name, identifier, and
  payload internals are not exposed. Returns 404 for invalid/expired tokens.
- Route: POST /api/registration-token (name: registration-token)
- CSRF exempt (same pattern as /api/token login endpoint)
- Also stores suggested fields in token payload so they survive the redirect
  (ResponseFactory change from previous commit)
- 15 integration tests: happy paths, field combinations, security
  (sensitive fields absent), expiry, and rejected HTTP methods
Username, Nickname (when flarum-nicknames is active), Email and Password
fields now display a <label> above the input so they are clearly
distinguishable when pre-filled from an OAuth redirect — previously
placeholder text disappears once a value is present, making Username and
Nickname look identical.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…o-link

When an OAuth login auto-links a user via email match, the redirect now
includes _flarum_linked={provider} so the frontend can show the
AccountLinkedModal informing the user their accounts have been connected.
@imorland imorland marked this pull request as ready for review March 15, 2026 21:20
@imorland imorland requested a review from a team as a code owner March 15, 2026 21:20
…Response protected, move _flarum_linked URL-building into make()

Extensions can now override either method to customise the login redirect
or registration handoff without needing to touch _flarum_linked logic.
Clean signature: makeLoggedInResponse(User, string) with no optional params.

Adds regression test asserting returning users do not get _flarum_linked appended.
@imorland imorland merged commit 928cf3a into 2.x Mar 15, 2026
27 checks passed
@imorland imorland deleted the im/oauth-overhaul branch March 15, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[2.x] Change OAuth Flow from Popup to Redirect

2 participants