[SDK-3633] Treat passing an empty string to SdkConfiguration as the d…

…efault undefined value type of NULL (#643) * [SDK-3633] Treat passing an empty string to SdkConfiguration as the default undefined value type of NULL * Adjustments to tests and domain validator to accommodate change * fix: Psalm/Stan linter warnings * tests: Update to use properly formatted domains * fix: Rector linter warnings * fix: Rector linter fix breaking unit tests
auth0 · Sep 22, 2022 · bcfdae7 · bcfdae7
1 parent 89b8d8f
commit bcfdae7
Show file tree

Hide file tree

Showing 12 changed files with 66 additions and 39 deletions.
diff --git a/rector.php b/rector.php
@@ -2,6 +2,7 @@
 
 declare(strict_types=1);
 
+use Rector\CodeQuality\Rector\Class_\CompleteDynamicPropertiesRector;
 use Rector\Config\RectorConfig;
 use Rector\Core\ValueObject\PhpVersion;
 use Rector\Php74\Rector\Property\TypedPropertyRector;
@@ -14,6 +15,13 @@
         SetList::CODE_QUALITY,
     ]);
 
+    $rectorConfig->skip([
+        CompleteDynamicPropertiesRector::class => [
+            // Breaks PEST
+            __DIR__ . '/tests/Utilities/MockApi.php'
+        ]
+    ]);
+
     $rectorConfig->rule(TypedPropertyRector::class);
 
     $rectorConfig->phpVersion(PhpVersion::PHP_74);

diff --git a/src/Configuration/SdkConfiguration.php b/src/Configuration/SdkConfiguration.php
@@ -473,18 +473,19 @@ private function onStateChange(
             throw \Auth0\SDK\Exception\ConfigurationException::validationFailed($propertyName);
         }
 
