Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like SecondFactorController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use SecondFactorController, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
55 | class SecondFactorController extends Controller |
||
|
|||
56 | { |
||
57 | public function selectSecondFactorForVerificationAction() |
||
129 | |||
130 | /** |
||
131 | * @Template |
||
132 | * @param Request $request |
||
133 | * @return array|RedirectResponse|Response |
||
134 | * @SuppressWarnings(PHPMD.ExcessiveMethodLength) |
||
135 | */ |
||
136 | public function chooseSecondFactorAction(Request $request) |
||
236 | |||
237 | public function verifyGssfAction() |
||
276 | |||
277 | public function gssfVerifiedAction() |
||
309 | |||
310 | /** |
||
311 | * @Template |
||
312 | * @param Request $request |
||
313 | * @return array|Response |
||
314 | */ |
||
315 | public function verifyYubiKeySecondFactorAction(Request $request) |
||
316 | { |
||
317 | $context = $this->getResponseContext(); |
||
318 | $originalRequestId = $context->getInResponseTo(); |
||
319 | |||
320 | /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */ |
||
321 | $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId); |
||
322 | |||
323 | $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger); |
||
324 | |||
325 | $logger->notice('Verifying possession of Yubikey second factor'); |
||
326 | |||
327 | $command = new VerifyYubikeyOtpCommand(); |
||
328 | $command->secondFactorId = $selectedSecondFactor; |
||
329 | |||
330 | $form = $this->createForm(VerifyYubikeyOtpType::class, $command)->handleRequest($request); |
||
331 | $cancelForm = $this->buildCancelAuthenticationForm()->handleRequest($request); |
||
332 | |||
333 | View Code Duplication | if ($form->isSubmitted() && !$form->isValid()) { |
|
334 | // OTP field is rendered empty in the template. |
||
335 | return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]; |
||
336 | } |
||
337 | |||
338 | $result = $this->getStepupService()->verifyYubikeyOtp($command); |
||
339 | |||
340 | if ($result->didOtpVerificationFail()) { |
||
341 | $form->addError(new FormError('gateway.form.verify_yubikey.otp_verification_failed')); |
||
342 | |||
343 | // OTP field is rendered empty in the template. |
||
344 | return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]; |
||
345 | } elseif (!$result->didPublicIdMatch()) { |
||
346 | $form->addError(new FormError('gateway.form.verify_yubikey.public_id_mismatch')); |
||
347 | |||
348 | // OTP field is rendered empty in the template. |
||
349 | return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]; |
||
350 | } |
||
351 | |||
352 | $this->getResponseContext()->markSecondFactorVerified(); |
||
353 | $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId); |
||
354 | |||
355 | $logger->info( |
||
356 | sprintf( |
||
357 | 'Marked Yubikey Second Factor "%s" as verified, forwarding to Saml Proxy to respond', |
||
358 | $selectedSecondFactor |
||
359 | ) |
||
360 | ); |
||
361 | |||
362 | return $this->forward($context->getResponseAction()); |
||
363 | } |
||
364 | |||
365 | /** |
||
366 | * @Template |
||
367 | * @param Request $request |
||
368 | * @return array|Response |
||
369 | */ |
||
370 | public function verifySmsSecondFactorAction(Request $request) |
||
371 | { |
||
372 | $context = $this->getResponseContext(); |
||
373 | $originalRequestId = $context->getInResponseTo(); |
||
374 | |||
375 | /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */ |
||
376 | $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId); |
||
377 | |||
378 | $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger); |
||
379 | |||
380 | $logger->notice('Verifying possession of SMS second factor, preparing to send'); |
||
381 | |||
382 | $command = new SendSmsChallengeCommand(); |
||
383 | $command->secondFactorId = $selectedSecondFactor; |
||
384 | |||
385 | $form = $this->createForm(SendSmsChallengeType::class, $command)->handleRequest($request); |
||
386 | $cancelForm = $this->buildCancelAuthenticationForm()->handleRequest($request); |
||
387 | |||
388 | $stepupService = $this->getStepupService(); |
||
389 | $phoneNumber = InternationalPhoneNumber::fromStringFormat( |
||
390 | $stepupService->getSecondFactorIdentifier($selectedSecondFactor) |
||
391 | ); |
||
392 | |||
393 | $otpRequestsRemaining = $stepupService->getSmsOtpRequestsRemainingCount(); |
||
394 | $maximumOtpRequests = $stepupService->getSmsMaximumOtpRequestsCount(); |
||
395 | $viewVariables = ['otpRequestsRemaining' => $otpRequestsRemaining, 'maximumOtpRequests' => $maximumOtpRequests]; |
||
396 | |||
397 | if ($form->isSubmitted() && !$form->isValid()) { |
||
398 | return array_merge( |
||
399 | $viewVariables, |
||
400 | ['phoneNumber' => $phoneNumber, 'form' => $form->createView(), 'cancelForm' => $cancelForm->createView()] |
||
401 | ); |
||
402 | } |
||
403 | |||
404 | $logger->notice('Verifying possession of SMS second factor, sending challenge per SMS'); |
||
405 | |||
406 | if (!$stepupService->sendSmsChallenge($command)) { |
||
407 | $form->addError(new FormError('gateway.form.send_sms_challenge.sms_sending_failed')); |
||
408 | |||
409 | return array_merge( |
||
410 | $viewVariables, |
||
411 | ['phoneNumber' => $phoneNumber, 'form' => $form->createView(), 'cancelForm' => $cancelForm->createView()] |
||
412 | ); |
||
413 | } |
||
414 | |||
415 | return $this->redirect($this->generateUrl('gateway_verify_second_factor_sms_verify_challenge')); |
||
416 | } |
||
417 | |||
418 | /** |
||
419 | * @Template |
||
420 | * @param Request $request |
||
421 | * @return array|Response |
||
422 | */ |
||
423 | public function verifySmsSecondFactorChallengeAction(Request $request) |
||
424 | { |
||
425 | $context = $this->getResponseContext(); |
||
426 | $originalRequestId = $context->getInResponseTo(); |
||
427 | |||
428 | /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */ |
||
429 | $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId); |
||
430 | |||
431 | $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger); |
||
432 | |||
433 | $command = new VerifyPossessionOfPhoneCommand(); |
||
434 | $form = $this->createForm(VerifySmsChallengeType::class, $command)->handleRequest($request); |
||
435 | $cancelForm = $this->buildCancelAuthenticationForm()->handleRequest($request); |
||
436 | |||
437 | View Code Duplication | if ($form->isSubmitted() && !$form->isValid()) { |
|
438 | return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]; |
||
439 | } |
||
440 | |||
441 | $logger->notice('Verifying input SMS challenge matches'); |
||
442 | |||
443 | $verification = $this->getStepupService()->verifySmsChallenge($command); |
||
444 | |||
445 | if ($verification->wasSuccessful()) { |
||
446 | $this->getStepupService()->clearSmsVerificationState(); |
||
447 | |||
448 | $this->getResponseContext()->markSecondFactorVerified(); |
||
449 | $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId); |
||
450 | |||
451 | $logger->info( |
||
452 | sprintf( |
||
453 | 'Marked Sms Second Factor "%s" as verified, forwarding to Saml Proxy to respond', |
||
454 | $selectedSecondFactor |
||
455 | ) |
||
456 | ); |
||
457 | |||
458 | return $this->forward($context->getResponseAction()); |
||
459 | } elseif ($verification->didOtpExpire()) { |
||
460 | $logger->notice('SMS challenge expired'); |
||
461 | $form->addError(new FormError('gateway.form.send_sms_challenge.challenge_expired')); |
||
462 | } elseif ($verification->wasAttemptedTooManyTimes()) { |
||
463 | $logger->notice('SMS challenge verification was attempted too many times'); |
||
464 | $form->addError(new FormError('gateway.form.send_sms_challenge.too_many_attempts')); |
||
465 | } else { |
||
466 | $logger->notice('SMS challenge did not match'); |
||
467 | $form->addError(new FormError('gateway.form.send_sms_challenge.sms_challenge_incorrect')); |
||
468 | } |
||
469 | |||
470 | return ['form' => $form->createView(), 'cancelForm' => $cancelForm->createView()]; |
||
471 | } |
||
472 | |||
473 | /** |
||
474 | * @Template |
||
475 | */ |
||
476 | public function initiateU2fAuthenticationAction() |
||
524 | |||
525 | /** |
||
526 | * @Template("SurfnetStepupGatewayGatewayBundle:SecondFactor:initiateU2fAuthentication.html.twig") |
||
527 | * |
||
528 | * @param Request $request |
||
529 | * @return array|Response |
||
530 | */ |
||
531 | public function verifyU2fAuthenticationAction(Request $request) |
||
532 | { |
||
533 | $context = $this->getResponseContext(); |
||
534 | $originalRequestId = $context->getInResponseTo(); |
||
535 | |||
536 | /** @var \Surfnet\SamlBundle\Monolog\SamlAuthenticationLogger $logger */ |
||
537 | $logger = $this->get('surfnet_saml.logger')->forAuthentication($originalRequestId); |
||
538 | |||
539 | $selectedSecondFactor = $this->getSelectedSecondFactor($context, $logger); |
||
540 | |||
541 | $logger->notice('Received sign response from device'); |
||
542 | |||
543 | /** @var AttributeBagInterface $session */ |
||
544 | $session = $this->get('gateway.session.u2f'); |
||
545 | $signRequest = $session->get('request'); |
||
546 | $signResponse = new SignResponse(); |
||
547 | |||
548 | $formAction = $this->generateUrl('gateway_verify_second_factor_u2f_verify_authentication'); |
||
549 | $form = $this |
||
550 | ->createForm( |
||
551 | VerifyDeviceAuthenticationType::class, |
||
552 | $signResponse, |
||
553 | ['sign_request' => $signRequest, 'action' => $formAction] |
||
554 | ) |
||
555 | ->handleRequest($request); |
||
556 | |||
557 | $cancelFormAction = $this->generateUrl('gateway_verify_second_factor_u2f_cancel_authentication'); |
||
558 | $cancelForm = |
||
559 | $this->createForm(CancelSecondFactorVerificationType::class, null, ['action' => $cancelFormAction]); |
||
560 | |||
561 | if ($form->isSubmitted() && !$form->isValid()) { |
||
562 | $logger->error('U2F authentication verification could not be started because device send illegal data'); |
||
563 | $this->addFlash('error', 'gateway.u2f.alert.error'); |
||
564 | |||
565 | return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()]; |
||
566 | } |
||
567 | |||
568 | $service = $this->get('surfnet_stepup_u2f_verification.service.u2f_verification'); |
||
569 | $result = $service->verifyAuthentication($signRequest, $signResponse); |
||
570 | |||
571 | if ($result->wasSuccessful()) { |
||
572 | $context->markSecondFactorVerified(); |
||
573 | $this->getAuthenticationLogger()->logSecondFactorAuthentication($originalRequestId); |
||
574 | |||
575 | $logger->info( |
||
576 | sprintf( |
||
577 | 'Marked U2F second factor "%s" as verified, forwarding to Saml Proxy to respond', |
||
578 | $selectedSecondFactor |
||
579 | ) |
||
580 | ); |
||
581 | |||
582 | return $this->forward($context->getResponseAction()); |
||
583 | } elseif ($result->didDeviceReportError()) { |
||
584 | $logger->error('U2F device reported error during authentication'); |
||
585 | $this->addFlash('error', 'gateway.u2f.alert.device_reported_an_error'); |
||
586 | } else { |
||
587 | $logger->error('U2F authentication verification failed'); |
||
588 | $this->addFlash('error', 'gateway.u2f.alert.error'); |
||
589 | } |
||
590 | |||
591 | return ['authenticationFailed' => true, 'cancelForm' => $cancelForm->createView()]; |
||
592 | } |
||
593 | |||
594 | public function cancelAuthenticationAction() |
||
598 | |||
599 | /** |
||
600 | * @return \Surfnet\StepupGateway\GatewayBundle\Service\StepupAuthenticationService |
||
601 | */ |
||
602 | private function getStepupService() |
||
606 | |||
607 | /** |
||
608 | * @return ResponseContext |
||
609 | */ |
||
610 | private function getResponseContext() |
||
614 | |||
615 | /** |
||
616 | * @return \Surfnet\StepupGateway\GatewayBundle\Monolog\Logger\AuthenticationLogger |
||
617 | */ |
||
618 | private function getAuthenticationLogger() |
||
622 | |||
623 | /** |
||
624 | * @param ResponseContext $context |
||
625 | * @param LoggerInterface $logger |
||
626 | * @return string |
||
627 | */ |
||
628 | private function getSelectedSecondFactor(ResponseContext $context, LoggerInterface $logger) |
||
640 | |||
641 | private function selectAndRedirectTo(SecondFactor $secondFactor, ResponseContext $context) |
||
659 | |||
660 | /** |
||
661 | * @return Form |
||
662 | */ |
||
663 | private function buildCancelAuthenticationForm() |
||
673 | } |
||
674 |
This class, trait or interface has been deprecated. The supplier of the file has supplied an explanatory message.
The explanatory message should give you some clue as to whether and when the type will be removed from the class and what other constant to use instead.