Issue 00 - pass QA and finish auth refactor

ThomasLdev · Apr 7, 2024 · 5c5c7c7 · 5c5c7c7
1 parent b2499d7
commit 5c5c7c7
Show file tree

Hide file tree

Showing 12 changed files with 184 additions and 122 deletions.
diff --git a/config/packages/google_apiclient.yaml b/config/packages/google_apiclient.yaml
@@ -7,5 +7,3 @@ services:
             # Authentication with "OAuth 2.0" using Client ID & Secret
             - [setClientId, ['%env(GOOGLE_CLIENT_ID)%']]
             - [setClientSecret, ['%env(GOOGLE_CLIENT_SECRET)%']]
-            - [setApplicationName, ['Instabot']]
-            - [setAccessType, ['offline']]
diff --git a/src/Controller/BaseController.php b/src/Controller/BaseController.php
@@ -16,7 +16,6 @@
 
 class BaseController extends AbstractController
 {
-    protected const LOGIN_ROUTE = 'app_login';
     protected const INDEX_ROUTE = 'app_index';
     protected const SETTINGS_ROUTE = 'app_settings';
 
@@ -32,34 +31,16 @@ public function flashOnRedirect(string $type, string $message, string $route): R
         return $this->redirectToRoute($route);
     }
 
-    public function getAppUser(): User
+    public function translateFlash(string $translateKey): string
     {
-        $user = $this->getUser();
-
-        if (false === $user instanceof User) {
-            $this->flashOnRedirect('error', 'errors.controller.user.not_logged',self::LOGIN_ROUTE);
-        }
-
-        return $user;
+        return $this->translator->trans($translateKey);
     }
 
-    public function getAppUserSettings(User $user): UserSettings
+    public function getUserSettings(): ?UserSettings
     {
-        $settings = $user->getSettings();
-
-        if (null === $settings) {
-            $this->flashOnRedirect(
-                'error',
-                'errors.controller.user.no_settings',
-                self::SETTINGS_ROUTE
-            );
-        }
-
-        return $settings;
-    }
+        /** @var User $user */
+        $user = $this->getUser();
 
-    public function translateFlash(string $translateKey): string
-    {
-        return $this->translator->trans($translateKey);
+        return $user->getSettings();
     }
 }
diff --git a/src/Controller/DriveFilesController.php b/src/Controller/DriveFilesController.php
@@ -9,24 +9,32 @@
 namespace App\Controller;
 
 use App\Service\Google\Drive\GoogleDriveClientService;
