Work on test updates

lib/Auth/Source/OAuth2.php

@@ -286,7 +286,7 @@ class OAuth2 extends Source
      */
     protected function retry(callable $function, ?int $retries = null, int $delay = 1)
     {
-        if ($retries == null) {
+        if ($retries === null) {
             $retries = $this->config->getOptionalInteger('retryOnError', 1);
         }
         $providerLabel = $this->getLabel();
@@ -297,7 +297,7 @@ class OAuth2 extends Source
             Logger::info('authoauth2: ' . $providerLabel . " Connection error. Retries left $retries. error: {$e->getMessage()}");
             if ($retries > 0) {
                 sleep($delay);
-                return $this->retry($function, $retries - 1, $retries);
+                return $this->retry($function, $retries - 1, $delay);
             } else {
                 Logger::info('authoauth2: ' . $providerLabel . ". Out of retries. Rethrowing error");
                 throw $e;
@@ -306,10 +306,10 @@ class OAuth2 extends Source
     }
 
     /**
-     * @param string $idToken
+     * @param ?string $idToken
      * @return string[] id token attributes
      */
-    protected function extraIdTokenAttributes(string $idToken): array
+    protected function extraIdTokenAttributes(?string $idToken): array
     {
         // We don't need to verify the signature on the id token since it was the token returned directly from
         // the OP over TLS
@@ -326,7 +326,7 @@ class OAuth2 extends Source
         return $data;
     }
 
-    protected function extraAndDecodeJwtPayload(string $jwt): ?string
+    protected function extraAndDecodeJwtPayload(?string $jwt): ?string
     {
         $parts = explode('.', $jwt);
         if ($parts === false || count($parts) < 3) {
@@ -365,4 +365,24 @@ class OAuth2 extends Source
     {
         return $this->config->getOptionalString('attributePrefix', '');
     }
+
+    /**
+     * Used to allow testing
+     * @return HTTP
+     */
+    public function getHttp(): HTTP
+    {
+        return $this->http;
+    }
+
+    /**
+     *  Used to allow testing
+     * @param HTTP $http
+     */
+    public function setHttp(HTTP $http): void
+    {
+        $this->http = $http;
+    }
+
+
 }

tests/config/config.php

@@ -3,4 +3,5 @@
 $config = [];
 // require a vanilla SSP config
 require "ssp2_0-config.php";
-$config['module.enable']['authoauth2'] = true;
\ No newline at end of file
+$config['module.enable']['authoauth2'] = true;
+$config['baseurlpath'] = '/';

tests/lib/Auth/Source/LinkedInV2AuthTest.php

@@ -14,7 +14,7 @@ class LinkedInV2AuthTest extends TestCase
 {
     public static function setUpBeforeClass(): void
     {
-        putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(dirname(dirname(__DIR__))) . '/config');
+        putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__, 3) . '/config');
          // When all tests are run at once, sometimes a Configuration is created prior to us
         // setting the one we want to use for the test.
         Configuration::clearInternalState();
@@ -33,7 +33,7 @@ class LinkedInV2AuthTest extends TestCase
         $this->assertEquals($expectedAttributes, $attributes);
     }
 
-    public function attributeConversionProvider()
+    public function attributeConversionProvider(): array
     {
         return [
             [["id" => "abc"], ["linkedin.id" => ["abc"]]],
@@ -56,7 +56,7 @@ class LinkedInV2AuthTest extends TestCase
         ];
     }
 
-    public function testNoEmailCallIfNotRequested()
+    public function testNoEmailCallIfNotRequested(): void
     {
         $linkedInAuth = new LinkedInV2Auth(['AuthId' => 'linked'], ['scopes' => ['r_liteprofile']]);
         $state = [];

tests/lib/Auth/Source/MicrosoftHybridAuthTest.php

@@ -2,7 +2,6 @@
 
 namespace Test\SimpleSAML\Auth\Source;
 
-use AspectMock\Test as test;
 use League\OAuth2\Client\Provider\AbstractProvider;
 use League\OAuth2\Client\Provider\GenericResourceOwner;
 use League\OAuth2\Client\Token\AccessToken;
@@ -16,21 +15,16 @@ class MicrosoftHybridAuthTest extends TestCase
 {
     public static function setUpBeforeClass(): void
     {
-        putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(dirname(dirname(__DIR__))) . '/config');
-    }
-
-    protected function tearDown(): void
-    {
-        test::clean(); // remove all registered test doubles
+        putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__, 3) . '/config');
     }
 
     /**
      * @dataProvider combineOidcAndGraphProfileProvider
-     * @param string $idToken The id_token response from the server
+     * @param ?string $idToken The id_token response from the server
      * @param array $expectedAttributes The expected attributes
      */
     public function testCombineOidcAndGraphProfile(
-        $idToken,
+        ?string $idToken,
         array $authenticatedRequestAttributes,
         array $expectedAttributes
     ) {

tests/lib/Auth/Source/OAuth2Test.php

@@ -2,17 +2,17 @@
 
 namespace Test\SimpleSAML\Auth\Source;
 
-use AspectMock\Test as test;
-use CirrusIdentity\SSP\Test\Capture\RedirectException;
-use CirrusIdentity\SSP\Test\MockHttp;
 use GuzzleHttp\Exception\ConnectException;
 use GuzzleHttp\HandlerStack;
 use League\OAuth2\Client\Provider\AbstractProvider;
 use League\OAuth2\Client\Provider\GenericResourceOwner;
 use League\OAuth2\Client\Token\AccessToken;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\RequestInterface;
+use SimpleSAML\Auth\State;
 use SimpleSAML\Module\authoauth2\Auth\Source\OAuth2;
+use SimpleSAML\Utils\HTTP;
 use Test\SimpleSAML\MockOAuth2Provider;
 
 /**
@@ -26,15 +26,10 @@ class OAuth2Test extends TestCase
 
     public static function setUpBeforeClass(): void
     {
-        putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(dirname(dirname(__DIR__))) . '/config');
+        putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__, 3) . '/config');
     }
 
-    protected function tearDown(): void
-    {
-        test::clean(); // remove all registered test doubles
-    }
-
-    protected function getInstance(array $config)
+    protected function getInstance(array $config): OAuth2
     {
         $info = ['AuthId' => static::AUTH_ID];
         return new OAuth2($info, $config);
@@ -99,7 +94,7 @@ class OAuth2Test extends TestCase
         );
     }
 
-    public function authenticateDataProvider()
+    public function authenticateDataProvider(): array
     {
         return [
             [
@@ -108,7 +103,7 @@ class OAuth2Test extends TestCase
                     'urlAccessToken' => 'https://example.com/token',
                     'urlResourceOwnerDetails' => 'https://example.com/userinfo'
                 ],
-                [\SimpleSAML\Auth\State::ID => 'stateId'],
+                [State::ID => 'stateId'],
                 // phpcs:ignore Generic.Files.LineLength.TooLong
                 'https://example.com/auth?state=authoauth2%7CstateId&response_type=code&approval_prompt=auto&redirect_uri=http%3A%2F%2Flocalhost%2Fmodule.php%2Fauthoauth2%2Flinkback.php'
             ]
@@ -124,21 +119,21 @@ class OAuth2Test extends TestCase
     public function testAuthenticatePerformsRedirect($config, $state, $expectedUrl)
     {
         // Override redirect behavior
-        MockHttp::throwOnRedirectTrustedURL();
+        $http = $this->createMock(HTTP::class);
+        $http->method('redirectTrustedURL')
+            ->with($expectedUrl)
+            ->willThrowException(
+                new \Exception('redirectTrustedURL')
+            );
 
         $authOAuth2 = $this->getInstance($config);
+        $authOAuth2->setHttp($http);
 
         try {
             $authOAuth2->authenticate($state);
             $this->fail("Redirect expected");
-        } catch (RedirectException $e) {
+        } catch (\Exception $e) {
             $this->assertEquals('redirectTrustedURL', $e->getMessage());
-            $this->assertEquals(
-                $expectedUrl,
-                $e->getUrl(),
-                "First argument should be the redirect url"
-            );
-            $this->assertEquals([], $e->getParams(), "query params are already added into url");
         }
 
         $this->assertEquals(static::AUTH_ID, $state[OAuth2::AUTHID], 'Ensure authsource name is presevered in state');
@@ -169,7 +164,7 @@ class OAuth2Test extends TestCase
     {
         // given: A mock Oauth2 provider
         $code = 'theCode';
-        $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
+        $state = [State::ID => 'stateId'];
 
         $mock = $this->getMockBuilder(AbstractProvider::class)
             ->disableOriginalConstructor()
@@ -223,7 +218,7 @@ class OAuth2Test extends TestCase
     {
         // given: A mock Oauth2 provider
         $code = 'theCode';
-        $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
+        $state = [State::ID => 'stateId'];
 
         $mock = $this->getMockBuilder(AbstractProvider::class)
             ->disableOriginalConstructor()
@@ -270,14 +265,14 @@ class OAuth2Test extends TestCase
     {
         // given: A mock Oauth2 provider
         $code = 'theCode';
-        $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
+        $state = [State::ID => 'stateId'];
 
-        /** @var $mock AbstractProvider|\PHPUnit_Framework_MockObject_MockObject */
+        /** @var $mock AbstractProvider|MockObject */
         $mock = $this->getMockBuilder(AbstractProvider::class)
             ->disableOriginalConstructor()
             ->getMock();
 
-        /** @var $mockRequest RequestInterface|\PHPUnit_Framework_MockObject_MockObject */
+        /** @var $mockRequest RequestInterface|MockObject */
         $mockRequest = $this->getMockBuilder(RequestInterface::class)
             ->getMock();
 
@@ -319,7 +314,7 @@ class OAuth2Test extends TestCase
     ) {
         // given: A mock Oauth2 provider
         $code = 'theCode';
-        $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
+        $state = [State::ID => 'stateId'];
 
         $mock = $this->getMockBuilder(AbstractProvider::class)
             ->disableOriginalConstructor()
@@ -376,14 +371,14 @@ class OAuth2Test extends TestCase
             'attributePrefix' => 'test.',
             'retryOnError' => 2,
         ];
