Completed
Push — feature/dont-clear-state-on-sa... ( 57a027...b19a4c )
by
unknown
05:27 queued 03:21
created

SecondFactorController::getAuthenticationLogger()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 4
Code Lines 2

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
c 0
b 0
f 0
dl 0
loc 4
rs 10
cc 1
eloc 2
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\LoaCannotBeGivenException;
31
use Surfnet\StepupGateway\GatewayBundle\Exception\RuntimeException;
32
use Surfnet\StepupGateway\GatewayBundle\Saml\ResponseContext;
33
use Surfnet\StepupGateway\U2fVerificationBundle\Value\KeyHandle;
34
use Surfnet\StepupU2fBundle\Dto\SignResponse;
35
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
36
use Symfony\Component\Form\FormError;
37
use Symfony\Component\HttpFoundation\RedirectResponse;
38
use Symfony\Component\HttpFoundation\Request;
39
use Symfony\Component\HttpFoundation\Response;
40
use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBagInterface;
41
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
42
43
/**
44
 * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
45
 */
46
class SecondFactorController extends Controller
47
{
48
    public function selectSecondFactorForVerificationAction()
49
    {
50
        $context = $this->getResponseContext();
51
        $originalRequestId = $context->getInResponseTo();
52
53
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
54
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
55
        $logger->notice('Determining which second factor to use...');
56
57
        try {
58
            // Retrieve all requirements to determine the required LoA
59
            $requestedLoa = $context->getRequiredLoa();
60
            $spConfiguredLoas = $context->getServiceProvider()->get('configuredLoas');
61
62
            $normalizedIdpSho = $context->getNormalizedSchacHomeOrganization();
63
            $normalizedUserSho = $this->getStepupService()->getNormalizedUserShoByIdentityNameId($context->getIdentityNameId());
0 ignored issues
show
Coding Style introduced by
This line exceeds maximum limit of 120 characters; contains 128 characters

Overly long lines are hard to read on any screen. Most code styles therefor impose a maximum limit on the number of characters in a line.

Loading history...
64
65
            $requiredLoa = $this
66
                ->getStepupService()
67
                ->resolveHighestRequiredLoa(
68
                    $requestedLoa,
69
                    $spConfiguredLoas,
70
                    $normalizedIdpSho,
71
                    $normalizedUserSho
72
                );
73
        } catch (LoaCannotBeGivenException $e) {
74
            // Log the message of the domain exception, this contains a meaningful message.
75
            $logger->notice($e->getMessage());
76
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
77
        }
78
79
        $logger->notice(sprintf('Determined that the required Loa is "%s"', $requiredLoa));
80
81
        if ($this->getStepupService()->isIntrinsicLoa($requiredLoa)) {
82
            $this->get('gateway.authentication_logger')->logIntrinsicLoaAuthentication($originalRequestId);
83
84
            return $this->forward($context->getResponseAction());
85
        }
86
87
        $secondFactorCollection = $this
88
            ->getStepupService()
89
            ->determineViableSecondFactors(
90
                $context->getIdentityNameId(),
91
                $requiredLoa,
92
                $this->get('gateway.service.whitelist')
93
            );
94
95
        switch (count($secondFactorCollection)) {
96
            case 0:
97
                $logger->notice('No second factors can give the determined Loa');
98
                return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
99
                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...
100
101
            case 1:
102
                $secondFactor = $secondFactorCollection->first();
103
                $logger->notice(sprintf(
104
                    'Found "%d" second factors, using second factor of type "%s"',
105
                    count($secondFactorCollection),
106
                    $secondFactor->secondFactorType
107
                ));
108
109
                return $this->selectAndRedirectTo($secondFactor, $context);
110
                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...
111
112
            default:
113
                return $this->forward(
114
                    'SurfnetStepupGatewayGatewayBundle:SecondFactor:chooseSecondFactor',
115
                    ['secondFactors' => $secondFactorCollection]
116
                );
117
                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...
118
        }
119
    }
120
121
    /**
122
     * @Template
123
     * @param Request $request
124
     * @return array|RedirectResponse|Response
125
     */
126
    public function chooseSecondFactorAction(Request $request)
127
    {
128
        $context = $this->getResponseContext();
129
        $originalRequestId = $context->getInResponseTo();
130
131
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
132
133
        // Retrieve all requirements to determine the required LoA
134
        $requestedLoa = $context->getRequiredLoa();
135
        $spConfiguredLoas = $context->getServiceProvider()->get('configuredLoas');
136
        $idpSho = $context->getSchacHomeOrganization();
0 ignored issues
show
Bug introduced by
The method getSchacHomeOrganization() does not seem to exist on object<Surfnet\StepupGat...e\Saml\ResponseContext>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
137
        $userSho = $this->getStepupService()->getUserShoByIdentityNameId($context->getIdentityNameId());
0 ignored issues
show
Bug introduced by
The method getUserShoByIdentityNameId() does not seem to exist on object<Surfnet\StepupGat...pAuthenticationService>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
138
139
        $this->getStepupService()->assertValidShoFormat($idpSho);
0 ignored issues
show
Bug introduced by
The method assertValidShoFormat() does not seem to exist on object<Surfnet\StepupGat...pAuthenticationService>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
140
        $this->getStepupService()->assertValidShoFormat($userSho);
0 ignored issues
show
Bug introduced by
The method assertValidShoFormat() does not seem to exist on object<Surfnet\StepupGat...pAuthenticationService>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
141
142
        try {
143
            $requiredLoa = $this
144
                ->getStepupService()
145
                ->resolveHighestRequiredLoa(
146
                    $requestedLoa,
147
                    $spConfiguredLoas,
148
                    $idpSho,
149
                    $userSho
150
                );
151
        } catch (LoaCannotBeGivenException $e) {
152
            $logger->notice($e->getMessage());
153
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendLoaCannotBeGiven');
154
        }
155
156
        $logger->notice(sprintf('Determined that the required Loa is "%s"', $requiredLoa));
157
158
        $secondFactors = $this
159
            ->getStepupService()
160
            ->determineViableSecondFactors(
161
                $context->getIdentityNameId(),
162
                $requiredLoa,
163
                $this->get('gateway.service.whitelist')
164
            );
165
166
        $command = new ChooseSecondFactorCommand();
167
        $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...
168
169
        $form = $this
170
            ->createForm(
171
                'gateway_choose_second_factor',
172
                $command,
173
                ['action' => $this->generateUrl('gateway_verify_second_factor_choose_second_factor')]
174
            )
175
            ->handleRequest($request);
176
177
        if ($form->get('cancel')->isClicked()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Form\FormInterface as the method isClicked() does only exist in the following implementations of said interface: Symfony\Component\Form\SubmitButton.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

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

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

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

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
178
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
179
        }
180
181
        if ($form->isSubmitted() && $form->isValid()) {
182
            $secondFactorIndex = $command->selectedSecondFactor;
183
            $secondFactor = $secondFactors->get($secondFactorIndex);
0 ignored issues
show
Documentation introduced by
$secondFactorIndex is of type object<Surfnet\StepupGat...le\Entity\SecondFactor>, but the function expects a string|integer.

It seems like the type of the argument is not accepted by the function/method which you are calling.

In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.

We suggest to add an explicit type cast like in the following example:

function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
Loading history...
184
            $logger->notice(sprintf(
185
                'User chose "%s" to use as second factor',
186
                $secondFactor->secondFactorType
187
            ));
188
189
            // Forward to action to verify possession of second factor
190
            return $this->selectAndRedirectTo($secondFactor, $context);
191
        }
192
193
        return ['form' => $form->createView()];
194
    }
195
196
    public function verifyGssfAction()
197
    {
198
        $context = $this->getResponseContext();
199
        $originalRequestId = $context->getInResponseTo();
200
201
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
202
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
203
        $logger->info('Received request to verify GSSF');
204
205
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
206
207
        $logger->info(sprintf(
208
            'Selected GSSF "%s" for verfication, forwarding to Saml handling',
209
            $selectedSecondFactor
210
        ));
211
212
        /** @var \Surfnet\StepupGateway\GatewayBundle\Service\SecondFactorService $secondFactorService */
213
        $secondFactorService = $this->get('gateway.service.second_factor_service');
214
        /** @var \Surfnet\StepupGateway\GatewayBundle\Entity\SecondFactor $secondFactor */
215
        $secondFactor = $secondFactorService->findByUuid($selectedSecondFactor);
216
        if (!$secondFactor) {
217
            $logger->critical(sprintf(
218
                'Requested verification of GSSF "%s", however that Second Factor no longer exists',
219
                $selectedSecondFactor
220
            ));
221
222
            throw new RuntimeException('Verification of selected second factor that no longer exists');
223
        }
224
225
        return $this->forward(
226
            'SurfnetStepupGatewaySamlStepupProviderBundle:SamlProxy:sendSecondFactorVerificationAuthnRequest',
227
            [
228
                'provider' => $secondFactor->secondFactorType,
229
                'subjectNameId' => $secondFactor->secondFactorIdentifier
230
            ]
231
        );
232
    }
233
234
    public function gssfVerifiedAction()
235
    {
236
        $context = $this->getResponseContext();
237
        $originalRequestId = $context->getInResponseTo();
238
239
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
240
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
241
        $logger->info('Attempting to mark GSSF as verified');
242
243
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
244
245
        /** @var \Surfnet\StepupGateway\GatewayBundle\Entity\SecondFactor $secondFactor */
246
        $secondFactor = $this->get('gateway.service.second_factor_service')->findByUuid($selectedSecondFactor);
247
        if (!$secondFactor) {
248
            $logger->critical(sprintf(
249
                'Verification of GSSF "%s" succeeded, however that Second Factor no longer exists',
250
                $selectedSecondFactor
251
            ));
252
253
            throw new RuntimeException('Verification of selected second factor that no longer exists');
254
        }
255
256
        $context->markSecondFactorVerified();
257
        $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId);
258
259
        $logger->info(sprintf(
260
            'Marked GSSF "%s" as verified, forwarding to Gateway controller to respond',
261
            $selectedSecondFactor
262
        ));
263
264
        return $this->forward($context->getResponseAction());
265
    }
