Skip to content

Commit 928cf3a

Browse files
imorlandStyleCIBotclaude
authored
[2.x] feat(core): redirect-only OAuth flow — rewrite ResponseFactory (#4461)
* feat(core): rewrite ResponseFactory for redirect-only OAuth flow 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. * Apply fixes from StyleCI * fix(tests): fix 3 unit test failures in OAuth-related classes - 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). * Apply fixes from StyleCI * chore(js): yarn format * feat(core): add RegistrationTokenResource for redirect-based OAuth pre-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. * Apply fixes from StyleCI * feat(core): add POST /api/registration-token endpoint for OAuth sign-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 * Apply fixes from StyleCI * feat(sign-up): add visible labels to all SignUpModal fields 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> * feat(ResponseFactory): append _flarum_linked param on email-match auto-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. * fix: resolve merge conflict markers in ResolveTest.php * Apply fixes from StyleCI * fix(test): update email_match test to expect _flarum_linked in redirect URL * refactor(ResponseFactory): make makeLoggedInResponse/makeRegistrationResponse 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. --------- Co-authored-by: StyleCI Bot <bot@styleci.io> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1c06575 commit 928cf3a

File tree

13 files changed

+816
-43
lines changed

13 files changed

+816
-43
lines changed

extensions/nicknames/js/src/forum/index.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,18 @@ app.initializers.add('flarum-nicknames', () => {
7272
if (app.forum.attribute('displayNameDriver') !== 'nickname') return;
7373

7474
if (app.forum.attribute('setNicknameOnRegistration')) {
75+
const nicknameLabel = extractText(app.translator.trans('flarum-nicknames.forum.sign_up.nickname_placeholder'));
76+
7577
items.add(
7678
'nickname',
7779
<div className="Form-group">
80+
<label className="label">{nicknameLabel}</label>
7881
<input
7982
className="FormControl"
8083
name="nickname"
8184
type="text"
82-
placeholder={extractText(app.translator.trans('flarum-nicknames.forum.sign_up.nickname_placeholder'))}
85+
placeholder={nicknameLabel}
86+
aria-label={nicknameLabel}
8387
bidi={this.nickname}
8488
disabled={this.loading || this.isProvided('nickname')}
8589
required={app.forum.attribute('randomizeUsernameOnRegistration')}

framework/core/js/src/forum/ForumApplication.tsx

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,4 @@ export default class ForumApplication extends Application {
149149
public viewingDiscussion(discussion: Discussion): boolean {
150150
return this.current.matches(DiscussionPage, { discussion });
151151
}
152-
153-
/**
154-
* Callback for when an external authenticator (social login) action has
155-
* completed.
156-
*
157-
* If the payload indicates that the user has been logged in, then the page
158-
* will be reloaded. Otherwise, a SignUpModal will be opened, prefilled
159-
* with the provided details.
160-
*/
161-
public authenticationComplete(payload: Record<string, unknown>): void {
162-
if (payload.loggedIn) {
163-
window.location.reload();
164-
} else {
165-
this.modal.show(() => import('./components/SignUpModal'), payload);
166-
}
167-
}
168152
}

framework/core/js/src/forum/components/SignUpModal.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export default class SignUpModal<CustomAttrs extends ISignupModalAttrs = ISignup
7474
items.add(
7575
'username',
7676
<div className="Form-group">
77+
<label className="label">{usernameLabel}</label>
7778
<input
7879
className="FormControl"
7980
name="username"
@@ -90,6 +91,7 @@ export default class SignUpModal<CustomAttrs extends ISignupModalAttrs = ISignup
9091
items.add(
9192
'email',
9293
<div className="Form-group">
94+
<label className="label">{emailLabel}</label>
9395
<input
9496
className="FormControl"
9597
name="email"
@@ -107,6 +109,7 @@ export default class SignUpModal<CustomAttrs extends ISignupModalAttrs = ISignup
107109
items.add(
108110
'password',
109111
<div className="Form-group">
112+
<label className="label">{passwordLabel}</label>
110113
<input
111114
className="FormControl"
112115
name="password"
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
/*
4+
* This file is part of Flarum.
5+
*
6+
* For detailed copyright and license information, please view the
7+
* LICENSE file that was distributed with this source code.
8+
*/
9+
10+
namespace Flarum\Api\Controller;
11+
12+
use Flarum\User\Exception\InvalidConfirmationTokenException;
13+
use Flarum\User\RegistrationToken;
14+
use Illuminate\Support\Arr;
15+
use Laminas\Diactoros\Response\JsonResponse;
16+
use Psr\Http\Message\ResponseInterface;
17+
use Psr\Http\Message\ServerRequestInterface;
18+
use Psr\Http\Server\RequestHandlerInterface;
19+
20+
/**
21+
* Resolves a registration token submitted in the POST body and returns the
22+
* non-sensitive fields needed to pre-populate the sign-up modal.
23+
*
24+
* The token is accepted in the body (not the URL) so it never appears in
25+
* server access logs, browser history, or Referer headers.
26+
*
27+
* Only username, email, and the list of pre-filled field names are returned.
28+
* Provider name, identifier, and payload internals are NOT exposed.
29+
*
30+
* No authentication is required — possession of the short-lived token is
31+
* proof of authorisation.
32+
*/
33+
class ResolveRegistrationTokenController implements RequestHandlerInterface
34+
{
35+
public function handle(ServerRequestInterface $request): ResponseInterface
36+
{
37+
$tokenValue = Arr::get($request->getParsedBody(), 'token');
38+
39+
try {
40+
$token = RegistrationToken::validOrFail((string) $tokenValue);
41+
} catch (InvalidConfirmationTokenException) {
42+
return new JsonResponse(['errors' => [['status' => '404', 'title' => 'Not Found']]], 404);
43+
}
44+
45+
$provided = array_keys($token->user_attributes ?? []);
46+
47+
return new JsonResponse([
48+
'username' => $token->user_attributes['username'] ?? ($token->payload['suggested']['username'] ?? null),
49+
'email' => $token->user_attributes['email'] ?? ($token->payload['suggested']['email'] ?? null),
50+
'provided' => $provided,
51+
]);
52+
}
53+
}

framework/core/src/Api/routes.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333
$route->toController(Controller\TerminateAllOtherSessionsController::class)
3434
);
3535

36+
// Resolve a registration token (returns username/email/provided for pre-populating sign-up modal)
37+
$map->post(
38+
'/registration-token',
39+
'registration-token',
40+
$route->toController(Controller\ResolveRegistrationTokenController::class)
41+
);
42+
3643
// Send forgot password email
3744
$map->post(
3845
'/forgot',

framework/core/src/Forum/Auth/Registration.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class Registration
1313
{
1414
protected array $provided = [];
1515
protected array $suggested = [];
16-
protected mixed $payload;
16+
protected mixed $payload = null;
1717

1818
public function getProvided(): array
1919
{

framework/core/src/Forum/Auth/ResponseFactory.php

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use Flarum\User\RegistrationToken;
1616
use Flarum\User\User;
1717
use Illuminate\Support\Arr;
18-
use Laminas\Diactoros\Response\HtmlResponse;
18+
use Laminas\Diactoros\Response\RedirectResponse;
1919
use Psr\Http\Message\ResponseInterface;
2020

2121
class ResponseFactory
@@ -25,10 +25,25 @@ public function __construct(
2525
) {
2626
}
2727

28-
public function make(string $provider, string $identifier, callable $configureRegistration): ResponseInterface
29-
{
28+
/**
29+
* Handle an OAuth callback by logging in an existing user or beginning
30+
* the registration flow for a new one.
31+
*
32+
* @param string $provider The OAuth provider name (e.g. 'github').
33+
* @param string $identifier The provider's unique identifier for this user.
34+
* @param callable $configureRegistration Callback that populates a {@see Registration} instance
35+
* with data from the provider (email, username, avatar, etc.).
36+
* @param string $returnTo Validated same-origin URL to redirect to after login.
37+
* Must be validated by the caller before being passed here.
38+
*/
39+
public function make(
40+
string $provider,
41+
string $identifier,
42+
callable $configureRegistration,
43+
string $returnTo = '/'
44+
): ResponseInterface {
3045
if ($user = LoginProvider::logIn($provider, $identifier)) {
31-
return $this->makeLoggedInResponse($user);
46+
return $this->makeLoggedInResponse($user, $returnTo);
3247
}
3348

3449
$configureRegistration($registration = new Registration);
@@ -38,38 +53,56 @@ public function make(string $provider, string $identifier, callable $configureRe
3853
if (! empty($provided['email']) && $user = User::where(Arr::only($provided, 'email'))->first()) {
3954
$user->loginProviders()->create(compact('provider', 'identifier'));
4055

41-
return $this->makeLoggedInResponse($user);
56+
// Append _flarum_linked so the frontend can show a confirmation modal.
57+
$separator = str_contains($returnTo, '?') ? '&' : '?';
58+
$returnTo .= $separator.'_flarum_linked='.urlencode($provider);
59+
60+
return $this->makeLoggedInResponse($user, $returnTo);
4261
}
4362

44-
$token = RegistrationToken::generate($provider, $identifier, $provided, $registration->getPayload());
63+
$payload = array_merge(
64+
['suggested' => $registration->getSuggested()],
65+
(array) $registration->getPayload()
66+
);
67+
68+
$token = RegistrationToken::generate($provider, $identifier, $provided, $payload);
4569
$token->save();
4670

47-
return $this->makeResponse(array_merge(
48-
$provided,
49-
$registration->getSuggested(),
50-
[
51-
'token' => $token->token,
52-
'provided' => array_keys($provided)
53-
]
54-
));
71+
return $this->makeRegistrationResponse($token->token, $returnTo);
5572
}
5673

57-
private function makeResponse(array $payload): HtmlResponse
74+
/**
75+
* Build a redirect response for a successfully authenticated existing user.
76+
* Sets the remember-me cookie so the session is established on the next request.
77+
*
78+
* Override this method to customise the login redirect or cookie behaviour.
79+
*/
80+
protected function makeLoggedInResponse(User $user, string $returnTo): ResponseInterface
5881
{
59-
$content = sprintf(
60-
'<script>window.close(); window.opener.app.authenticationComplete(%s);</script>',
61-
json_encode($payload)
62-
);
82+
$token = RememberAccessToken::generate($user->id);
83+
84+
$response = new RedirectResponse($returnTo ?: '/');
6385

64-
return new HtmlResponse($content);
86+
return $this->rememberer->remember($response, $token);
6587
}
6688

67-
private function makeLoggedInResponse(User $user): ResponseInterface
89+
/**
90+
* Build a redirect response that triggers the registration modal on the frontend.
91+
*
92+
* The `_flarum_auth` query parameter carries the registration token. The frontend
93+
* detects this parameter on boot, strips it from the URL, and opens the SignUpModal
94+
* pre-populated with data from the provider.
95+
*
96+
* Override this method to customise how the registration handoff is communicated
97+
* to the frontend.
98+
*/
99+
protected function makeRegistrationResponse(string $token, string $returnTo): ResponseInterface
68100
{
69-
$response = $this->makeResponse(['loggedIn' => true]);
101+
$base = $returnTo ?: '/';
70102

71-
$token = RememberAccessToken::generate($user->id);
103+
// Append _flarum_auth without clobbering any existing query params on returnTo.
104+
$separator = str_contains($base, '?') ? '&' : '?';
72105

73-
return $this->rememberer->remember($response, $token);
106+
return new RedirectResponse($base.$separator.'_flarum_auth='.urlencode($token));
74107
}
75108
}

framework/core/src/Http/HttpServiceProvider.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class HttpServiceProvider extends AbstractServiceProvider
2626
public function register(): void
2727
{
2828
$this->container->singleton('flarum.http.csrfExemptPaths', function () {
29-
return ['token'];
29+
return ['token', 'registration-token'];
3030
});
3131

3232
$this->container->bind(Middleware\CheckCsrfToken::class, function (Container $container) {

0 commit comments

Comments
 (0)