OXDEV-10037 Get provider dynamicaly and redirect to correct page

TitaKoleva · tkcreateit · commit 33ec3efb075b · 2026-02-11T12:36:42.000+01:00
diff --git a/src/Authentication/OAuth2/Controller/OAuthController.php b/src/Authentication/OAuth2/Controller/OAuthController.php
@@ -11,24 +11,28 @@
 use OxidEsales\Eshop\Core\Registry;
 use OxidEsales\SecurityModule\Authentication\OAuth2\Service\ProviderCollectorInterface;
 use OxidEsales\SecurityModule\Authentication\OAuth2\Service\UserServiceInterface;
+use OxidEsales\SecurityModule\Authentication\Session\SessionKeys;
 
 class OAuthController extends FrontendController
 {
     public function login(): void
     {
+        $providerName = $_GET['provider'] ?? '';
+
         $providerCollector = $this->getService(ProviderCollectorInterface::class);
 
-        $provider = $providerCollector->getProvider($_GET['provider']);
+        $provider = $providerCollector->getProvider($providerName);
 
         Registry::getUtils()->redirect($provider->getAuthorizationUrl());
     }
 
     public function redirect(): void
     {
-        //todo: get provider dynamically
+        $providerName = $_GET['provider'] ?? '';
+
         $provider = $this
             ->getService(ProviderCollectorInterface::class)
-            ->getProvider('google');
+            ->getProvider($providerName);
 
         $accessToken = $provider->getAccessToken($_GET['code']);
 
@@ -38,6 +42,11 @@ public function redirect(): void
             ->getService(UserServiceInterface::class)
             ->login($userDTO);
 
-        Registry::getUtils()->redirect('');
+        $redirectUrl = Registry::getSession()->getVariable(SessionKeys::AUTH_REDIRECT_URL);
+        if (!$redirectUrl) {
+            $redirectUrl = Registry::getConfig()->getShopHomeUrl();
+        }
+
+        Registry::getUtils()->redirect($redirectUrl, false);
     }
 }
diff --git a/src/Authentication/OAuth2/Infrastructure/Provider/Google/GoogleProviderFactory.php b/src/Authentication/OAuth2/Infrastructure/Provider/Google/GoogleProviderFactory.php
@@ -26,7 +26,7 @@ public function create(): GoogleProvider
     {
         return new GoogleProvider([
             'clientId'        => $this->moduleSettings->getGoogleClientId(),
-            'clientSecret'    => $this->moduleSettings->getFacebookClientSecret(),
+            'clientSecret'    => $this->moduleSettings->getGoogleClientSecret(),
             'redirectUri'     => $this->moduleSettings->getGoogleRedirectUrl(),
         ]);
     }
diff --git a/src/Authentication/OAuth2/Service/UserService.php b/src/Authentication/OAuth2/Service/UserService.php
@@ -9,6 +9,8 @@
 
 namespace OxidEsales\SecurityModule\Authentication\OAuth2\Service;
 
+use OxidEsales\Eshop\Application\Model\User;
+use OxidEsales\Eshop\Core\Registry;
 use OxidEsales\EshopCommunity\Internal\Framework\Session\SessionInterface;
 use OxidEsales\SecurityModule\Authentication\OAuth2\DTO\OAuth2UserDTOInterface;
 use OxidEsales\SecurityModule\Authentication\OAuth2\Exception\UserBlockedException;
diff --git a/src/Authentication/Session/SessionKeys.php b/src/Authentication/Session/SessionKeys.php
@@ -0,0 +1,15 @@
+<?php
+
+/**
+ * Copyright © OXID eSales AG. All rights reserved.
+ * See LICENSE file for license details.
+ */
+
+declare(strict_types=1);
+
+namespace OxidEsales\SecurityModule\Authentication\Session;
+
+final class SessionKeys
+{
+    public const AUTH_REDIRECT_URL = 'otp_target_url';
+}
diff --git a/src/Authentication/Subscriber/StoreCurrentUrlSubscriber.php b/src/Authentication/Subscriber/StoreCurrentUrlSubscriber.php
@@ -7,29 +7,33 @@
 
 declare(strict_types=1);
 
-namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Subscriber;
+namespace OxidEsales\SecurityModule\Authentication\Subscriber;
 
 use OxidEsales\Eshop\Core\Config;
 use OxidEsales\Eshop\Core\Request;
 use OxidEsales\EshopCommunity\Internal\Framework\Session\SessionInterface;
 use OxidEsales\EshopCommunity\Internal\Transition\ShopEvents\ViewRenderedEvent;