266
267
    /**
268
     * @Template
269
     * @param Request $request
270
     * @return array|Response
271
     */
272
    public function verifyYubiKeySecondFactorAction(Request $request)
273
    {
274
        $context = $this->getResponseContext();
275
        $originalRequestId = $context->getInResponseTo();
276
277
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
278
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
279
280
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
281
282
        $logger->notice('Verifying possession of Yubikey second factor');
283
284
        $command = new VerifyYubikeyOtpCommand();
285
        $command->secondFactorId = $selectedSecondFactor;
286
287
        $form = $this->createForm('gateway_verify_yubikey_otp', $command)->handleRequest($request);
288
289
        if ($form->get('cancel')->isClicked()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Form\FormInterface as the method isClicked() does only exist in the following implementations of said interface: Symfony\Component\Form\SubmitButton.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

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

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

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

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
290
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
291
        }
292
293
        if (!$form->isValid()) {
294
            // OTP field is rendered empty in the template.
295
            return ['form' => $form->createView()];
296
        }
297
298
        $result = $this->getStepupService()->verifyYubikeyOtp($command);
299
300
        if ($result->didOtpVerificationFail()) {
301
            $form->addError(new FormError('gateway.form.verify_yubikey.otp_verification_failed'));
302
303
            // OTP field is rendered empty in the template.
304
            return ['form' => $form->createView()];
305
        } elseif (!$result->didPublicIdMatch()) {
306
            $form->addError(new FormError('gateway.form.verify_yubikey.public_id_mismatch'));
307
308
            // OTP field is rendered empty in the template.
309
            return ['form' => $form->createView()];
310
        }
311
312
        $this->getResponseContext()->markSecondFactorVerified();
313
        $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId);
