Completed
Pull Request — develop (#193)
by Michiel
01:51
created

SecondFactorController   C

Complexity

Total Complexity 53

Size/Duplication

Total Lines 690
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 28

Importance

Changes 0
Metric Value
wmc 53
lcom 1
cbo 28
dl 0
loc 690
rs 6.87
c 0
b 0
f 0

17 Methods

Rating   Name   Duplication   Size   Complexity  
B selectSecondFactorForVerificationAction() 0 74 5
C chooseSecondFactorAction() 0 101 8
A verifyGssfAction() 0 45 3
A gssfVerifiedAction() 0 35 2
B verifyYubiKeySecondFactorAction() 0 54 5
A verifySmsSecondFactorAction() 0 52 4
B verifySmsSecondFactorChallengeAction() 0 54 6
A initiateU2fAuthenticationAction() 0 53 2
B verifyU2fAuthenticationAction() 0 70 4
A cancelAuthenticationAction() 0 7 1
A getStepupService() 0 4 1
A getResponseContext() 0 11 3
A getAuthenticationLogger() 0 4 1
A getSelectedSecondFactor() 0 12 2
A selectAndRedirectTo() 0 18 2
A buildCancelAuthenticationForm() 0 13 1
A supportsAuthenticationMode() 0 7 3

How to fix   Complexity   

Complex Class

Complex classes like SecondFactorController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use SecondFactorController, and based on these observations, apply Extract Interface, too.

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\Form;
45
use Symfony\Component\Form\FormError;
46
use Symfony\Component\Form\FormInterface;
47
use Symfony\Component\HttpFoundation\RedirectResponse;
48
use Symfony\Component\HttpFoundation\Request;
49
use Symfony\Component\HttpFoundation\Response;
50
use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBagInterface;
51
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
52
53
/**
54
 * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
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($this->generateUrl('gateway_verify_second_factor_sms_verify_challenge', ['authenticationMode' => $authenticationMode]));
444
    }
445
446
    /**
447
     * @Template
448
     * @param Request $request
449
     * @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...
450
     * @return array|Response
451
     */
452
    public function verifySmsSecondFactorChallengeAction(Request $request)
453
    {
454
        if (!$request->get('authenticationMode', false)) {
455
            throw new RuntimeException('Unable to determine the authentication mode in the SMS challenge action');
456
        }
457
        $authenticationMode = $request->get('authenticationMode');
458
        $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...
459
        $context = $this->getResponseContext($authenticationMode);
460
        $originalRequestId = $context->getInResponseTo();
461
462
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
463
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
464
465
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 459 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...
466
467
        $command = new VerifyPossessionOfPhoneCommand();
468
        $form = $this->createForm(VerifySmsChallengeType::class, $command)->handleRequest($request);
469
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
470
471
        if (!$form->isValid()) {
472
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
473
        }
474
475
        $logger->notice('Verifying input SMS challenge matches');
476
477
        $verification = $this->getStepupService()->verifySmsChallenge($command);
478
479
        if ($verification->wasSuccessful()) {
480
            $this->getStepupService()->clearSmsVerificationState();
481
482
            $this->getResponseContext($authenticationMode)->markSecondFactorVerified();
483
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
484
485
            $logger->info(
486
                sprintf(
487
                    'Marked Sms Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
488
                    $selectedSecondFactor
489
                )
490
            );
491
492
            return $this->forward($context->getResponseAction());
493
        } elseif ($verification->didOtpExpire()) {
494
            $logger->notice('SMS challenge expired');
495
            $form->addError(new FormError('gateway.form.send_sms_challenge.challenge_expired'));
496
        } elseif ($verification->wasAttemptedTooManyTimes()) {
497
            $logger->notice('SMS challenge verification was attempted too many times');
498
            $form->addError(new FormError('gateway.form.send_sms_challenge.too_many_attempts'));
499
        } else {
500
            $logger->notice('SMS challenge did not match');
501
            $form->addError(new FormError('gateway.form.send_sms_challenge.sms_challenge_incorrect'));
502
        }
503
504
        return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
505
    }
506
507
    /**
508
     * @Template
509
     * @param string $authenticationMode
510
     * @return array
511
     */
512
    public function initiateU2fAuthenticationAction($authenticationMode)
513
    {
514
        $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...
515
        $context = $this->getResponseContext($authenticationMode);
516
517
        $originalRequestId = $context->getInResponseTo();
518
519
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
520
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
521
522
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 515 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...
523
        $stepupService = $this->getStepupService();
524
525
        $cancelFormAction = $this->generateUrl('gateway_verify_second_factor_u2f_cancel_authentication', ['authenticationMode' => $authenticationMode]);
526
        $cancelForm =
527
            $this->createForm(CancelSecondFactorVerificationType::class, null, ['action' => $cancelFormAction]);
528
529
        $logger->notice('Verifying possession of U2F second factor, looking for registration matching key handle');
530
531
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
532
        $keyHandle = new KeyHandle($stepupService->getSecondFactorIdentifier($selectedSecondFactor));
533
        $registration = $service->findRegistrationByKeyHandle($keyHandle);
534
535
        if ($registration === null) {
536
            $logger->critical(
537
                sprintf('No known registration for key handle of second factor "%s"', $selectedSecondFactor)
538
            );
539
            $this->addFlash('error', 'gateway.u2f.alert.unknown_registration');
540
541
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
542
        }
543
544
        $logger->notice('Creating sign request');
545
546
        $signRequest = $service->createSignRequest($registration);
547
        $signResponse = new SignResponse();
548
549
        /** @var AttributeBagInterface $session */
550
        $session = $this->get('gateway.session.u2f');
551
        $session->set('request', $signRequest);
552
553
        $formAction = $this->generateUrl(
554
            'gateway_verify_second_factor_u2f_verify_authentication',
555
            ['authenticationMode' => $authenticationMode]
556
        );
557
        $form = $this->createForm(
558
            VerifyDeviceAuthenticationType::class,
559
            $signResponse,
560
            ['sign_request' => $signRequest, 'action' => $formAction]
561
        );
562
563
        return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
564
    }
565
566
    /**
567
     * @Template("SurfnetStepupGatewayGatewayBundle:SecondFactor:initiateU2fAuthentication.html.twig")
568
     *
569
     * @param Request $request
570
     * @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...
571
     * @return array|Response
572
     */
573
    public function verifyU2fAuthenticationAction(Request $request)
574
    {
575
        $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...
576
        $context = $this->getResponseContext($authenticationMode);
577
578
        $originalRequestId = $context->getInResponseTo();
579
580
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
581
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
582
583
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 576 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...
584
585
        $logger->notice('Received sign response from device');
586
587
        /** @var AttributeBagInterface $session */
588
        $session = $this->get('gateway.session.u2f');
589
        $signRequest = $session->get('request');
590
        $signResponse = new SignResponse();
591
592
        $formAction = $this->generateUrl(
593
            'gateway_verify_second_factor_u2f_verify_authentication',
594
            ['authenticationMode' => $authenticationMode]
595
        );
596
        $form = $this
597
            ->createForm(
598
                VerifyDeviceAuthenticationType::class,
599
                $signResponse,
600
                ['sign_request' => $signRequest, 'action' => $formAction]
601
            )
602
            ->handleRequest($request);
603
604
        $cancelFormAction = $this->generateUrl(
605
            'gateway_verify_second_factor_u2f_cancel_authentication',
606
            ['authenticationMode' => $authenticationMode]
607
        );
608
        $cancelForm =
609
            $this->createForm(CancelSecondFactorVerificationType::class, null, ['action' => $cancelFormAction]);
610
611
        if (!$form->isValid()) {
612
            $logger->error('U2F authentication verification could not be started because device send illegal data');
613
            $this->addFlash('error', 'gateway.u2f.alert.error');
614
615
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
616
        }
617
618
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
619
        $result = $service->verifyAuthentication($signRequest, $signResponse);
620
621
        if ($result->wasSuccessful()) {
622
            $context->markSecondFactorVerified();
623
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
624
625
            $logger->info(
626
                sprintf(
627
                    'Marked U2F second factor "%s" as verified, forwarding to Saml Proxy to respond',
628
                    $selectedSecondFactor
629
                )
630
            );
631
632
            return $this->forward($context->getResponseAction());
633
        } elseif ($result->didDeviceReportError()) {
634
            $logger->error('U2F device reported error during authentication');
635
            $this->addFlash('error', 'gateway.u2f.alert.device_reported_an_error');
636
        } else {
637
            $logger->error('U2F authentication verification failed');
638
            $this->addFlash('error', 'gateway.u2f.alert.error');
639
        }
640
641
        return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
642
    }
643
644
    public function cancelAuthenticationAction($authenticationMode)
645
    {
646
        // Might not need the authenticationMode?
647
        $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...
648
649
        return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
650
    }
651
652
    /**
653
     * @return \Surfnet\StepupGateway\GatewayBundle\Service\StepupAuthenticationService
654
     */
655
    private function getStepupService()
656
    {
657
        return $this->get('gateway.service.stepup_authentication');
658
    }
659
660
    /**
661
     * @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...
662
     */
663
    private function getResponseContext($authenticationMode)
664
    {
665
        switch ($authenticationMode) {
666
            case self::MODE_SFO:
667
                return $this->get($this->get('gateway.proxy.sfo.state_handler')->getResponseContextServiceId());
668
                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...
669
            case self::MODE_SSO:
670
                return $this->get($this->get('gateway.proxy.state_handler')->getResponseContextServiceId());
671
                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...
672
        }
673
    }
674
675
    /**
676
     * @return \Surfnet\StepupGateway\GatewayBundle\Monolog\Logger\AuthenticationLogger
677
     */
678
    private function getAuthenticationLogger()
679
    {
680
        return $this->get('gateway.authentication_logger');
681
    }
682
683
    /**
684
     * @param ResponseContext $context
685
     * @param LoggerInterface $logger
686
     * @return string
687
     */
688
    private function getSelectedSecondFactor(ResponseContext $context, LoggerInterface $logger)
689
    {
690
        $selectedSecondFactor = $context->getSelectedSecondFactor();
691
692
        if (!$selectedSecondFactor) {
693
            $logger->error('Cannot verify possession of an unknown second factor');
694
695
            throw new BadRequestHttpException('Cannot verify possession of an unknown second factor.');
696
        }
697
698
        return $selectedSecondFactor;
699
    }
700
701
    private function selectAndRedirectTo(SecondFactor $secondFactor, ResponseContext $context, $authenticationMode)
702
    {
703
        $context->saveSelectedSecondFactor($secondFactor);
704
705
        $this->getStepupService()->clearSmsVerificationState();
706
707
        $secondFactorTypeService = $this->get('surfnet_stepup.service.second_factor_type');
708
        $secondFactorType = new SecondFactorType($secondFactor->secondFactorType);
709
710
        $route = 'gateway_verify_second_factor_';
711
        if ($secondFactorTypeService->isGssf($secondFactorType)) {
712
            $route .= 'gssf';
713
        } else {
714
            $route .= strtolower($secondFactor->secondFactorType);
715
        }
716
717
        return $this->redirect($this->generateUrl($route, ['authenticationMode' => $authenticationMode]));
718
    }
719
720
    /**
721
     * @param string $authenticationMode
722
     * @return FormInterface
723
     */
724
    private function buildCancelAuthenticationForm($authenticationMode)
725
    {
726
        $cancelFormAction = $this->generateUrl(
727
            'gateway_cancel_authentication',
728
            ['authenticationMode' => $authenticationMode]
729
        );
730
731
        return $this->createForm(
732
            CancelAuthenticationType::class,
733
            null,
734
            ['action' => $cancelFormAction]
735
        );
736
    }
737
738
    private function supportsAuthenticationMode($authenticationMode)
739
    {
740
        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...
741
            return true;
742
        }
743
        return false;
744
    }
745
}
746