SecondFactorController::selectSecondFactorForVerificationAction()

Complexity

Conditions	6
Paths	11

Size

Total Lines	97
Code Lines	65

Duplication

Lines	0
Ratio	0 %

Importance

Changes	4
Bugs	0	Features	0

<?php
    public function selectSecondFactorForVerificationAction($authenticationMode, Request $request)

Missing doc comment for function selectSecondFactorForVerificationAction()

    {
        $this->supportsAuthenticationMode($authenticationMode);
        $context = $this->getResponseContext($authenticationMode);

        $originalRequestId = $context->getInResponseTo();

        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */

The open comment tag must be the only content on the line

Missing short description in doc comment

The close comment tag must be the only content on the line

        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
        $logger->notice('Determining which second factor to use...');

        try {
            // Retrieve all requirements to determine the required LoA
            $requestedLoa = $context->getRequiredLoa();
            $spConfiguredLoas = $context->getServiceProvider()->get('configuredLoas');

            $identityNameId = $context->getIdentityNameId();
            $normalizedIdpSho = $context->getNormalizedSchacHomeOrganization();
            $normalizedUserSho = $this->getStepupService()->getNormalizedUserShoByIdentityNameId($identityNameId);

            $requiredLoa = $this
                ->getStepupService()
                ->resolveHighestRequiredLoa(
                    $requestedLoa,
                    $spConfiguredLoas,

It seems like $spConfiguredLoas can also be of type null; however, parameter $spConfiguredLoas of Surfnet\StepupGateway\GatewayBundle\Service\StepUpAuthenticationService::resolveHighestRequiredLoa() does only seem to accept array, maybe add an additional type check?

                    $normalizedIdpSho,
                    $normalizedUserSho
                );
        } catch (LoaCannotBeGivenException $e) {
            // Log the message of the domain exception, this contains a meaningful message.
            $logger->notice($e->getMessage());

            return $this->forward(
                'SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven',
                ['authenticationMode' => $authenticationMode]
            );
        }

        $logger->notice(sprintf('Determined that the required Loa is "%s"', $requiredLoa));

        if ($this->getStepupService()->isIntrinsicLoa($requiredLoa)) {
            $this->get('gateway.authentication_logger')->logIntrinsicLoaAuthentication($originalRequestId);
            return $this->forward($context->getResponseAction());

It seems like $context->getResponseAction() can also be of type null; however, parameter $controller of Symfony\Bundle\FrameworkBundle\Controller\Controller::forward() does only seem to accept string, maybe add an additional type check?

        }

        $secondFactorCollection = $this
            ->getStepupService()
            ->determineViableSecondFactors(
                $context->getIdentityNameId(),
                $requiredLoa,
                $this->get('gateway.service.whitelist')
            );

        /**

Missing short description in doc comment

         * @var CookieService $ssoCookieService
         */
        $ssoCookieService = $this->get('gateway.service.sso_2fa_cookie');
        if ($ssoCookieService->shouldSkip2faAuthentication(
            $context,
            $requiredLoa->getLevel(),
            $identityNameId,
            $secondFactorCollection,
            $request
        )) {

Closing parenthesis of a multi-line IF statement must be on a new line

            $logger->notice('Skipping second factor authentication. Required LoA was met by the LoA recorded in the cookie');
            $this->getResponseContext($authenticationMode)->markSecondFactorVerified();
            $this->getResponseContext($authenticationMode)->markVerifiedBySsoOn2faCookie($ssoCookieService->getCookieFingerprint($request));
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);

It seems like $originalRequestId can also be of type null; however, parameter $requestId of Surfnet\StepupGateway\GatewayBundle\Monolog\Logger\AuthenticationLogger::logSecondFactorAuthentication() does only seem to accept string, maybe add an additional type check?

            return $this->forward($context->getResponseAction());
        }
        switch (count($secondFactorCollection)) {
            case 0:

Line indented incorrectly; expected 8 spaces, found 12

                $logger->notice('No second factors can give the determined Loa');

                return $this->forward(
                    'SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven',
                    ['authenticationMode' => $authenticationMode]
                );
                break;

break is not strictly necessary here and could be removed.

            case 1:

Line indented incorrectly; expected 8 spaces, found 12

                $secondFactor = $secondFactorCollection->first();
                $logger->notice(sprintf(

The opening parenthesis of a multi-line function call should be the last content on the line.

                    'Found "%d" second factors, using second factor of type "%s"',
                    count($secondFactorCollection),
                    $secondFactor->secondFactorType
                ));

For multi-line function calls, the closing parenthesis should be on a new line.

                return $this->selectAndRedirectTo($secondFactor, $context, $authenticationMode);
                break;

            default:

Line indented incorrectly; expected 8 spaces, found 12

                return $this->forward(
                    'SurfnetStepupGatewayGatewayBundle:SecondFactor:chooseSecondFactor',
                    ['authenticationMode' => $authenticationMode, 'secondFactors' => $secondFactorCollection]
                );
                break;
        }