Completed
Push — feature/implement-state-handli... ( 10b49e...43ce66 )
by Michiel
02:04
created

verifySmsSecondFactorAction()   B

Complexity

Conditions 4
Paths 4

Size

Total Lines 57

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 57
rs 8.9381
c 0
b 0
f 0
cc 4
nc 4
nop 1

How to fix   Long Method   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

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 Psr\Log\LoggerInterface;
22
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
23
use Surfnet\StepupBundle\Command\VerifyPossessionOfPhoneCommand;
24
use Surfnet\StepupBundle\Value\PhoneNumber\InternationalPhoneNumber;
25
use Surfnet\StepupBundle\Value\SecondFactorType;
26
use Surfnet\StepupGateway\GatewayBundle\Command\ChooseSecondFactorCommand;
27
use Surfnet\StepupGateway\GatewayBundle\Command\SendSmsChallengeCommand;
28
use Surfnet\StepupGateway\GatewayBundle\Command\VerifyYubikeyOtpCommand;
29
use Surfnet\StepupGateway\GatewayBundle\Entity\SecondFactor;
30
use Surfnet\StepupGateway\GatewayBundle\Exception\InvalidArgumentException;
31
use Surfnet\StepupGateway\GatewayBundle\Exception\LoaCannotBeGivenException;
32
use Surfnet\StepupGateway\GatewayBundle\Exception\RuntimeException;
33
use Surfnet\StepupGateway\GatewayBundle\Form\Type\CancelAuthenticationType;
34
use Surfnet\StepupGateway\GatewayBundle\Form\Type\CancelSecondFactorVerificationType;
35
use Surfnet\StepupGateway\GatewayBundle\Form\Type\ChooseSecondFactorType;
36
use Surfnet\StepupGateway\GatewayBundle\Form\Type\SendSmsChallengeType;
37
use Surfnet\StepupGateway\GatewayBundle\Form\Type\VerifySmsChallengeType;
38
use Surfnet\StepupGateway\GatewayBundle\Form\Type\VerifyYubikeyOtpType;
39
use Surfnet\StepupGateway\GatewayBundle\Saml\ResponseContext;
40
use Surfnet\StepupGateway\U2fVerificationBundle\Value\KeyHandle;
41
use Surfnet\StepupU2fBundle\Dto\SignResponse;
42
use Surfnet\StepupU2fBundle\Form\Type\VerifyDeviceAuthenticationType;
43
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
44
use Symfony\Component\Form\FormError;
45
use Symfony\Component\Form\FormInterface;
46
use Symfony\Component\HttpFoundation\RedirectResponse;
47
use Symfony\Component\HttpFoundation\Request;
48
use Symfony\Component\HttpFoundation\Response;
49
use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBagInterface;
50
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
51
52
/**
53
 * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
54
 * @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
55
 */
