Skip to content
Snippets Groups Projects
Unverified Commit 3016234c authored by Danny's avatar Danny
Browse files

fix: remove debug log (+2 squashed commits)

Squashed commits:
[cebc113] fix: tests (+2 squashed commits)
Squashed commits:
[16f467c] fix: undo changes
[0f25a49] fix: error message (+1 squashed commit)
Squashed commits:
[389a40d] fix: whites spaces (+1 squashed commit)
Squashed commits:
[08a1836] Adding additional tests, and clean up code.
add logic that graph api call, has priority compared to token
[43a6af7] Add authenticated api requests support to query multiple endpoints
parent f0d65b02
No related branches found
No related tags found
No related merge requests found
...@@ -10,6 +10,7 @@ use GuzzleHttp\Middleware; ...@@ -10,6 +10,7 @@ use GuzzleHttp\Middleware;
use League\OAuth2\Client\Provider\AbstractProvider; use League\OAuth2\Client\Provider\AbstractProvider;
use League\OAuth2\Client\Provider\ResourceOwnerInterface; use League\OAuth2\Client\Provider\ResourceOwnerInterface;
use League\OAuth2\Client\Token\AccessToken; use League\OAuth2\Client\Token\AccessToken;
use League\OAuth2\Client\Token\AccessTokenInterface;
use SimpleSAML\Logger; use SimpleSAML\Logger;
use SimpleSAML\Module; use SimpleSAML\Module;
use SimpleSAML\Module\authoauth2\AttributeManipulator; use SimpleSAML\Module\authoauth2\AttributeManipulator;
...@@ -186,6 +187,9 @@ class OAuth2 extends \SimpleSAML\Auth\Source ...@@ -186,6 +187,9 @@ class OAuth2 extends \SimpleSAML\Auth\Source
$provider = $this->getProvider($this->config); $provider = $this->getProvider($this->config);
/**
* @var AccessTokenInterface $accessToken
*/
$accessToken = $this->retry( $accessToken = $this->retry(
function () use ($provider, $oauth2Code) { function () use ($provider, $oauth2Code) {
return $provider->getAccessToken('authorization_code', [ return $provider->getAccessToken('authorization_code', [
...@@ -213,6 +217,34 @@ class OAuth2 extends \SimpleSAML\Auth\Source ...@@ -213,6 +217,34 @@ class OAuth2 extends \SimpleSAML\Auth\Source
); );
$attributes = $resourceOwner->toArray(); $attributes = $resourceOwner->toArray();
$authenticatedApiRequests = $this->config->getArray('authenticatedApiRequests', []);
foreach ($authenticatedApiRequests as $apiUrl) {
try {
$apiAttributes = $this->retry(function () use ($provider, $accessToken, $apiUrl) {
/** @var Psr\Http\Message\RequestInterface $request */
$apiRequest = $provider->getAuthenticatedRequest(
'GET',
$apiUrl,
$accessToken
);
return $provider->getParsedResponse($apiRequest);
},
$this->config->getInteger('retryOnError', 1)
);
if (!empty($apiAttributes)) {
$attributes = array_replace_recursive($attributes, $apiAttributes);
}
} catch (\Exception $e) {
// not retrieving additional resources, should not fail the authentication
Logger::error(
'OAuth2: ' . $this->getLabel() . ' exception authenticatedApiRequests ' . $e->getMessage()
);
}
}
$prefix = $this->getAttributePrefix(); $prefix = $this->getAttributePrefix();
$state['Attributes'] = $this->convertResourceOwnerAttributes($attributes, $prefix); $state['Attributes'] = $this->convertResourceOwnerAttributes($attributes, $prefix);
$this->postFinalStep($accessToken, $provider, $state); $this->postFinalStep($accessToken, $provider, $state);
......
...@@ -6,6 +6,7 @@ use AspectMock\Test as test; ...@@ -6,6 +6,7 @@ use AspectMock\Test as test;
use League\OAuth2\Client\Provider\AbstractProvider; use League\OAuth2\Client\Provider\AbstractProvider;
use League\OAuth2\Client\Provider\GenericResourceOwner; use League\OAuth2\Client\Provider\GenericResourceOwner;
use League\OAuth2\Client\Token\AccessToken; use League\OAuth2\Client\Token\AccessToken;
use Psr\Http\Message\RequestInterface;
use SimpleSAML\Module\authoauth2\Auth\Source\MicrosoftHybridAuth; use SimpleSAML\Module\authoauth2\Auth\Source\MicrosoftHybridAuth;
use Test\SimpleSAML\MockOAuth2Provider; use Test\SimpleSAML\MockOAuth2Provider;
...@@ -27,7 +28,7 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase ...@@ -27,7 +28,7 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase
* @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 * @param array $expectedAttributes The expected attributes
*/ */
public function testCombineOidcAndGraphProfile($idToken, array $expectedAttributes) public function testCombineOidcAndGraphProfile($idToken, array $authenticatedRequestAttributes, array $expectedAttributes)
{ {
// given: A mock Oauth2 provider // given: A mock Oauth2 provider
$code = 'theCode'; $code = 'theCode';
...@@ -35,6 +36,7 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase ...@@ -35,6 +36,7 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase
$config = [ $config = [
'template' => 'MicrosoftGraphV1', 'template' => 'MicrosoftGraphV1',
'providerClass' => MockOAuth2Provider::class, 'providerClass' => MockOAuth2Provider::class,
'authenticatedApiRequests' => ['https://mock.com/v1.0/me/memberOf'],
]; ];
$state = [\SimpleSAML\Auth\State::ID => 'stateId']; $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
...@@ -58,6 +60,15 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase ...@@ -58,6 +60,15 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase
->with($token) ->with($token)
->willReturn($user); ->willReturn($user);
$mockRequest = $this->createMock(RequestInterface::class);
$mock->method('getAuthenticatedRequest')
->with('GET', 'https://mock.com/v1.0/me/memberOf', $token)
->willReturn($mockRequest);
$mock->method('getParsedResponse')
->with($mockRequest)
->willReturn($authenticatedRequestAttributes);
MockOAuth2Provider::setDelegate($mock); MockOAuth2Provider::setDelegate($mock);
// when: turning a code into a token and then into a resource owner attributes // when: turning a code into a token and then into a resource owner attributes
...@@ -68,26 +79,62 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase ...@@ -68,26 +79,62 @@ class MicrosoftHybridAuthTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($expectedAttributes, $state['Attributes']); $this->assertEquals($expectedAttributes, $state['Attributes']);
} }
public function combineOidcAndGraphProfileProvider() public function combineOidcAndGraphProfileProvider()
{ {
$expectedGraphAttributes = ['microsoft.id' => ['a76d6a7a097c1e9d']]; $expectedGraphAttributes = ['microsoft.id' => ['a76d6a7a097c1e9d'],
'microsoft.@odata.context' => ['https://graph.microsoft.com/v1.0/$metadata#directoryObjects'],
'microsoft.value.0.@odata.type' => ['#microsoft.graph.group'],
'microsoft.value.0.id' => ['11111111-1111-1111-1111-111111111111']];
// A Graph Id token. note: only the payload is valid. Header and signature are not // A Graph Id token. note: only the payload is valid. Header and signature are not
// phpcs:ignore Generic.Files.LineLength.TooLong // phpcs:ignore Generic.Files.LineLength.TooLong
$validIdToken = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiIsImtpZCI6IjFMVE16YWtpaGlSbGFfOHoyQkVKVlhlV01xbyJ9.eyJ2ZXIiOiIyLjAiLCJpc3MiOiJodHRwczovL2xvZ2luLm1pY3Jvc29mdG9ubGluZS5jb20vOTE4ODA0MGQtNmM2Ny00YzViLWIxMTItMzZhMzA0YjY2ZGFkL3YyLjAiLCJzdWIiOiJBQUFBQUFBQUFBQUFBQUFBQUFBQUFORHBFcUNHa3lPVVFDTXpHOHRGYUUiLCJhdWQiOiI5ZTdkZTIyZS0zYTE3LTQ0ZmQtODdjNy1jNmVjZWIxYmVlMGUiLCJleHAiOjE1Mzk5NjUwNDUsImlhdCI6MTUzOTg3ODM0NSwibmJmIjoxNTM5ODc4MzQ1LCJuYW1lIjoiU3RldmUgU3RyYXR1cyIsInByZWZlcnJlZF91c2VybmFtZSI6InN0ZXZlLnN0cmF0dXNAb3V0bG9vay5jb20iLCJvaWQiOiIwMDAwMDAwMC0wMDAwLTAwMDAtYTc2ZDZhN2EwOTdjMWU5ZCIsImVtYWlsIjoic3RldmUuc3RyYXR1c0BvdXRsb29rLmNvbSIsInRpZCI6IjkxODgwNDBkLTZjNjctNGM1Yi1iMTEyMzZhMzA0YjY2ZGFkIiwiYWlvIjoiRGI1YmRMSHBaSkdla0h3czlxaHlkUkFHSGR1cSFvUDdpS1cxYzFFQkd2dWhDWnZXS2luS0FoVnFZV3NtYSEwT3ZiRTFmV1J2TUF3NHFLUVBud3N6akQwKkd2N1RsbFpOY2FxcDQ0eTM0ZyJ9.SjNeBS11Qa2eXKLhxSApShFMLQ9nDjTXT27JZm3cctM'; $validIdToken = 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiIsImtpZCI6IjFMVE16YWtpaGlSbGFfOHoyQkVKVlhlV01xbyJ9.eyJ2ZXIiOiIyLjAiLCJpc3MiOiJodHRwczovL2xvZ2luLm1pY3Jvc29mdG9ubGluZS5jb20vOTE4ODA0MGQtNmM2Ny00YzViLWIxMTItMzZhMzA0YjY2ZGFkL3YyLjAiLCJzdWIiOiJBQUFBQUFBQUFBQUFBQUFBQUFBQUFORHBFcUNHa3lPVVFDTXpHOHRGYUUiLCJhdWQiOiI5ZTdkZTIyZS0zYTE3LTQ0ZmQtODdjNy1jNmVjZWIxYmVlMGUiLCJleHAiOjE1Mzk5NjUwNDUsImlhdCI6MTUzOTg3ODM0NSwibmJmIjoxNTM5ODc4MzQ1LCJuYW1lIjoiU3RldmUgU3RyYXR1cyIsInByZWZlcnJlZF91c2VybmFtZSI6InN0ZXZlLnN0cmF0dXNAb3V0bG9vay5jb20iLCJvaWQiOiIwMDAwMDAwMC0wMDAwLTAwMDAtYTc2ZDZhN2EwOTdjMWU5ZCIsImVtYWlsIjoic3RldmUuc3RyYXR1c0BvdXRsb29rLmNvbSIsInRpZCI6IjkxODgwNDBkLTZjNjctNGM1Yi1iMTEyMzZhMzA0YjY2ZGFkIiwiYWlvIjoiRGI1YmRMSHBaSkdla0h3czlxaHlkUkFHSGR1cSFvUDdpS1cxYzFFQkd2dWhDWnZXS2luS0FoVnFZV3NtYSEwT3ZiRTFmV1J2TUF3NHFLUVBud3N6akQwKkd2N1RsbFpOY2FxcDQ0eTM0ZyJ9.SjNeBS11Qa2eXKLhxSApShFMLQ9nDjTXT27JZm3cctM';
$authenticatedRequestAttributes = [
'@odata.context' => 'https://graph.microsoft.com/v1.0/$metadata#directoryObjects',
'value' => [
0 => [
'@odata.type' => '#microsoft.graph.group',
'id' => '11111111-1111-1111-1111-111111111111',
],
],
];
$conflictedRequestAttributes = [
'id' => ['11111111'],
'name' => ['Steve Stratus'],
'mail' => ['steve.stratus@outlook.com'],
'@odata.context' => 'https://graph.microsoft.com/v1.0/$metadata#directoryObjects',
'value' => [
0 => [
'@odata.type' => '#microsoft.graph.group',
'id' => '11111111-1111-1111-1111-111111111111',
],
],
];
return [ return [
// jwt, expected attributes // jwt, expected attributes
['invalidJwt', $expectedGraphAttributes], ['invalidJwt', $authenticatedRequestAttributes, $expectedGraphAttributes],
['', $expectedGraphAttributes], ['', $authenticatedRequestAttributes, $expectedGraphAttributes],
[null, $expectedGraphAttributes], [null, $authenticatedRequestAttributes, $expectedGraphAttributes],
['blah.abc.egd', $expectedGraphAttributes], ['blah.abc.egd', $authenticatedRequestAttributes, $expectedGraphAttributes],
[$validIdToken, [$validIdToken, $authenticatedRequestAttributes,
[ [
'microsoft.name' => ['Steve Stratus'], 'microsoft.name' => ['Steve Stratus'],
'microsoft.mail' => ['steve.stratus@outlook.com'], 'microsoft.mail' => ['steve.stratus@outlook.com'],
'microsoft.id' => ['a76d6a7a097c1e9d'] 'microsoft.id' => ['a76d6a7a097c1e9d'],
'microsoft.@odata.context' => ['https://graph.microsoft.com/v1.0/$metadata#directoryObjects'],
'microsoft.value.0.@odata.type' => ['#microsoft.graph.group'],
'microsoft.value.0.id' => ['11111111-1111-1111-1111-111111111111'],
] ]
],
[$validIdToken, $conflictedRequestAttributes, [
'microsoft.name' => ['Steve Stratus'],
'microsoft.mail' => ['steve.stratus@outlook.com'],
'microsoft.id' => ['11111111'],
'microsoft.@odata.context' => ['https://graph.microsoft.com/v1.0/$metadata#directoryObjects'],
'microsoft.value.0.@odata.type' => ['#microsoft.graph.group'],
'microsoft.value.0.id' => ['11111111-1111-1111-1111-111111111111'],
] ]
],
]; ];
} }
......
...@@ -98,7 +98,8 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase ...@@ -98,7 +98,8 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase
); );
} }
public function authenticateDataProvider() { public function authenticateDataProvider()
{
return [ return [
[ [
[ [
...@@ -142,7 +143,8 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase ...@@ -142,7 +143,8 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase
$this->assertEquals(static::AUTH_ID, $state[OAuth2::AUTHID], 'Ensure authsource name is presevered in state'); $this->assertEquals(static::AUTH_ID, $state[OAuth2::AUTHID], 'Ensure authsource name is presevered in state');
} }
public function finalStepsDataProvider() { public function finalStepsDataProvider()
{
return [ return [
[ [
[ [
...@@ -191,6 +193,70 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase ...@@ -191,6 +193,70 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase
$this->assertEquals($expectedAttributes, $state['Attributes']); $this->assertEquals($expectedAttributes, $state['Attributes']);
} }
public function finalStepsDataProviderWithAuthenticatedApiRequest()
{
return [
[
[
'providerClass' => MockOAuth2Provider::class,
'attributePrefix' => 'test.',
'retryOnError' => 1,
'authenticatedApiRequests' => ['https://mock.com/v1.0/me/memberOf'],
],
new AccessToken(['access_token' => 'stubToken']),
['test.name' => ['Bob'], 'test.additionalResource' => ['info']],
]
];
}
/**
* @dataProvider finalStepsDataProviderWithAuthenticatedApiRequest
* @param $config
* @param $accessToken
* @param $expectedAttributes
*/
public function testFinalStepsWithAuthenticatedApiRequest($config, $accessToken, $expectedAttributes)
{
// given: A mock Oauth2 provider
$code = 'theCode';
$state = [\SimpleSAML\Auth\State::ID => 'stateId'];
$mock = $this->getMockBuilder(AbstractProvider::class)
->disableOriginalConstructor()
->getMock();
$mock->method('getAccessToken')
->with('authorization_code', ['code' => $code])
->willReturn($accessToken);
$attributes = ['name' => 'Bob'];
$user = new GenericResourceOwner($attributes, 'userId');
$mock->method('getResourceOwner')
->with($accessToken)
->willReturn($user);
$authenticatedRequestAttributes = [
'additionalResource' => ['info']
];
$mockRequest = $this->createMock(RequestInterface::class);
$mock->method('getAuthenticatedRequest')
->with('GET', 'https://mock.com/v1.0/me/memberOf', $accessToken)
->willReturn($mockRequest);
$mock->method('getParsedResponse')
->with($mockRequest)
->willReturn($authenticatedRequestAttributes);
MockOAuth2Provider::setDelegate($mock);
// when: turning a code into a token and then into a resource owner attributes
$authOAuth2 = $this->getInstance($config);
$authOAuth2->finalStep($state, $code);
// then: The attributes should be returned based on the getResourceOwner call
$this->assertEquals($expectedAttributes, $state['Attributes']);
}
/** /**
* @dataProvider finalStepsDataProvider * @dataProvider finalStepsDataProvider
* @param $config * @param $config
...@@ -203,12 +269,12 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase ...@@ -203,12 +269,12 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase
$code = 'theCode'; $code = 'theCode';
$state = [\SimpleSAML\Auth\State::ID => 'stateId']; $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
/** @var $mock AbstractProvider|\PHPUnit_Framework_MockObject_MockObject*/ /** @var $mock AbstractProvider|\PHPUnit_Framework_MockObject_MockObject */
$mock = $this->getMockBuilder(AbstractProvider::class) $mock = $this->getMockBuilder(AbstractProvider::class)
->disableOriginalConstructor() ->disableOriginalConstructor()
->getMock(); ->getMock();
/** @var $mockRequest RequestInterface|\PHPUnit_Framework_MockObject_MockObject*/ /** @var $mockRequest RequestInterface|\PHPUnit_Framework_MockObject_MockObject */
$mockRequest = $this->getMockBuilder(RequestInterface::class) $mockRequest = $this->getMockBuilder(RequestInterface::class)
->getMock(); ->getMock();
...@@ -237,6 +303,57 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase ...@@ -237,6 +303,57 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase
$this->assertEquals($expectedAttributes, $state['Attributes']); $this->assertEquals($expectedAttributes, $state['Attributes']);
} }
/**
* @dataProvider finalStepsDataProviderWithAuthenticatedApiRequest
* @param $config
* @param $accessToken
* @param $expectedAttributes
*/
public function testFinalStepsWithAuthenticatedApiRequestWithNetworkErrors($config, $accessToken, $expectedAttributes)
{
// given: A mock Oauth2 provider
$code = 'theCode';
$state = [\SimpleSAML\Auth\State::ID => 'stateId'];
$mock = $this->getMockBuilder(AbstractProvider::class)
->disableOriginalConstructor()
->getMock();
$mock->method('getAccessToken')
->with('authorization_code', ['code' => $code])
->willReturn($accessToken);
$attributes = ['name' => 'Bob'];
$user = new GenericResourceOwner($attributes, 'userId');
$mock->method('getResourceOwner')
->with($accessToken)
->willReturn($user);
$authenticatedRequestAttributes = [
'additionalResource' => ['info']
];
$mockRequest = $this->createMock(RequestInterface::class);
$mockRequestAuthenticatedRequest = $this->createMock(RequestInterface::class);
$mock->method('getAuthenticatedRequest')
->with('GET', 'https://mock.com/v1.0/me/memberOf', $accessToken)
->will($this->onConsecutiveCalls(
$this->throwException(new ConnectException('getAuthenticatedRequest', $mockRequestAuthenticatedRequest)),
$mockRequest
));
$mock->method('getParsedResponse')
->with($mockRequest)
->willReturn($authenticatedRequestAttributes);
MockOAuth2Provider::setDelegate($mock);
// when: turning a code into a token and then into a resource owner attributes
$authOAuth2 = $this->getInstance($config);
$authOAuth2->finalStep($state, $code);
// then: The attributes should be returned based on the getResourceOwner call
$this->assertEquals($expectedAttributes, $state['Attributes']);
}
public function testTooManyErrorsForRetry() public function testTooManyErrorsForRetry()
{ {
// Exception expected on the 3rd attempt // Exception expected on the 3rd attempt
...@@ -252,12 +369,12 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase ...@@ -252,12 +369,12 @@ class OAuth2Test extends \PHPUnit_Framework_TestCase
]; ];
$state = [\SimpleSAML\Auth\State::ID => 'stateId']; $state = [\SimpleSAML\Auth\State::ID => 'stateId'];
/** @var $mock AbstractProvider|\PHPUnit_Framework_MockObject_MockObject*/ /** @var $mock AbstractProvider|\PHPUnit_Framework_MockObject_MockObject */
$mock = $this->getMockBuilder(AbstractProvider::class) $mock = $this->getMockBuilder(AbstractProvider::class)
->disableOriginalConstructor() ->disableOriginalConstructor()
->getMock(); ->getMock();
/** @var $mockRequest RequestInterface|\PHPUnit_Framework_MockObject_MockObject*/ /** @var $mockRequest RequestInterface|\PHPUnit_Framework_MockObject_MockObject */
$mockRequest = $this->getMockBuilder(RequestInterface::class) $mockRequest = $this->getMockBuilder(RequestInterface::class)
->getMock(); ->getMock();
......
...@@ -4,6 +4,7 @@ namespace Test\SimpleSAML; ...@@ -4,6 +4,7 @@ namespace Test\SimpleSAML;
use League\OAuth2\Client\Provider\GenericProvider; use League\OAuth2\Client\Provider\GenericProvider;
use League\OAuth2\Client\Provider\AbstractProvider; use League\OAuth2\Client\Provider\AbstractProvider;
use Psr\Http\Message\RequestInterface;
class MockOAuth2Provider extends GenericProvider implements \SimpleSAML\Utils\ClearableState class MockOAuth2Provider extends GenericProvider implements \SimpleSAML\Utils\ClearableState
{ {
...@@ -36,11 +37,20 @@ class MockOAuth2Provider extends GenericProvider implements \SimpleSAML\Utils\Cl ...@@ -36,11 +37,20 @@ class MockOAuth2Provider extends GenericProvider implements \SimpleSAML\Utils\Cl
return self::$delegate->getResourceOwner($token); return self::$delegate->getResourceOwner($token);
} }
public function getAuthenticatedRequest($method, $url, $token, array $options = [])
{
return self::$delegate->getAuthenticatedRequest($method, $url, $token, $options);
}
public static function setDelegate(AbstractProvider $delegate) public static function setDelegate(AbstractProvider $delegate)
{ {
self::$delegate = $delegate; self::$delegate = $delegate;
} }
public function getParsedResponse(RequestInterface $request)
{
return self::$delegate->getParsedResponse($request);
}
/** /**
* Clear any cached internal state. * Clear any cached internal state.
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment