Skip to content

Commit a59a4bb

Browse files
authored
fix(oauth2): prevent profile redirect during OIDC consent flow (#120)
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 72d6c09 commit a59a4bb

5 files changed

Lines changed: 853 additions & 5 deletions

File tree

.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)