Skip to content

Commit

Permalink
Ensure consume_assertion.html.twig is rendered once
Browse files Browse the repository at this point in the history
Previously we would always render the regular consume_assertion.html.twig
template, and additionally render the adfs variant when dealing with an
ADFS response. That caused the JS from not ending up in the template, as
Twig concluded that the (reused) JS was already on the page (as it was
just rendered).

This ensures the render method is called only once. Circumventing the
problem. But not realy solving the issue at hand. Which is that the
controller logic is getting growingly more complex and harder to
understand.

For that problem, I'm opening a technical debt issue in Pivotal.

#312
  • Loading branch information
MKodde committed Dec 20, 2023
1 parent 098c675 commit 5a9d4c9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,33 +132,33 @@ public function respondAction(Request $request)
// Reset state
$this->getSecondFactorRespondService()->resetRespondState($responseContext);

// Handle SAML response
$httpResponse = $responseRendering->renderResponse($responseContext, $response, $request);

$ssoCookieService = $this->get('gateway.service.sso_2fa_cookie');
$ssoCookieService->handleSsoOn2faCookieStorage($responseContext, $request, $httpResponse);

// We can now forget the selected second factor.
$responseContext->finalizeAuthentication();

// Check if ADFS response
// Check if ADFS response, if it is, we use the ADFS ACS twig template
$adfsParameters = $this->getSecondFactorAdfsService()->handleAdfsResponse($logger, $responseContext);

if (!is_null($adfsParameters)) {
// Handle Adfs response
$responseRendering = $this->get('second_factor_only.response_rendering');
$xmlResponse = $responseRendering->getResponseAsXML($response);

return $this->render(
$httpResponse = $this->render(
'@SurfnetStepupGatewaySecondFactorOnly/adfs/consume_assertion.html.twig',
[
'acu' => $responseContext->getDestinationForAdfs(),
'samlResponse' => $xmlResponse,
'adfs' => $adfsParameters,
]
);
} else {
// Render the regular SAML response, we do not return it yet, the SSO on 2FA handler will use it to store
// the SSO on 2FA cookie.
$httpResponse = $responseRendering->renderResponse($responseContext, $response, $request);
}

$ssoCookieService = $this->get('gateway.service.sso_2fa_cookie');
$ssoCookieService->handleSsoOn2faCookieStorage($responseContext, $request, $httpResponse);

// We can now forget the selected second factor.
$responseContext->finalizeAuthentication();

return $httpResponse;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,13 @@ public function handleAdfsRequest(LoggerInterface $logger, Request $httpRequest,
}

/**
* This method detectds if we need to return a ADFS response, If so ADFS parameters are returned.
* This method detects if we need to return a ADFS response, If so ADFS parameters are returned.
*
* Second factor verification handled by SecondFactorController is
* finished. The user was forwarded back to this action with an internal
* redirect. This method sends a AuthnResponse back to the service
* provider in response to the AuthnRequest received in ssoAction().
*
* @param LoggerInterface $logger
* @param ResponseContext $responseContext
* @return null|\Surfnet\StepupGateway\SecondFactorOnlyBundle\Adfs\ValueObject\Response
* @throws InvalidAdfsResponseException
*/
Expand Down

0 comments on commit 5a9d4c9

Please sign in to comment.