From 83b4c3236be35352071051f6ed7653412e05f6b2 Mon Sep 17 00:00:00 2001 From: Sebastian Marcet Date: Thu, 5 Jan 2017 19:12:04 -0300 Subject: [PATCH] Fix on User Consent Service when end user on any OAuth2.0 flow especified severals scopes on the auth request, the permutation alg carried on user consent service grow until occasionate a stack overflow, now its fixed with a simplier approach. Change-Id: I2206ee2c7bcd04c21f3119da27ed27bd917edbd1 --- app/Services/OAuth2/UserConsentService.php | 71 +++------ tests/OIDCProtocolTest.php | 177 ++++++++++++++++++++- 2 files changed, 193 insertions(+), 55 deletions(-) diff --git a/app/Services/OAuth2/UserConsentService.php b/app/Services/OAuth2/UserConsentService.php index 05695557..68ac3ab4 100644 --- a/app/Services/OAuth2/UserConsentService.php +++ b/app/Services/OAuth2/UserConsentService.php @@ -33,57 +33,27 @@ class UserConsentService implements IUserConsentService */ public function get($user_id, $client_id, $scopes) { + $scope_set = explode(' ', $scopes); + sort($scope_set); - $set = explode(' ', $scopes); - $size = count($set) - 1; - $perm = range(0, $size); - $j = 0; - $perms = []; + $consent = UserConsent + ::where('user_id', '=', $user_id) + ->where('client_id', '=', $client_id) + ->where('scopes', 'like', '%' . join(' ', $scope_set).'%')->first(); - do - { - foreach ($perm as $i) - { - $perms[$j][] = $set[$i]; + if(is_null($consent)){ + $consents = UserConsent + ::where('user_id', '=', $user_id) + ->where('client_id', '=', $client_id)->get(); + + foreach($consents as $aux_consent){ + // check if the requested scopes are on the former consent present + if(str_contains($aux_consent->scopes, $scope_set)){ + $consent = $aux_consent; + break; + } } - } while ($perm = MathUtils::nextPermutation($perm, $size) and ++$j); - - - $query1 = UserConsent::where('user_id', '=', $user_id)->where('client_id', '=', $client_id); - - $query2 = UserConsent::where('user_id', '=', $user_id)->where('client_id', '=', $client_id); - - - $query1 = $query1->where(function ($query) use($perms) - { - foreach ($perms as $p) - { - $str = join(' ', $p); - $query = $query->orWhere('scopes', '=', $str); - } - - return $query; - }); - - - $query2 = $query2->where(function ($query) use($perms) - { - foreach ($perms as $p) - { - $str = join(' ', $p); - $query = $query->orWhere('scopes', 'like', '%'.$str.'%'); - } - - return $query; - }); - - - $consent = $query1->first(); - - if (is_null($consent)) { - $consent = $query2->first(); } - return $consent; } @@ -96,15 +66,16 @@ class UserConsentService implements IUserConsentService */ public function add($user_id, $client_id, $scopes) { - $consent = new UserConsent(); - + $consent = new UserConsent(); + $scope_set = explode(' ', $scopes); + sort($scope_set); if (is_null(Client::find($client_id))) { throw new EntityNotFoundException(); } $consent->client_id = $client_id; $consent->user_id = $user_id; - $consent->scopes = $scopes; + $consent->scopes = join(' ', $scope_set); $consent->Save(); } } \ No newline at end of file diff --git a/tests/OIDCProtocolTest.php b/tests/OIDCProtocolTest.php index 7f3add4a..051cfe5c 100644 --- a/tests/OIDCProtocolTest.php +++ b/tests/OIDCProtocolTest.php @@ -695,6 +695,167 @@ class OIDCProtocolTest extends OpenStackIDBaseTest return $access_token; } + public function testTokenSeveralScopes + ( + $client_id = 'Jiz87D8/Vcvr6fvQbH4HyNgwTlfSyQ3x.openstack.client', + $client_secret = 'ITc/6Y5N7kOtGKhgITc/6Y5N7kOtGKhgITc/6Y5N7kOtGKhgITc/6Y5N7kOtGKhg', + $use_enc = true + ) { + + + $params = array( + 'client_id' => $client_id, + 'redirect_uri' => 'https://www.test.com/oauth2', + 'response_type' => 'code', + 'scope' => + join(" ", [ + OAuth2Protocol::OpenIdConnect_Scope, + 'profile', + 'email', + 'address', + OAuth2Protocol::OfflineAccess_Scope, + sprintf('%s/resource-server/read', $this->current_realm), + sprintf('%s/resource-server/read.page', $this->current_realm), + sprintf('%s/resource-server/write', $this->current_realm), + sprintf('%s/resource-server/delete', $this->current_realm), + sprintf('%s/resource-server/update', $this->current_realm), + sprintf('%s/resource-server/update.status', $this->current_realm), + sprintf('%s/resource-server/regenerate.secret', $this->current_realm), + ]), + OAuth2Protocol::OAuth2Protocol_LoginHint => 'sebastian@tipit.net', + OAuth2Protocol::OAuth2Protocol_Nonce => 'test_nonce', + OAuth2Protocol::OAuth2Protocol_Prompt => OAuth2Protocol::OAuth2Protocol_Prompt_Consent, + OAuth2Protocol::OAuth2Protocol_MaxAge => 3200 + ); + + $response = $this->action("POST", "OAuth2\OAuth2ProviderController@auth", + $params, + array(), + array(), + array()); + + $this->assertResponseStatus(302); + + $url = $response->getTargetUrl(); + + $response = $this->call('GET', $url); + + $this->assertResponseStatus(200); + + // verify that login hint (email) is populated + $this->assertTrue(str_contains($response->getContent(), 'sebastian@tipit.net')); + + // do login + $response = $this->call('POST', $url, + array + ( + 'username' => 'sebastian@tipit.net', + 'password' => '1qaz2wsx', + '_token' => Session::token() + ) + ); + + $this->assertResponseStatus(302); + + $response = $this->action("GET", "OAuth2\OAuth2ProviderController@auth", + array(), + array(), + array(), + array()); + + $this->assertResponseStatus(302); + + $response = $this->action('GET', 'UserController@getConsent'); + + $this->assertResponseStatus(200); + + $response = $this->action('POST', 'UserController@getConsent', array( + 'trust' => 'AllowOnce', + '_token' => Session::token() + )); + + $this->assertResponseStatus(302); + + // get auth code + + $response = $this->action("GET", "OAuth2\OAuth2ProviderController@auth", + array(), + array(), + array(), + array()); + + $this->assertResponseStatus(302); + + $url = $response->getTargetUrl(); + + $comps = @parse_url($url); + $query = $comps['query']; + $output = array(); + parse_str($query, $output); + + $this->assertTrue(array_key_exists('code', $output)); + $this->assertTrue(!empty($output['code'])); + + $params = array( + 'code' => $output['code'], + 'redirect_uri' => 'https://www.test.com/oauth2', + 'grant_type' => OAuth2Protocol::OAuth2Protocol_GrantType_AuthCode, + ); + + $response = $this->action("POST", "OAuth2\OAuth2ProviderController@token", + $params, + array(), + array(), + array(), + // Symfony interally prefixes headers with "HTTP", so + array("HTTP_Authorization" => " Basic " . base64_encode($client_id . ':' . $client_secret))); + + + $this->assertResponseStatus(200); + + $this->assertEquals('application/json;charset=UTF-8', $response->headers->get('Content-Type')); + + $content = $response->getContent(); + + $response = json_decode($content); + $access_token = $response->access_token; + $refresh_token = $response->refresh_token; + $id_token = $response->id_token; + + $this->assertTrue(!empty($access_token)); + $this->assertTrue(!empty($refresh_token)); + $this->assertTrue(!empty($id_token)); + + $jwt = BasicJWTFactory::build($id_token); + + if ($use_enc) { + $this->assertTrue($jwt instanceof IJWE); + + $recipient_key = RSAJWKFactory::build + ( + new RSAJWKPEMPrivateKeySpecification + ( + TestSeeder::$client_private_key_1, + RSAJWKPEMPrivateKeySpecification::WithoutPassword, + $jwt->getJOSEHeader()->getAlgorithm()->getString() + ) + ); + + $recipient_key->setKeyUse(JSONWebKeyPublicKeyUseValues::Encryption)->setId('recipient_public_key'); + + + $jwt->setRecipientKey($recipient_key); + + $payload = $jwt->getPlainText(); + + $jwt = BasicJWTFactory::build($payload); + + $this->assertTrue($jwt instanceof IJWS); + } + + return $access_token; + } + public function testGetRefreshTokenWithPromptSetToConsentLogin(){ $client_id = 'Jiz87D8/Vcvr6fvQbH4HyNgwTlfSyQ3x.openstack.client'; @@ -1941,8 +2102,12 @@ class OIDCProtocolTest extends OpenStackIDBaseTest 'client_id' => $client_id, 'redirect_uri' => 'https://www.test.com/oauth2', 'response_type' => OAuth2Protocol::OAuth2Protocol_ResponseType_IdToken, - 'scope' => sprintf('%s profile email %s', OAuth2Protocol::OpenIdConnect_Scope, - OAuth2Protocol::OfflineAccess_Scope), + 'scope' =>join(' ', [ + OAuth2Protocol::OpenIdConnect_Scope, + 'profile', + 'email', + OAuth2Protocol::OfflineAccess_Scope + ]), OAuth2Protocol::OAuth2Protocol_LoginHint => 'sebastian@tipit.net', OAuth2Protocol::OAuth2Protocol_Prompt => OAuth2Protocol::OAuth2Protocol_Prompt_Consent, OAuth2Protocol::OAuth2Protocol_MaxAge => 1000, @@ -1969,7 +2134,6 @@ class OIDCProtocolTest extends OpenStackIDBaseTest ) ); - $this->assertResponseStatus(302); $response = $this->action("GET", "OAuth2\OAuth2ProviderController@auth", @@ -2012,7 +2176,11 @@ class OIDCProtocolTest extends OpenStackIDBaseTest sleep(10); $params[OAuth2Protocol::OAuth2Protocol_Prompt] = OAuth2Protocol::OAuth2Protocol_Prompt_None; - $params['scope'] = sprintf('%s profile email', OAuth2Protocol::OpenIdConnect_Scope); + $params['scope'] =join(' ', [ + OAuth2Protocol::OpenIdConnect_Scope, + 'profile', + 'email', + ]); $response = $this->action("POST", "OAuth2\OAuth2ProviderController@auth", $params, @@ -2034,7 +2202,6 @@ class OIDCProtocolTest extends OpenStackIDBaseTest $this->assertTrue(empty($output2['access_token'])); $this->assertTrue(array_key_exists('id_token', $output2)); $this->assertTrue(!empty($output2['id_token'])); - } public function testImplicitFlowAccessToken()