feat(OpenId): Added force_ssl_on_redirect_uri open_id configuration…

… to allow `http` redirectUri
roadiz · May 14, 2024 · 352bf79 · 352bf79
1 parent 9023668
commit 352bf79
Show file tree

Hide file tree

Showing 12 changed files with 51 additions and 90 deletions.
diff --git a/config/packages/roadiz_rozier.yaml b/config/packages/roadiz_rozier.yaml
@@ -18,6 +18,7 @@ roadiz_rozier:
         # OpenID identity provider OAuth2 client secret
         oauth_client_secret: '%env(string:OPEN_ID_CLIENT_SECRET)%'
         requires_local_user: true
+        force_ssl_on_redirect_uri: true
         # Only when local users are not required, creating virtual users
         # with following roles.
         granted_roles:

diff --git a/lib/OpenId/src/Authentication/OpenIdAuthenticator.php b/lib/OpenId/src/Authentication/OpenIdAuthenticator.php
@@ -35,56 +35,28 @@ final class OpenIdAuthenticator extends AbstractAuthenticator
 {
     use TargetPathTrait;
 
-    private HttpUtils $httpUtils;
-    private ?Discovery $discovery;
     private Client $client;
-    private JwtRoleStrategy $roleStrategy;
-    private OpenIdJwtConfigurationFactory $jwtConfigurationFactory;
-    private UrlGeneratorInterface $urlGenerator;
-    private string $returnPath;
-    private string $defaultRoute;
-    private ?string $oauthClientId;
-    private ?string $oauthClientSecret;
-    private string $usernameClaim;
-    private string $targetPathParameter;
-    private array $defaultRoles;
-    private bool $forceSsl;
-    private bool $requiresLocalUsers;
 
     public function __construct(
-        HttpUtils $httpUtils,
-        ?Discovery $discovery,
-        JwtRoleStrategy $roleStrategy,
-        OpenIdJwtConfigurationFactory $jwtConfigurationFactory,
-        UrlGeneratorInterface $urlGenerator,
-        string $returnPath,
-        string $defaultRoute,
-        ?string $oauthClientId,
-        ?string $oauthClientSecret,
-        bool $requiresLocalUsers = true,
-        string $usernameClaim = 'email',
-        string $targetPathParameter = '_target_path',
-        array $defaultRoles = [],
-        bool $forceSsl = true
+        private readonly HttpUtils $httpUtils,
+        private readonly ?Discovery $discovery,
+        private readonly JwtRoleStrategy $roleStrategy,
+        private readonly OpenIdJwtConfigurationFactory $jwtConfigurationFactory,
+        private readonly UrlGeneratorInterface $urlGenerator,
+        private readonly string $returnPath,
+        private readonly string $defaultRoute,
+        private readonly ?string $oauthClientId,
+        private readonly ?string $oauthClientSecret,
+        private readonly bool $forceSslOnRedirectUri,
+        private readonly bool $requiresLocalUsers,
+        private readonly string $usernameClaim = 'email',
+        private readonly string $targetPathParameter = '_target_path',
+        private readonly array $defaultRoles = []
     ) {
-        $this->httpUtils = $httpUtils;
-        $this->discovery = $discovery;
         $this->client = new Client([
             // You can set any number of default request options.
             'timeout'  => 2.0,
         ]);
-        $this->roleStrategy = $roleStrategy;
-        $this->returnPath = $returnPath;
-        $this->oauthClientId = $oauthClientId;
-        $this->oauthClientSecret = $oauthClientSecret;
-        $this->usernameClaim = $usernameClaim;
-        $this->targetPathParameter = $targetPathParameter;
-        $this->defaultRoles = $defaultRoles;
-        $this->defaultRoute = $defaultRoute;
-        $this->urlGenerator = $urlGenerator;
-        $this->jwtConfigurationFactory = $jwtConfigurationFactory;
-        $this->forceSsl = $forceSsl;
-        $this->requiresLocalUsers = $requiresLocalUsers;
     }
 
     /**
@@ -142,7 +114,7 @@ public function authenticate(Request $request): Passport
             /*
              * Redirect URI should always use SSL
              */
-            if ($this->forceSsl && str_starts_with($redirectUri, 'http://')) {
+            if ($this->forceSslOnRedirectUri && str_starts_with($redirectUri, 'http://')) {
                 $redirectUri = str_replace('http://', 'https://', $redirectUri);
             }
 

diff --git a/lib/OpenId/src/Authentication/Provider/SettingsRoleStrategy.php b/lib/OpenId/src/Authentication/Provider/SettingsRoleStrategy.php
@@ -10,14 +10,9 @@ class SettingsRoleStrategy implements JwtRoleStrategy
 {
     public const SETTING_NAME = 'openid_default_roles';
 
-    protected ParameterBag $settingsBag;
-
-    /**
-     * @param ParameterBag $settingsBag
-     */
-    public function __construct(ParameterBag $settingsBag)
-    {
-        $this->settingsBag = $settingsBag;
+    public function __construct(
+        protected readonly ParameterBag $settingsBag
+    ) {
     }
 
     public function supports(): bool

diff --git a/lib/OpenId/src/Discovery.php b/lib/OpenId/src/Discovery.php
@@ -20,20 +20,17 @@
 class Discovery extends LazyParameterBag
 {
     public const CACHE_KEY = 'rz_openid_discovery_parameters';
-
-    protected string $discoveryUri;
-    protected CacheItemPoolInterface $cacheAdapter;
     protected ?array $jwksData = null;
 
     /**
      * @param string  $discoveryUri
      * @param CacheItemPoolInterface $cacheAdapter
      */
-    public function __construct(string $discoveryUri, CacheItemPoolInterface $cacheAdapter)
-    {
+    public function __construct(
+        protected readonly string $discoveryUri,
+        protected readonly CacheItemPoolInterface $cacheAdapter
+    ) {
         parent::__construct();
-        $this->discoveryUri = $discoveryUri;
-        $this->cacheAdapter = $cacheAdapter;
     }
 
     public function isValid(): bool

diff --git a/lib/OpenId/src/OAuth2LinkGenerator.php b/lib/OpenId/src/OAuth2LinkGenerator.php
@@ -12,24 +12,16 @@
 class OAuth2LinkGenerator
 {
     public const OAUTH_STATE_TOKEN = 'openid_state';
-
-    protected ?Discovery $discovery;
-    protected CsrfTokenManagerInterface $csrfTokenManager;
-    private ?string $openIdHostedDomain;
-    private ?string $oauthClientId;
     private array $openIdScopes;
 
     public function __construct(
-        ?Discovery $discovery,
-        CsrfTokenManagerInterface $csrfTokenManager,
-        ?string $openIdHostedDomain,
-        ?string $oauthClientId,
-        ?array $openIdScopes
+        protected readonly ?Discovery $discovery,
+        protected readonly CsrfTokenManagerInterface $csrfTokenManager,
+        protected readonly ?string $openIdHostedDomain,
+        protected readonly ?string $oauthClientId,
+        ?array $openIdScopes,
+        protected readonly bool $forceSslOnRedirectUri
     ) {
-        $this->discovery = $discovery;
-        $this->csrfTokenManager = $csrfTokenManager;
-        $this->openIdHostedDomain = $openIdHostedDomain;
-        $this->oauthClientId = $oauthClientId;
         $this->openIdScopes = array_filter($openIdScopes ?? []);
     }
 
@@ -47,8 +39,7 @@ public function generate(
         Request $request,
         string $redirectUri,
         array $state = [],
-        string $responseType = 'code',
-        bool $forceSsl = true
+        string $responseType = 'code'
     ): string {
         if (null === $this->discovery) {
             throw new DiscoveryNotAvailableException(
@@ -66,7 +57,7 @@ public function generate(
         /*
          * Redirect URI should always use SSL
          */
-        if ($forceSsl && str_starts_with($redirectUri, 'http://')) {
+        if ($this->forceSslOnRedirectUri && str_starts_with($redirectUri, 'http://')) {
             $redirectUri = str_replace('http://', 'https://', $redirectUri);
         }
 
@@ -83,7 +74,7 @@ public function generate(
         }
         $stateToken = $this->csrfTokenManager->getToken(static::OAUTH_STATE_TOKEN);
         return $this->discovery->get('authorization_endpoint') . '?' . http_build_query([
-            'response_type' => 'code',
+            'response_type' => $responseType,
             'hd' => $this->openIdHostedDomain,
             'state' => http_build_query(array_merge($state, [
                 'token' => $stateToken->getValue()

diff --git a/lib/OpenId/src/OpenIdJwtConfigurationFactory.php b/lib/OpenId/src/OpenIdJwtConfigurationFactory.php
@@ -18,21 +18,12 @@
 
 final class OpenIdJwtConfigurationFactory implements JwtConfigurationFactory
 {
-    private ?Discovery $discovery;
-    private bool $verifyUserInfo;
-    private ?string $openIdHostedDomain;
-    private ?string $oauthClientId;
-
     public function __construct(
-        ?Discovery $discovery,
-        ?string $openIdHostedDomain,
-        ?string $oauthClientId,
-        bool $verifyUserInfo = false
+        private readonly ?Discovery $discovery,
+        private readonly ?string $openIdHostedDomain,
+        private readonly ?string $oauthClientId,
+        private readonly bool $verifyUserInfo
     ) {
-        $this->discovery = $discovery;
-        $this->verifyUserInfo = $verifyUserInfo;
-        $this->openIdHostedDomain = $openIdHostedDomain;
-        $this->oauthClientId = $oauthClientId;
     }
 
     /**

diff --git a/lib/OpenId/src/User/OpenIdAccount.php b/lib/OpenId/src/User/OpenIdAccount.php
@@ -114,7 +114,9 @@ public function __construct(
         $this->givenName = $this->getStringClaim($claims, 'given_name');
         $this->familyName = $this->getStringClaim($claims, 'family_name');
         $this->middleName = $this->getStringClaim($claims, 'middle_name');
-        $this->nickname = $this->getStringClaim($claims, 'nickname');
+        $this->nickname = $this->getStringClaim($claims, 'nickname') ??
+            $this->getStringClaim($claims, 'preferred_username') ??
+            null;
         $this->profile = $this->getStringClaim($claims, 'profile');
         $this->picture = $this->getStringClaim($claims, 'picture');
         $this->locale = $this->getStringClaim($claims, 'locale');

diff --git a/lib/RoadizRozierBundle/README.md b/lib/RoadizRozierBundle/README.md
@@ -106,6 +106,8 @@ roadiz_rozier:
     open_id:
         # Verify User info in JWT at each login
         verify_user_info: false
+        # Force generating redirect uri with https scheme. (required by some OpenID providers)
+        force_ssl_on_redirect_uri: true
         # Standard OpenID autodiscovery URL, required to enable OpenId login in Roadiz CMS.
         discovery_url: '%env(string:OPEN_ID_DISCOVERY_URL)%'
         # For public identity providers (such as Google), restrict users emails by their domain.

diff --git a/lib/RoadizRozierBundle/config/packages/roadiz_rozier.yaml b/lib/RoadizRozierBundle/config/packages/roadiz_rozier.yaml
@@ -18,6 +18,7 @@ roadiz_rozier:
         # OpenID identity provider OAuth2 client secret
         oauth_client_secret: '%env(string:OPEN_ID_CLIENT_SECRET)%'
         requires_local_user: false
+        force_ssl_on_redirect_uri: true
         # Only when local users are not required, creating virtual users
         # with following roles.
         granted_roles:

diff --git a/lib/RoadizRozierBundle/config/services.yaml b/lib/RoadizRozierBundle/config/services.yaml
@@ -12,6 +12,7 @@ services:
             $googleServerId: '%roadiz_core.medias.google_server_id%'
             $soundcloudClientId: '%roadiz_core.medias.soundcloud_client_id%'
             $allowNodeTypeEdition: '%kernel.debug%'
+            $forceSslOnRedirectUri: '%roadiz_rozier.open_id.force_ssl_on_redirect_uri%'
 
     RZ\Roadiz\RozierBundle\:
         resource: '../src/'

diff --git a/lib/RoadizRozierBundle/src/DependencyInjection/Configuration.php b/lib/RoadizRozierBundle/src/DependencyInjection/Configuration.php
@@ -76,6 +76,13 @@ protected function addOpenIdNode()
                     ->defaultTrue()
                     ->info(<<<EOD
 Verify User info in JWT at each login
+EOD
+                    )
+                ->end()
+                ->booleanNode('force_ssl_on_redirect_uri')
+                    ->defaultTrue()
+                    ->info(<<<EOD
+Force generating redirect uri with https scheme. (required by some OpenID providers)
 EOD
                     )
                 ->end()

diff --git a/lib/RoadizRozierBundle/src/DependencyInjection/RoadizRozierExtension.php b/lib/RoadizRozierBundle/src/DependencyInjection/RoadizRozierExtension.php
@@ -44,6 +44,7 @@ public function load(array $configs, ContainerBuilder $container): void
     private function registerOpenId(array $config, ContainerBuilder $container): void
     {
         $container->setParameter('roadiz_rozier.open_id.verify_user_info', $config['open_id']['verify_user_info']);
+        $container->setParameter('roadiz_rozier.open_id.force_ssl_on_redirect_uri', $config['open_id']['force_ssl_on_redirect_uri']);
         $container->setParameter('roadiz_rozier.open_id.discovery_url', $config['open_id']['discovery_url']);
         $container->setParameter('roadiz_rozier.open_id.hosted_domain', $config['open_id']['hosted_domain']);
         $container->setParameter('roadiz_rozier.open_id.oauth_client_id', $config['open_id']['oauth_client_id']);