-use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\AuthorizeService;
+use OxidEsales\SecurityModule\Authentication\Session\SessionKeys;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 readonly class StoreCurrentUrlSubscriber implements EventSubscriberInterface
 {
-    public function __construct(private SessionInterface $session, private Config $config, private Request $request)
-    {
-    }
-
     private const EXCLUDED_CONTROLLERS = [
         'twofactorauth',
+        'oauth',
     ];
 
     private const EXCLUDED_FUNCTIONS = [
         'logout',
     ];
 
+    public function __construct(
+        private SessionInterface $session,
+        private Config $config,
+        private Request $request
+    ) {
+    }
+
     public static function getSubscribedEvents(): array
     {
         return [
@@ -49,7 +53,6 @@ public function onViewRendered(ViewRenderedEvent $event): void
         $currentController = $this->getCurrentController();
         $currentFunction = $this->getCurrentFunction();
 
-        // Skip widgets, excluded controllers, and excluded functions (like logout)
         if (
             $this->isWidget($currentController)
             || $this->shouldExcludeController($currentController)
@@ -60,7 +63,7 @@ public function onViewRendered(ViewRenderedEvent $event): void
 
         $currentUrl = $this->getCurrentPageUrl();
         if ($currentUrl) {
-            $this->session->set(AuthorizeService::OTP_TARGET_URL, $currentUrl);
+            $this->session->set(SessionKeys::AUTH_REDIRECT_URL, $currentUrl);
         }
     }
 
diff --git a/src/Authentication/Subscriber/services.yaml b/src/Authentication/Subscriber/services.yaml
@@ -6,6 +6,6 @@ services:
       OxidEsales\Eshop\Core\Config: '@=service("OxidEsales\\SecurityModule\\Core\\Registry").getConfig()'
       OxidEsales\Eshop\Core\Request: '@=service("OxidEsales\\SecurityModule\\Core\\Registry").getRequest()'
 
-  OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Subscriber\StoreCurrentUrlSubscriber:
+  OxidEsales\SecurityModule\Authentication\Subscriber\StoreCurrentUrlSubscriber:
     tags:
       - { name: kernel.event_subscriber }
diff --git a/src/Authentication/TwoFactorAuth/Service/AuthorizeService.php b/src/Authentication/TwoFactorAuth/Service/AuthorizeService.php
@@ -16,8 +16,6 @@ class AuthorizeService implements AuthorizeServiceInterface
 {
     public const USER_SESSION_KEY = 'pending_authorized_user';
 
-    public const OTP_TARGET_URL = 'otp_target_url';
-
     public function __construct(
         private ModuleSettingsServiceInterface $moduleSettings,
         private VerificationCollectorServiceInterface $verifyCollector,
diff --git a/src/Authentication/TwoFactorAuth/Service/UserService.php b/src/Authentication/TwoFactorAuth/Service/UserService.php
@@ -12,6 +12,7 @@
 use OxidEsales\Eshop\Core\Config;
 use OxidEsales\Eshop\Core\Utils;
 use OxidEsales\EshopCommunity\Internal\Framework\Session\SessionInterface;
+use OxidEsales\SecurityModule\Authentication\Session\SessionKeys;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Infrastructure\Factory\UserFactoryInterface;
 
 readonly class UserService implements UserServiceInterface
@@ -52,13 +53,13 @@ public function finalizeLogin(): void
     public function clearOTPSessionVariables(): void
     {
         $this->session->remove(AuthorizeService::USER_SESSION_KEY);
-        $this->session->remove(AuthorizeService::OTP_TARGET_URL);
+        $this->session->remove(SessionKeys::AUTH_REDIRECT_URL);
         $this->session->remove('OTP_PASS');
     }
 
     private function getRedirectUrl(): string
     {
-        $storedUrl = $this->session->get(AuthorizeService::OTP_TARGET_URL);
+        $storedUrl = $this->session->get(SessionKeys::AUTH_REDIRECT_URL);
 
         if ($storedUrl && $this->isInternalUrl($storedUrl)) {
             return $storedUrl;
diff --git a/src/Authentication/TwoFactorAuth/services.yaml b/src/Authentication/TwoFactorAuth/services.yaml
@@ -3,4 +3,3 @@ imports:
   - { resource: Service/services.yaml }
   - { resource: Transput/services.yaml }
   - { resource: Controller/services.yaml }
-  - { resource: Subscriber/services.yaml }
diff --git a/src/Authentication/services.yaml b/src/Authentication/services.yaml
@@ -1,3 +1,4 @@
 imports:
   - { resource: OAuth2/services.yaml }
   - { resource: TwoFactorAuth/services.yaml }
+  - { resource: Subscriber/services.yaml }
diff --git a/src/Shared/Model/User.php b/src/Shared/Model/User.php
@@ -85,7 +85,7 @@ protected function onLogin($userName, $password)
     {
         parent::onLogin($userName, $password);
 
-        if (!$this->isOTPEnabled()) {
+        if (!$this->isOTPEnabled() || $this->isAdmin()) {
             return;
         }
 
diff --git a/tests/Integration/Authentication/OAuth2/Infrastructure/Factory/OAuth2UserDTOFactoryTest.php b/tests/Integration/Authentication/OAuth2/Infrastructure/Factory/OAuth2UserDTOFactoryTest.php
@@ -10,6 +10,7 @@
 namespace OxidEsales\SecurityModule\Tests\Integration\Authentication\OAuth2\Infrastructure\Factory;
 
 use League\OAuth2\Client\Provider\FacebookUser;
+use League\OAuth2\Client\Provider\GoogleUser;
 use OxidEsales\SecurityModule\Authentication\OAuth2\Infrastructure\Factory\OAuth2UserDTOFactory;
 use PHPUnit\Framework\TestCase;
 
@@ -34,4 +35,24 @@ public function testCreateFromFacebookUser(): void
         $this->assertSame($expectedLastName, $result->getLastName());
         $this->assertSame($expectedEmail, $result->getEmail());
     }
+
+    public function testCreateFromGoogleUser(): void
+    {
+        $expectedFirstName = uniqid();
+        $expectedLastName = uniqid();
+        $expectedEmail = uniqid();
+
+        $googleUserStub = $this->createStub(GoogleUser::class);
+        $googleUserStub->method('getFirstName')->willReturn($expectedFirstName);
+        $googleUserStub->method('getLastName')->willReturn($expectedLastName);
+        $googleUserStub->method('getEmail')->willReturn($expectedEmail);
+
+        $sut = new OAuth2UserDTOFactory();
+
+        $result = $sut->createFromGoogleUser($googleUserStub);
+
+        $this->assertSame($expectedFirstName, $result->getFirstName());
+        $this->assertSame($expectedLastName, $result->getLastName());
+        $this->assertSame($expectedEmail, $result->getEmail());
+    }
 }
diff --git a/tests/Unit/Authentication/OAuth2/Service/ModuleSettingsServiceTest.php b/tests/Unit/Authentication/OAuth2/Service/ModuleSettingsServiceTest.php
@@ -75,4 +75,26 @@ private static function prepareStringTestItem(
             'expectedValue' => $exampleValue
         ];
     }
+
+    public static function saveBooleanDataProvider(): array
+    {
+        return [
+            ['saveFacebookEnabled', ModuleSettingsService::FACEBOOK_LOGIN_ENABLED, true],
+            ['saveFacebookEnabled', ModuleSettingsService::FACEBOOK_LOGIN_ENABLED, false],
+            ['saveGoogleEnabled', ModuleSettingsService::GOOGLE_LOGIN_ENABLED, true],
+            ['saveGoogleEnabled', ModuleSettingsService::GOOGLE_LOGIN_ENABLED, false],
+        ];
+    }
+
+    #[DataProvider('saveBooleanDataProvider')]
+    public function testSaveBooleanSettings(string $method, string $key, bool $value): void
+    {
+        $mssMock = $this->createPartialMock(ModuleSettingService::class, ['saveBoolean']);
+        $mssMock->expects($this->once())
+            ->method('saveBoolean')
+            ->with($key, $value, Module::MODULE_ID);
+
+        $sut = new ModuleSettingsService($mssMock);
+        $sut->$method($value);
+    }
 }
diff --git a/tests/Unit/Authentication/Subscriber/StoreCurrentUrlSubscriberTest.php b/tests/Unit/Authentication/Subscriber/StoreCurrentUrlSubscriberTest.php
@@ -7,16 +7,16 @@
 
 declare(strict_types=1);
 
-namespace OxidEsales\SecurityModule\Tests\Unit\Authentication\TwoFactorAuth\Subscriber;
+namespace OxidEsales\SecurityModule\Tests\Unit\Authentication\Subscriber;
 
 use Exception;
 use OxidEsales\Eshop\Core\Config;
 use OxidEsales\Eshop\Core\Request;
 use OxidEsales\EshopCommunity\Application\Controller\FrontendController;
 use OxidEsales\EshopCommunity\Internal\Framework\Session\SessionInterface;
 use OxidEsales\EshopCommunity\Internal\Transition\ShopEvents\ViewRenderedEvent;
-use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\AuthorizeService;
-use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Subscriber\StoreCurrentUrlSubscriber;
+use OxidEsales\SecurityModule\Authentication\Session\SessionKeys;
+use OxidEsales\SecurityModule\Authentication\Subscriber\StoreCurrentUrlSubscriber;
 use PHPUnit\Framework\TestCase;
 
 class StoreCurrentUrlSubscriberTest extends TestCase
@@ -47,7 +47,7 @@ public function testOnViewRenderedStoresCurrentUrl(): void
         $sessionMock = $this->createMock(SessionInterface::class);
         $sessionMock->expects($this->once())
             ->method('set')
-            ->with(AuthorizeService::OTP_TARGET_URL, $currentUrl);
+            ->with(SessionKeys::AUTH_REDIRECT_URL, $currentUrl);
 
         $sut = $this->getSut(
             session: $sessionMock,
@@ -122,6 +122,31 @@ public function testOnViewRenderedSkipsTwoFactorAuthController(): void
         $sut->onViewRendered($eventStub);
     }
 
+    public function testOnViewRenderedSkipsOAuthController(): void
+    {
+        $viewStub = $this->createStub(FrontendController::class);
+        $viewStub->method('getClassKey')->willReturn('oauth');
+
+        $configStub = $this->createStub(Config::class);
+        $configStub->method('isAdmin')->willReturn(false);
+        $configStub->method('getTopActiveView')->willReturn($viewStub);
+
+        $requestStub = $this->createStub(Request::class);
+        $requestStub->method('getRequestParameter')->with('fnc')->willReturn(null);
+
+        $sessionMock = $this->createMock(SessionInterface::class);
+        $sessionMock->expects($this->never())->method('set');
+
+        $sut = $this->getSut(
+            session: $sessionMock,
+            config: $configStub,
+            request: $requestStub,
+        );
+        $eventStub = $this->createStub(ViewRenderedEvent::class);
+
+        $sut->onViewRendered($eventStub);
+    }
+
     public function testOnViewRenderedSkipsWhenNoActiveView(): void
     {
         $configStub = $this->createStub(Config::class);
@@ -234,7 +259,7 @@ public function testOnViewRenderedStoresUrlWithOtherFunction(): void
         $sessionMock = $this->createMock(SessionInterface::class);
         $sessionMock->expects($this->once())
             ->method('set')
-            ->with(AuthorizeService::OTP_TARGET_URL, $currentUrl);
+            ->with(SessionKeys::AUTH_REDIRECT_URL, $currentUrl);
 
         $sut = $this->getSut(
             session: $sessionMock,
diff --git a/tests/Unit/Authentication/TwoFactorAuth/Service/ModuleSettingsServiceTest.php b/tests/Unit/Authentication/TwoFactorAuth/Service/ModuleSettingsServiceTest.php
diff --git a/tests/Unit/Authentication/TwoFactorAuth/Service/UserServiceTest.php b/tests/Unit/Authentication/TwoFactorAuth/Service/UserServiceTest.php
diff --git a/views/admin_twig/de/oesecuritymodule_lang.php b/views/admin_twig/de/oesecuritymodule_lang.php
diff --git a/views/admin_twig/en/oesecuritymodule_lang.php b/views/admin_twig/en/oesecuritymodule_lang.php

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ public function create(): GoogleProvider`
`26`	`26`	`{`
`27`	`27`	`return new GoogleProvider([`
`28`	`28`	`'clientId' => $this->moduleSettings->getGoogleClientId(),`
`29`		`- 'clientSecret' => $this->moduleSettings->getFacebookClientSecret(),`
	`29`	`+ 'clientSecret' => $this->moduleSettings->getGoogleClientSecret(),`
`30`	`30`	`'redirectUri' => $this->moduleSettings->getGoogleRedirectUrl(),`
`31`	`31`	`]);`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,8 +16,6 @@ class AuthorizeService implements AuthorizeServiceInterface`
`16`	`16`	`{`
`17`	`17`	`public const USER_SESSION_KEY = 'pending_authorized_user';`
`18`	`18`
`19`		`- public const OTP_TARGET_URL = 'otp_target_url';`
`20`		`-`
`21`	`19`	`public function __construct(`
`22`	`20`	`private ModuleSettingsServiceInterface $moduleSettings,`
`23`	`21`	`private VerificationCollectorServiceInterface $verifyCollector,`