314
315
        $logger->info(
316
            sprintf(
317
                'Marked Yubikey Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
318
                $selectedSecondFactor
319
            )
320
        );
321
322
        return $this->forward($context->getResponseAction());
323
    }
324
325
    /**
326
     * @Template
327
     * @param Request $request
328
     * @return array|Response
329
     */
330
    public function verifySmsSecondFactorAction(Request $request)
331
    {
332
        $context = $this->getResponseContext();
333
        $originalRequestId = $context->getInResponseTo();
334
335
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
336
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
337
338
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
339
340
        $logger->notice('Verifying possession of SMS second factor, preparing to send');
341
342
        $command = new SendSmsChallengeCommand();
343
        $command->secondFactorId = $selectedSecondFactor;
344
345
        $form = $this->createForm('gateway_send_sms_challenge', $command)->handleRequest($request);
346
347
        $stepupService = $this->getStepupService();
348
        $phoneNumber = InternationalPhoneNumber::fromStringFormat(
349
            $stepupService->getSecondFactorIdentifier($selectedSecondFactor)
350
        );
351
352
        $otpRequestsRemaining = $stepupService->getSmsOtpRequestsRemainingCount();
353
        $maximumOtpRequests = $stepupService->getSmsMaximumOtpRequestsCount();
354
        $viewVariables = ['otpRequestsRemaining' => $otpRequestsRemaining, 'maximumOtpRequests' => $maximumOtpRequests];
355
356
        if ($form->get('cancel')->isClicked()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Form\FormInterface as the method isClicked() does only exist in the following implementations of said interface: Symfony\Component\Form\SubmitButton.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

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

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

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

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
357
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
358
        }
359
360
        if (!$form->isValid()) {
361
            return array_merge($viewVariables, ['phoneNumber' => $phoneNumber, 'form' => $form->createView()]);
362
        }
363
364
        $logger->notice('Verifying possession of SMS second factor, sending challenge per SMS');
365
366
        if (!$stepupService->sendSmsChallenge($command)) {
367
            $form->addError(new FormError('gateway.form.send_sms_challenge.sms_sending_failed'));
368
369
            return array_merge($viewVariables, ['phoneNumber' => $phoneNumber, 'form' => $form->createView()]);
370
        }
371
372
        return $this->redirect($this->generateUrl('gateway_verify_second_factor_sms_verify_challenge'));
373
    }