56
class SecondFactorController extends Controller
57
{
58
    const MODE_SFO = 'sfo';
59
    const MODE_SSO = 'sso';
60
61
    public function selectSecondFactorForVerificationAction($authenticationMode)
62
    {
63
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
64
        $context = $this->getResponseContext($authenticationMode);
65
66
        $originalRequestId = $context->getInResponseTo();
67
68
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
69
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
70
        $logger->notice('Determining which second factor to use...');
71
72
        try {
73
            // Retrieve all requirements to determine the required LoA
74
            $requestedLoa = $context->getRequiredLoa();
75
            $spConfiguredLoas = $context->getServiceProvider()->get('configuredLoas');
76
77
            $normalizedIdpSho = $context->getNormalizedSchacHomeOrganization();
78
            $normalizedUserSho = $this->getStepupService()->getNormalizedUserShoByIdentityNameId($context->getIdentityNameId());
79
80
            $requiredLoa = $this
81
                ->getStepupService()
82
                ->resolveHighestRequiredLoa(
83
                    $requestedLoa,
84
                    $spConfiguredLoas,
85
                    $normalizedIdpSho,
86
                    $normalizedUserSho
87
                );
88
        } catch (LoaCannotBeGivenException $e) {
89
            // Log the message of the domain exception, this contains a meaningful message.
90
            $logger->notice($e->getMessage());
91
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
92
        }
93
94
        $logger->notice(sprintf('Determined that the required Loa is "%s"', $requiredLoa));
95
96
        if ($this->getStepupService()->isIntrinsicLoa($requiredLoa)) {
97
            $this->get('gateway.authentication_logger')->logIntrinsicLoaAuthentication($originalRequestId);
98
99
            return $this->forward($context->getResponseAction());
100
        }
101
102
        $secondFactorCollection = $this
103
            ->getStepupService()
104
            ->determineViableSecondFactors(
105
                $context->getIdentityNameId(),
106
                $requiredLoa,
107
                $this->get('gateway.service.whitelist')
108
            );
109
110
        switch (count($secondFactorCollection)) {
111
            case 0:
112
                $logger->notice('No second factors can give the determined Loa');
113
                return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
114
                break;
0 ignored issues
show
Unused Code introduced by
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...
115
116
            case 1:
117
                $secondFactor = $secondFactorCollection->first();
118
                $logger->notice(sprintf(
119
                    'Found "%d" second factors, using second factor of type "%s"',
120
                    count($secondFactorCollection),
121
                    $secondFactor->secondFactorType
122
                ));
123
124
                return $this->selectAndRedirectTo($secondFactor, $context, $authenticationMode);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 64 can be null; however, Surfnet\StepupGateway\Ga...::selectAndRedirectTo() 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...
125
                break;
0 ignored issues
show
Unused Code introduced by
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...
126
127
            default:
128
                return $this->forward(
129
                    'SurfnetStepupGatewayGatewayBundle:SecondFactor:chooseSecondFactor',
130
                    ['authenticationMode' => $authenticationMode, 'secondFactors' => $secondFactorCollection]
131
                );
132
                break;
0 ignored issues
show
Unused Code introduced by
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...
133
        }
134
    }
135
136
    /**
137
     * @Template
138
     * @param Request $request
139
     * @param string $authenticationMode
140
     * @return array|RedirectResponse|Response
141
     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
142
     */
143
    public function chooseSecondFactorAction(Request $request, $authenticationMode)
144
    {
145
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
146
        $context = $this->getResponseContext($authenticationMode);
147
        $originalRequestId = $context->getInResponseTo();
148
149
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
150
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
151
        $logger->notice('Ask the user which one of his suitable second factor tokens to use...');
152
153
        try {
154
            // Retrieve all requirements to determine the required LoA
155
            $requestedLoa = $context->getRequiredLoa();
156
            $spConfiguredLoas = $context->getServiceProvider()->get('configuredLoas');
157
158
            $normalizedIdpSho = $context->getNormalizedSchacHomeOrganization();
159
            $normalizedUserSho = $this->getStepupService()->getNormalizedUserShoByIdentityNameId($context->getIdentityNameId());
160
161
            $requiredLoa = $this
162
                ->getStepupService()
163
                ->resolveHighestRequiredLoa(
164
                    $requestedLoa,
165
                    $spConfiguredLoas,
166
                    $normalizedIdpSho,
167
                    $normalizedUserSho
168
                );
169
        } catch (LoaCannotBeGivenException $e) {
170
            // Log the message of the domain exception, this contains a meaningful message.
171
            $logger->notice($e->getMessage());
172
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
173
        }
174
175
        $logger->notice(sprintf('Determined that the required Loa is "%s"', $requiredLoa));
176
177
        $secondFactors = $this
178
            ->getStepupService()
179
            ->determineViableSecondFactors(
180
                $context->getIdentityNameId(),
181
                $requiredLoa,
182
                $this->get('gateway.service.whitelist')
183
            );
184
185
        $command = new ChooseSecondFactorCommand();
186
        $command->secondFactors = $secondFactors;
0 ignored issues
show
Documentation Bug introduced by
It seems like $secondFactors of type object<Doctrine\Common\Collections\Collection> is incompatible with the declared type array<integer,object<Sur...e\Entity\SecondFactor>> of property $secondFactors.

Our type inference engine has found an assignment to a property that is incompatible with the declared type of that property.

Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property..

Loading history...
187
188
        $form = $this
189
            ->createForm(
190
                ChooseSecondFactorType::class,
191
                $command,
192
                ['action' => $this->generateUrl('gateway_verify_second_factor_choose_second_factor', ['authenticationMode' => $authenticationMode])]
193
            )
194
            ->handleRequest($request);
195
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
196
197
        if ($form->isSubmitted() && $form->isValid()) {
198
            $buttonName = $form->getClickedButton()->getName();
199
            $formResults = $request->request->get('gateway_choose_second_factor', false);
200
201
            if (!isset($formResults[$buttonName])) {
202
                throw new InvalidArgumentException(
203
                    sprintf(
204
                        'Second factor type "%s" could not be found in the posted form results.',
205
                        $buttonName
206
                    )
207
                );
208
            }
209
210
            $secondFactorType = $formResults[$buttonName];
211
212
            // Filter the selected second factor from the array collection
213
            $secondFactorFiltered = $secondFactors->filter(
214
                function ($secondFactor) use ($secondFactorType) {
215
                    return $secondFactorType === $secondFactor->secondFactorType;
216
                }
217
            );
218
219
            if ($secondFactorFiltered->isEmpty()) {
220
                throw new InvalidArgumentException(
221
                    sprintf(
222
                        'Second factor type "%s" could not be found in the collection of available second factors.',
223
                        $secondFactorType
224
                    )
225
                );
226
            }
227
228
            $secondFactor = $secondFactorFiltered->first();
229
230
            $logger->notice(sprintf('User chose "%s" to use as second factor', $secondFactorType));
231
232
            // Forward to action to verify possession of second factor
233
            return $this->selectAndRedirectTo($secondFactor, $context, $authenticationMode);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 146 can be null; however, Surfnet\StepupGateway\Ga...::selectAndRedirectTo() 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...
234
        } else if ($form->isSubmitted() && !$form->isValid()) {
235
            $form->addError(new FormError('gateway.form.gateway_choose_second_factor.unknown_second_factor_type'));
236
        }
237
238
        return [
239
            'form' => $form->createView(),
240
            'cancelForm' => $cancelForm->createView(),
241
            'secondFactors' => $secondFactors,
242
        ];
243
    }
244
245
    public function verifyGssfAction(Request $request)
246
    {
247
        if (!$request->get('authenticationMode', false)) {
248
            throw new RuntimeException('Unable to determine the authentication mode in the GSSP verification action');
249
        }
250
        $authenticationMode = $request->get('authenticationMode');
251
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
252
        $context = $this->getResponseContext($authenticationMode);
253
254
        $originalRequestId = $context->getInResponseTo();
255
256
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
257
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
258
        $logger->info('Received request to verify GSSF');
259
260
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 252 can be null; however, Surfnet\StepupGateway\Ga...tSelectedSecondFactor() 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...
261
262
        $logger->info(sprintf(
263
            'Selected GSSF "%s" for verfication, forwarding to Saml handling',
264
            $selectedSecondFactor
265
        ));
266
267
        /** @var \Surfnet\StepupGateway\GatewayBundle\Service\SecondFactorService $secondFactorService */
268
        $secondFactorService = $this->get('gateway.service.second_factor_service');
269
        /** @var \Surfnet\StepupGateway\GatewayBundle\Entity\SecondFactor $secondFactor */
270
        $secondFactor = $secondFactorService->findByUuid($selectedSecondFactor);
271
        if (!$secondFactor) {
272
            throw new RuntimeException(sprintf(
273
                'Requested verification of GSSF "%s", however that Second Factor no longer exists',
274
                $selectedSecondFactor
275
            ));
276
        }
277
278
        // Also send the response context service id, as later we need to know if this is regular SSO or SFO authn.
279
        $responseContextServiceId = $context->getResponseContextServiceId();
280
281
        return $this->forward(
282
            'SurfnetStepupGatewaySamlStepupProviderBundle:SamlProxy:sendSecondFactorVerificationAuthnRequest',
283
            [
284
                'provider' => $secondFactor->secondFactorType,
285
                'subjectNameId' => $secondFactor->secondFactorIdentifier,
286
                'responseContextServiceId' => $responseContextServiceId,
287
            ]
288
        );
289
    }
290
291
    public function gssfVerifiedAction(Request $request)
292
    {
293
        $authenticationMode = $request->get('authenticationMode');
294
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
295
        $context = $this->getResponseContext($authenticationMode);
296
297
        $originalRequestId = $context->getInResponseTo();
298
299
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
300
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
301
        $logger->info('Attempting to mark GSSF as verified');
302
303
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 295 can be null; however, Surfnet\StepupGateway\Ga...tSelectedSecondFactor() 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...
304
305
        /** @var \Surfnet\StepupGateway\GatewayBundle\Entity\SecondFactor $secondFactor */
306
        $secondFactor = $this->get('gateway.service.second_factor_service')->findByUuid($selectedSecondFactor);
307
        if (!$secondFactor) {
308
            throw new RuntimeException(
309
                sprintf(
310
                    'Verification of GSSF "%s" succeeded, however that Second Factor no longer exists',
311
                    $selectedSecondFactor
312
                )
313
            );
314
        }
315
316
        $context->markSecondFactorVerified();
317
        $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
318
319
        $logger->info(sprintf(
320
            'Marked GSSF "%s" as verified, forwarding to Gateway controller to respond',
321
            $selectedSecondFactor
322
        ));
323
324
        return $this->forward($context->getResponseAction());
325
    }
326
327
    /**
328
     * @Template
329
     * @param Request $request
330
     * @return array|Response
331
     */
332
    public function verifyYubiKeySecondFactorAction(Request $request)
333
    {
334
        if (!$request->get('authenticationMode', false)) {
335
            throw new RuntimeException('Unable to determine the authentication mode in Yubikey verification action');
336
        }
337
        $authenticationMode = $request->get('authenticationMode');
338
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
339
        $context = $this->getResponseContext($authenticationMode);
340
        $originalRequestId = $context->getInResponseTo();
341
342
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
343
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
344
345
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 339 can be null; however, Surfnet\StepupGateway\Ga...tSelectedSecondFactor() 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...
346
347
        $logger->notice('Verifying possession of Yubikey second factor');
348
349
        $command = new VerifyYubikeyOtpCommand();
350
        $command->secondFactorId = $selectedSecondFactor;
351
352
        $form = $this->createForm(VerifyYubikeyOtpType::class, $command)->handleRequest($request);
353
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
354
355
        if (!$form->isValid()) {
356
            // OTP field is rendered empty in the template.
357
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
358
        }
359
360
        $result = $this->getStepupService()->verifyYubikeyOtp($command);
361
362
        if ($result->didOtpVerificationFail()) {
363
            $form->addError(new FormError('gateway.form.verify_yubikey.otp_verification_failed'));
364
365
            // OTP field is rendered empty in the template.
366
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
367
        } elseif (!$result->didPublicIdMatch()) {
368
            $form->addError(new FormError('gateway.form.verify_yubikey.public_id_mismatch'));
369
370
            // OTP field is rendered empty in the template.
371
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
372
        }
373
374
        $this->getResponseContext($authenticationMode)->markSecondFactorVerified();
375
        $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
376
377
        $logger->info(
378
            sprintf(
379
                'Marked Yubikey Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
380
                $selectedSecondFactor
381
            )
382
        );
383
384
        return $this->forward($context->getResponseAction());
385
    }
386
387
    /**
388
     * @Template
389
     * @param Request $request
390
     * @param string $authenticationMode
0 ignored issues
show
Bug introduced by
There is no parameter named $authenticationMode. Was it maybe removed?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function.

Consider the following example. The parameter $italy is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $island
 * @param array $italy
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was removed, but the annotation was not.

Loading history...
391
     * @return array|Response
392
     */
393
    public function verifySmsSecondFactorAction(Request $request)
394
    {
395
        if (!$request->get('authenticationMode', false)) {
396
            throw new RuntimeException('Unable to determine the authentication mode in the SMS verification action');
397
        }
398
        $authenticationMode = $request->get('authenticationMode');
399
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
400
        $context = $this->getResponseContext($authenticationMode);
401
        $originalRequestId = $context->getInResponseTo();
402
403
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
404
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
405
406
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 400 can be null; however, Surfnet\StepupGateway\Ga...tSelectedSecondFactor() 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...
407
408
        $logger->notice('Verifying possession of SMS second factor, preparing to send');
409
410
        $command = new SendSmsChallengeCommand();
411
        $command->secondFactorId = $selectedSecondFactor;
412
413
        $form = $this->createForm(SendSmsChallengeType::class, $command)->handleRequest($request);
414
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
415
416
        $stepupService = $this->getStepupService();
417
        $phoneNumber = InternationalPhoneNumber::fromStringFormat(
418
            $stepupService->getSecondFactorIdentifier($selectedSecondFactor)
419
        );
420
421
        $otpRequestsRemaining = $stepupService->getSmsOtpRequestsRemainingCount();
422
        $maximumOtpRequests = $stepupService->getSmsMaximumOtpRequestsCount();
423
        $viewVariables = ['otpRequestsRemaining' => $otpRequestsRemaining, 'maximumOtpRequests' => $maximumOtpRequests];
424
425
        if (!$form->isValid()) {
426
            return array_merge(
427
                $viewVariables,
428
                ['phoneNumber' => $phoneNumber, 'form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]
429
            );
430
        }
431
432
        $logger->notice('Verifying possession of SMS second factor, sending challenge per SMS');
433
434
        if (!$stepupService->sendSmsChallenge($command)) {
435
            $form->addError(new FormError('gateway.form.send_sms_challenge.sms_sending_failed'));
436
437
            return array_merge(
438
                $viewVariables,
439
                ['phoneNumber' => $phoneNumber, 'form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]
440
            );
441
        }
442
443
        return $this->redirect(
444
            $this->generateUrl(
445
                'gateway_verify_second_factor_sms_verify_challenge',
446
                ['authenticationMode' => $authenticationMode]
447
            )
448
        );
449
    }
450
451
    /**
452
     * @Template
453
     * @param Request $request
454
     * @param string $authenticationMode
0 ignored issues
show
Bug introduced by
There is no parameter named $authenticationMode. Was it maybe removed?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function.

Consider the following example. The parameter $italy is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $island
 * @param array $italy
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was removed, but the annotation was not.

Loading history...
455
     * @return array|Response
456
     */
457
    public function verifySmsSecondFactorChallengeAction(Request $request)
458
    {
459
        if (!$request->get('authenticationMode', false)) {
460
            throw new RuntimeException('Unable to determine the authentication mode in the SMS challenge action');
461
        }
462
        $authenticationMode = $request->get('authenticationMode');
463
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
464
        $context = $this->getResponseContext($authenticationMode);
465
        $originalRequestId = $context->getInResponseTo();
466
467
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
468
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
469
470
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 464 can be null; however, Surfnet\StepupGateway\Ga...tSelectedSecondFactor() 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...
471
472
        $command = new VerifyPossessionOfPhoneCommand();
473
        $form = $this->createForm(VerifySmsChallengeType::class, $command)->handleRequest($request);
474
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
475
476
        if (!$form->isValid()) {
477
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
478
        }
479
480
        $logger->notice('Verifying input SMS challenge matches');
481
482
        $verification = $this->getStepupService()->verifySmsChallenge($command);
483
484
        if ($verification->wasSuccessful()) {
485
            $this->getStepupService()->clearSmsVerificationState();
486
487
            $this->getResponseContext($authenticationMode)->markSecondFactorVerified();
488
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
489
490
            $logger->info(
491
                sprintf(
492
                    'Marked Sms Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
493
                    $selectedSecondFactor
494
                )
495
            );
496
497
            return $this->forward($context->getResponseAction());
498
        } elseif ($verification->didOtpExpire()) {
499
            $logger->notice('SMS challenge expired');
500
            $form->addError(new FormError('gateway.form.send_sms_challenge.challenge_expired'));
501
        } elseif ($verification->wasAttemptedTooManyTimes()) {
502
            $logger->notice('SMS challenge verification was attempted too many times');
503
            $form->addError(new FormError('gateway.form.send_sms_challenge.too_many_attempts'));
504
        } else {
505
            $logger->notice('SMS challenge did not match');
506
            $form->addError(new FormError('gateway.form.send_sms_challenge.sms_challenge_incorrect'));
507
        }
508
509
        return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
510
    }
511
512
    /**
513
     * @Template
514
     * @param string $authenticationMode
515
     * @return array
516
     */
517
    public function initiateU2fAuthenticationAction($authenticationMode)
518
    {
519
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
520
        $context = $this->getResponseContext($authenticationMode);
521
522
        $originalRequestId = $context->getInResponseTo();
523
524
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
525
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
526
527
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 520 can be null; however, Surfnet\StepupGateway\Ga...tSelectedSecondFactor() 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...
528
        $stepupService = $this->getStepupService();
529
530
        $cancelFormAction = $this->generateUrl(
531
            'gateway_verify_second_factor_u2f_cancel_authentication',
532
            ['authenticationMode' => $authenticationMode]
533
        );
534
        $cancelForm =
535
            $this->createForm(CancelSecondFactorVerificationType::class, null, ['action' => $cancelFormAction]);
536
537
        $logger->notice('Verifying possession of U2F second factor, looking for registration matching key handle');
538
539
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
540
        $keyHandle = new KeyHandle($stepupService->getSecondFactorIdentifier($selectedSecondFactor));
541
        $registration = $service->findRegistrationByKeyHandle($keyHandle);
542
543
        if ($registration === null) {
544
            $logger->critical(
545
                sprintf('No known registration for key handle of second factor "%s"', $selectedSecondFactor)
546
            );
547
            $this->addFlash('error', 'gateway.u2f.alert.unknown_registration');
548
549
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
550
        }
551
552
        $logger->notice('Creating sign request');
553
554
        $signRequest = $service->createSignRequest($registration);
555
        $signResponse = new SignResponse();
556
557
        /** @var AttributeBagInterface $session */
558
        $session = $this->get('gateway.session.u2f');
559
        $session->set('request', $signRequest);
560
561
        $formAction = $this->generateUrl(
562
            'gateway_verify_second_factor_u2f_verify_authentication',
563
            ['authenticationMode' => $authenticationMode]
564
        );
565
        $form = $this->createForm(
566
            VerifyDeviceAuthenticationType::class,
567
            $signResponse,
568
            ['sign_request' => $signRequest, 'action' => $formAction]
569
        );
570
571
        return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
572
    }
573
574
    /**
575
     * @Template("SurfnetStepupGatewayGatewayBundle:SecondFactor:initiateU2fAuthentication.html.twig")
576
     *
577
     * @param Request $request
578
     * @param string $authenticationMode
0 ignored issues
show
Bug introduced by
There is no parameter named $authenticationMode. Was it maybe removed?

This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function.

Consider the following example. The parameter $italy is not defined by the method finale(...).

/**
 * @param array $germany
 * @param array $island
 * @param array $italy
 */
function finale($germany, $island) {
    return "2:1";
}

The most likely cause is that the parameter was removed, but the annotation was not.

Loading history...
579
     * @return array|Response
580
     */
581
    public function verifyU2fAuthenticationAction(Request $request)
582
    {
583
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Bug introduced by
The variable $authenticationMode does not exist. Did you forget to declare it?

This check marks access to variables or properties that have not been declared yet. While PHP has no explicit notion of declaring a variable, accessing it before a value is assigned to it is most likely a bug.

Loading history...
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
584
        $context = $this->getResponseContext($authenticationMode);
585
586
        $originalRequestId = $context->getInResponseTo();
587
588
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
589
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
590
591
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 584 can be null; however, Surfnet\StepupGateway\Ga...tSelectedSecondFactor() 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...
592
593
        $logger->notice('Received sign response from device');
594
595
        /** @var AttributeBagInterface $session */
596
        $session = $this->get('gateway.session.u2f');
597
        $signRequest = $session->get('request');
598
        $signResponse = new SignResponse();
599
600
        $formAction = $this->generateUrl(
601
            'gateway_verify_second_factor_u2f_verify_authentication',
602
            ['authenticationMode' => $authenticationMode]
603
        );
604
        $form = $this
605
            ->createForm(
606
                VerifyDeviceAuthenticationType::class,
607
                $signResponse,
608
                ['sign_request' => $signRequest, 'action' => $formAction]
609
            )
610
            ->handleRequest($request);
611
612
        $cancelFormAction = $this->generateUrl(
613
            'gateway_verify_second_factor_u2f_cancel_authentication',
614
            ['authenticationMode' => $authenticationMode]
615
        );
616
        $cancelForm =
617
            $this->createForm(CancelSecondFactorVerificationType::class, null, ['action' => $cancelFormAction]);
618
619
        if (!$form->isValid()) {
620
            $logger->error('U2F authentication verification could not be started because device send illegal data');
621
            $this->addFlash('error', 'gateway.u2f.alert.error');
622
623
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
624
        }
625
626
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
627
        $result = $service->verifyAuthentication($signRequest, $signResponse);
628
629
        if ($result->wasSuccessful()) {
630
            $context->markSecondFactorVerified();
631
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
632
633
            $logger->info(
634
                sprintf(
635
                    'Marked U2F second factor "%s" as verified, forwarding to Saml Proxy to respond',
636
                    $selectedSecondFactor
637
                )
638
            );
639
640
            return $this->forward($context->getResponseAction());
641
        } elseif ($result->didDeviceReportError()) {
642
            $logger->error('U2F device reported error during authentication');
643
            $this->addFlash('error', 'gateway.u2f.alert.device_reported_an_error');
644
        } else {
645
            $logger->error('U2F authentication verification failed');
646
            $this->addFlash('error', 'gateway.u2f.alert.error');
647
        }
648
649
        return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
650
    }
651
652
    public function cancelAuthenticationAction($authenticationMode)
653
    {
654
        // Might not need the authenticationMode?
655
        $this->supportsAuthenticationMode($authenticationMode);
0 ignored issues
show
Unused Code introduced by
The call to the method Surfnet\StepupGateway\Ga...rtsAuthenticationMode() seems un-needed as the method has no side-effects.

PHP Analyzer performs a side-effects analysis of your code. A side-effect is basically anything that might be visible after the scope of the method is left.

Let’s take a look at an example:

class User
{
    private $email;

    public function getEmail()
    {
        return $this->email;
    }

    public function setEmail($email)
    {
        $this->email = $email;
    }
}

If we look at the getEmail() method, we can see that it has no side-effect. Whether you call this method or not, no future calls to other methods are affected by this. As such code as the following is useless:

$user = new User();
$user->getEmail(); // This line could safely be removed as it has no effect.

On the hand, if we look at the setEmail(), this method _has_ side-effects. In the following case, we could not remove the method call:

$user = new User();
$user->setEmail('email@domain'); // This line has a side-effect (it changes an
                                 // instance variable).
Loading history...
656
657
        return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
658
    }
659
660
    /**
661
     * @return \Surfnet\StepupGateway\GatewayBundle\Service\StepupAuthenticationService
662
     */
663
    private function getStepupService()
664
    {
665
        return $this->get('gateway.service.stepup_authentication');
666
    }
667
668
    /**
669
     * @return ResponseContext
0 ignored issues
show
Documentation introduced by
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...
670
     */
671
    private function getResponseContext($authenticationMode)
672
    {
673
        switch ($authenticationMode) {
674
            case self::MODE_SFO:
675
                return $this->get($this->get('gateway.proxy.sfo.state_handler')->getResponseContextServiceId());
676
                break;
0 ignored issues
show
Unused Code introduced by
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...
677
            case self::MODE_SSO:
678
                return $this->get($this->get('gateway.proxy.state_handler')->getResponseContextServiceId());
679
                break;
0 ignored issues
show
Unused Code introduced by
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...
680
        }
681
    }
682
683
    /**
684
     * @return \Surfnet\StepupGateway\GatewayBundle\Monolog\Logger\AuthenticationLogger
685
     */
686
    private function getAuthenticationLogger()
687
    {
688
        return $this->get('gateway.authentication_logger');
689
    }
690
691
    /**
692
     * @param ResponseContext $context
693
     * @param LoggerInterface $logger
694
     * @return string
695
     */
696
    private function getSelectedSecondFactor(ResponseContext $context, LoggerInterface $logger)
697
    {
698
        $selectedSecondFactor = $context->getSelectedSecondFactor();
699
700
        if (!$selectedSecondFactor) {
701
            $logger->error('Cannot verify possession of an unknown second factor');
702
703
            throw new BadRequestHttpException('Cannot verify possession of an unknown second factor.');
704
        }
705
706
        return $selectedSecondFactor;
707
    }
708
709
    private function selectAndRedirectTo(SecondFactor $secondFactor, ResponseContext $context, $authenticationMode)
710
    {
711
        $context->saveSelectedSecondFactor($secondFactor);
712
713
        $this->getStepupService()->clearSmsVerificationState();
714
715
        $secondFactorTypeService = $this->get('surfnet_stepup.service.second_factor_type');
716
        $secondFactorType = new SecondFactorType($secondFactor->secondFactorType);
717
718
        $route = 'gateway_verify_second_factor_';
719
        if ($secondFactorTypeService->isGssf($secondFactorType)) {
720
            $route .= 'gssf';
721
        } else {
722
            $route .= strtolower($secondFactor->secondFactorType);
723
        }
724
725
        return $this->redirect($this->generateUrl($route, ['authenticationMode' => $authenticationMode]));
726
    }
727
728
    /**
729
     * @param string $authenticationMode
730
     * @return FormInterface
731
     */
732
    private function buildCancelAuthenticationForm($authenticationMode)
733
    {
734
        $cancelFormAction = $this->generateUrl(
735
            'gateway_cancel_authentication',
736
            ['authenticationMode' => $authenticationMode]
737
        );
738
739
        return $this->createForm(
740
            CancelAuthenticationType::class,
741
            null,
742
            ['action' => $cancelFormAction]
743
        );
744
    }
745
746
    private function supportsAuthenticationMode($authenticationMode)
747
    {
748
        if ($authenticationMode === self::MODE_SSO || $authenticationMode === self::MODE_SFO) {
0 ignored issues
show
Unused Code introduced by
This if statement, and the following return statement can be replaced with return $authenticationMo...ode === self::MODE_SFO;.
Loading history...
749
            return true;
750
        }
751
        return false;
752
    }
753
}
754