GatewayController::sendLoaCannotBeGivenAction() - Code Metrics - Inspection of "Add sso unhappy flow tests" - OpenConext/Stepup-Gateway - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — bugfix/saml-error-response-0-t... ( b34353...3f23cd )

by Michiel

created 2020-04-14 14:32 UTC

GatewayController::sendLoaCannotBeGivenAction() A

↳ Parent: GatewayController

Complexity

Conditions	2
Paths	2

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	14
rs	9.7998
c	0
b	0
f	0
cc	2
nc	2
nop	1

<?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 SAML2\Response as SAMLResponse;
use Surfnet\StepupGateway\GatewayBundle\Exception\InvalidArgumentException;
use Surfnet\StepupGateway\GatewayBundle\Exception\RequesterFailureException;
use Surfnet\StepupGateway\GatewayBundle\Exception\ResponseFailureException;
use Surfnet\StepupGateway\GatewayBundle\Exception\RuntimeException;
use Surfnet\StepupGateway\GatewayBundle\Saml\ResponseContext;
use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\ConsumeAssertionService;
use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\FailedResponseService;
use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\LoginService;
use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\RespondService;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\HttpException;

/**
 * Entry point for the Stepup login flow.
 *
 * See docs/GatewayState.md for a high-level diagram on how this controller
 * interacts with outside actors and other parts of Stepup.
 */
class GatewayController extends Controller
{
    const RESPONSE_CONTEXT_SERVICE_ID = 'gateway.proxy.response_context';

    const MODE_SFO = 'sfo';
    const MODE_SSO = 'sso';

    /**
     * Receive an AuthnRequest from a service provider.
     *
     * The service provider is either a Stepup component (SelfService, RA) or
     * an external service provider.
     *
     * This single sign-on action will start a new SAML request to the remote
     * IDP configured in Stepup (most likely to be an instance of OpenConext
     * EngineBlock).
     *
     * @param Request $httpRequest
     * @return \Symfony\Component\HttpFoundation\RedirectResponse|Response
     */
    public function ssoAction(Request $httpRequest)
    {
        /** @var \Psr\Log\LoggerInterface $logger */
        $logger = $this->get('logger');

        $redirectBinding = $this->get('surfnet_saml.http.redirect_binding');
        $gatewayLoginService = $this->getGatewayLoginService();

        $logger->notice('Received AuthnRequest, started processing');

        try {
            $proxyRequest = $gatewayLoginService->singleSignOn($httpRequest);
        } catch (RequesterFailureException $e) {
            $response = $this->getGatewayFailedResponseService()->createRequesterFailureResponse(
                $this->getResponseContext(self::MODE_SSO)
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
            );

            return $this->renderSamlResponse('consumeAssertion', $response, self::MODE_SSO);
        }

        return $redirectBinding->createResponseFor($proxyRequest);
    }

    /**
     *
     */
    public function proxySsoAction()
    {
        throw new HttpException(418, 'Not Yet Implemented');
    }

    /**
     * Receive an AuthnResponse from an identity provider.
     *
     * The AuthnRequest started in ssoAction() resulted in an AuthnResponse
     * from the IDP. This method handles the assertion and forwards the user
     * using an internal redirect to the SecondFactorController to start the
     * actual second factor verification.
     *
     * @param Request $request
     * @return \Symfony\Component\HttpFoundation\Response
     */
    public function consumeAssertionAction(Request $request)
    {
        $responseContext = $this->getResponseContext(self::MODE_SSO);
        $gatewayLoginService = $this->getGatewayConsumeAssertionService();

        try {
            $gatewayLoginService->consumeAssertion($request, $responseContext);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
        } catch (ResponseFailureException $e) {
            $response = $this->getGatewayFailedResponseService()->createResponseFailureResponse($responseContext);

            return $this->renderSamlResponse('unprocessableResponse', $response, self::MODE_SSO);
        }

        // Forward to the selectSecondFactorForVerificationSsoAction, this in turn will forward to the correct
        // verification action (based on authentication type sso/sfo)
        return $this->forward('SurfnetStepupGatewayGatewayBundle:SecondFactor:selectSecondFactorForVerificationSso');
    }

