YubikeyController::verifyYubiKeySecondFactorAction() - Code Metrics - Inspection of "Feature/second factor only" - OpenConext/Stepup-Gateway - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — develop (#91)

by Boy

created 2016-05-26 10:18 UTC

verifyYubiKeySecondFactorAction() B

↳ Parent: YubikeyController

Complexity

Conditions	5
Paths	5

Size

Total Lines	55
Code Lines	29

Duplication

Lines	0
Ratio	0 %

Importance

Changes	3
Bugs	0	Features	0

Metric	Value
c	3
b	0
f	0
dl	0
loc	55
rs	8.7752
cc	5
eloc	29
nc	5
nop	1

How to fix Long Method

<?php

/**
 * Copyright 2014 SURFnet bv
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

namespace Surfnet\StepupGateway\GatewayBundle\Controller;

use Surfnet\StepupGateway\GatewayBundle\Saml\ResponseContext;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\Form\FormError;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Surfnet\StepupGateway\GatewayBundle\Command\VerifyYubikeyOtpCommand;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;

class YubikeyController extends Controller
{
    /**
     * @Template
     * @param Request $request
     * @return array|Response
     */
    public function verifyYubiKeySecondFactorAction(Request $request)
    {
        /** @var ResponseContext $responseContext */
        $context = $this->get(
          $this->get('gateway.proxy.state_handler')->getResponseContextServiceId()
        );
        $originalRequestId = $context->getInResponseTo();

        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);

        $selectedSecondFactor = $this->get('gateway.service.require_selected_factor')
          ->requireSelectedSecondFactor($logger);

        $logger->notice('Verifying possession of Yubikey second factor');

        $command = new VerifyYubikeyOtpCommand();
        $command->secondFactorId = $selectedSecondFactor;

        $form = $this->createForm('gateway_verify_yubikey_otp', $command)->handleRequest($request);

        if ($form->get('cancel')->isClicked()) {
interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Failure:sendAuthenticationCancelledByUser');
        }

        if (!$form->isValid()) {
            // OTP field is rendered empty in the template.
            return ['form' => $form->createView()];
        }

        $result = $this->get('gateway.service.stepup_authentication')->verifyYubikeyOtp($command);

        if ($result->didOtpVerificationFail()) {
            $form->addError(new FormError('gateway.form.verify_yubikey.otp_verification_failed'));

            // OTP field is rendered empty in the template.
            return ['form' => $form->createView()];
        } elseif (!$result->didPublicIdMatch()) {
            $form->addError(new FormError('gateway.form.verify_yubikey.public_id_mismatch'));

            // OTP field is rendered empty in the template.
            return ['form' => $form->createView()];
        }

        $context->markSecondFactorVerified();
        $this->get('gateway.authentication_logger')->logSecondFactorAuthentication($originalRequestId);

        $logger->info(
          sprintf(
            'Marked Yubikey Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
            $selectedSecondFactor
          )
        );

        return $this->forward($context->getResponseAction());
    }
}


1			<?php
2
3			/**
4			* Copyright 2014 SURFnet bv
5			*
6			* Licensed under the Apache License, Version 2.0 (the "License");
7			* you may not use this file except in compliance with the License.
8			* You may obtain a copy of the License at
9			*
10			* http://www.apache.org/licenses/LICENSE-2.0
11			*
12			* Unless required by applicable law or agreed to in writing, software
13			* distributed under the License is distributed on an "AS IS" BASIS,
14			* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15			* See the License for the specific language governing permissions and
16			* limitations under the License.
17			*/
18
19			namespace Surfnet\StepupGateway\GatewayBundle\Controller;
20
21			use Surfnet\StepupGateway\GatewayBundle\Saml\ResponseContext;
22			use Symfony\Bundle\FrameworkBundle\Controller\Controller;
23			use Symfony\Component\Form\FormError;
24			use Symfony\Component\HttpFoundation\Request;
25			use Symfony\Component\HttpFoundation\Response;
26			use Surfnet\StepupGateway\GatewayBundle\Command\VerifyYubikeyOtpCommand;
27			use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
28
29			class YubikeyController extends Controller
30			{
31			/**
32			* @Template
33			* @param Request $request
34			* @return array\|Response
35			*/
36			public function verifyYubiKeySecondFactorAction(Request $request)
37			{
38			/** @var ResponseContext $responseContext */
39			$context = $this->get(
40			$this->get('gateway.proxy.state_handler')->getResponseContextServiceId()
41			);
42			$originalRequestId = $context->getInResponseTo();
43
44			$logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
45
46			$selectedSecondFactor = $this->get('gateway.service.require_selected_factor')
47			->requireSelectedSecondFactor($logger);
48
49			$logger->notice('Verifying possession of Yubikey second factor');
50
51			$command = new VerifyYubikeyOtpCommand();
52			$command->secondFactorId = $selectedSecondFactor;
53
54			$form = $this->createForm('gateway_verify_yubikey_otp', $command)->handleRequest($request);
55
56			if ($form->get('cancel')->isClicked()) {
			0 ignored issues – show Bug introduced 2015-12-15 13:35 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Symfony\Component\Form\FormInterface` as the method `isClicked()` does only exist in the following implementations of said interface: `Symfony\Component\Form\SubmitButton`. Let’s take a look at an example: interface User { /** @return string / public function getPassword(); } class MyUser implements User { public function getPassword() { // return something } public function getDisplayName() { // return some name. } } class AuthSystem { public function authenticate(User $user) { $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName())); // do something. } } In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } Add an additional type-check: class AuthSystem { public function authenticate(User $user) { if ($user instanceof MyUser) { $this->logger->info(/* ... /); } // or alternatively if ( ! $user instanceof MyUser) { throw new \LogicException( '$user must be an instance of MyUser, ' .'other instances are not supported.' ); } } } Note:* PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case. Add the method to the interface: interface User { /** @return string / public function getPassword(); /* @return string */ public function getDisplayName(); } Loading history...
57			return $this->forward('SurfnetStepupGatewayGatewayBundle:Failure:sendAuthenticationCancelledByUser');
58			}
59
60			if (!$form->isValid()) {
61			// OTP field is rendered empty in the template.
62			return ['form' => $form->createView()];
63			}
64
65			$result = $this->get('gateway.service.stepup_authentication')->verifyYubikeyOtp($command);
66
67			if ($result->didOtpVerificationFail()) {
68			$form->addError(new FormError('gateway.form.verify_yubikey.otp_verification_failed'));
69
70			// OTP field is rendered empty in the template.
71			return ['form' => $form->createView()];
72			} elseif (!$result->didPublicIdMatch()) {
73			$form->addError(new FormError('gateway.form.verify_yubikey.public_id_mismatch'));
74
75			// OTP field is rendered empty in the template.
76			return ['form' => $form->createView()];
77			}
78
79			$context->markSecondFactorVerified();
80			$this->get('gateway.authentication_logger')->logSecondFactorAuthentication($originalRequestId);
81
82			$logger->info(
83			sprintf(
84			'Marked Yubikey Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
85			$selectedSecondFactor
86			)
87			);
88
89			return $this->forward($context->getResponseAction());
90			}
91			}
92

OpenConext / Stepup-Gateway

Pull Request — develop (#91)

verifyYubiKeySecondFactorAction() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Available Fixes

Duplication Side-by-Side

Filter issues like