Merge pull request #69 from OpenConext/feature/temporarily-firebase-s…

…upport Update tiqr-server-libphp to support firebase fallback
OpenConext · Mar 6, 2019 · 01c8168 · 01c8168
2 parents f983c4b + f420a95
commit 01c8168
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 63 deletions.
diff --git a/app/config/parameters.yml.dist b/app/config/parameters.yml.dist
@@ -38,6 +38,10 @@ parameters:
     # PCRE as accepted by preg_match (http://php.net/preg_match).
     mobile_app_user_agent_pattern: "/^.*$/"
 
+    # 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
     tiqr_library_options:
         general:
@@ -57,6 +61,8 @@ parameters:
             apns:
                 certificate: 'absolute path to certificate'
                 environment: production
+            firebase:
+                apikey: 'Your Firebase API KEY'
         accountblocking:
             maxAttempts: 5
         storage:

diff --git 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
diff --git a/src/AppBundle/Controller/AuthenticationController.php b/src/AppBundle/Controller/AuthenticationController.php
@@ -48,21 +48,27 @@ class AuthenticationController extends Controller
     private $authenticationRateLimitService;
     private $userRepository;
     private $stateHandler;
+    /**
+     * @var bool
+     */
+    private $useFirebaseFallbackForGcm;
 
     public function __construct(
         AuthenticationService $authenticationService,
         StateHandlerInterface $stateHandler,
         TiqrServiceInterface $tiqrService,
         TiqrUserRepositoryInterface $userRepository,
         AuthenticationRateLimitServiceInterface $authenticationRateLimitService,
-        LoggerInterface $logger
+        LoggerInterface $logger,
+        $useFirebaseFallbackForGcm
     ) {
         $this->authenticationService = $authenticationService;
         $this->stateHandler = $stateHandler;
         $this->tiqrService = $tiqrService;
         $this->logger = $logger;
         $this->authenticationRateLimitService = $authenticationRateLimitService;
         $this->userRepository = $userRepository;
+        $this->useFirebaseFallbackForGcm = $useFirebaseFallbackForGcm;
     }
 
     /**
@@ -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,64 @@ 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->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',
+                    $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
@@ -150,6 +150,16 @@ private function createLibraryConfig()
                     ->isRequired()
                     ->info(<<<TEXT
 The application identifier of your custom android app, typically com.yourorganization.yourapp
+TEXT
+                    )
+                ->end()
+            ->end()
+        ->end()
+        ->arrayNode('firebase')
+            ->children()
+                ->scalarNode('apikey')
+                    ->info(<<<TEXT
+        The API key you use for your Google Cloud Messaging (Firebase) account (android push notifications).
 TEXT
                     )
                 ->end()

diff --git 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]
@@ -55,5 +54,9 @@ services:
           - '@logger'
 
     AppBundle\Service\UserAgentMatcher:
-      arguments:
-        - '%mobile_app_user_agent_pattern%'
+      bind:
+        $pattern: '%mobile_app_user_agent_pattern%'
+
+    AppBundle\Controller\AuthenticationController:
+      bind:
+        $useFirebaseFallbackForGcm: '%use_firebase_fallback_for_gcm%'
diff --git 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];