-        if ($propertyName === 'cookieSecret') {
-            if (is_string($propertyValue) && mb_strlen($propertyValue) !== 0) {
-                return $propertyValue;
-            }
+        if ($propertyName === 'domain' || $propertyName === 'customDomain') {
+            if (is_string($propertyValue)) {
+                $propertyValue = trim($propertyValue);
 
-            throw \Auth0\SDK\Exception\ConfigurationException::validationFailed($propertyName);
-        }
+                if (strlen($propertyValue) !== 0) {
+                    $host = preg_replace('#^[^:/.]*[:/]+#i', '', $propertyValue);
+                    $host = parse_url('https://' . $host, PHP_URL_HOST);
+                    $host = filter_var($host, FILTER_SANITIZE_URL, FILTER_NULL_ON_FAILURE);
 
-        if ($propertyName === 'domain' || $propertyName === 'customDomain') {
-            if (is_string($propertyValue) && mb_strlen($propertyValue) !== 0) {
-                $host = parse_url($propertyValue, PHP_URL_HOST);
-                return $host ?? $propertyValue;
+                    if (is_string($host) && strlen($host) !== 0 && filter_var($host, FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME) !== false) {
+                        return $host;
+                    }
+                }
             }
 
             throw \Auth0\SDK\Exception\ConfigurationException::validationFailed($propertyName);

diff --git a/src/Mixins/ConfigurableMixin.php b/src/Mixins/ConfigurableMixin.php
@@ -264,6 +264,10 @@ private function changeState(
             throw \Auth0\SDK\Exception\ConfigurationException::setIncompatible($propertyName, (string) $expectedType, $propertyType);
         }
 
+        if ($propertyType === 'string' && is_string($propertyValue) && trim($propertyValue) === '' && in_array('NULL', $allowedTypes, true)) {
+            $propertyValue = null;
+        }
+
         if (method_exists($this, 'onStateChange')) {
             $propertyValue = $this->onStateChange($propertyName, $propertyValue);
         }

diff --git a/tests/Pest.php b/tests/Pest.php
@@ -4,7 +4,7 @@
 
 define('AUTH0_TESTS_DIR', dirname(__FILE__));
 
-require_once join(DIRECTORY_SEPARATOR, [AUTH0_TESTS_DIR, '..', 'vendor', 'autoload.php']);
+require_once implode(DIRECTORY_SEPARATOR, [AUTH0_TESTS_DIR, '..', 'vendor', 'autoload.php']);
 
 // For unit tests, use a mock network client rather than sending real requests.
 uses()

diff --git a/tests/Unit/Auth0Test.php b/tests/Unit/Auth0Test.php
@@ -15,7 +15,7 @@
     $_COOKIE = [];
 
     $this->configuration = [
-        'domain' => '__test_domain__',
+        'domain' => 'domain.test',
         'clientId' => '__test_client_id__',
         'cookieSecret' => uniqid(),
         'clientSecret' => '__test_client_secret__',
@@ -110,7 +110,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('scope=openid%20profile%20email')
@@ -134,7 +134,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('scope=openid%20profile%20email')
@@ -161,7 +161,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('scope=' . $params['scope'])
@@ -178,7 +178,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('state=')
@@ -192,7 +192,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('code_challenge=')
@@ -208,7 +208,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('max_age=1000');
@@ -225,7 +225,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('max_age=1001');
@@ -238,7 +238,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('screen_hint=signup');
@@ -255,7 +255,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/authorize')
         ->query
             ->toContain('invitation=__test_invitation__')
@@ -280,7 +280,7 @@ public function defer(
 
     expect($url)
         ->scheme->toEqual('https')
-        ->host->toEqual('__test_domain__')
+        ->host->toEqual('domain.test')
         ->path->toEqual('/v2/logout')
         ->query
             ->toContain('returnTo=' . $returnUrl)
@@ -684,7 +684,7 @@ public function defer(
     expect($requestBody['client_secret'])->toEqual('__test_client_secret__');
     expect($requestBody['client_id'])->toEqual('__test_client_id__');
     expect($requestBody['refresh_token'])->toEqual('2.3.4');
-    expect($request->getUri()->__toString())->toEqual('https://__test_domain__/oauth/token');
+    expect($request->getUri()->__toString())->toEqual('https://domain.test/oauth/token');
 });
 
 test('getCredentials() returns null when a session is not available', function(): void {
@@ -1002,7 +1002,7 @@ public function defer(
     $_GET[$testParameterName] = $candidate->token;
 
     $auth0 = new \Auth0\SDK\Auth0(array_merge($this->configuration, [
-        'domain' => '__bad_domain__',
+        'domain' => 'domain.bad',
         'tokenJwksUri' => $candidate->jwks,
         'tokenCache' => $candidate->cached
     ]));

diff --git a/tests/Unit/Configuration/SdkConfigurationTest.php b/tests/Unit/Configuration/SdkConfigurationTest.php
@@ -74,6 +74,19 @@
         'clientId' => $clientId,
         'redirectUri' => $redirectUri,
     ]);
+})->throws(\Auth0\SDK\Exception\ConfigurationException::class, \Auth0\SDK\Exception\ConfigurationException::MSG_REQUIRES_DOMAIN);
+
+test('__construct() throws an exception if domain is an invalid uri', function(): void {
+    $cookieSecret = uniqid();
+    $clientId = uniqid();
+    $redirectUri = uniqid();
+
+    $sdk = new SdkConfiguration([
+        'domain' => '₠',
+        'cookieSecret' => $cookieSecret,
+        'clientId' => $clientId,
+        'redirectUri' => $redirectUri,
+    ]);
 })->throws(\Auth0\SDK\Exception\ConfigurationException::class, sprintf(\Auth0\SDK\Exception\ConfigurationException::MSG_VALIDATION_FAILED, 'domain'));
 
 test('__construct() throws an exception if cookieSecret is undefined', function(): void {
@@ -100,7 +113,7 @@
         'clientId' => $clientId,
         'redirectUri' => $redirectUri,
     ]);
-})->throws(\Auth0\SDK\Exception\ConfigurationException::class, sprintf(\Auth0\SDK\Exception\ConfigurationException::MSG_VALIDATION_FAILED, 'cookieSecret'));
+})->throws(\Auth0\SDK\Exception\ConfigurationException::class, \Auth0\SDK\Exception\ConfigurationException::MSG_REQUIRES_COOKIE_SECRET);
 
 test('__construct() throws an exception if an invalid token algorithm is specified', function(): void {
     $domain = uniqid();
@@ -311,12 +324,12 @@
 
 test('formatDomain() returns the custom domain when a custom domain is configured', function(): void
 {
-    $domain = uniqid();
-    $customDomain = uniqid();
+    $domain = uniqid() . '.test';
+    $customDomain = uniqid() . '.test';
 
     $sdk = new SdkConfiguration([
         'domain' => $domain,
-        'customDomain' => 'test://' . $customDomain,
+        'customDomain' => $customDomain,
         'cookieSecret' => uniqid(),
         'clientId' => uniqid(),
         'redirectUri' => uniqid(),

diff --git a/tests/Unit/Token/VerifierTest.php b/tests/Unit/Token/VerifierTest.php
@@ -26,7 +26,7 @@
 dataset('tokenHs256', static function () {
     $token = (new TokenGenerator())->withHs256([]);
     [$headers, $claims, $signature] = explode('.', $token);
-    $payload = join('.', [$headers, $claims]);
+    $payload = implode('.', [$headers, $claims]);
     $signature = TokenGenerator::decodePart($signature, false);
 
     yield [ $token, $payload, $signature, $headers ];
@@ -36,7 +36,7 @@
     $keyPair = TokenGenerator::generateRsaKeyPair();
     $token = (new TokenGenerator())->withRs256([], $keyPair['private'], ['kid' => '__test_kid__']);
     [$headers, $claims, $signature] = explode('.', $token);
-    $payload = join('.', [$headers, $claims]);
+    $payload = implode('.', [$headers, $claims]);
     $signature = TokenGenerator::decodePart($signature, false);
 
     // Mimic JWKS response format: strip opening and closing comment lines from public key, remove line breaks.

diff --git a/tests/Unit/TokenTest.php b/tests/Unit/TokenTest.php
@@ -142,7 +142,7 @@ function(): SdkConfiguration {
     expect($token->validate(null, null, ['__test_org__'], $claims['nonce'], 100))->toEqual($token);
 })->with(['mocked data' => [
     function(): SdkConfiguration {
-        $this->configuration->setDomain('__test_domain__');
+        $this->configuration->setDomain('domain.test');
         $this->configuration->setClientId('__test_client_id__');
         $this->configuration->setTokenAlgorithm('HS256');
         $this->configuration->setClientSecret('__test_client_secret__');
@@ -161,7 +161,7 @@ function(): SdkConfiguration {
     $token->validate(null, [ $claims['aud'] ]);
 })->with(['mocked data' => [
     function(): SdkConfiguration {
-        $this->configuration->setDomain('__test_domain__');
+        $this->configuration->setDomain('domain.test');
         $this->configuration->setClientId('__diff_client_id__');
         $this->configuration->setTokenAlgorithm('HS256');
         return $this->configuration;

diff --git a/tests/Unit/Utility/HttpClientTest.php b/tests/Unit/Utility/HttpClientTest.php
@@ -67,9 +67,9 @@
 
     for ($i=0; $i < 10; $i++) {
         $this->client->mockResponse(clone $this->httpResponse429);
-        $baseWait = intval(100 * pow(2, $i));
+        $baseWait = (int) (100 * pow(2, $i));
         $baseWaits[] = $baseWait;
-        $baseWaitSum = $baseWaitSum + $baseWait;
+        $baseWaitSum += $baseWait;
     }
 
     $response = $this->client->method('get')

diff --git a/tests/Unit/Utility/ToolkitTest.php b/tests/Unit/Utility/ToolkitTest.php
@@ -28,7 +28,7 @@
     $items = ['a', 'b', 'c'];
 
     Toolkit::each($items, function(&$item, $key) {
-        $item = $item . $key;
+        $item .= $key;
     });
 
     expect($items)->toEqual(['a0', 'b1', 'c2']);
@@ -38,7 +38,7 @@
     $items = ['a', 'b', 'c'];
 
     Toolkit::each($items, function(&$item, $key) {
-        $item = $item . $key;
+        $item .= $key;
         return false;
     });
 

diff --git a/tests/Utilities/MockApi.php b/tests/Utilities/MockApi.php
@@ -4,6 +4,7 @@
 
 namespace Auth0\Tests\Utilities;
 
+use Auth0\SDK\API\Authentication;
 use Auth0\SDK\Utility\HttpClient;
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
@@ -29,7 +30,7 @@ public function __construct(
         // Create an instance of the intended API.
         $this->setClient();
 
-        if ($responses !== null && ! count($responses)) {
+        if ($responses !== null && $responses === []) {
             $responses[] = HttpResponseGenerator::create();
         }
 

diff --git a/tests/Utilities/TokenGenerator.php b/tests/Utilities/TokenGenerator.php
@@ -37,7 +37,7 @@ protected static function getIdTokenClaims(
     ): array {
         $defaults = [
             'sub' => '__test_sub__',
-            'iss' => 'https://__test_domain__/',
+            'iss' => 'https://domain.test/',
             'aud' => '__test_client_id__',
             'nonce' => '__test_nonce__',
             'auth_time' => time() - 100,
@@ -157,7 +157,7 @@ public static function create(
         }
 
         [$headers, $claims, $signature] = explode('.', $token);
-        $payload = join('.', [$headers, $claims]);
+        $payload = implode('.', [$headers, $claims]);
         $signature = (string) TokenGenerator::decodePart($signature, false);
         $claims = (array) TokenGenerator::decodePart($claims, true);
         $headers = (array) TokenGenerator::decodePart($headers, true);