-        $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
+        $state = [State::ID => 'stateId'];
 
-        /** @var $mock AbstractProvider|\PHPUnit_Framework_MockObject_MockObject */
+        /** @var $mock AbstractProvider|MockObject */
         $mock = $this->getMockBuilder(AbstractProvider::class)
             ->disableOriginalConstructor()
             ->getMock();
 
-        /** @var $mockRequest RequestInterface|\PHPUnit_Framework_MockObject_MockObject */
+        /** @var $mockRequest RequestInterface|MockObject */
         $mockRequest = $this->getMockBuilder(RequestInterface::class)
             ->getMock();
 

tests/lib/MockOAuth2Provider.php

@@ -4,9 +4,11 @@ namespace Test\SimpleSAML;
 
 use League\OAuth2\Client\Provider\GenericProvider;
 use League\OAuth2\Client\Provider\AbstractProvider;
+use League\OAuth2\Client\Token\AccessToken;
 use Psr\Http\Message\RequestInterface;
+use SimpleSAML\Utils\ClearableState;
 
-class MockOAuth2Provider extends GenericProvider implements \SimpleSAML\Utils\ClearableState
+class MockOAuth2Provider extends GenericProvider implements ClearableState
 {
     /**
      * @var AbstractProvider
@@ -32,7 +34,7 @@ class MockOAuth2Provider extends GenericProvider implements \SimpleSAML\Utils\Cl
         return self::$delegate->getAccessToken($grant, $options);
     }
 
-    public function getResourceOwner(\League\OAuth2\Client\Token\AccessToken $token)
+    public function getResourceOwner(AccessToken $token)
     {
         return self::$delegate->getResourceOwner($token);
     }
@@ -55,7 +57,7 @@ class MockOAuth2Provider extends GenericProvider implements \SimpleSAML\Utils\Cl
     /**
      * Clear any cached internal state.
      */
-    public static function clearInternalState()
+    public static function clearInternalState(): void
     {
         self::$delegate = null;
     }