From 9438e61dd91b3abc47c17868f1a9321b4ddb0f4f Mon Sep 17 00:00:00 2001 From: Bas Strooband Date: Wed, 20 Feb 2019 10:41:13 +0100 Subject: [PATCH 1/4] Add FCM fallback for GCM A fallback (firebase/fcm) mechanism for gcm is added. This in order to support the new Android push notifications system. * Update the tiqr library * Add use_gcm_fallback flag to parameters.yml * Changed log level from info to notice for push messages processing --- app/config/parameters.yml.dist | 5 + composer.lock | 15 ++- .../Controller/AuthenticationController.php | 106 ++++++++++++------ .../DependencyInjection/Configuration.php | 10 ++ src/AppBundle/Resources/config/services.yml | 5 +- src/AppBundle/Tiqr/TiqrConfiguration.php | 5 + 6 files changed, 102 insertions(+), 44 deletions(-) diff --git a/app/config/parameters.yml.dist b/app/config/parameters.yml.dist index 28995439..e1b48078 100644 --- a/app/config/parameters.yml.dist +++ b/app/config/parameters.yml.dist @@ -38,6 +38,9 @@ parameters: # PCRE as accepted by preg_match (http://php.net/preg_match). mobile_app_user_agent_pattern: "/^.*$/" + # Flag for GCM fallback with FCM on failure + use_gcm_fallback: true + # Options for the tiqr library tiqr_library_options: general: @@ -57,6 +60,8 @@ parameters: apns: certificate: 'absolute path to certificate' environment: production + firebase: + apikey: 'Your Firebase API KEY' accountblocking: maxAttempts: 5 storage: diff --git a/composer.lock b/composer.lock index cf508296..35699261 100644 --- a/composer.lock +++ b/composer.lock @@ -1,7 +1,7 @@ { "_readme": [ "This file locks the dependencies of your project to a known state", - "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", + "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], "content-hash": "248f98a1778700b5f817d9805d9c93bc", @@ -2627,20 +2627,22 @@ }, { "name": "tiqr/tiqr-server-libphp", - "version": "v1.1.6", + "version": "v1.1.10", "source": { "type": "git", "url": "https://github.com/SURFnet/tiqr-server-libphp.git", - "reference": "9d3fc594c5070c0427f998bd27f6be71e99a3f36" + "reference": "70fa11c41d68be4cde040902eae202dcdea369b5" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/SURFnet/tiqr-server-libphp/zipball/9d3fc594c5070c0427f998bd27f6be71e99a3f36", - "reference": "9d3fc594c5070c0427f998bd27f6be71e99a3f36", + "url": "https://api.github.com/repos/SURFnet/tiqr-server-libphp/zipball/70fa11c41d68be4cde040902eae202dcdea369b5", + "reference": "70fa11c41d68be4cde040902eae202dcdea369b5", "shasum": "" }, "require": { + "ext-curl": "*", "ext-gd": "*", + "ext-json": "*", "kairos/phpqrcode": "~1.0", "zendframework/zendframework1": "~1.12" }, @@ -2650,7 +2652,7 @@ "BSD-3-Clause" ], "description": "php library for tiqr authentication.", - "time": "2018-09-04T11:58:24+00:00" + "time": "2019-02-28T15:49:53+00:00" }, { "name": "twig/twig", @@ -4671,6 +4673,7 @@ "mock", "xunit" ], + "abandoned": true, "time": "2017-06-30T09:13:00+00:00" }, { diff --git a/src/AppBundle/Controller/AuthenticationController.php b/src/AppBundle/Controller/AuthenticationController.php index b452d0ae..5f2c44fc 100644 --- a/src/AppBundle/Controller/AuthenticationController.php +++ b/src/AppBundle/Controller/AuthenticationController.php @@ -48,6 +48,10 @@ class AuthenticationController extends Controller private $authenticationRateLimitService; private $userRepository; private $stateHandler; + /** + * @var bool + */ + private $useGcmFallback; public function __construct( AuthenticationService $authenticationService, @@ -55,7 +59,8 @@ public function __construct( TiqrServiceInterface $tiqrService, TiqrUserRepositoryInterface $userRepository, AuthenticationRateLimitServiceInterface $authenticationRateLimitService, - LoggerInterface $logger + LoggerInterface $logger, + $useGcmFallback ) { $this->authenticationService = $authenticationService; $this->stateHandler = $stateHandler; @@ -63,6 +68,7 @@ public function __construct( $this->logger = $logger; $this->authenticationRateLimitService = $authenticationRateLimitService; $this->userRepository = $userRepository; + $this->useGcmFallback = $useGcmFallback; } /** @@ -317,13 +323,7 @@ public function authenticationNotificationAction() $notificationType = $user->getNotificationType(); $notificationAddress = $user->getNotificationAddress(); if ($notificationType && $notificationAddress) { - $this->logger->info(sprintf( - 'Sending client notification for type "%s" and address "%s"', - $notificationType, - $notificationAddress - )); - $result = $this->tiqrService->sendNotification($notificationType, $notificationAddress); - $this->logNotificationResponse($result, $notificationType, $notificationAddress); + $result = $this->sendNotification($notificationType, $notificationAddress); if ($result) { return $this->generateNotificationResponse('success'); } @@ -350,35 +350,6 @@ private function handleInvalidResponse(TiqrUserInterface $user, $response, Logge ]); } - /** - * @param boolean $result - * @param string $notificationType - * @param string $notificationAddress - */ - private function logNotificationResponse($result, $notificationType, $notificationAddress) - { - if ($result) { - $this->logger->info(sprintf( - 'Push notification successfully send for type "%s" and address "%s"', - $notificationType, - $notificationAddress - )); - - return; - } - - $this->logger->warning( - sprintf( - 'Failed to send push notification for type "%s" and address "%s"', - $notificationType, - $notificationAddress - ), - [ - 'error_info' => $this->tiqrService->getNotificationError(), - ] - ); - } - private function showUserIsBlockedErrorPage($isBlockedPermanently) { $exception = new UserTemporarilyBlockedException(); @@ -394,4 +365,65 @@ private function showUserIsBlockedErrorPage($isBlockedPermanently) ] ); } + + /** + * @param $notificationType + * @param $notificationAddress + * @return bool + */ + private function sendNotification($notificationType, $notificationAddress) + { + $this->logger->notice(sprintf( + 'Sending client notification for type "%s" and address "%s"', + $notificationType, + $notificationAddress + )); + + $result = $this->tiqrService->sendNotification($notificationType, $notificationAddress); + if (!$result + && $notificationType == 'GCM' + && $this->useGcmFallback + && $this->tiqrService->getNotificationError()['message'] == 'MismatchSenderId' + ) { + // Retry with FCM if GCM + + $this->logger->notice( + sprintf( + 'Failed to send push notification for type "%s" and address "%s" retrying with FCM', + $notificationType, + $notificationAddress + ), + [ + 'error_info' => $this->tiqrService->getNotificationError(), + ] + ); + + $notificationType = 'FCM'; + $result = $this->tiqrService->sendNotification($notificationType, $notificationAddress); + } + + if (!$result) { + $this->logger->warning( + sprintf( + 'Failed to send push notification for type "%s" and address "%s"', + $notificationType, + $notificationAddress + ), + [ + 'error_info' => $this->tiqrService->getNotificationError(), + ] + ); + return false; + } + + $this->logger->notice( + sprintf( + 'Successfully send push notification for type "%s" and address "%s"', + $notificationType, + $notificationAddress + ) + ); + + return true; + } } diff --git a/src/AppBundle/DependencyInjection/Configuration.php b/src/AppBundle/DependencyInjection/Configuration.php index c6caa1ef..423d3721 100644 --- a/src/AppBundle/DependencyInjection/Configuration.php +++ b/src/AppBundle/DependencyInjection/Configuration.php @@ -150,6 +150,16 @@ private function createLibraryConfig() ->isRequired() ->info(<<end() + ->end() + ->end() + ->arrayNode('firebase') + ->children() + ->scalarNode('apikey') + ->info(<<end() diff --git a/src/AppBundle/Resources/config/services.yml b/src/AppBundle/Resources/config/services.yml index 8a02c511..68da5ffe 100644 --- a/src/AppBundle/Resources/config/services.yml +++ b/src/AppBundle/Resources/config/services.yml @@ -1,4 +1,3 @@ -services: services: AppBundle\Twig\GsspExtension: tags: [twig.extension] @@ -57,3 +56,7 @@ services: AppBundle\Service\UserAgentMatcher: arguments: - '%mobile_app_user_agent_pattern%' + + + AppBundle\Controller\AuthenticationController: + $useGcmFallback: '%use_gcm_fallback%' \ No newline at end of file diff --git a/src/AppBundle/Tiqr/TiqrConfiguration.php b/src/AppBundle/Tiqr/TiqrConfiguration.php index 08cfc5c7..f22ff9c2 100644 --- a/src/AppBundle/Tiqr/TiqrConfiguration.php +++ b/src/AppBundle/Tiqr/TiqrConfiguration.php @@ -70,6 +70,11 @@ public function __construct($configuration) $this->options['gcm.application'] = $configuration['library']['gcm']['application']; } + if (isset($configuration['library']['firebase'])) { + Assertion::string($configuration['library']['firebase']['apikey']); + $this->options['firebase.apikey'] = $configuration['library']['firebase']['apikey']; + } + if (isset($configuration['accountblocking'][self::MAX_ATTEMPTS])) { Assertion::digit($configuration['accountblocking'][self::MAX_ATTEMPTS]); $this->options[self::MAX_ATTEMPTS] = $configuration['accountblocking'][self::MAX_ATTEMPTS]; From 060b1f3256b3648faecdcf6d4baa07e473e00d36 Mon Sep 17 00:00:00 2001 From: Bas Strooband Date: Fri, 1 Mar 2019 13:50:56 +0100 Subject: [PATCH 2/4] Fix security checker deprecation --- composer.json | 4 ++-- composer.lock | 31 ++++++++++++++++--------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/composer.json b/composer.json index 9b7643ed..b9ae1e33 100644 --- a/composer.json +++ b/composer.json @@ -64,7 +64,7 @@ "phpunit/phpunit": "^5.7", "sebastian/phpcpd": "^3.0", "sensio/generator-bundle": "^3.0", - "sensiolabs/security-checker": "^4.1", + "sensiolabs/security-checker": "^5.0", "squizlabs/php_codesniffer": "^3.1", "symfony/phpunit-bridge": "^3.0" }, @@ -95,7 +95,7 @@ "phpunit": "vendor/bin/phpunit tests", "behat": ["vendor/bin/behat --config behat.yml --tags '~skip'"], - "security-tests": "vendor/bin/security-checker security:check --end-point=http://security.sensiolabs.org/check_lock", + "security-tests": "vendor/bin/security-checker security:check", "coverage": [ "@phpunit-coverage", diff --git a/composer.lock b/composer.lock index 35699261..0b1b745e 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "248f98a1778700b5f817d9805d9c93bc", + "content-hash": "626f229659f60927420af6f59a39ea6d", "packages": [ { "name": "beberlei/assert", @@ -1392,21 +1392,21 @@ }, { "name": "sensio/distribution-bundle", - "version": "v5.0.22", + "version": "v5.0.24", "source": { "type": "git", "url": "https://github.com/sensiolabs/SensioDistributionBundle.git", - "reference": "209b11f8cee5bf71986dd703e45e27d3ed7a6d15" + "reference": "59eac70f15f97ee945924948a6f5e2f6f86b7a4b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sensiolabs/SensioDistributionBundle/zipball/209b11f8cee5bf71986dd703e45e27d3ed7a6d15", - "reference": "209b11f8cee5bf71986dd703e45e27d3ed7a6d15", + "url": "https://api.github.com/repos/sensiolabs/SensioDistributionBundle/zipball/59eac70f15f97ee945924948a6f5e2f6f86b7a4b", + "reference": "59eac70f15f97ee945924948a6f5e2f6f86b7a4b", "shasum": "" }, "require": { "php": ">=5.3.9", - "sensiolabs/security-checker": "~3.0|~4.0", + "sensiolabs/security-checker": "~5.0", "symfony/class-loader": "~2.3|~3.0", "symfony/config": "~2.3|~3.0", "symfony/dependency-injection": "~2.3|~3.0", @@ -1440,7 +1440,7 @@ "configuration", "distribution" ], - "time": "2018-06-07T06:22:12+00:00" + "time": "2018-12-14T17:36:15+00:00" }, { "name": "sensio/framework-extra-bundle", @@ -1514,20 +1514,21 @@ }, { "name": "sensiolabs/security-checker", - "version": "v4.1.8", + "version": "v5.0.3", "source": { "type": "git", "url": "https://github.com/sensiolabs/security-checker.git", - "reference": "dc270d5fec418cc6ac983671dba5d80ffaffb142" + "reference": "46be3f58adac13084497961e10eed9a7fb4d44d1" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/sensiolabs/security-checker/zipball/dc270d5fec418cc6ac983671dba5d80ffaffb142", - "reference": "dc270d5fec418cc6ac983671dba5d80ffaffb142", + "url": "https://api.github.com/repos/sensiolabs/security-checker/zipball/46be3f58adac13084497961e10eed9a7fb4d44d1", + "reference": "46be3f58adac13084497961e10eed9a7fb4d44d1", "shasum": "" }, "require": { "composer/ca-bundle": "^1.0", + "php": ">=5.5.9", "symfony/console": "~2.7|~3.0|~4.0" }, "bin": [ @@ -1536,12 +1537,12 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "4.1-dev" + "dev-master": "5.0-dev" } }, "autoload": { - "psr-0": { - "SensioLabs\\Security": "" + "psr-4": { + "SensioLabs\\Security\\": "SensioLabs/Security" } }, "notification-url": "https://packagist.org/downloads/", @@ -1555,7 +1556,7 @@ } ], "description": "A security checker for your composer.lock", - "time": "2018-02-28T22:10:01+00:00" + "time": "2018-12-19T17:14:59+00:00" }, { "name": "simplesamlphp/saml2", From 1454511998b982ece1ba2f0ae45a09e1ed0b9494 Mon Sep 17 00:00:00 2001 From: Bas Strooband Date: Mon, 4 Mar 2019 11:01:59 +0100 Subject: [PATCH 3/4] Add parameter optimizations Changed the name of the gcm fallback configuration parameter to be more descriptive. Add the bind parameter key to the servie.yml config to be more precise while still using the full potential of autowiring. --- app/config/parameters.yml.dist | 2 +- src/AppBundle/Controller/AuthenticationController.php | 9 ++++----- src/AppBundle/Resources/config/services.yml | 8 ++++---- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/config/parameters.yml.dist b/app/config/parameters.yml.dist index e1b48078..f96b7e50 100644 --- a/app/config/parameters.yml.dist +++ b/app/config/parameters.yml.dist @@ -39,7 +39,7 @@ parameters: mobile_app_user_agent_pattern: "/^.*$/" # Flag for GCM fallback with FCM on failure - use_gcm_fallback: true + use_firebase_fallback_for_gcm: true # Options for the tiqr library tiqr_library_options: diff --git a/src/AppBundle/Controller/AuthenticationController.php b/src/AppBundle/Controller/AuthenticationController.php index 5f2c44fc..ce4938da 100644 --- a/src/AppBundle/Controller/AuthenticationController.php +++ b/src/AppBundle/Controller/AuthenticationController.php @@ -51,7 +51,7 @@ class AuthenticationController extends Controller /** * @var bool */ - private $useGcmFallback; + private $useFirebaseFallbackForGcm; public function __construct( AuthenticationService $authenticationService, @@ -60,7 +60,7 @@ public function __construct( TiqrUserRepositoryInterface $userRepository, AuthenticationRateLimitServiceInterface $authenticationRateLimitService, LoggerInterface $logger, - $useGcmFallback + $useFirebaseFallbackForGcm ) { $this->authenticationService = $authenticationService; $this->stateHandler = $stateHandler; @@ -68,7 +68,7 @@ public function __construct( $this->logger = $logger; $this->authenticationRateLimitService = $authenticationRateLimitService; $this->userRepository = $userRepository; - $this->useGcmFallback = $useGcmFallback; + $this->useFirebaseFallbackForGcm = $useFirebaseFallbackForGcm; } /** @@ -382,11 +382,10 @@ private function sendNotification($notificationType, $notificationAddress) $result = $this->tiqrService->sendNotification($notificationType, $notificationAddress); if (!$result && $notificationType == 'GCM' - && $this->useGcmFallback + && $this->useFirebaseFallbackForGcm && $this->tiqrService->getNotificationError()['message'] == 'MismatchSenderId' ) { // Retry with FCM if GCM - $this->logger->notice( sprintf( 'Failed to send push notification for type "%s" and address "%s" retrying with FCM', diff --git a/src/AppBundle/Resources/config/services.yml b/src/AppBundle/Resources/config/services.yml index 68da5ffe..3337773f 100644 --- a/src/AppBundle/Resources/config/services.yml +++ b/src/AppBundle/Resources/config/services.yml @@ -54,9 +54,9 @@ services: - '@logger' AppBundle\Service\UserAgentMatcher: - arguments: - - '%mobile_app_user_agent_pattern%' - + bind: + $pattern: '%mobile_app_user_agent_pattern%' AppBundle\Controller\AuthenticationController: - $useGcmFallback: '%use_gcm_fallback%' \ No newline at end of file + bind: + $useFirebaseFallbackForGcm: '%use_firebase_fallback_for_gcm%' \ No newline at end of file From f420a953d5ee8494fe0acc002c3d298a4595e5b9 Mon Sep 17 00:00:00 2001 From: Bas Strooband Date: Mon, 4 Mar 2019 11:08:53 +0100 Subject: [PATCH 4/4] Add more descriptive comment to the GCM fallback parameter The comment for the GCM fallback should be more descriptive and therefore the comment was updated to be more descriptive. --- app/config/parameters.yml.dist | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/config/parameters.yml.dist b/app/config/parameters.yml.dist index f96b7e50..89dd0002 100644 --- a/app/config/parameters.yml.dist +++ b/app/config/parameters.yml.dist @@ -38,7 +38,8 @@ parameters: # PCRE as accepted by preg_match (http://php.net/preg_match). mobile_app_user_agent_pattern: "/^.*$/" - # Flag for GCM fallback with FCM on failure + # Flag to use Firebse as fallback when GCM fails with a MismatchSenderId. + # This is used to handle the push notification method rollover from GCM to Firebase. use_firebase_fallback_for_gcm: true # Options for the tiqr library