Skip to content

Commit

Permalink
Simplify cookie write conditions
Browse files Browse the repository at this point in the history
When:
 - SP allows storing of cookies
 - Institution allows use of the SSO on 2FA feature
 - The user performed an actual 2FA authentication

Then the cookie can be written

Previously other (incorrect) preconditions were checked.
  • Loading branch information
MKodde committed Sep 14, 2023
1 parent 9c6ecd5 commit 8c2baaa
Showing 1 changed file with 3 additions and 55 deletions.
58 changes: 3 additions & 55 deletions src/Surfnet/StepupGateway/GatewayBundle/Sso2fa/CookieService.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,13 @@ public function handleSsoOn2faCookieStorage(
$secondFactor->institution
)
);

if ($isEnabled) {
// The cookie reader can return a NullCookie if the cookie is not present, was expired or was otherwise
// deemed invalid. See the CookieHelper::read implementation for details.
$ssoCookie = $this->read($request);
$identityId = $responseContext->getIdentityNameId();
$loa = $secondFactor->getLoaLevel($this->secondFactorTypeService);
$isValid = $this->isCookieValid($ssoCookie, $loa, $identityId);
$isVerifiedBySsoOn2faCookie = $responseContext->isVerifiedBySsoOn2faCookie();
// Did the LoA requirement change? If a higher LoA was requested, or did a new token authentication
// take place? In that case create a new SSO on 2FA cookie
if ($this->shouldAddCookie($ssoCookie, $isValid, $loa, $isVerifiedBySsoOn2faCookie)) {
// Did the user perform a new second factor authentication?
if (!$isVerifiedBySsoOn2faCookie) {
$cookie = CookieValue::from($identityId, $secondFactor->secondFactorId, $loa);
$this->store($httpResponse, $cookie);
}
Expand Down Expand Up @@ -200,54 +196,6 @@ public function getCookieFingerprint(Request $request): string
return $this->cookieHelper->fingerprint($request);
}

/**
* This method determines if an SSO on 2FA cookie should be created.
*
* The comments in the code block should give a good feel for what business rules
* are applied in this method.
*
* @param CookieValueInterface $ssoCookie The SSO on 2FA cookie as read from the HTTP response
* @param float $loa The LoA that was requested for this authentication, used to
* compare to the LoA stored in the SSO cookie
* @param bool $wasAuthenticatedWithSsoOn2faCookie Indicator if the currently running authentication was performed
* with the SSO on 2FA cookie
*/
private function shouldAddCookie(
CookieValueInterface $ssoCookie,
bool $isCookieValid,
float $loa,
bool $wasAuthenticatedWithSsoOn2faCookie
): bool {
// When the cookie is not yet set, was expired or was otherwise deemed invalid, we get a NullCookieValue
// back from the reader. Indicating there is no valid cookie present.
$cookieNotSet = $ssoCookie instanceof NullCookieValue;
// OR the existing cookie does exist, but the LoA stored in that cookie does not match the required LoA
$cookieDoesNotMeetLoaRequirement = ($ssoCookie instanceof CookieValue && !$ssoCookie->meetsRequiredLoa($loa));
if (!$ssoCookie instanceof NullCookieValue && $cookieDoesNotMeetLoaRequirement) {
$this->logger->notice(
sprintf(
'Storing new SSO on 2FA cookie as LoA requirement (%d changed to %d) changed',
$ssoCookie->getLoa(),
$loa
)
);
}
// OR when a new authentication took place, we replace the existing cookie with a new one
if (!$wasAuthenticatedWithSsoOn2faCookie) {
$this->logger->notice('Storing new SSO on 2FA cookie as a new authentication took place');
}

// Or when the cookie is not valid for some reason (see logs for the specific error)
if (!$isCookieValid) {
$this->logger->notice('Storing new SSO on 2FA cookie, the current cookie is invalid');
}

return $cookieNotSet ||
!$isCookieValid ||
$cookieDoesNotMeetLoaRequirement ||
!$wasAuthenticatedWithSsoOn2faCookie;
}

private function store(Response $response, CookieValueInterface $cookieValue)
{
$this->cookieHelper->write($response, $cookieValue);
Expand Down

0 comments on commit 8c2baaa

Please sign in to comment.