    /**
     * Send a SAML response back to the service provider.
     *
     * 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().
     */
    public function respondAction()
    {
        $responseContext = $this->getResponseContext(self::MODE_SSO);
        $gatewayLoginService = $this->getGatewayRespondService();

        $response = $gatewayLoginService->respond($responseContext);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
        $gatewayLoginService->resetRespondState($responseContext);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

        return $this->renderSamlResponse('consumeAssertion', $response, self::MODE_SSO);
    }

    /**
     * This action is also used from the context of SecondFactorOnly authentications
     * @param $authenticationMode
     * @return Response
     */
    public function sendLoaCannotBeGivenAction(Request $request)
    {
        if (!$request->get('authenticationMode', false)) {
            throw new RuntimeException('Unable to determine the authentication mode in the sendLoaCannotBeGiven action');
        }
        $authenticationMode = $request->get('authenticationMode');
        $this->supportsAuthenticationMode($authenticationMode);
        $responseContext = $this->getResponseContext($authenticationMode);
        $gatewayLoginService = $this->getGatewayFailedResponseService();

        $response = $gatewayLoginService->sendLoaCannotBeGiven($responseContext);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

        return $this->renderSamlResponse('consumeAssertion', $response, $authenticationMode);
    }

    /**
     * @return Response
     */
    public function sendAuthenticationCancelledByUserAction()
    {
        // The authentication mode is read from the parent request, in the meantime a forward was followed, making
        // reading the auth mode from the current request impossible.
        // @see: \Surfnet\StepupGateway\GatewayBundle\Controller\SecondFactorController::cancelAuthenticationAction
        $requestStack = $this->get('request_stack');
        $request = $requestStack->getParentRequest();
        if (!$request->get('authenticationMode', false)) {
            throw new RuntimeException('Unable to determine the authentication mode in the sendAuthenticationCancelledByUser action');
        }
        $authenticationMode = $request->get('authenticationMode');

        $this->supportsAuthenticationMode($authenticationMode);
        $responseContext = $this->getResponseContext($authenticationMode);
        $gatewayLoginService = $this->getGatewayFailedResponseService();

        $response = $gatewayLoginService->sendAuthenticationCancelledByUser($responseContext);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

        return $this->renderSamlResponse('consumeAssertion', $response, $authenticationMode);
    }

    /**
     * @param string $view
     * @param SAMLResponse $response
     * @param $authenticationMode
     * @return Response
     */
    public function renderSamlResponse($view, SAMLResponse $response, $authenticationMode)
    {
        $this->supportsAuthenticationMode($authenticationMode);
        $responseContext = $this->getResponseContext($authenticationMode);

        return $this->render($view, [
            'acu'        => $responseContext->getDestination(),
            'response'   => $this->getResponseAsXML($response),
            'relayState' => $responseContext->getRelayState()
        ]);
    }

    /**
     * @param string   $view
     * @param array    $parameters
     * @param Response $response

     * @return Response
     */
    public function render($view, array $parameters = array(), Response $response = null)
    {
        return parent::render(
            'SurfnetStepupGatewayGatewayBundle:Gateway:' . $view . '.html.twig',
            $parameters,
            $response
        );
    }

    /**
     * @return ResponseContext

     */
    public function getResponseContext($authenticationMode)

