Completed
Push — feature/implement-state-handli... ( a0fda0...18e737 )
by Michiel
02:06
created

selectSecondFactorForVerificationSfoAction()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 4

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 4
rs 10
c 0
b 0
f 0
cc 1
nc 1
nop 0
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
 * @SuppressWarnings(PHPMD.TooManyPublicMethods)
56
 */
57
class SecondFactorController extends Controller
58
{
59
    const MODE_SFO = 'sfo';
60
    const MODE_SSO = 'sso';
61
62
    public function selectSecondFactorForVerificationSsoAction()
63
    {
64
        return $this->selectSecondFactorForVerificationAction(self::MODE_SSO);
65
    }
66
67
    public function selectSecondFactorForVerificationSfoAction()
68
    {
69
        return $this->selectSecondFactorForVerificationAction(self::MODE_SFO);
70
    }
71
72
    public function selectSecondFactorForVerificationAction($authenticationMode)
73
    {
74
        $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...
75
        $context = $this->getResponseContext($authenticationMode);
76
77
        $originalRequestId = $context->getInResponseTo();
78
79
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
80
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
81
        $logger->notice('Determining which second factor to use...');
82
83
        try {
84
            // Retrieve all requirements to determine the required LoA
85
            $requestedLoa = $context->getRequiredLoa();
86
            $spConfiguredLoas = $context->getServiceProvider()->get('configuredLoas');
87
88
            $normalizedIdpSho = $context->getNormalizedSchacHomeOrganization();
89
            $normalizedUserSho = $this->getStepupService()->getNormalizedUserShoByIdentityNameId($context->getIdentityNameId());
90
91
            $requiredLoa = $this
92
                ->getStepupService()
93
                ->resolveHighestRequiredLoa(
94
                    $requestedLoa,
95
                    $spConfiguredLoas,
96
                    $normalizedIdpSho,
97
                    $normalizedUserSho
98
                );
99
        } catch (LoaCannotBeGivenException $e) {
100
            // Log the message of the domain exception, this contains a meaningful message.
101
            $logger->notice($e->getMessage());
102
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
103
        }
104
105
        $logger->notice(sprintf('Determined that the required Loa is "%s"', $requiredLoa));
106
107
        if ($this->getStepupService()->isIntrinsicLoa($requiredLoa)) {
108
            $this->get('gateway.authentication_logger')->logIntrinsicLoaAuthentication($originalRequestId);
109
110
            return $this->forward($context->getResponseAction());
111
        }
112
113
        $secondFactorCollection = $this
114
            ->getStepupService()
115
            ->determineViableSecondFactors(
116
                $context->getIdentityNameId(),
117
                $requiredLoa,
118
                $this->get('gateway.service.whitelist')
119
            );
120
121
        switch (count($secondFactorCollection)) {
122
            case 0:
123
                $logger->notice('No second factors can give the determined Loa');
124
                return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
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
            case 1:
128
                $secondFactor = $secondFactorCollection->first();
129
                $logger->notice(sprintf(
130
                    'Found "%d" second factors, using second factor of type "%s"',
131
                    count($secondFactorCollection),
132
                    $secondFactor->secondFactorType
133
                ));
134
135
                return $this->selectAndRedirectTo($secondFactor, $context, $authenticationMode);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 75 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...
136
                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...
137
138
            default:
139
                return $this->forward(
140
                    'SurfnetStepupGatewayGatewayBundle:SecondFactor:chooseSecondFactor',
141
                    ['authenticationMode' => $authenticationMode, 'secondFactors' => $secondFactorCollection]
142
                );
143
                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...
144
        }
145
    }
146
147
    /**
148
     * @Template
149
     * @param Request $request
150
     * @param string $authenticationMode
151
     * @return array|RedirectResponse|Response
152
     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
153
     */
154
    public function chooseSecondFactorAction(Request $request, $authenticationMode)
155
    {
156
        $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...
157
        $context = $this->getResponseContext($authenticationMode);
158
        $originalRequestId = $context->getInResponseTo();
159
160
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
161
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
162
        $logger->notice('Ask the user which one of his suitable second factor tokens to use...');
163
164
        try {
165
            // Retrieve all requirements to determine the required LoA
166
            $requestedLoa = $context->getRequiredLoa();
167
            $spConfiguredLoas = $context->getServiceProvider()->get('configuredLoas');
168
169
            $normalizedIdpSho = $context->getNormalizedSchacHomeOrganization();
170
            $normalizedUserSho = $this->getStepupService()->getNormalizedUserShoByIdentityNameId($context->getIdentityNameId());
171
172
            $requiredLoa = $this
173
                ->getStepupService()
174
                ->resolveHighestRequiredLoa(
175
                    $requestedLoa,
176
                    $spConfiguredLoas,
177
                    $normalizedIdpSho,
178
                    $normalizedUserSho
179
                );
180
        } catch (LoaCannotBeGivenException $e) {
181
            // Log the message of the domain exception, this contains a meaningful message.
182
            $logger->notice($e->getMessage());
183
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
184
        }
185
186
        $logger->notice(sprintf('Determined that the required Loa is "%s"', $requiredLoa));
187
188
        $secondFactors = $this
189
            ->getStepupService()
190
            ->determineViableSecondFactors(
191
                $context->getIdentityNameId(),
192
                $requiredLoa,
193
                $this->get('gateway.service.whitelist')
194
            );
195
196
        $command = new ChooseSecondFactorCommand();
197
        $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...
198
199
        $form = $this
200
            ->createForm(
201
                ChooseSecondFactorType::class,
202
                $command,
203
                ['action' => $this->generateUrl('gateway_verify_second_factor_choose_second_factor', ['authenticationMode' => $authenticationMode])]
204
            )
205
            ->handleRequest($request);
206
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
207
208
        if ($form->isSubmitted() && $form->isValid()) {
209
            $buttonName = $form->getClickedButton()->getName();
210
            $formResults = $request->request->get('gateway_choose_second_factor', false);
211
212
            if (!isset($formResults[$buttonName])) {
213
                throw new InvalidArgumentException(
214
                    sprintf(
215
                        'Second factor type "%s" could not be found in the posted form results.',
216
                        $buttonName
217
                    )
218
                );
219
            }
220
221
            $secondFactorType = $formResults[$buttonName];
222
223
            // Filter the selected second factor from the array collection
224
            $secondFactorFiltered = $secondFactors->filter(
225
                function ($secondFactor) use ($secondFactorType) {
226
                    return $secondFactorType === $secondFactor->secondFactorType;
227
                }
228
            );
229
230
            if ($secondFactorFiltered->isEmpty()) {
231
                throw new InvalidArgumentException(
232
                    sprintf(
233
                        'Second factor type "%s" could not be found in the collection of available second factors.',
234
                        $secondFactorType
235
                    )
236
                );
237
            }
238
239
            $secondFactor = $secondFactorFiltered->first();
240
241
            $logger->notice(sprintf('User chose "%s" to use as second factor', $secondFactorType));
242
243
            // Forward to action to verify possession of second factor
244
            return $this->selectAndRedirectTo($secondFactor, $context, $authenticationMode);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 157 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...
245
        } else if ($form->isSubmitted() && !$form->isValid()) {
246
            $form->addError(new FormError('gateway.form.gateway_choose_second_factor.unknown_second_factor_type'));
247
        }
248
249
        return [
250
            'form' => $form->createView(),
251
            'cancelForm' => $cancelForm->createView(),
252
            'secondFactors' => $secondFactors,
253
        ];
254
    }
255
256
    public function verifyGssfAction(Request $request)
257
    {
258
        if (!$request->get('authenticationMode', false)) {
259
            throw new RuntimeException('Unable to determine the authentication mode in the GSSP verification action');
260
        }
261
        $authenticationMode = $request->get('authenticationMode');
262
        $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...
263
        $context = $this->getResponseContext($authenticationMode);
264
265
        $originalRequestId = $context->getInResponseTo();
266
267
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
268
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
269
        $logger->info('Received request to verify GSSF');
270
271
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 263 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...
272
273
        $logger->info(sprintf(
274
            'Selected GSSF "%s" for verfication, forwarding to Saml handling',
275
            $selectedSecondFactor
276
        ));
277
278
        /** @var \Surfnet\StepupGateway\GatewayBundle\Service\SecondFactorService $secondFactorService */
279
        $secondFactorService = $this->get('gateway.service.second_factor_service');
280
        /** @var \Surfnet\StepupGateway\GatewayBundle\Entity\SecondFactor $secondFactor */
281
        $secondFactor = $secondFactorService->findByUuid($selectedSecondFactor);
282
        if (!$secondFactor) {
283
            throw new RuntimeException(sprintf(
284
                'Requested verification of GSSF "%s", however that Second Factor no longer exists',
285
                $selectedSecondFactor
286
            ));
287
        }
288
289
        // Also send the response context service id, as later we need to know if this is regular SSO or SFO authn.
290
        $responseContextServiceId = $context->getResponseContextServiceId();
291
292
        return $this->forward(
293
            'SurfnetStepupGatewaySamlStepupProviderBundle:SamlProxy:sendSecondFactorVerificationAuthnRequest',
294
            [
295
                'provider' => $secondFactor->secondFactorType,
296
                'subjectNameId' => $secondFactor->secondFactorIdentifier,
297
                'responseContextServiceId' => $responseContextServiceId,
298
            ]
299
        );
300
    }
301
302
    public function gssfVerifiedAction(Request $request)
303
    {
304
        $authenticationMode = $request->get('authenticationMode');
305
        $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...
306
        $context = $this->getResponseContext($authenticationMode);
307
308
        $originalRequestId = $context->getInResponseTo();
309
310
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
311
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
312
        $logger->info('Attempting to mark GSSF as verified');
313
314
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 306 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...
315
316
        /** @var \Surfnet\StepupGateway\GatewayBundle\Entity\SecondFactor $secondFactor */
317
        $secondFactor = $this->get('gateway.service.second_factor_service')->findByUuid($selectedSecondFactor);
318
        if (!$secondFactor) {
319
            throw new RuntimeException(
320
                sprintf(
321
                    'Verification of GSSF "%s" succeeded, however that Second Factor no longer exists',
322
                    $selectedSecondFactor
323
                )
324
            );
325
        }
326
327
        $context->markSecondFactorVerified();
328
        $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
329
330
        $logger->info(sprintf(
331
            'Marked GSSF "%s" as verified, forwarding to Gateway controller to respond',
332
            $selectedSecondFactor
333
        ));
334
335
        return $this->forward($context->getResponseAction());
336
    }
337
338
    /**
339
     * @Template
340
     * @param Request $request
341
     * @return array|Response
342
     */
343
    public function verifyYubiKeySecondFactorAction(Request $request)
344
    {
345
        if (!$request->get('authenticationMode', false)) {
346
            throw new RuntimeException('Unable to determine the authentication mode in Yubikey verification action');
347
        }
348
        $authenticationMode = $request->get('authenticationMode');
349
        $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...
350
        $context = $this->getResponseContext($authenticationMode);
351
        $originalRequestId = $context->getInResponseTo();
352
353
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
354
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
355
356
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 350 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...
357
358
        $logger->notice('Verifying possession of Yubikey second factor');
359
360
        $command = new VerifyYubikeyOtpCommand();
361
        $command->secondFactorId = $selectedSecondFactor;
362
363
        $form = $this->createForm(VerifyYubikeyOtpType::class, $command)->handleRequest($request);
364
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
365
366
        if (!$form->isValid()) {
367
            // OTP field is rendered empty in the template.
368
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
369
        }
370
371
        $result = $this->getStepupService()->verifyYubikeyOtp($command);
372
373
        if ($result->didOtpVerificationFail()) {
374
            $form->addError(new FormError('gateway.form.verify_yubikey.otp_verification_failed'));
375
376
            // OTP field is rendered empty in the template.
377
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
378
        } elseif (!$result->didPublicIdMatch()) {
379
            $form->addError(new FormError('gateway.form.verify_yubikey.public_id_mismatch'));
380
381
            // OTP field is rendered empty in the template.
382
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
383
        }
384
385
        $this->getResponseContext($authenticationMode)->markSecondFactorVerified();
386
        $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
387
388
        $logger->info(
389
            sprintf(
390
                'Marked Yubikey Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
391
                $selectedSecondFactor
392
            )
393
        );
394
395
        return $this->forward($context->getResponseAction());
396
    }
397
398
    /**
399
     * @Template
400
     * @param Request $request
401
     * @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...
402
     * @return array|Response
403
     */
404
    public function verifySmsSecondFactorAction(Request $request)
405
    {
406
        if (!$request->get('authenticationMode', false)) {
407
            throw new RuntimeException('Unable to determine the authentication mode in the SMS verification action');
408
        }
409
        $authenticationMode = $request->get('authenticationMode');
410
        $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...
411
        $context = $this->getResponseContext($authenticationMode);
412
        $originalRequestId = $context->getInResponseTo();
413
414
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
415
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
416
417
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 411 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...
418
419
        $logger->notice('Verifying possession of SMS second factor, preparing to send');
420
421
        $command = new SendSmsChallengeCommand();
422
        $command->secondFactorId = $selectedSecondFactor;
423
424
        $form = $this->createForm(SendSmsChallengeType::class, $command)->handleRequest($request);
425
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
426
427
        $stepupService = $this->getStepupService();
428
        $phoneNumber = InternationalPhoneNumber::fromStringFormat(
429
            $stepupService->getSecondFactorIdentifier($selectedSecondFactor)
430
        );
431
432
        $otpRequestsRemaining = $stepupService->getSmsOtpRequestsRemainingCount();
433
        $maximumOtpRequests = $stepupService->getSmsMaximumOtpRequestsCount();
434
        $viewVariables = ['otpRequestsRemaining' => $otpRequestsRemaining, 'maximumOtpRequests' => $maximumOtpRequests];
435
436
        if (!$form->isValid()) {
437
            return array_merge(
438
                $viewVariables,
439
                ['phoneNumber' => $phoneNumber, 'form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]
440
            );
441
        }
442
443
        $logger->notice('Verifying possession of SMS second factor, sending challenge per SMS');
444
445
        if (!$stepupService->sendSmsChallenge($command)) {
446
            $form->addError(new FormError('gateway.form.send_sms_challenge.sms_sending_failed'));
447
448
            return array_merge(
449
                $viewVariables,
450
                ['phoneNumber' => $phoneNumber, 'form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]
451
            );
452
        }
453
454
        return $this->redirect(
455
            $this->generateUrl(
456
                'gateway_verify_second_factor_sms_verify_challenge',
457
                ['authenticationMode' => $authenticationMode]
458
            )
459
        );
460
    }
461
462
    /**
463
     * @Template
464
     * @param Request $request
465
     * @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...
466
     * @return array|Response
467
     */
468
    public function verifySmsSecondFactorChallengeAction(Request $request)
469
    {
470
        if (!$request->get('authenticationMode', false)) {
471
            throw new RuntimeException('Unable to determine the authentication mode in the SMS challenge action');
472
        }
473
        $authenticationMode = $request->get('authenticationMode');
474
        $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...
475
        $context = $this->getResponseContext($authenticationMode);
476
        $originalRequestId = $context->getInResponseTo();
477
478
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
479
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
480
481
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 475 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...
482
483
        $command = new VerifyPossessionOfPhoneCommand();
484
        $form = $this->createForm(VerifySmsChallengeType::class, $command)->handleRequest($request);
485
        $cancelForm = $this->buildCancelAuthenticationForm($authenticationMode)->handleRequest($request);
486
487
        if (!$form->isValid()) {
488
            return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
489
        }
490
491
        $logger->notice('Verifying input SMS challenge matches');
492
493
        $verification = $this->getStepupService()->verifySmsChallenge($command);
494
495
        if ($verification->wasSuccessful()) {
496
            $this->getStepupService()->clearSmsVerificationState();
497
498
            $this->getResponseContext($authenticationMode)->markSecondFactorVerified();
499
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
500
501
            $logger->info(
502
                sprintf(
503
                    'Marked Sms Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
504
                    $selectedSecondFactor
505
                )
506
            );
507
508
            return $this->forward($context->getResponseAction());
509
        } elseif ($verification->didOtpExpire()) {
510
            $logger->notice('SMS challenge expired');
511
            $form->addError(new FormError('gateway.form.send_sms_challenge.challenge_expired'));
512
        } elseif ($verification->wasAttemptedTooManyTimes()) {
513
            $logger->notice('SMS challenge verification was attempted too many times');
514
            $form->addError(new FormError('gateway.form.send_sms_challenge.too_many_attempts'));
515
        } else {
516
            $logger->notice('SMS challenge did not match');
517
            $form->addError(new FormError('gateway.form.send_sms_challenge.sms_challenge_incorrect'));
518
        }
519
520
        return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
521
    }
522
523
    /**
524
     * @Template
525
     * @param string $authenticationMode
526
     * @return array
527
     */
528
    public function initiateU2fAuthenticationAction($authenticationMode)
529
    {
530
        $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...
531
        $context = $this->getResponseContext($authenticationMode);
532
533
        $originalRequestId = $context->getInResponseTo();
534
535
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
536
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
537
538
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 531 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...
539
        $stepupService = $this->getStepupService();
540
541
        $cancelFormAction = $this->generateUrl(
542
            'gateway_verify_second_factor_u2f_cancel_authentication',
543
            ['authenticationMode' => $authenticationMode]
544
        );
545
        $cancelForm =
546
            $this->createForm(CancelSecondFactorVerificationType::class, null, ['action' => $cancelFormAction]);
547
548
        $logger->notice('Verifying possession of U2F second factor, looking for registration matching key handle');
549
550
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
551
        $keyHandle = new KeyHandle($stepupService->getSecondFactorIdentifier($selectedSecondFactor));
552
        $registration = $service->findRegistrationByKeyHandle($keyHandle);
553
554
        if ($registration === null) {
555
            $logger->critical(
556
                sprintf('No known registration for key handle of second factor "%s"', $selectedSecondFactor)
557
            );
558
            $this->addFlash('error', 'gateway.u2f.alert.unknown_registration');
559
560
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
561
        }
562
563
        $logger->notice('Creating sign request');
564
565
        $signRequest = $service->createSignRequest($registration);
566
        $signResponse = new SignResponse();
567
568
        /** @var AttributeBagInterface $session */
569
        $session = $this->get('gateway.session.u2f');
570
        $session->set('request', $signRequest);
571
572
        $formAction = $this->generateUrl(
573
            'gateway_verify_second_factor_u2f_verify_authentication',
574
            ['authenticationMode' => $authenticationMode]
575
        );
576
        $form = $this->createForm(
577
            VerifyDeviceAuthenticationType::class,
578
            $signResponse,
579
            ['sign_request' => $signRequest, 'action' => $formAction]
580
        );
581
582
        return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
583
    }
584
585
    /**
586
     * @Template("SurfnetStepupGatewayGatewayBundle:SecondFactor:initiateU2fAuthentication.html.twig")
587
     *
588
     * @param Request $request
589
     * @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...
590
     * @return array|Response
591
     */
592
    public function verifyU2fAuthenticationAction(Request $request)
593
    {
594
        $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...
595
        $context = $this->getResponseContext($authenticationMode);
596
597
        $originalRequestId = $context->getInResponseTo();
598
599
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
600
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
601
602
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
0 ignored issues
show
Bug introduced by
It seems like $context defined by $this->getResponseContext($authenticationMode) on line 595 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...
603
604
        $logger->notice('Received sign response from device');
605
606
        /** @var AttributeBagInterface $session */
607
        $session = $this->get('gateway.session.u2f');
608
        $signRequest = $session->get('request');
609
        $signResponse = new SignResponse();
610
611
        $formAction = $this->generateUrl(
612
            'gateway_verify_second_factor_u2f_verify_authentication',
613
            ['authenticationMode' => $authenticationMode]
614
        );
615
        $form = $this
616
            ->createForm(
617
                VerifyDeviceAuthenticationType::class,
618
                $signResponse,
619
                ['sign_request' => $signRequest, 'action' => $formAction]
620
            )
621
            ->handleRequest($request);
622
623
        $cancelFormAction = $this->generateUrl(
624
            'gateway_verify_second_factor_u2f_cancel_authentication',
625
            ['authenticationMode' => $authenticationMode]
626
        );
627
        $cancelForm =
628
            $this->createForm(CancelSecondFactorVerificationType::class, null, ['action' => $cancelFormAction]);
629
630
        if (!$form->isValid()) {
631
            $logger->error('U2F authentication verification could not be started because device send illegal data');
632
            $this->addFlash('error', 'gateway.u2f.alert.error');
633
634
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
635
        }
636
637
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
638
        $result = $service->verifyAuthentication($signRequest, $signResponse);
639
640
        if ($result->wasSuccessful()) {
641
            $context->markSecondFactorVerified();
642
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId, $authenticationMode);
643
644
            $logger->info(
645
                sprintf(
646
                    'Marked U2F second factor "%s" as verified, forwarding to Saml Proxy to respond',
647
                    $selectedSecondFactor
648
                )
649
            );
650
651
            return $this->forward($context->getResponseAction());
652
        } elseif ($result->didDeviceReportError()) {
653
            $logger->error('U2F device reported error during authentication');
654
            $this->addFlash('error', 'gateway.u2f.alert.device_reported_an_error');
655
        } else {
656
            $logger->error('U2F authentication verification failed');
657
            $this->addFlash('error', 'gateway.u2f.alert.error');
658
        }
659
660
        return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
661
    }
662
663
    public function cancelAuthenticationAction($authenticationMode)
664
    {
665
        // Might not need the authenticationMode?
666
        $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...
667
668
        return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
669
    }
670
671
    /**
672
     * @return \Surfnet\StepupGateway\GatewayBundle\Service\StepupAuthenticationService
673
     */
674
    private function getStepupService()
675
    {
676
        return $this->get('gateway.service.stepup_authentication');
677
    }
678
679
    /**
680
     * @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...
681
     */
682
    private function getResponseContext($authenticationMode)
683
    {
684
        switch ($authenticationMode) {
685
            case self::MODE_SFO:
686
                return $this->get($this->get('gateway.proxy.sfo.state_handler')->getResponseContextServiceId());
687
                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...
688
            case self::MODE_SSO:
689
                return $this->get($this->get('gateway.proxy.sso.state_handler')->getResponseContextServiceId());
690
                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...
691
        }
692
    }
693
694
    /**
695
     * @return \Surfnet\StepupGateway\GatewayBundle\Monolog\Logger\AuthenticationLogger
696
     */
697
    private function getAuthenticationLogger()
698
    {
699
        return $this->get('gateway.authentication_logger');
700
    }
701
702
    /**
703
     * @param ResponseContext $context
704
     * @param LoggerInterface $logger
705
     * @return string
706
     */
707
    private function getSelectedSecondFactor(ResponseContext $context, LoggerInterface $logger)
708
    {
709
        $selectedSecondFactor = $context->getSelectedSecondFactor();
710
711
        if (!$selectedSecondFactor) {
712
            $logger->error('Cannot verify possession of an unknown second factor');
713
714
            throw new BadRequestHttpException('Cannot verify possession of an unknown second factor.');
715
        }
716
717
        return $selectedSecondFactor;
718
    }
719
720
    private function selectAndRedirectTo(SecondFactor $secondFactor, ResponseContext $context, $authenticationMode)
721
    {
722
        $context->saveSelectedSecondFactor($secondFactor);
723
724
        $this->getStepupService()->clearSmsVerificationState();
725
726
        $secondFactorTypeService = $this->get('surfnet_stepup.service.second_factor_type');
727
        $secondFactorType = new SecondFactorType($secondFactor->secondFactorType);
728
729
        $route = 'gateway_verify_second_factor_';
730
        if ($secondFactorTypeService->isGssf($secondFactorType)) {
731
            $route .= 'gssf';
732
        } else {
733
            $route .= strtolower($secondFactor->secondFactorType);
734
        }
735
736
        return $this->redirect($this->generateUrl($route, ['authenticationMode' => $authenticationMode]));
737
    }
738
739
    /**
740
     * @param string $authenticationMode
741
     * @return FormInterface
742
     */
743
    private function buildCancelAuthenticationForm($authenticationMode)
744
    {
745
        $cancelFormAction = $this->generateUrl(
746
            'gateway_cancel_authentication',
747
            ['authenticationMode' => $authenticationMode]
748
        );
749
750
        return $this->createForm(
751
            CancelAuthenticationType::class,
752
            null,
753
            ['action' => $cancelFormAction]
754
        );
755
    }
756
757
    private function supportsAuthenticationMode($authenticationMode)
758
    {
759
        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...
760
            return true;
761
        }
762
        return false;
763
    }
764
}
765