374
375
    /**
376
     * @Template
377
     * @param Request $request
378
     * @return array|Response
379
     */
380
    public function verifySmsSecondFactorChallengeAction(Request $request)
381
    {
382
        $context = $this->getResponseContext();
383
        $originalRequestId = $context->getInResponseTo();
384
385
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
386
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
387
388
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
389
390
        $command = new VerifyPossessionOfPhoneCommand();
391
        $form = $this->createForm('gateway_verify_sms_challenge', $command)->handleRequest($request);
392
393
        if ($form->get('cancel')->isClicked()) {
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Form\FormInterface as the method isClicked() does only exist in the following implementations of said interface: Symfony\Component\Form\SubmitButton.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

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

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

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

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
394
            return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
395
        }
396
397
        if (!$form->isValid()) {
398
            return ['form' => $form->createView()];
399
        }
400
401
        $logger->notice('Verifying input SMS challenge matches');
402
403
        $verification = $this->getStepupService()->verifySmsChallenge($command);
404
405
        if ($verification->wasSuccessful()) {
406
            $this->getStepupService()->clearSmsVerificationState();
407
408
            $this->getResponseContext()->markSecondFactorVerified();
409
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId);
410
411
            $logger->info(
412
                sprintf(
413
                    'Marked Sms Second Factor "%s" as verified, forwarding to Saml Proxy to respond',
414
                    $selectedSecondFactor
415
                )
416
            );
417
418
            return $this->forward($context->getResponseAction());
419
        } elseif ($verification->didOtpExpire()) {
420
            $logger->notice('SMS challenge expired');
421
            $form->addError(new FormError('gateway.form.send_sms_challenge.challenge_expired'));
422
        } elseif ($verification->wasAttemptedTooManyTimes()) {
423
            $logger->notice('SMS challenge verification was attempted too many times');
424
            $form->addError(new FormError('gateway.form.send_sms_challenge.too_many_attempts'));
425
        } else {
426
            $logger->notice('SMS challenge did not match');
427
            $form->addError(new FormError('gateway.form.send_sms_challenge.sms_challenge_incorrect'));
428
        }