-use Psr\Container\ContainerExceptionInterface;
-use Psr\Container\NotFoundExceptionInterface;
+use Exception;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Attribute\Route;
 
 class DriveFilesController extends BaseController
 {
     /**
-     * @throws ContainerExceptionInterface
-     * @throws NotFoundExceptionInterface
+     * @throws Exception
      */
     #[Route('/drive/files', name: 'app_drive_files')]
     public function index(GoogleDriveClientService $driveService): Response|RedirectResponse
     {
         $this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
 
-        $response = $driveService->getFilesForUser($this->getAppUserSettings($this->getAppUser()));
+        $settings = $this->getUserSettings();
+
+        if (null === $settings) {
+            return $this->flashOnRedirect(
+                'error',
+                'errors.controller.drive.no_settings',
+                self::SETTINGS_ROUTE
+            );
+        }
+
+        $response = $driveService->getFilesForUser($settings);
 
         if (false === $response->getSuccess()) {
             return $this->flashOnRedirect('error', $response->getMessage(), self::SETTINGS_ROUTE);

diff --git a/src/Controller/GoogleAuthorizeController.php b/src/Controller/GoogleAuthorizeController.php
@@ -25,21 +25,20 @@ public function index(GoogleClientService $clientService): RedirectResponse
     {
         $this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
 
-        $client = null;
-        $settings = $this->getAppUserSettings($this->getAppUser());
+        $settings = $this->getUserSettings();
 
-        try {
-            $client = $clientService->getClientForUser($settings);
-        } catch (Exception $e) {
-            $this->flashOnRedirect(
+        if (null === $settings) {
+            return $this->flashOnRedirect(
                 'error',
-                'errors.controller.google.authorization_failed',
-                BaseController::SETTINGS_ROUTE
+                'errors.controller.drive.no_settings',
+                self::SETTINGS_ROUTE
             );
         }
 
-        if (null === $client) {
-            $this->flashOnRedirect(
+        try {
+            $client = $clientService->getClientForUser($settings);
+        } catch (Exception $e) {
+            return $this->flashOnRedirect(
                 'error',
                 'errors.controller.google.authorization_failed',
                 BaseController::SETTINGS_ROUTE
@@ -58,17 +57,27 @@ public function response(Request $request, GoogleOAuthTokenService $tokenService
     {
         $this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
 
-        $settings = $this->getAppUserSettings($this->getAppUser());
+        $settings = $this->getUserSettings();
+
+        if (null === $settings) {
+            return $this->flashOnRedirect(
+                'error',
+                'errors.controller.drive.no_settings',
+                self::SETTINGS_ROUTE
+            );
+        }
+
         $authCode = $request->query->get('code');
 
         if (false === is_string($authCode)) {
-            $this->flashOnRedirect(
+            return $this->flashOnRedirect(
                 'error',
                 'errors.controller.google.no_code',
                 BaseController::SETTINGS_ROUTE
             );
         }
 
+        /** @var string $authCode */
         $response = $tokenService->storeAuthCodeForUser($settings, $authCode);
 
         if (false === $response->getSuccess()) {
@@ -91,7 +100,16 @@ public function revokeAuthCodeForUser(EntityManagerInterface $entityManager): Re
     {
         $this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY');
 
-        $settings = $this->getAppUserSettings($this->getAppUser());
+        $settings = $this->getUserSettings();
+
+        if (null === $settings) {
+            return $this->flashOnRedirect(
+                'error',
+                'errors.controller.drive.no_settings',
+                self::SETTINGS_ROUTE
+            );
+        }
+
         $settings->setGoogleDriveAuthCode(null);
         $settings->setGoogleDriveToken(null);
 

diff --git a/src/Controller/RegistrationController.php b/src/Controller/RegistrationController.php
@@ -35,8 +35,7 @@ public function register(
         Request $request,
         UserPasswordHasherInterface $userPasswordHasher,
         EntityManagerInterface $entityManager
-    ): Response
-    {
+    ): Response {
         $user = new User();
         $form = $this->createForm(RegistrationFormType::class, $user);
         $form->handleRequest($request);

diff --git a/src/Controller/SettingsController.php b/src/Controller/SettingsController.php
@@ -20,7 +20,17 @@ class SettingsController extends BaseController
     #[Route('/settings', name: 'app_settings')]
     public function index(Request $request, EntityManagerInterface $entityManager): Response
     {
-        $form = $this->createForm(UserSettingsType::class, $this->getAppUserSettings($this->getAppUser()));
+        $settings = $this->getUserSettings();
+
+        if (null === $settings) {
+            return $this->flashOnRedirect(
+                'error',
+                'errors.controller.user.no_settings',
+                self::SETTINGS_ROUTE
+            );
+        }
+
+        $form = $this->createForm(UserSettingsType::class, $settings);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {

diff --git a/src/Helper/GoogleClientParametersHelper.php b/src/Helper/GoogleClientParametersHelper.php
@@ -0,0 +1,68 @@
+<?php
+
+/**
+ * @author ThomasLdev
+ */
+
+declare(strict_types=1);
+
+namespace App\Helper;
+
+use Google\Client;
+use Google\Service\Drive;
+use Psr\Container\ContainerExceptionInterface;
+use Psr\Container\NotFoundExceptionInterface;
+use RuntimeException;
+use Symfony\Component\DependencyInjection\ParameterBag\ContainerBagInterface;
+
+class GoogleClientParametersHelper
+{
+    public const GOOGLE_API_KEY = 'google_api_key';
+    public const GOOGLE_CLIENT_ID = 'google_client_id';
+    public const GOOGLE_CLIENT_SECRET = 'google_client_secret';
+    public const GOOGLE_APP_NAME = 'Instabot';
+
+    public function __construct(
+        private readonly ContainerBagInterface $containerBag
+    ) {
+    }
+
+    /**
+     * @throws ContainerExceptionInterface
+     * @throws NotFoundExceptionInterface
+     */
+    public function setExtraParameters(Client $client): void
+    {
+        $params = $this->getRequiredParameters();
+
+        // for some reason unable to force https via RouterInterface.
+        $client->setRedirectUri('https://' . $_SERVER['HTTP_HOST'] . '/google/authorize-response');
+        $client->setApplicationName(self::GOOGLE_APP_NAME);
+        $client->setDeveloperKey($params[self::GOOGLE_API_KEY]);
+        $client->setClientId($params[self::GOOGLE_CLIENT_ID]);
+        $client->setClientSecret($params[self::GOOGLE_CLIENT_SECRET]);
+        $client->setAccessType('offline');
+        $client->addScope(Drive::DRIVE);
+    }
+
+    /**
+     * @throws NotFoundExceptionInterface
+     * @throws ContainerExceptionInterface
+     */
+    private function getRequiredParameters(): array
+    {
+        $envParams = [
+            self::GOOGLE_API_KEY => $this->containerBag->get(self::GOOGLE_API_KEY),
+            self::GOOGLE_CLIENT_ID => $this->containerBag->get(self::GOOGLE_CLIENT_ID),
+            self::GOOGLE_CLIENT_SECRET => $this->containerBag->get(self::GOOGLE_CLIENT_SECRET),
+        ];
+
+        foreach ($envParams as $param) {
+            if (false === is_string($param)) {
+                throw new RuntimeException('Google API parameters must be strings.');
+            }
+        }
+
+        return $envParams;
+    }
+}
diff --git a/src/Service/Google/Drive/GoogleDriveClientService.php b/src/Service/Google/Drive/GoogleDriveClientService.php
@@ -17,10 +17,9 @@
 class GoogleDriveClientService
 {
     public function __construct(
-        private readonly GoogleClientService        $clientService,
+        private readonly GoogleClientService $clientService,
         private readonly GoogleDriveResponseService $responseService
-    )
-    {
+    ) {
     }
 
     /**
@@ -81,7 +80,7 @@ private function getFiles(UserSettings $userSettings, string $folderId): array
     private function getQueryParameters(string $folderId): array
     {
         return [
-            'q' => "'".$folderId."' in parents and trashed = false",
+            'q' => "'" . $folderId . "' in parents and trashed = false",
         ];
     }
 }
diff --git a/src/Service/Google/GoogleClientService.php b/src/Service/Google/GoogleClientService.php
@@ -9,12 +9,13 @@
 namespace App\Service\Google;
 
 use App\Entity\UserSettings;
+use App\Helper\GoogleClientParametersHelper;
 use App\Service\Security\EncryptionService;
 use Exception;
 use Google\Client;
-use Google\Service\Drive;
-use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
-use Symfony\Component\Routing\Router;
+use Psr\Container\ContainerExceptionInterface;
+use Psr\Container\NotFoundExceptionInterface;
+use RuntimeException;
 
 /**
  * Initiate the connexion with Google SDK client.
@@ -23,7 +24,7 @@ class GoogleClientService
 {
     public function __construct(
         private readonly EncryptionService $encryptionService,
-        private readonly Router            $router,
+        private readonly GoogleClientParametersHelper $parametersHelper,
     ) {
     }
 
@@ -35,28 +36,26 @@ public function getClientForUser(UserSettings $userSettings): Client
         $client = new Client();
         $accessToken = $userSettings->getGoogleDriveToken();
 
-        $this->setClientExtraData($client);
+        try {
+            $this->parametersHelper->setExtraParameters($client);
+        } catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) {
+            throw new RuntimeException('Google API parameters not found.');
+        }
 
         // first time authorization won't have token.
-        if (null === $accessToken | '' === $accessToken) {
+        if (false === is_string($accessToken) | '' === $accessToken) {
             return $client;
         }
 
-        $client->setAccessToken($this->encryptionService->decrypt($accessToken));
+        /** @var string $accessToken */
+        $plainToken = $this->encryptionService->decrypt($accessToken);
 
-        return $client;
-    }
+        if (null === $plainToken) {
+            throw new RuntimeException('Token decryption failed.');
+        }
 
-    // the rest is set in google_apiclient.yaml.
-    private function setClientExtraData(Client $client): void
-    {
-        $client->setRedirectUri(
-            $this->router->generate(
-                'app_google_authorize_response', [],
-                UrlGeneratorInterface::ABSOLUTE_URL
-            )
-        );
-
-        $client->addScope(Drive::DRIVE);
+        $client->setAccessToken($plainToken);
+
+        return $client;
     }
 }
diff --git a/src/Service/Google/GoogleResponseInterface.php b/src/Service/Google/GoogleResponseInterface.php
@@ -12,5 +12,8 @@
 
 interface GoogleResponseInterface
 {
+    public const ERROR_KEY = 'error';
+    public const ACCESS_TOKEN_KEY = 'access_token';
+
     public function handleResponse(array $data): BaseGoogleResponse;
 }
diff --git a/src/Service/Google/OAuth/GoogleOAuthResponseService.php b/src/Service/Google/OAuth/GoogleOAuthResponseService.php
@@ -13,13 +13,10 @@
 
 class GoogleOAuthResponseService implements GoogleResponseInterface
 {
-    private const ERROR_KEY = 'error';
-    private const ACCESS_TOKEN_KEY = 'access_token';
-
     public function handleResponse(array $data): GoogleClientResponse
     {
         if (array_key_exists(self::ERROR_KEY, $data)) {
-            return $this->setResponse(false, $data['error']);
+            return $this->setResponse(false, $data[self::ERROR_KEY]);
         }
 
         if (false === array_key_exists(self::ACCESS_TOKEN_KEY, $data)) {