Skip to content

Commit d6c7ac2

Browse files
committed
fix(2fa): apply the local MFA policy to social login
Social login (SocialLoginController::callback) established a full session for any user that passed canLogin(), so an account with shouldRequire2FA() true could log in and obtain tokens without ever passing the 2FA challenge the password flow enforces. Extract the password flow's challenge issuance (gate check, resend rate-limit, OTP issuance scoped to the pending OAuth2 client, audit log, session bookkeeping) from UserController::postLogin into a dedicated TwoFactorChallengeService, and call it from the social callback before Auth::login(). An enforced user now gets the same email-OTP challenge and completes it through the unchanged verify2FA endpoint; non-enforced users and the two_factor.enabled kill-switch behave as before. Drop the now-unused ITwoFactorGateService / ITwoFactorRateLimitService dependencies from UserController.
1 parent 5d6cb70 commit d6c7ac2

6 files changed

Lines changed: 292 additions & 57 deletions

File tree

‎app/Http/Controllers/SocialLoginController.php‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
* limitations under the License.
1313
**/
1414

15+
use App\Http\Controllers\Traits\MFACookieManager;
1516
use App\libs\Auth\SocialLoginProviders;
17+
use App\Services\Auth\ITwoFactorChallengeService;
1618
use App\Services\Auth\IUserService;
1719
use Auth\Exceptions\AuthenticationException;
1820
use Illuminate\Support\Facades\Auth;
@@ -29,6 +31,7 @@
2931
*/
3032
final class SocialLoginController extends Controller
3133
{
34+
use MFACookieManager;
3235

3336
/**
3437
* @var IAuthService
@@ -48,22 +51,30 @@ final class SocialLoginController extends Controller
4851
*/
4952
private $memento_service;
5053

54+
/**
55+
* @var ITwoFactorChallengeService
56+
*/
57+
private $two_factor_challenge_service;
58+
5159
/**
5260
* SocialLoginController constructor.
5361
* @param IAuthService $auth_service
5462
* @param IUserService $user_service
5563
* @param IMementoOAuth2SerializerService $memento_service
5664
* @param ILoginStrategyFactory $login_strategy_factory
65+
* @param ITwoFactorChallengeService $two_factor_challenge_service
5766
*/
5867
public function __construct(
5968
IAuthService $auth_service,
6069
IUserService $user_service,
6170
IMementoOAuth2SerializerService $memento_service,
62-
ILoginStrategyFactory $login_strategy_factory
71+
ILoginStrategyFactory $login_strategy_factory,
72+
ITwoFactorChallengeService $two_factor_challenge_service
6373
){
6474
$this->auth_service = $auth_service;
6575
$this->user_service = $user_service;
6676
$this->memento_service = $memento_service;
77+
$this->two_factor_challenge_service = $two_factor_challenge_service;
6778
$this->middleware(function ($request, $next) use($login_strategy_factory){
6879
// we do it here just to ensure that user session is loaded
6980
Log::debug(sprintf("SocialLoginController::middleware"));
@@ -161,6 +172,24 @@ public function callback($provider)
161172
);
162173
}
163174

175+
// Social login is primary authentication only - the local MFA
176+
// policy still applies on return from the provider, exactly as
177+
// it does for the password flow. An enforced user gets the same
178+
// challenge (rate-limited, audited, session-flashed) instead of
179+
// an immediate session.
180+
$payload = $this->two_factor_challenge_service->issueChallengeIfRequired($user, $this->getCookieToken(), true);
181+
if (!is_null($payload)) {
182+
$payload = array_merge($payload, [
183+
'username' => $user_email,
184+
'user_fullname' => $user->getFullName(),
185+
'user_pic' => $user->getPic(),
186+
'user_verified' => true,
187+
'user_is_active' => $user->isActive() ? 1 : 0,
188+
]);
189+
190+
return $this->login_strategy->challengeRequired($payload);
191+
}
192+
164193
Auth::login($user, true);
165194
// and continue the usual flow
166195
return $this->login_strategy->postLogin(['provider' => $provider]);

‎app/Http/Controllers/UserController.php‎

Lines changed: 9 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@
2323
use App\Services\Auth\IDeviceTrustService;
2424
use App\Services\Auth\IRecoveryCodeService;
2525
use App\Services\Auth\ITwoFactorAuditService;
26-
use App\Services\Auth\ITwoFactorGateService;
27-
use App\Services\Auth\ITwoFactorRateLimitService;
26+
use App\Services\Auth\ITwoFactorChallengeService;
2827
use App\Services\Auth\IUserService as AuthUserService;
2928
use Auth\Exceptions\AuthenticationException;
3029
use Auth\Exceptions\UnverifiedEmailMemberException;
@@ -153,19 +152,14 @@ final class UserController extends OpenIdController
153152
private $two_factor_audit_service;
154153

155154
/**
156-
* @var ITwoFactorGateService
157-
*/
158-
private $mfa_gate_service;
159-
160-
/**
161-
* @var ITwoFactorRateLimitService
155+
* @var IRecoveryCodeService
162156
*/
163-
private $two_factor_rate_limit_service;
157+
private $recovery_code_service;
164158

165159
/**
166-
* @var IRecoveryCodeService
160+
* @var ITwoFactorChallengeService
167161
*/
168-
private $recovery_code_service;
162+
private $two_factor_challenge_service;
169163

170164
/**
171165
* @param IMementoOpenIdSerializerService $openid_memento_service
@@ -205,9 +199,8 @@ public function __construct
205199
LoginHintProcessStrategy $login_hint_process_strategy,
206200
IDeviceTrustService $device_trust_service,
207201
ITwoFactorAuditService $two_factor_audit_service,
208-
ITwoFactorGateService $mfa_gate_service,
209-
ITwoFactorRateLimitService $two_factor_rate_limit_service,
210202
IRecoveryCodeService $recovery_code_service,
203+
ITwoFactorChallengeService $two_factor_challenge_service,
211204
)
212205
{
213206
$this->openid_memento_service = $openid_memento_service;
@@ -227,9 +220,8 @@ public function __construct
227220
$this->security_context_service = $security_context_service;
228221
$this->device_trust_service = $device_trust_service;
229222
$this->two_factor_audit_service = $two_factor_audit_service;
230-
$this->mfa_gate_service = $mfa_gate_service;
231-
$this->two_factor_rate_limit_service = $two_factor_rate_limit_service;
232223
$this->recovery_code_service = $recovery_code_service;
224+
$this->two_factor_challenge_service = $two_factor_challenge_service;
233225

234226
$this->middleware(function ($request, $next) use($login_hint_process_strategy){
235227

@@ -547,47 +539,8 @@ public function postLogin()
547539

548540
$cookieToken = $this->getCookieToken();
549541

550-
if ($this->mfa_gate_service->requiresChallenge($user, $cookieToken)) {
551-
// Initial issuance shares the resend rate-limit window
552-
// (SDS idp-mfa.md §4.12) - without this, this route
553-
// would be an unthrottled way to mail-bomb the account
554-
// owner with OTP codes.
555-
if ($this->two_factor_rate_limit_service->isRateLimited(
556-
ITwoFactorRateLimitService::ActionResend,
557-
$user->getId()
558-
)) {
559-
throw new AuthenticationException(ITwoFactorRateLimitService::RATE_LIMIT_MESSAGE);
560-
}
561-
562-
// Issue a challenge and stop short of session creation.
563-
$client = $this->resolveClientFromMemento();
564-
$method = $user->getTwoFactorMethod();
565-
$strategy = MFAChallengeStrategyFactory::create($method);
566-
$payload = $this->auth_service->issueMFAChallenge($user, $strategy, $client, $remember);
567-
$this->two_factor_rate_limit_service->increment(ITwoFactorRateLimitService::ActionResend, $user->getId());
568-
569-
// Best-effort: the challenge was already issued and the OTP
570-
// sent, so an audit-logging failure must not 500 the user
571-
// out of the mfa_required response they need to proceed.
572-
try {
573-
$this->two_factor_audit_service->log(
574-
$user,
575-
TwoFactorAuditLog::EventChallengeIssued,
576-
$method,
577-
IPHelper::getUserIp()
578-
);
579-
} catch (\Throwable $ex) {
580-
Log::warning($ex);
581-
}
582-
583-
// Restore-on-refresh: a subsequent GET /login can rehydrate
584-
// the 2FA screen from session instead of dropping back to the
585-
// password form. otp_length/otp_lifetime (part of $payload)
586-
// are flashed by challengeRequired() itself; flow/mfa_method
587-
// aren't part of the challenge payload, so they're set here.
588-
Session::put('flow', IAuthService::AuthenticationFlowMFA);
589-
Session::put('mfa_method', $method);
590-
542+
$payload = $this->two_factor_challenge_service->issueChallengeIfRequired($user, $cookieToken, $remember);
543+
if (!is_null($payload)) {
591544
// The password step now submits as a native form POST, so this
592545
// response is a fresh page load, not a client-side transition -
593546
// without these, the React app remounts with no identity state
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
namespace App\Services\Auth;
4+
5+
/**
6+
* Copyright 2026 OpenStack Foundation
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
**/
17+
18+
use Auth\Exceptions\AuthenticationException;
19+
use Auth\User;
20+
21+
/**
22+
* Interface ITwoFactorChallengeService
23+
*
24+
* Single source of truth for issuing the local MFA challenge on a primary
25+
* authentication that just succeeded (password or social login) - shared
26+
* so every primary-auth flow that must honor User::shouldRequire2FA()
27+
* applies the exact same challenge (rate limit, OTP issuance, audit log,
28+
* session bookkeeping) instead of drifting out of sync.
29+
*
30+
* @package App\Services\Auth
31+
*/
32+
interface ITwoFactorChallengeService
33+
{
34+
/**
35+
* @param User $user the user who just completed primary authentication
36+
* @param string|null $cookieToken the device-trust cookie value, if any
37+
* @param bool $remember
38+
* @return array|null null when no challenge is required (the caller should
39+
* complete the login); otherwise the challenge payload
40+
* the caller should pass to login_strategy->challengeRequired()
41+
* @throws AuthenticationException when the resend rate limit is exceeded
42+
*/
43+
public function issueChallengeIfRequired(User $user, ?string $cookieToken, bool $remember): ?array;
44+
}
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
<?php
2+
3+
namespace App\Services\Auth;
4+
5+
/**
6+
* Copyright 2026 OpenStack Foundation
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
**/
17+
18+
use App\libs\Auth\Models\TwoFactorAuditLog;
19+
use Auth\Exceptions\AuthenticationException;
20+
use Auth\User;
21+
use Illuminate\Support\Facades\Log;
22+
use Illuminate\Support\Facades\Session;
23+
use models\exceptions\ValidationException;
24+
use Models\OAuth2\Client;
25+
use OAuth2\Factories\OAuth2AuthorizationRequestFactory;
26+
use OAuth2\OAuth2Message;
27+
use OAuth2\Repositories\IClientRepository;
28+
use OAuth2\Services\IMementoOAuth2SerializerService;
29+
use Strategies\MFA\MFAChallengeStrategyFactory;
30+
use Utils\IPHelper;
31+
use Utils\Services\IAuthService;
32+
33+
/**
34+
* Class TwoFactorChallengeService
35+
* @package App\Services\Auth
36+
*/
37+
final class TwoFactorChallengeService implements ITwoFactorChallengeService
38+
{
39+
public function __construct(
40+
private readonly ITwoFactorGateService $mfa_gate_service,
41+
private readonly ITwoFactorRateLimitService $rate_limit_service,
42+
private readonly ITwoFactorAuditService $audit_service,
43+
private readonly IAuthService $auth_service,
44+
private readonly IMementoOAuth2SerializerService $oauth2_memento_service,
45+
private readonly IClientRepository $client_repository
46+
) {
47+
}
48+
49+
public function issueChallengeIfRequired(User $user, ?string $cookieToken, bool $remember): ?array
50+
{
51+
if (!$this->mfa_gate_service->requiresChallenge($user, $cookieToken)) {
52+
return null;
53+
}
54+
55+
// Initial issuance shares the resend rate-limit window
56+
// (SDS idp-mfa.md §4.12) - without this, this would be an
57+
// unthrottled way to mail-bomb the account owner with OTP codes.
58+
if ($this->rate_limit_service->isRateLimited(
59+
ITwoFactorRateLimitService::ActionResend,
60+
$user->getId()
61+
)) {
62+
throw new AuthenticationException(ITwoFactorRateLimitService::RATE_LIMIT_MESSAGE);
63+
}
64+
65+
// Issue a challenge and stop short of session creation.
66+
$client = $this->resolveClientFromMemento();
67+
$method = $user->getTwoFactorMethod();
68+
$strategy = MFAChallengeStrategyFactory::create($method);
69+
$payload = $this->auth_service->issueMFAChallenge($user, $strategy, $client, $remember);
70+
$this->rate_limit_service->increment(ITwoFactorRateLimitService::ActionResend, $user->getId());
71+
72+
// Best-effort: the challenge was already issued and the OTP sent, so
73+
// an audit-logging failure must not fail the caller out of the
74+
// mfa_required response the user needs to proceed.
75+
try {
76+
$this->audit_service->log(
77+
$user,
78+
TwoFactorAuditLog::EventChallengeIssued,
79+
$method,
80+
IPHelper::getUserIp()
81+
);
82+
} catch (\Throwable $ex) {
83+
Log::warning($ex);
84+
}
85+
86+
// Restore-on-refresh: a subsequent GET /login can rehydrate the 2FA
87+
// screen from session instead of dropping back to the primary-auth
88+
// screen. otp_length/otp_lifetime (part of $payload) are flashed by
89+
// challengeRequired() itself; flow/mfa_method aren't part of the
90+
// challenge payload, so they're set here.
91+
Session::put('flow', IAuthService::AuthenticationFlowMFA);
92+
Session::put('mfa_method', $method);
93+
94+
return $payload;
95+
}
96+
97+
/**
98+
* Own copy of UserController::resolveClientFromMemento() - resolves the
99+
* OAuth2 client for a pending authorization request, if any, so the
100+
* challenge (and its later verification) can be scoped to that client
101+
* the same way the password flow already does.
102+
*/
103+
private function resolveClientFromMemento(): ?Client
104+
{
105+
if (!$this->oauth2_memento_service->exists()) {
106+
return null;
107+
}
108+
109+
$oauth_auth_request = OAuth2AuthorizationRequestFactory::getInstance()->build(
110+
OAuth2Message::buildFromMemento($this->oauth2_memento_service->load())
111+
);
112+
113+
if (!$oauth_auth_request->isValid()) {
114+
return null;
115+
}
116+
117+
$client = $this->client_repository->getClientById($oauth_auth_request->getClientId());
118+
if (is_null($client)) {
119+
throw new ValidationException("client does not exists");
120+
}
121+
122+
$this->oauth2_memento_service->serialize($oauth_auth_request->getMessage()->createMemento());
123+
124+
return $client;
125+
}
126+
}

‎app/Services/Auth/TwoFactorServiceProvider.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public function register(): void
4343
$this->app->singleton(ITwoFactorGateService::class, MFAGateService::class);
4444
$this->app->singleton(ITwoFactorRateLimitService::class, TwoFactorRateLimitService::class);
4545
$this->app->singleton(IRecoveryCodeService::class, RecoveryCodeService::class);
46+
$this->app->singleton(ITwoFactorChallengeService::class, TwoFactorChallengeService::class);
4647
}
4748

4849
/**
@@ -112,6 +113,7 @@ public function provides(): array
112113
ITwoFactorGateService::class,
113114
ITwoFactorRateLimitService::class,
114115
IRecoveryCodeService::class,
116+
ITwoFactorChallengeService::class,
115117
];
116118
}
117119
}

0 commit comments

Comments
 (0)