Skip to content

Commit

Permalink
FIX Users without CMS access are now able to log in correctly
Browse files Browse the repository at this point in the history
This fixes an issue that would continue to redirect the user to MFA after they
logged in, ending up in a redirect loop. This patch checks whether it should
redirect to MFA as well as checking if the registration is not complete yet
before executing this logic.
  • Loading branch information
robbieaverill committed Jun 24, 2019
1 parent 2195c28 commit d9f8e8c
Showing 1 changed file with 24 additions and 9 deletions.
33 changes: 24 additions & 9 deletions src/Authenticator/LoginHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ public function redirectAfterSuccessfulLogin(): HTTPResponse
_t(__CLASS__ . '.MFA_LOGIN_INCOMPLETE', 'You must provide MFA login details'),
ValidationResult::TYPE_ERROR
);
return $this->redirect($loginUrl);
return $this->redirect($this->getBackURL() ?: $loginUrl);
}

$request = $this->getRequest();
Expand All @@ -431,7 +431,9 @@ public function redirectAfterSuccessfulLogin(): HTTPResponse
// This is potentially redundant logic as the member should only be logged in if they've fully registered.
// They're allowed to login if they can skip - so only do assertions if they're not allowed to skip
// We'll also check that they've registered the required MFA details
if (!$enforcementManager->canSkipMFA($member) && !$enforcementManager->hasCompletedRegistration($member)) {
if (!$enforcementManager->hasCompletedRegistration($member)
&& $enforcementManager->shouldRedirectToMFA($member)
) {
// Log them out again
/** @var IdentityStore $identityStore */
$identityStore = Injector::inst()->get(IdentityStore::class);
Expand All @@ -441,18 +443,12 @@ public function redirectAfterSuccessfulLogin(): HTTPResponse
_t(__CLASS__ . '.INVALID_REGISTRATION', 'You must complete MFA registration'),
ValidationResult::TYPE_ERROR
);
return $this->redirect($loginUrl);
return $this->redirect($this->getBackURL() ?: $loginUrl);
}

// Clear the "additional data"
$data = $request->getSession()->get(static::SESSION_KEY . '.additionalData') ?: [];
$request->getSession()->clear(static::SESSION_KEY . '.additionalData');

// Redirecting after successful login expects a getVar to be set
if (!empty($data['BackURL'])) {
$request['BackURL'] = $data['BackURL'];
}

// Ensure any left over session state is cleaned up
$store = $this->getStore();
if ($store) {
Expand Down Expand Up @@ -504,6 +500,25 @@ public function getLogger(): ?LoggerInterface
return $this->logger;
}

/**
* Adds another option for the back URL to be returned from a current MFA session store
*
* @return string|null
*/
public function getBackURL(): ?string
{
$backURL = parent::getBackURL();

if (!$backURL && $this->getRequest()) {
$data = $this->getRequest()->getSession()->get(static::SESSION_KEY . '.additionalData');
if (isset($data['BackURL'])) {
$backURL = $data['BackURL'];
}
}

return $backURL;
}

/**
* Respond with the given array as a JSON response
*
Expand Down

0 comments on commit d9f8e8c

Please sign in to comment.