From 9198ac13998f1e234a29bfdac737dbf9b95ee77b Mon Sep 17 00:00:00 2001 From: DRvanR Date: Mon, 13 Jul 2015 11:59:38 +0200 Subject: [PATCH 01/12] Configure X-Frame-Options:DENY header --- app/config/config.yml | 3 +++ app/config/security.yml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/config/config.yml b/app/config/config.yml index 9277fbfe8..d10ca3077 100644 --- a/app/config/config.yml +++ b/app/config/config.yml @@ -61,6 +61,9 @@ swiftmailer: spool: { type: memory } nelmio_security: + clickjacking: + paths: + '^/.*': DENY # Content Security Policy csp: report_uri: /csp/report diff --git a/app/config/security.yml b/app/config/security.yml index a15943a1e..d20640b66 100644 --- a/app/config/security.yml +++ b/app/config/security.yml @@ -20,5 +20,5 @@ security: invalidate_session: true access_control: - - { path: ^/authentication, roles: IS_AUTHENTICATED_ANONYMOUSLY, requires_channel:https } + - { path: ^/authentication, roles: IS_AUTHENTICATED_ANONYMOUSLY, requires_channel: https } - { path: ^/, roles: ROLE_USER, requires_channel: https } From 51daa6d918bcb0c0797b4583f1aa818d7761a0e4 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Mon, 13 Jul 2015 12:18:08 +0200 Subject: [PATCH 02/12] Regenerate Sessions ID after login to prevent fixation --- .../Security/Authentication/SessionHandler.php | 9 +++++++++ .../SelfServiceBundle/Security/Firewall/SamlListener.php | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SessionHandler.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SessionHandler.php index 727a24113..d73ef16f7 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SessionHandler.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SessionHandler.php @@ -91,4 +91,13 @@ public function clearRequestId() { $this->session->remove(self::SAML_SESSION_KEY . 'request_id'); } + + /** + * Migrates the current session to a new session id while maintaining all + * session attributes. + */ + public function migrate() + { + $this->session->migrate(); + } } diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Firewall/SamlListener.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Firewall/SamlListener.php index 04c698c47..cb4653824 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Firewall/SamlListener.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Firewall/SamlListener.php @@ -28,6 +28,7 @@ use Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication\Token\SamlToken; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; @@ -151,6 +152,9 @@ private function handleEvent(GetResponseEvent $event) $this->tokenStorage->setToken($authToken); + // migrate the session to prevent session hijacking + $this->sessionHandler->migrate(); + $event->setResponse(new RedirectResponse($this->sessionHandler->getCurrentRequestUri())); $logger->notice('Authentication succeeded, redirecting to original location'); From 6be8afe532a1c93431fad671856f775390dd9aab Mon Sep 17 00:00:00 2001 From: DRvanR Date: Mon, 13 Jul 2015 14:48:39 +0200 Subject: [PATCH 03/12] Update Stepup SAML bundle to fix dependency collision --- composer.json | 3 +- composer.lock | 122 +++++++++++++++++++++++--------------------------- 2 files changed, 57 insertions(+), 68 deletions(-) diff --git a/composer.json b/composer.json index 24b2a1894..5839db61c 100644 --- a/composer.json +++ b/composer.json @@ -25,8 +25,7 @@ "jms/di-extra-bundle": "~1.4.0", "surfnet/stepup-middleware-client-bundle": "^1.0", "guzzlehttp/guzzle": "~4", - "simplesamlphp/saml2": "dev-master", - "surfnet/stepup-saml-bundle": "^1.0", + "surfnet/stepup-saml-bundle": "^1.2.0", "surfnet/stepup-bundle": "^1.0", "symfony/swiftmailer-bundle": "~2.3" }, diff --git a/composer.lock b/composer.lock index 8bd41c036..ee0f03f43 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "dde1874086fc44d0301b0c1fead90c0b", + "hash": "b92753df8af9c7022a90afc7e9e7bc94", "packages": [ { "name": "beberlei/assert", @@ -1190,7 +1190,7 @@ }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phiamo/MopaBootstrapBundle/zipball/4d4b6291b47fc70491a4f92105be39d6bcccbb95", + "url": "https://api.github.com/repos/phiamo/MopaBootstrapBundle/zipball/ee161a9c8ebc41358466076ff065d2df7a852ab7", "reference": "818b0f47ebd352559950e9a64431ff9472e8a9dd", "shasum": "" }, @@ -1487,6 +1487,47 @@ ], "time": "2014-11-09 18:42:56" }, + { + "name": "robrichards/xmlseclibs", + "version": "1.4.0", + "source": { + "type": "git", + "url": "https://github.com/robrichards/xmlseclibs.git", + "reference": "e91b4272f43b7708020c995f3c58056e68bfddf0" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/robrichards/xmlseclibs/zipball/e91b4272f43b7708020c995f3c58056e68bfddf0", + "reference": "e91b4272f43b7708020c995f3c58056e68bfddf0", + "shasum": "" + }, + "require": { + "php": ">= 5.2" + }, + "suggest": { + "ext/mcrypt": "MCrypt extension", + "ext/openssl": "OpenSSL extension" + }, + "type": "library", + "autoload": { + "classmap": [ + "src/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "description": "A PHP library for XML Security", + "homepage": "https://github.com/robrichards/xmlseclibs", + "keywords": [ + "security", + "signature", + "xml", + "xmldsig" + ], + "time": "2015-06-23 18:50:25" + }, { "name": "sensio/distribution-bundle", "version": "v3.0.30", @@ -1649,16 +1690,16 @@ }, { "name": "simplesamlphp/saml2", - "version": "dev-master", + "version": "v1.6.1", "source": { "type": "git", "url": "https://github.com/simplesamlphp/saml2.git", - "reference": "fd11ea8c7d8de2537b1b8e06033e0db2664bb373" + "reference": "da4f356dfee6eaabac4fb8afb0de045e571c476b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/simplesamlphp/saml2/zipball/fd11ea8c7d8de2537b1b8e06033e0db2664bb373", - "reference": "fd11ea8c7d8de2537b1b8e06033e0db2664bb373", + "url": "https://api.github.com/repos/simplesamlphp/saml2/zipball/da4f356dfee6eaabac4fb8afb0de045e571c476b", + "reference": "da4f356dfee6eaabac4fb8afb0de045e571c476b", "shasum": "" }, "require": { @@ -1695,58 +1736,7 @@ } ], "description": "SAML2 PHP library from SimpleSAMLphp", - "time": "2015-06-22 14:31:51" - }, - { - "name": "simplesamlphp/xmlseclibs", - "version": "v1.3.2", - "source": { - "type": "git", - "url": "https://github.com/simplesamlphp/xmlseclibs.git", - "reference": "734e80899ade295b979de08553161cad63c2dd98" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/simplesamlphp/xmlseclibs/zipball/734e80899ade295b979de08553161cad63c2dd98", - "reference": "734e80899ade295b979de08553161cad63c2dd98", - "shasum": "" - }, - "replace": { - "cdatazone/xmlseclibs": "self.version", - "fr3d/xmlseclibs": "self.version", - "robrichards/xmlseclibs": "self.version" - }, - "suggest": { - "ext/mcrypt": "", - "ext/openssl": "" - }, - "type": "library", - "autoload": { - "files": [ - "xmlseclibs.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "BSD-3-Clause" - ], - "authors": [ - { - "name": "Rob Richards" - } - ], - "description": "A PHP library for XML Security", - "homepage": "http://code.google.com/p/xmlseclibs/", - "keywords": [ - "certificate", - "security", - "signature", - "signing", - "x.509", - "xml", - "xmlsec" - ], - "time": "2013-06-19 00:00:00" + "time": "2015-07-13 10:00:26" }, { "name": "surfnet/stepup-bundle", @@ -1852,22 +1842,23 @@ }, { "name": "surfnet/stepup-saml-bundle", - "version": "1.0.0", + "version": "1.2.0", "source": { "type": "git", "url": "https://github.com/SURFnet/Stepup-saml-bundle.git", - "reference": "8f336c442d3dc9d045bff498c4b27de65213565a" + "reference": "abc48cd40ed5abebf74d12467c439db54d0aa5f5" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/SURFnet/Stepup-saml-bundle/zipball/8f336c442d3dc9d045bff498c4b27de65213565a", - "reference": "8f336c442d3dc9d045bff498c4b27de65213565a", + "url": "https://api.github.com/repos/SURFnet/Stepup-saml-bundle/zipball/abc48cd40ed5abebf74d12467c439db54d0aa5f5", + "reference": "abc48cd40ed5abebf74d12467c439db54d0aa5f5", "shasum": "" }, "require": { "ext-openssl": "*", "php": "~5.4", - "simplesamlphp/saml2": "dev-master", + "robrichards/xmlseclibs": "^1.4.0", + "simplesamlphp/saml2": "^1.6.1", "symfony/dependency-injection": "^2.6", "symfony/framework-bundle": "~2.5" }, @@ -1895,7 +1886,7 @@ "stepup", "surfnet" ], - "time": "2015-06-19 10:11:01" + "time": "2015-07-13 12:44:50" }, { "name": "swiftmailer/swiftmailer", @@ -4263,8 +4254,7 @@ "aliases": [], "minimum-stability": "stable", "stability-flags": { - "mopa/bootstrap-bundle": 20, - "simplesamlphp/saml2": 20 + "mopa/bootstrap-bundle": 20 }, "prefer-stable": false, "prefer-lowest": false, From 55ba72fc865f0c4048b1637d6b2695c0215f3e96 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 10:14:04 +0200 Subject: [PATCH 04/12] Update SAML bundle to include XXE defense scoping --- composer.json | 2 +- composer.lock | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/composer.json b/composer.json index 5839db61c..6c39cfbcb 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,7 @@ "jms/di-extra-bundle": "~1.4.0", "surfnet/stepup-middleware-client-bundle": "^1.0", "guzzlehttp/guzzle": "~4", - "surfnet/stepup-saml-bundle": "^1.2.0", + "surfnet/stepup-saml-bundle": "^1.3.0", "surfnet/stepup-bundle": "^1.0", "symfony/swiftmailer-bundle": "~2.3" }, diff --git a/composer.lock b/composer.lock index ee0f03f43..af5a3a2e2 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "b92753df8af9c7022a90afc7e9e7bc94", + "hash": "db58f82f70cfd4430ec8dfc43e96b065", "packages": [ { "name": "beberlei/assert", @@ -1842,16 +1842,16 @@ }, { "name": "surfnet/stepup-saml-bundle", - "version": "1.2.0", + "version": "1.3.0", "source": { "type": "git", "url": "https://github.com/SURFnet/Stepup-saml-bundle.git", - "reference": "abc48cd40ed5abebf74d12467c439db54d0aa5f5" + "reference": "e5417e683a47377177135d788199b8be072b6de1" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/SURFnet/Stepup-saml-bundle/zipball/abc48cd40ed5abebf74d12467c439db54d0aa5f5", - "reference": "abc48cd40ed5abebf74d12467c439db54d0aa5f5", + "url": "https://api.github.com/repos/SURFnet/Stepup-saml-bundle/zipball/e5417e683a47377177135d788199b8be072b6de1", + "reference": "e5417e683a47377177135d788199b8be072b6de1", "shasum": "" }, "require": { @@ -1886,7 +1886,7 @@ "stepup", "surfnet" ], - "time": "2015-07-13 12:44:50" + "time": "2015-07-13 13:45:47" }, { "name": "swiftmailer/swiftmailer", From 47b565364e57ce622efff243ff3f4e0b00ee641a Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 10:25:33 +0200 Subject: [PATCH 05/12] Verify that the SF to be revoked belongs to the current identity --- .../Controller/SecondFactorController.php | 9 ++++++ .../Service/SecondFactorService.php | 29 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/SecondFactorController.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/SecondFactorController.php index f35fa42c9..1cac5a953 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/SecondFactorController.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/SecondFactorController.php @@ -63,6 +63,15 @@ public function revokeAction(Request $request, $state, $secondFactorId) /** @var SecondFactorService $service */ $service = $this->get('surfnet_stepup_self_service_self_service.service.second_factor'); + if (!$service->identityHasSecondFactorOfStateWithId($this->getIdentity()->id, $state, $secondFactorId)) { + $this->get('logger')->error(sprintf( + 'Identity "%s" tried to revoke "%s" second factor "%s", but does not own that second factor', + $this->getIdentity()->id, + $state, + $secondFactorId + )); + throw new NotFoundHttpException(); + } if ($form->isValid()) { /** @var FlashBagInterface $flashBag */ diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SecondFactorService.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SecondFactorService.php index 805048151..f0a18ac87 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SecondFactorService.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Service/SecondFactorService.php @@ -107,6 +107,35 @@ public function doSecondFactorsExistForIdentity($identityId) $vettedSecondFactors->getTotalItems() > 0; } + public function identityHasSecondFactorOfStateWithId($identityId, $state, $secondFactorId) + { + switch ($state) { + case 'unverified': + $secondFactors = $this->findUnverifiedByIdentity($identityId); + break; + case 'verified': + $secondFactors = $this->findVerifiedByIdentity($identityId); + break; + case 'vetted': + $secondFactors = $this->findVettedByIdentity($identityId); + break; + default: + throw new \LogicException(sprintf('Invalid second factor state "%s" given.', $state)); + } + + if (count($secondFactors->getElements()) === 0) { + return false; + } + + foreach ($secondFactors->getElements() as $secondFactor) { + if ($secondFactor->id === $secondFactorId) { + return true; + } + } + + return false; + } + /** * Returns the given registrant's unverified second factors. * From 16aed397faa7b7d29373f9c41abfed3c698c6bf4 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 10:25:56 +0200 Subject: [PATCH 06/12] Ensure switch locale return-url points to the same domain --- .../SelfServiceBundle/Controller/LocaleController.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/LocaleController.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/LocaleController.php index 447e67241..ca4784927 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/LocaleController.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/LocaleController.php @@ -22,6 +22,7 @@ use Surfnet\StepupBundle\Command\SwitchLocaleCommand; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; final class LocaleController extends Controller { @@ -29,6 +30,16 @@ public function switchLocaleAction(Request $request) { $returnUrl = $request->query->get('return-url'); + $domain = $request->getSchemeAndHttpHost(); + if (strpos($returnUrl, $domain) !== 0) { + $this->get('logger')->error(sprintf( + 'Identity "%s" used illegal return-url for redirection after changing locale, aborting request', + $this->getIdentity()->id + )); + + throw new BadRequestHttpException('Invalid return-url given'); + } + /** @var LoggerInterface $logger */ $logger = $this->get('logger'); $logger->info('Switching locale...'); From d6d4903db45dc05906c42c0ccc2cff9db5a28e09 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 11:29:26 +0200 Subject: [PATCH 07/12] Use SF form for GSSF initiate action for CSRF protection --- .../Registration/GssfController.php | 33 +++++------ .../Form/Type/InitiateGssfType.php | 59 +++++++++++++++++++ .../Resources/config/services.yml | 12 ++-- .../Registration/Gssf/initiate.html.twig | 17 +++--- 4 files changed, 87 insertions(+), 34 deletions(-) create mode 100644 src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Registration/GssfController.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Registration/GssfController.php index c27415c67..a28e6c76c 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Registration/GssfController.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Registration/GssfController.php @@ -19,7 +19,6 @@ namespace Surfnet\StepupSelfService\SelfServiceBundle\Controller\Registration; use Exception; -use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template; use Surfnet\SamlBundle\Http\XMLResponse; use Surfnet\SamlBundle\SAML2\AuthnRequestFactory; use Surfnet\SamlBundle\SAML2\Response\Assertion\InResponseTo; @@ -34,14 +33,12 @@ final class GssfController extends Controller { /** - * @Template - * @param Request $request * @param string $provider * @return array|Response */ - public function initiateAction(Request $request, $provider) + public function initiateAction($provider) { - return ['provider' => $this->getProvider($provider)->getName()]; + return $this->renderInitiateForm($provider); } /** @@ -101,10 +98,7 @@ public function consumeAssertionAction(Request $httpRequest, $provider) sprintf('Could not process received Response, error: "%s"', $exception->getMessage()) ); - return $this->render( - 'SurfnetStepupSelfServiceSelfServiceBundle:Registration/Gssf:initiate.html.twig', - ['provider' => $provider->getName(), 'authenticationFailed' => true] - ); + return $this->renderInitiateForm($provider->getName(), ['authenticationFailed' => true]); } $expectedResponseTo = $provider->getStateHandler()->getRequestId(); @@ -116,10 +110,7 @@ public function consumeAssertionAction(Request $httpRequest, $provider) ($expectedResponseTo ? 'expected "' . $expectedResponseTo . '"' : ' no response expected') )); - return $this->render( - 'SurfnetStepupSelfServiceSelfServiceBundle:Registration/Gssf:initiate.html.twig', - ['provider' => $provider->getName(), 'authenticationFailed' => true] - ); + return $this->renderInitiateForm($provider->getName(), ['authenticationFailed' => true]); } $this->get('logger')->notice( @@ -145,10 +136,7 @@ public function consumeAssertionAction(Request $httpRequest, $provider) $this->getLogger()->error('Unable to prove GSSF possession'); - return $this->render( - 'SurfnetStepupSelfServiceSelfServiceBundle:Registration/Gssf:initiate.html.twig', - ['provider' => $provider->getName(), 'proofOfPossessionFailed' => true] - ); + return $this->renderInitiateForm($provider->getName(), ['proofOfPossessionFailed' => true]); } /** @@ -191,4 +179,15 @@ private function getLogger() { return $this->get('logger'); } + + private function renderInitiateForm($provider, array $parameters = []) + { + $form = $this->createForm('ss_initiate_gssf', null, ['provider' => $provider]); + $templateParameters = array_merge($parameters, ['form' => $form->createView(), 'provider' => $provider]); + + return $this->render( + 'SurfnetStepupSelfServiceSelfServiceBundle:Registration/Gssf:initiate.html.twig', + $templateParameters + ); + } } diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php new file mode 100644 index 000000000..e3a139b3c --- /dev/null +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php @@ -0,0 +1,59 @@ +router = $router; + } + + public function buildForm(FormBuilderInterface $builder, array $options) + { + $action = $this->router->generate('ss_registration_gssf_authenticate', ['provider' => $options['provider']]); + + $builder + ->add('submit', 'button', [ + 'attr' => ['class' => 'btn btn-primary'], + 'label' => 'ss.registration.gssf.initiate.' . $options['provider'] . '.button.initiate' + ]) + ->setAction($action); + } + + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setRequired(['provider']); + } + + public function getName() + { + return 'ss_initiate_gssf'; + } +} diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/services.yml b/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/services.yml index cdab0ec47..78b65c48d 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/services.yml +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/services.yml @@ -35,13 +35,11 @@ services: class: Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\RevokeSecondFactorType tags: [{ name: form.type, alias: ss_revoke_second_factor }] - surfnet_stepup_self_service_self_serivce.form.type.phone_number: - class: Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\PhoneNumberType - tags: [{ name: form.type, alias: ss_phone_number }] - - surfnet_stepup_self_service_self_service.form.type.initiate_registration_with_gssp: - class: Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\InitiateRegistrationWithGsspType - tags: [{ name: form.type, alias: ss_initiate_registration_with_gssp }] + surfnet_stepup_self_service_self_service.form.type.initiate_gssf: + class: Surfnet\StepupSelfService\SelfServiceBundle\Form\Type\InitiateGssfType + arguments: + - @router + tags: [{ name: form.type, alias: ss_initiate_gssf }] surfnet_stepup_self_service_self_service.service.yubikey: public: false diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/views/Registration/Gssf/initiate.html.twig b/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/views/Registration/Gssf/initiate.html.twig index 5b558ab08..e8ff389db 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/views/Registration/Gssf/initiate.html.twig +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/views/Registration/Gssf/initiate.html.twig @@ -15,15 +15,12 @@
-
- {% if authenticationFailed is defined %} -
{{ ('ss.registration.gssf.initiate.' ~ provider ~ '.error.authn_failed')|trans }}
- {% endif %} - {% if proofOfPossessionFailed is defined %} -
{{ ('ss.registration.gssf.initiate.' ~ provider ~ '.error.proof_of_possession_failed')|trans }}
- {% endif %} - - -
+ {% if authenticationFailed is defined %} +
{{ ('ss.registration.gssf.initiate.' ~ provider ~ '.error.authn_failed')|trans }}
+ {% endif %} + {% if proofOfPossessionFailed is defined %} +
{{ ('ss.registration.gssf.initiate.' ~ provider ~ '.error.proof_of_possession_failed')|trans }}
+ {% endif %} + {{ form(form) }} {% endblock %} From 516f7660ed6de89f48acebc4d62a513148e811b3 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 13:41:54 +0200 Subject: [PATCH 08/12] Actually submit the Initiate GSSF form --- .../SelfServiceBundle/Form/Type/InitiateGssfType.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php index e3a139b3c..56978f37f 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Form/Type/InitiateGssfType.php @@ -40,7 +40,7 @@ public function buildForm(FormBuilderInterface $builder, array $options) $action = $this->router->generate('ss_registration_gssf_authenticate', ['provider' => $options['provider']]); $builder - ->add('submit', 'button', [ + ->add('submit', 'submit', [ 'attr' => ['class' => 'btn btn-primary'], 'label' => 'ss.registration.gssf.initiate.' . $options['provider'] . '.button.initiate' ]) From cbb03d24984a08508e823d1c02355a4492ddc8f0 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 13:42:20 +0200 Subject: [PATCH 09/12] Update StepupBundle to include cache headers update --- composer.json | 2 +- composer.lock | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/composer.json b/composer.json index 6c39cfbcb..ed22440c1 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,7 @@ "surfnet/stepup-middleware-client-bundle": "^1.0", "guzzlehttp/guzzle": "~4", "surfnet/stepup-saml-bundle": "^1.3.0", - "surfnet/stepup-bundle": "^1.0", + "surfnet/stepup-bundle": "^1.1.0", "symfony/swiftmailer-bundle": "~2.3" }, "require-dev": { diff --git a/composer.lock b/composer.lock index af5a3a2e2..c072f0391 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "db58f82f70cfd4430ec8dfc43e96b065", + "hash": "51d6418f9f2d66e3080217706d490010", "packages": [ { "name": "beberlei/assert", @@ -1740,16 +1740,16 @@ }, { "name": "surfnet/stepup-bundle", - "version": "1.0.0", + "version": "1.1.0", "source": { "type": "git", "url": "https://github.com/SURFnet/Stepup-bundle.git", - "reference": "7e201ac7f5b870a142a85d3a44f1e2ebb8b31933" + "reference": "5282a745516ba34cdd164c8d09b0add9fb859615" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/SURFnet/Stepup-bundle/zipball/7e201ac7f5b870a142a85d3a44f1e2ebb8b31933", - "reference": "7e201ac7f5b870a142a85d3a44f1e2ebb8b31933", + "url": "https://api.github.com/repos/SURFnet/Stepup-bundle/zipball/5282a745516ba34cdd164c8d09b0add9fb859615", + "reference": "5282a745516ba34cdd164c8d09b0add9fb859615", "shasum": "" }, "require": { @@ -1789,7 +1789,7 @@ "suaas", "surfnet" ], - "time": "2015-06-19 11:44:48" + "time": "2015-07-14 11:33:20" }, { "name": "surfnet/stepup-middleware-client-bundle", From c8630ac6f8a0ce20012dc902ea8621770029e487 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 14:33:35 +0200 Subject: [PATCH 10/12] Small coding style fixes, deprecated warning fix --- .../SelfServiceBundle/Controller/Controller.php | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Controller.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Controller.php index f4f7b22cd..0e1dc750b 100644 --- a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Controller.php +++ b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/Controller.php @@ -21,7 +21,7 @@ use Surfnet\StepupMiddlewareClientBundle\Identity\Dto\Identity; use Symfony\Bundle\FrameworkBundle\Controller\Controller as FrameworkController; use Symfony\Component\Security\Core\Exception\AccessDeniedException; -use Symfony\Component\Security\Core\SecurityContextInterface; +use UnexpectedValueException; class Controller extends FrameworkController { @@ -31,16 +31,13 @@ class Controller extends FrameworkController */ protected function getIdentity() { - /** @var SecurityContextInterface $tokenStorage */ - $tokenStorage = $this->get('security.context'); - $token = $tokenStorage->getToken(); - - $user = $token->getUser(); + $token = $this->get('security.token_storage')->getToken(); + $user = $token->getUser(); if (!$user instanceof Identity) { $actualType = is_object($token) ? get_class($token) : gettype($token); - throw new \UnexpectedValueException( + throw new UnexpectedValueException( sprintf( "Token did not contain user of type '%s', but one of type '%s'", 'Surfnet\StepupMiddlewareClientBundle\Identity\Dto\Identity', From ce64c8a435371e1e2872fd2d9cc1e48164372c14 Mon Sep 17 00:00:00 2001 From: DRvanR Date: Tue, 14 Jul 2015 14:34:04 +0200 Subject: [PATCH 11/12] Add CSRF protection to logout form --- app/Resources/views/base.html.twig | 2 +- app/config/security.yml | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/Resources/views/base.html.twig b/app/Resources/views/base.html.twig index b7800e196..5deab8a7e 100644 --- a/app/Resources/views/base.html.twig +++ b/app/Resources/views/base.html.twig @@ -27,7 +27,7 @@ {% if app.user %}
-
+