Skip to content

Commit 8c2742a

Browse files
committed
fix(oauth2): prevent profile redirect during OIDC consent flow
During an OIDC authorization code flow, users were intermittently redirected to the IDP profile page instead of back to the client application after submitting the consent form.
1 parent ed5f869 commit 8c2742a

File tree

5 files changed

+853
-5
lines changed

5 files changed

+853
-5
lines changed

.github/workflows/pull_request_unit_tests.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ jobs:
3535
SSL_ENABLED: false
3636
SESSION_DRIVER: redis
3737
PHP_VERSION: 8.3
38+
OTEL_SDK_DISABLED: true
39+
OTEL_SERVICE_ENABLED: false
3840
services:
3941
mysql:
4042
image: mysql:8.0

app/Strategies/OAuth2LoginStrategy.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ public function getLogin()
6161
{
6262
Log::debug("OAuth2LoginStrategy::getLogin");
6363

64-
if (!Auth::guest())
65-
return Redirect::action("UserController@getProfile");
64+
if (!Auth::guest()) {
65+
Log::debug("OAuth2LoginStrategy::getLogin user already authenticated, redirecting to auth endpoint");
66+
return Redirect::action("OAuth2\OAuth2ProviderController@auth");
67+
}
6668

6769
$this->login_hint_process_strategy->process();
6870

app/libs/OAuth2/GrantTypes/InteractiveGrantType.php

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,16 @@ protected function processUserHint(OAuth2AuthenticationRequest $request, Client
461461
$token_hint = $request->getIdTokenHint();
462462
$otp_login_hint = $request->getOTPLoginHint();
463463

464-
Log::debug(sprintf("InteractiveGrant::processUserHint request %s client %s", $request->__toString(), $client->getId()));
464+
Log::debug
465+
(
466+
sprintf
467+
(
468+
"InteractiveGrant::processUserHint request %s client %s",
469+
$request->__toString(),
470+
$client->getId()
471+
)
472+
);
473+
465474
// process login hint
466475
$user = null;
467476
if (!empty($otp_login_hint) && !empty ($login_hint)
@@ -484,7 +493,7 @@ protected function processUserHint(OAuth2AuthenticationRequest $request, Client
484493
$user = $this->auth_service->getUserById($user_id);
485494
}
486495
$request->markParamAsProcessed(OAuth2Protocol::OAuth2Protocol_LoginHint);
487-
} else if(!empty($token_hint)) {
496+
} else if(!empty($token_hint) && !$request->isProcessedParam(OAuth2Protocol::OAuth2Protocol_IDTokenHint)) {
488497
Log::debug("InteractiveGrant::processUserHint processing Token hint...");
489498

490499
$jwt = BasicJWTFactory::build($token_hint);
@@ -544,6 +553,7 @@ protected function processUserHint(OAuth2AuthenticationRequest $request, Client
544553

545554
$this->auth_service->reloadSession($jti->getValue());
546555

556+
$request->markParamAsProcessed(OAuth2Protocol::OAuth2Protocol_IDTokenHint);
547557
}
548558
if(!is_null($user))
549559
{
@@ -689,6 +699,15 @@ protected function shouldForceReLogin(OAuth2AuthorizationRequest $request, IClie
689699
*/
690700
protected function mustAuthenticateUser(OAuth2AuthorizationRequest $request, Client $client)
691701
{
702+
Log::debug
703+
(
704+
sprintf
705+
(
706+
"InteractiveGrant::mustAuthenticateUser: request %s client %s",
707+
$request->__toString(),
708+
$client->getClientId()
709+
)
710+
);
692711

693712
if ($request instanceof OAuth2AuthenticationRequest) {
694713
try {
@@ -702,19 +721,22 @@ protected function mustAuthenticateUser(OAuth2AuthorizationRequest $request, Cli
702721
throw $ex;
703722
}
704723
catch (Exception $ex){
705-
Log::warning($ex);
724+
Log::warning("InteractiveGrant::mustAuthenticateUser processUserHint generic error", [ 'error' => $ex]);
725+
$this->auth_service->logout(false);
706726
return true;
707727
}
708728
}
709729

710730
if($this->shouldPromptLogin($request))
711731
{
732+
Log::warning("InteractiveGrant::mustAuthenticateUser: shouldPromptLogin");
712733
$this->auth_service->logout(false);
713734
return true;
714735
}
715736

716737
if($this->shouldForceReLogin($request, $client))
717738
{
739+
Log::warning("InteractiveGrant::mustAuthenticateUser: shouldForceReLogin");
718740
$this->auth_service->logout(false);
719741
return true;
720742
}

0 commit comments

Comments
 (0)