429
430
        return ['form' => $form->createView()];
431
    }
432
433
    /**
434
     * @Template
435
     */
436
    public function initiateU2fAuthenticationAction()
437
    {
438
        $context = $this->getResponseContext();
439
        $originalRequestId = $context->getInResponseTo();
440
441
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
442
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
443
444
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
445
        $stepupService = $this->getStepupService();
446
447
        $cancelFormAction = $this->generateUrl('gateway_verify_second_factor_u2f_cancel_authentication');
448
        $cancelForm =
449
            $this->createForm('gateway_cancel_second_factor_verification', null, ['action' => $cancelFormAction]);
450
451
        $logger->notice('Verifying possession of U2F second factor, looking for registration matching key handle');
452
453
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
454
        $keyHandle = new KeyHandle($stepupService->getSecondFactorIdentifier($selectedSecondFactor));
455
        $registration = $service->findRegistrationByKeyHandle($keyHandle);
456
457
        if ($registration === null) {
458
            $logger->critical(
459
                sprintf('No known registration for key handle of second factor "%s"', $selectedSecondFactor)
460
            );
461
            $this->addFlash('error', 'gateway.u2f.alert.unknown_registration');
462
463
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
464
        }
465
466
        $logger->notice('Creating sign request');
467
468
        $signRequest = $service->createSignRequest($registration);
469
        $signResponse = new SignResponse();
470
471
        /** @var AttributeBagInterface $session */
472
        $session = $this->get('gateway.session.u2f');
473
        $session->set('request', $signRequest);
474
475
        $formAction = $this->generateUrl('gateway_verify_second_factor_u2f_verify_authentication');
476
        $form = $this->createForm(
477
            'surfnet_stepup_u2f_verify_device_authentication',
478
            $signResponse,
479
            ['sign_request' => $signRequest, 'action' => $formAction]
480
        );
481
482
        return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()];
483
    }
484
485
    /**
486
     * @Template("SurfnetStepupGatewayGatewayBundle:SecondFactor:initiateU2fAuthentication.html.twig")
487
     *
488
     * @param Request $request
489
     * @return array|Response
490
     */
491
    public function verifyU2fAuthenticationAction(Request $request)
492
    {
493
        $context = $this->getResponseContext();
494
        $originalRequestId = $context->getInResponseTo();
495
496
        /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */
497
        $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId);
498
499
        $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger);
500
501
        $logger->notice('Received sign response from device');
502
503
        /** @var AttributeBagInterface $session */
504
        $session = $this->get('gateway.session.u2f');
505
        $signRequest = $session->get('request');
506
        $signResponse = new SignResponse();
507
508
        $formAction = $this->generateUrl('gateway_verify_second_factor_u2f_verify_authentication');
509
        $form = $this