    {
        switch ($authenticationMode) {
            case self::MODE_SFO:
                return $this->get($this->get('gateway.proxy.sfo.state_handler')->getResponseContextServiceId());
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}
            case self::MODE_SSO:
                return $this->get($this->get('gateway.proxy.sso.state_handler')->getResponseContextServiceId());
                break;
switch ($x) {
    case 1:
        return 'foo';
        break; // This break is not necessary and can be left off.
}
        }
    }

    /**
     * @param SAMLResponse $response
     * @return string
     */
    private function getResponseAsXML(SAMLResponse $response)
    {
        return base64_encode($response->toUnsignedXML()->ownerDocument->saveXML());
    }

    /**
     * @return LoginService
     */
    private function getGatewayLoginService()
    {
        return $this->get('gateway.service.gateway.login');
    }

    /**
     * @return ConsumeAssertionService
     */
    private function getGatewayConsumeAssertionService()
    {
        return $this->get('gateway.service.gateway.consume_assertion');
    }

    /**
     * @return RespondService
     */
    private function getGatewayRespondService()
    {
        return $this->get('gateway.service.gateway.respond');
    }

    /**
     * @return FailedResponseService
     */
    private function getGatewayFailedResponseService()
    {
        return $this->get('gateway.service.gateway.failed_response');
    }

    private function supportsAuthenticationMode($authenticationMode)
    {
        if (!($authenticationMode === self::MODE_SSO || $authenticationMode === self::MODE_SFO)) {
            throw new InvalidArgumentException('Invalid authentication mode requested');
        }
    }
}


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 SAML2\Response as SAMLResponse;
22		use Surfnet\StepupGateway\GatewayBundle\Exception\InvalidArgumentException;
23		use Surfnet\StepupGateway\GatewayBundle\Exception\RequesterFailureException;
24		use Surfnet\StepupGateway\GatewayBundle\Exception\ResponseFailureException;
25		use Surfnet\StepupGateway\GatewayBundle\Exception\RuntimeException;
26		use Surfnet\StepupGateway\GatewayBundle\Saml\ResponseContext;
27		use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\ConsumeAssertionService;
28		use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\FailedResponseService;
29		use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\LoginService;
30		use Surfnet\StepupGateway\GatewayBundle\Service\Gateway\RespondService;
31		use Symfony\Bundle\FrameworkBundle\Controller\Controller;
32		use Symfony\Component\HttpFoundation\Request;
33		use Symfony\Component\HttpFoundation\RequestStack;
34		use Symfony\Component\HttpFoundation\Response;
35		use Symfony\Component\HttpKernel\Exception\HttpException;
36
37		/**
38		* Entry point for the Stepup login flow.
39		*
40		* See docs/GatewayState.md for a high-level diagram on how this controller
41		* interacts with outside actors and other parts of Stepup.
42		*/
43		class GatewayController extends Controller
44		{
45		const RESPONSE_CONTEXT_SERVICE_ID = 'gateway.proxy.response_context';
46
47		const MODE_SFO = 'sfo';
48		const MODE_SSO = 'sso';
49
50		/**
51		* Receive an AuthnRequest from a service provider.
52		*
53		* The service provider is either a Stepup component (SelfService, RA) or
54		* an external service provider.
55		*
56		* This single sign-on action will start a new SAML request to the remote
57		* IDP configured in Stepup (most likely to be an instance of OpenConext
58		* EngineBlock).
59		*
60		* @param Request $httpRequest
61		* @return \Symfony\Component\HttpFoundation\RedirectResponse\|Response
62		*/
63		public function ssoAction(Request $httpRequest)
64		{
65		/** @var \Psr\Log\LoggerInterface $logger */
66		$logger = $this->get('logger');
67
68		$redirectBinding = $this->get('surfnet_saml.http.redirect_binding');
69		$gatewayLoginService = $this->getGatewayLoginService();
70
71		$logger->notice('Received AuthnRequest, started processing');
72
73		try {
74		$proxyRequest = $gatewayLoginService->singleSignOn($httpRequest);
75		} catch (RequesterFailureException $e) {
76		$response = $this->getGatewayFailedResponseService()->createRequesterFailureResponse(
77		$this->getResponseContext(self::MODE_SSO)
		0 ignored issues – show Bug introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report It seems like `$this->getResponseContext(self::MODE_SSO)` can be `null`; however, `createRequesterFailureResponse()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
78		);
79
80		return $this->renderSamlResponse('consumeAssertion', $response, self::MODE_SSO);
81		}
82
83		return $redirectBinding->createResponseFor($proxyRequest);
84		}
85
86		/**
87		*
88		*/
89		public function proxySsoAction()
90		{
91		throw new HttpException(418, 'Not Yet Implemented');
92		}
93
94		/**
95		* Receive an AuthnResponse from an identity provider.
96		*
97		* The AuthnRequest started in ssoAction() resulted in an AuthnResponse
98		* from the IDP. This method handles the assertion and forwards the user
99		* using an internal redirect to the SecondFactorController to start the
100		* actual second factor verification.
101		*
102		* @param Request $request
103		* @return \Symfony\Component\HttpFoundation\Response
104		*/
105		public function consumeAssertionAction(Request $request)
106		{
107		$responseContext = $this->getResponseContext(self::MODE_SSO);
108		$gatewayLoginService = $this->getGatewayConsumeAssertionService();
109
110		try {
111		$gatewayLoginService->consumeAssertion($request, $responseContext);
		0 ignored issues – show Bug introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report It seems like `$responseContext` defined by `$this->getResponseContext(self::MODE_SSO)` on line `107` can be `null`; however, `Surfnet\StepupGateway\Ga...ice::consumeAssertion()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
112		} catch (ResponseFailureException $e) {
113		$response = $this->getGatewayFailedResponseService()->createResponseFailureResponse($responseContext);
114
115		return $this->renderSamlResponse('unprocessableResponse', $response, self::MODE_SSO);
116		}
117
118		// Forward to the selectSecondFactorForVerificationSsoAction, this in turn will forward to the correct
119		// verification action (based on authentication type sso/sfo)
120		return $this->forward('SurfnetStepupGatewayGatewayBundle:SecondFactor:selectSecondFactorForVerificationSso');
121		}
122
123		/**
124		* Send a SAML response back to the service provider.
125		*
126		* Second factor verification handled by SecondFactorController is
127		* finished. The user was forwarded back to this action with an internal
128		* redirect. This method sends a AuthnResponse back to the service
129		* provider in response to the AuthnRequest received in ssoAction().
130		*/
131		public function respondAction()
132		{
133		$responseContext = $this->getResponseContext(self::MODE_SSO);
134		$gatewayLoginService = $this->getGatewayRespondService();
135
136		$response = $gatewayLoginService->respond($responseContext);
		0 ignored issues – show Bug introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report It seems like `$responseContext` defined by `$this->getResponseContext(self::MODE_SSO)` on line `133` can be `null`; however, `Surfnet\StepupGateway\Ga...spondService::respond()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
137		$gatewayLoginService->resetRespondState($responseContext);
		0 ignored issues – show Bug introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report It seems like `$responseContext` defined by `$this->getResponseContext(self::MODE_SSO)` on line `133` can be `null`; however, `Surfnet\StepupGateway\Ga...ce::resetRespondState()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
138
139		return $this->renderSamlResponse('consumeAssertion', $response, self::MODE_SSO);
140		}
141
142		/**
143		* This action is also used from the context of SecondFactorOnly authentications
144		* @param $authenticationMode
145		* @return Response
146		*/
147		public function sendLoaCannotBeGivenAction(Request $request)
148		{
149		if (!$request->get('authenticationMode', false)) {
150		throw new RuntimeException('Unable to determine the authentication mode in the sendLoaCannotBeGiven action');
151		}
152		$authenticationMode = $request->get('authenticationMode');
153		$this->supportsAuthenticationMode($authenticationMode);
154		$responseContext = $this->getResponseContext($authenticationMode);
155		$gatewayLoginService = $this->getGatewayFailedResponseService();
156
157		$response = $gatewayLoginService->sendLoaCannotBeGiven($responseContext);
		0 ignored issues – show Bug introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report It seems like `$responseContext` defined by `$this->getResponseContext($authenticationMode)` on line `154` can be `null`; however, `Surfnet\StepupGateway\Ga...:sendLoaCannotBeGiven()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
158
159		return $this->renderSamlResponse('consumeAssertion', $response, $authenticationMode);
160		}
161
162		/**
163		* @return Response
164		*/
165		public function sendAuthenticationCancelledByUserAction()
166		{
167		// The authentication mode is read from the parent request, in the meantime a forward was followed, making
168		// reading the auth mode from the current request impossible.
169		// @see: \Surfnet\StepupGateway\GatewayBundle\Controller\SecondFactorController::cancelAuthenticationAction
170		$requestStack = $this->get('request_stack');
171		$request = $requestStack->getParentRequest();
172		if (!$request->get('authenticationMode', false)) {
173		throw new RuntimeException('Unable to determine the authentication mode in the sendAuthenticationCancelledByUser action');
174		}
175		$authenticationMode = $request->get('authenticationMode');
176
177		$this->supportsAuthenticationMode($authenticationMode);
178		$responseContext = $this->getResponseContext($authenticationMode);
179		$gatewayLoginService = $this->getGatewayFailedResponseService();
180
181		$response = $gatewayLoginService->sendAuthenticationCancelledByUser($responseContext);
		0 ignored issues – show Bug introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report It seems like `$responseContext` defined by `$this->getResponseContext($authenticationMode)` on line `178` can be `null`; however, `Surfnet\StepupGateway\Ga...cationCancelledByUser()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
182
183		return $this->renderSamlResponse('consumeAssertion', $response, $authenticationMode);
184		}
185
186		/**
187		* @param string $view
188		* @param SAMLResponse $response
189		* @param $authenticationMode
190		* @return Response
191		*/
192		public function renderSamlResponse($view, SAMLResponse $response, $authenticationMode)
193		{
194		$this->supportsAuthenticationMode($authenticationMode);
195		$responseContext = $this->getResponseContext($authenticationMode);
196
197		return $this->render($view, [
198		'acu' => $responseContext->getDestination(),
199		'response' => $this->getResponseAsXML($response),
200		'relayState' => $responseContext->getRelayState()
201		]);
202		}
203
204		/**
205		* @param string $view
206		* @param array $parameters
207		* @param Response $response
		0 ignored issues – show Documentation introduced 2015-12-15 13:35 UTC by Report Bug Copy Issue Report Should the type for parameter `$response` not be `null\|Response`? This check looks for `@param` annotations where the type inferred by our type inference engine differs from the declared type. It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
208		* @return Response
209		*/
210		public function render($view, array $parameters = array(), Response $response = null)
211		{
212		return parent::render(
213		'SurfnetStepupGatewayGatewayBundle:Gateway:' . $view . '.html.twig',
214		$parameters,
215		$response
216		);
217		}
218
219		/**
220		* @return ResponseContext
		0 ignored issues – show Documentation introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report Should the return type not be `ResponseContext\|null`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
221		*/
222	View Code Duplication	public function getResponseContext($authenticationMode)
		0 ignored issues – show Duplication introduced 2020-04-14 14:34 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
223		{
224		switch ($authenticationMode) {
225		case self::MODE_SFO:
226		return $this->get($this->get('gateway.proxy.sfo.state_handler')->getResponseContextServiceId());
227		break;
		0 ignored issues – show Unused Code introduced 2020-04-02 12:58 UTC by Report Bug Copy Issue Report `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
228		case self::MODE_SSO:
229		return $this->get($this->get('gateway.proxy.sso.state_handler')->getResponseContextServiceId());
230		break;
		0 ignored issues – show Unused Code introduced 2020-04-02 12:58 UTC by Report Bug Copy Issue Report `break` is not strictly necessary here and could be removed. The `break` statement is not necessary if it is preceded for example by a `return` statement: switch ($x) { case 1: return 'foo'; break; // This break is not necessary and can be left off. } If you would like to keep this construct to be consistent with other `case` statements, you can safely mark this issue as a false-positive. Loading history...
231		}
232		}
233
234		/**
235		* @param SAMLResponse $response
236		* @return string
237		*/
238		private function getResponseAsXML(SAMLResponse $response)
239		{
240		return base64_encode($response->toUnsignedXML()->ownerDocument->saveXML());
241		}
242
243		/**
244		* @return LoginService
245		*/
246		private function getGatewayLoginService()
247		{
248		return $this->get('gateway.service.gateway.login');
249		}
250
251		/**
252		* @return ConsumeAssertionService
253		*/
254		private function getGatewayConsumeAssertionService()
255		{
256		return $this->get('gateway.service.gateway.consume_assertion');
257		}
258
259		/**
260		* @return RespondService
261		*/
262		private function getGatewayRespondService()
263		{
264		return $this->get('gateway.service.gateway.respond');
265		}
266
267		/**
268		* @return FailedResponseService
269		*/
270		private function getGatewayFailedResponseService()
271		{
272		return $this->get('gateway.service.gateway.failed_response');
273		}
274
275		private function supportsAuthenticationMode($authenticationMode)
276		{
277		if (!($authenticationMode === self::MODE_SSO \|\| $authenticationMode === self::MODE_SFO)) {
278		throw new InvalidArgumentException('Invalid authentication mode requested');
279		}
280		}
281		}
282

OpenConext / Stepup-Gateway

Push — bugfix/saml-error-response-0-t... ( b34353...3f23cd )

GatewayController::sendLoaCannotBeGivenAction() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like