510
            ->createForm(
511
                'surfnet_stepup_u2f_verify_device_authentication',
512
                $signResponse,
513
                ['sign_request' => $signRequest, 'action' => $formAction]
514
            )
515
            ->handleRequest($request);
516
517
        $cancelFormAction = $this->generateUrl('gateway_verify_second_factor_u2f_cancel_authentication');
518
        $cancelForm =
519
            $this->createForm('gateway_cancel_second_factor_verification', null, ['action' => $cancelFormAction]);
520
521
        if (!$form->isValid()) {
522
            $logger->error('U2F authentication verification could not be started because device send illegal data');
523
            $this->addFlash('error', 'gateway.u2f.alert.error');
524
525
            return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
526
        }
527
528
        $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification');
529
        $result = $service->verifyAuthentication($signRequest, $signResponse);
530
531
        if ($result->wasSuccessful()) {
532
            $context->markSecondFactorVerified();
533
            $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId);
534
535
            $logger->info(
536
                sprintf(
537
                    'Marked U2F second factor "%s" as verified, forwarding to Saml Proxy to respond',
538
                    $selectedSecondFactor
539
                )
540
            );
541
542
            return $this->forward($context->getResponseAction());
543
        } elseif ($result->didDeviceReportError()) {
544
            $logger->error('U2F device reported error during authentication');
545
            $this->addFlash('error', 'gateway.u2f.alert.device_reported_an_error');
546
        } else {
547
            $logger->error('U2F authentication verification failed');
548
            $this->addFlash('error', 'gateway.u2f.alert.error');
549
        }
550
551
        return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()];
552
    }
553
554
    public function cancelU2fAuthenticationAction()
555
    {
556
        return $this->forward('SurfnetStepupGatewayGatewayBundle:Gateway:sendAuthenticationCancelledByUser');
557
    }
558
559
    /**
560
     * @return \Surfnet\StepupGateway\GatewayBundle\Service\StepupAuthenticationService
561
     */
562
    private function getStepupService()
563
    {
564
        return $this->get('gateway.service.stepup_authentication');
565
    }
566
567
    /**
568
     * @return ResponseContext
569
     */
570
    private function getResponseContext()
571
    {
572
        return $this->get($this->get('gateway.proxy.state_handler')->getResponseContextServiceId());
573
    }
574
575
    /**
576
     * @return \Surfnet\StepupGateway\GatewayBundle\Monolog\Logger\AuthenticationLogger
577
     */
578
    private function getAuthenticationLogger()
579
    {
580
        return $this->get('gateway.authentication_logger');
581
    }
582
583
    /**
584
     * @param ResponseContext $context
585
     * @param LoggerInterface $logger
586
     * @return string
587
     */
588
    private function getSelectedSecondFactor(ResponseContext $context, LoggerInterface $logger)
589
    {
590
        $selectedSecondFactor = $context->getSelectedSecondFactor();
591
592
        if (!$selectedSecondFactor) {
593
            $logger->error('Cannot verify possession of an unknown second factor');
594
595
            throw new BadRequestHttpException('Cannot verify possession of an unknown second factor.');
596
        }
597
598
        return $selectedSecondFactor;
599
    }
600
601
    private function selectAndRedirectTo(SecondFactor $secondFactor, ResponseContext $context)
602
    {
603
        $context->saveSelectedSecondFactor($secondFactor);
604
605
        $this->getStepupService()->clearSmsVerificationState();
606
607
        $secondFactorTypeService = $this->get('surfnet_stepup.service.second_factor_type');
608
        $secondFactorType = new SecondFactorType($secondFactor->secondFactorType);
609
610
        $route = 'gateway_verify_second_factor_';
611
        if ($secondFactorTypeService->isGssf($secondFactorType)) {
612
            $route .= 'gssf';
613
        } else {
614
            $route .= strtolower($secondFactor->secondFactorType);
615
        }
616
617
        return $this->redirect($this->generateUrl($route));
618
    }
619
}
620