From 99ede0898a4b2a785a98a2e3072080844c5c721d Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Fri, 28 Aug 2020 09:33:00 +0200 Subject: [PATCH 1/5] Update codebase to parse XML in a more efficient maneer. --- src/Security/Core/User/EuLoginUser.php | 72 ++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/src/Security/Core/User/EuLoginUser.php b/src/Security/Core/User/EuLoginUser.php index 47fcca1..eaa0e0d 100644 --- a/src/Security/Core/User/EuLoginUser.php +++ b/src/Security/Core/User/EuLoginUser.php @@ -6,6 +6,8 @@ use EcPhp\CasBundle\Security\Core\User\CasUserInterface; +use function array_key_exists; + final class EuLoginUser implements EuLoginUserInterface { /** @@ -56,7 +58,31 @@ public function getAttribute(string $key, $default = null) public function getAttributes(): array { $attributes = $this->user->getAttributes(); - $attributes['extendedAttributes'] = $this->getExtendedAttributes(); + + $propertyToMangle = [ + ['extendedAttributes', 'extendedAttribute'], + ['groups', 'group'], + ['strengths', 'strength'], + ['authenticationFactors', 'authenticationFactor'], + ]; + + foreach ($propertyToMangle as $properties) { + if (!array_key_exists($properties[0], $attributes)) { + continue; + } + + if (!array_key_exists($properties[1], $attributes[$properties[0]])) { + continue; + } + + $attributes[$properties[0]][$properties[1]] = (array) $attributes[$properties[0]][$properties[1]]; + + if (array_key_exists(0, $attributes[$properties[0]][$properties[1]])) { + continue; + } + + $attributes[$properties[0]][$properties[1]] = [$attributes[$properties[0]][$properties[1]]]; + } return $attributes; } @@ -119,8 +145,22 @@ public function getEmployeeType(): ?string public function getExtendedAttributes(): array { + $attributes = $this->getAttributes(); + + if (!array_key_exists('extendedAttributes', $attributes)) { + return []; + } + + $extendedAttributes = $attributes['extendedAttributes']; + + if (!array_key_exists('extendedAttribute', $extendedAttributes)) { + return []; + } + + $extendedAttributes = $attributes['extendedAttributes']['extendedAttribute']; + return array_reduce( - $this->user->getAttribute('extendedAttributes', []), + $extendedAttributes, static function (array $carry, array $item): array { $carry[$item['@attributes']['name']] = $item['attributeValue']; @@ -143,7 +183,19 @@ public function getFirstName(): ?string */ public function getGroups(): array { - return $this->user->getAttribute('groups', ['group' => []])['group']; + $attributes = $this->getAttributes(); + + if (!array_key_exists('groups', $attributes)) { + return []; + } + + $groups = $attributes['groups']; + + if (!array_key_exists('group', $groups)) { + return []; + } + + return $groups['group']; } /** @@ -225,7 +277,19 @@ public function getSso(): ?string */ public function getStrengths(): array { - return $this->user->getAttribute('strengths', []); + $attributes = $this->getAttributes(); + + if (!array_key_exists('strengths', $attributes)) { + return []; + } + + $strengths = $attributes['strengths']; + + if (!array_key_exists('strength', $strengths)) { + return []; + } + + return (array) $strengths['strength']; } /** From 41a4f853b35281baa20acf1a318bb4e6ba210f9a Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Fri, 28 Aug 2020 09:33:20 +0200 Subject: [PATCH 2/5] Update tests. --- .../Core/User/EuLoginUserProviderSpec.php | 351 ++++++++++++++---- .../Security/Core/User/EuLoginUserSpec.php | 30 +- 2 files changed, 299 insertions(+), 82 deletions(-) diff --git a/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserProviderSpec.php b/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserProviderSpec.php index 9b7ac89..e1bb829 100644 --- a/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserProviderSpec.php +++ b/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserProviderSpec.php @@ -10,8 +10,8 @@ use EcPhp\EuLoginBundle\Security\Core\User\EuLoginUser; use EcPhp\EuLoginBundle\Security\Core\User\EuLoginUserInterface; use EcPhp\EuLoginBundle\Security\Core\User\EuLoginUserProvider; +use Nyholm\Psr7\Response; use PhpSpec\ObjectBehavior; -use Psr\Http\Message\ResponseInterface; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\User\User; @@ -28,93 +28,200 @@ public function it_can_check_if_the_user_class_is_supported() ->shouldReturn(false); } - public function it_can_load_a_user_from_a_response(ResponseInterface $response) + public function it_can_load_a_user_from_a_response(): void { - $response - ->getStatusCode() - ->willReturn(200); - - $response - ->hasHeader('Content-Type') - ->willReturn(true); - - $response - ->getHeaderLine('Content-Type') - ->willReturn('application/xml'); - - $body = <<<'EOF' - - - username - bar - - foo - - - group1 - group2 - - - - rex - snoopy - - - - -EOF; + // TestBody1 + $response = new Response(200, ['content-type' => 'application/xml'], $this->getTestBody1()); - $response - ->getBody() - ->willReturn($body); - - $this - ->loadUserByResponse($response) - ->shouldReturnAnInstanceOf(EuLoginUser::class); + $user = $this->loadUserByResponse($response); - $this - ->loadUserByResponse($response) + $user ->getAttributes() - ->shouldEqual([ - 'foo' => 'bar', - 'groups' => [ - 'group' => [ - 'group1', - 'group2', + ->shouldReturn( + [ + 'departmentNumber' => 'departmentNumber', + 'email' => 'email', + 'employeeNumber' => 'employeeNumber', + 'employeeType' => 'employeeType', + 'firstName' => 'firstName', + 'lastName' => 'lastName', + 'domain' => 'domain', + 'domainUsername' => 'domainUsername', + 'telephoneNumber' => 'telephoneNumber', + 'userManager' => 'userManager', + 'timeZone' => 'timeZone', + 'locale' => 'locale', + 'assuranceLevel' => 'assuranceLevel', + 'uid' => 'uid', + 'orgId' => 'orgId', + 'teleworkingPriority' => 'teleworkingPriority', + 'extendedAttributes' => [ + 'extendedAttribute' => [ + [ + 'attributeValue' => [ + 'value1', + 'value2', + ], + '@attributes' => [ + 'name' => 'extendedAttribute1', + ], + ], + [ + 'attributeValue' => [ + 'value3', + 'value4', + ], + '@attributes' => [ + 'name' => 'extendedAttribute2', + ], + ], + ], ], - ], - 'extendedAttributes' => [ - 'http://stork.eu/motherInLawDogName' => [ - 'rex', - 'snoopy', + 'groups' => [ + 'group' => [ + 'group1', + 'group2', + ], + '@attributes' => [ + 'number' => '2', + ], + ], + 'strengths' => [ + 'strength' => [ + 'strength1', + 'strength2', + ], + '@attributes' => [ + 'number' => '2', + ], + ], + 'authenticationFactors' => [ + 'moniker' => 'moniker1', + '@attributes' => [ + 'number' => '1', + ], ], + 'loginDate' => '', + 'sso' => 'sso', + 'ticketType' => 'ticketType', + 'proxyGrantingProtocol' => 'proxyGrantingProtocol', + ] + ); + + $user + ->getExtendedAttributes() + ->shouldReturn([ + 'extendedAttribute1' => [ + 'value1', + 'value2', + ], + 'extendedAttribute2' => [ + 'value3', + 'value4', ], ]); - $this - ->loadUserByResponse($response) - ->getUsername() - ->shouldReturn('username'); - - $this - ->loadUserByResponse($response) - ->getRoles() + $user + ->getGroups() ->shouldReturn([ 'group1', 'group2', - 'ROLE_CAS_AUTHENTICATED', ]); - $this->loadUserByResponse($response) - ->getExtendedAttributes() + $user + ->getStrengths() + ->shouldReturn([ + 'strength1', + 'strength2', + ]); + + // TestBody2 + $response = new Response(200, ['content-type' => 'application/xml'], $this->getTestBody2()); + + $user = $this->loadUserByResponse($response); + + $user + ->getAttributes() ->shouldReturn( [ - 'http://stork.eu/motherInLawDogName' => [ - 'rex', - 'snoopy', + 'departmentNumber' => 'departmentNumber', + 'email' => 'email', + 'employeeNumber' => 'employeeNumber', + 'employeeType' => 'employeeType', + 'firstName' => 'firstName', + 'lastName' => 'lastName', + 'domain' => 'domain', + 'domainUsername' => 'domainUsername', + 'telephoneNumber' => 'telephoneNumber', + 'userManager' => 'userManager', + 'timeZone' => 'timeZone', + 'locale' => 'locale', + 'assuranceLevel' => 'assuranceLevel', + 'uid' => 'uid', + 'orgId' => 'orgId', + 'teleworkingPriority' => 'teleworkingPriority', + 'extendedAttributes' => [ + 'extendedAttribute' => [ + [ + 'attributeValue' => [ + 'value1', + 'value2', + ], + '@attributes' => [ + 'name' => 'extendedAttribute1', + ], + ], + ], ], + 'groups' => [ + 'group' => [ + 'group1', + ], + '@attributes' => [ + 'number' => '1', + ], + ], + 'strengths' => [ + 'strength' => [ + 'strength1', + ], + '@attributes' => [ + 'number' => '1', + ], + ], + 'authenticationFactors' => [ + 'moniker' => 'moniker1', + '@attributes' => [ + 'number' => '1', + ], + ], + 'loginDate' => '', + 'sso' => 'sso', + 'ticketType' => 'ticketType', + 'proxyGrantingProtocol' => 'proxyGrantingProtocol', ] ); + + $user + ->getExtendedAttributes() + ->shouldReturn([ + 'extendedAttribute1' => [ + 'value1', + 'value2', + ], + ]); + + $user + ->getGroups() + ->shouldReturn([ + 'group1', + ]); + + $user + ->getStrengths() + ->shouldReturn([ + 'strength1', + ]); } public function it_can_refresh_a_user(EuLoginUserInterface $user) @@ -145,4 +252,110 @@ public function let() $this ->beConstructedWith(new CasUserProvider(new EcasIntrospector(new Introspector()))); } + + private function getTestBody1() + { + return <<< 'EOF' + + + + username + departmentNumber + email + employeeNumber + employeeType + firstName + lastName + domain + domainUsername + telephoneNumber + userManager + timeZone + locale + assuranceLevel + uid + orgId + teleworkingPriority + + + value1 + value2 + + + value3 + value4 + + + + group1 + group2 + + + strength1 + strength2 + + + moniker1 + + + sso + ticketType + proxyGrantingProtocol + + proxy1 + + + +EOF; + } + + private function getTestBody2() + { + return <<< 'EOF' + + + + username + departmentNumber + email + employeeNumber + employeeType + firstName + lastName + domain + domainUsername + telephoneNumber + userManager + timeZone + locale + assuranceLevel + uid + orgId + teleworkingPriority + + + value1 + value2 + + + + group1 + + + strength1 + + + moniker1 + + + sso + ticketType + proxyGrantingProtocol + + proxy1 + + + +EOF; + } } diff --git a/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserSpec.php b/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserSpec.php index de55069..63e32be 100644 --- a/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserSpec.php +++ b/spec/EcPhp/EuLoginBundle/Security/Core/User/EuLoginUserSpec.php @@ -24,10 +24,7 @@ public function it_can_get_groups_when_no_groups_are_available() foo - - group1 - group2 - + rex @@ -41,7 +38,6 @@ public function it_can_get_groups_when_no_groups_are_available() $response = new Response(200, ['Content-Type' => 'application/xml'], $body); $data = (new Introspector())->parse($response)['serviceResponse']['authenticationSuccess']; - unset($data['attributes']['groups']); $casUser = new CasUser($data); @@ -203,7 +199,7 @@ public function let(CasUserInterface $user) 40 - + group1 group2 @@ -218,7 +214,7 @@ public function let(CasUserInterface $user) EOF; - $response = new Response(200, ['Content-Type' => 'application/json'], $body); + $response = new Response(200, ['Content-Type' => 'application/xml'], $body); $data = (new Introspector())->parse($response)['serviceResponse']['authenticationSuccess']; $user @@ -362,9 +358,13 @@ private function getAttributesData(): array 'employeeNumber' => 'employeeNumber', 'employeeType' => 'employeeType', 'extendedAttributes' => [ - 'attr1' => [ - 'value1', - 'value2', + 'extendedAttribute' => [ + [ + 'attr1' => [ + 'value1', + 'value2', + ], + ], ], ], 'firstName' => 'firstName', @@ -378,11 +378,15 @@ private function getAttributesData(): array 'orgId' => 'orgId', 'teleworkingPriority' => 'teleworkingPriority', 'groups' => [ - 'group1', - 'group2', + 'group' => [ + 'group1', + 'group2', + ], ], 'strengths' => [ - 'bar', + 'strength' => [ + 'bar', + ], ], 'authenticationFactors' => [ 'ecphp@ec.europa.eu', From d01a1234220efc38118a3030d734f0913c2a4573 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Tue, 1 Sep 2020 14:30:29 +0200 Subject: [PATCH 3/5] Add a todo for later. --- src/Security/Core/User/EuLoginUser.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Security/Core/User/EuLoginUser.php b/src/Security/Core/User/EuLoginUser.php index eaa0e0d..cdef28d 100644 --- a/src/Security/Core/User/EuLoginUser.php +++ b/src/Security/Core/User/EuLoginUser.php @@ -59,6 +59,7 @@ public function getAttributes(): array { $attributes = $this->user->getAttributes(); + // @Todo Ugly. Refactor this when JSON format will be available. $propertyToMangle = [ ['extendedAttributes', 'extendedAttribute'], ['groups', 'group'], From 6e70a923ee85e5ec2100b80dcf6da18486c47b62 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Tue, 1 Sep 2020 14:32:12 +0200 Subject: [PATCH 4/5] Bump dev packages. --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 6a007d5..947b0b9 100644 --- a/composer.json +++ b/composer.json @@ -21,11 +21,11 @@ "symfony/framework-bundle": "^5.1" }, "require-dev": { - "drupol/php-conventions": "^1.8.16", + "drupol/php-conventions": "^1.8.18", "friends-of-phpspec/phpspec-code-coverage": "^4.3.2", "infection/infection": "^0.15.3", "phpspec/phpspec": "^6.2.1", - "vimeo/psalm": "^3.13" + "vimeo/psalm": "^3.14" }, "autoload": { "psr-4": { From 57cc9815310231c88a9810fa9538fe86fa4223d2 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Tue, 1 Sep 2020 14:36:08 +0200 Subject: [PATCH 5/5] Minor code simplification. --- src/Security/Core/User/EuLoginUser.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Security/Core/User/EuLoginUser.php b/src/Security/Core/User/EuLoginUser.php index cdef28d..3864a4f 100644 --- a/src/Security/Core/User/EuLoginUser.php +++ b/src/Security/Core/User/EuLoginUser.php @@ -59,7 +59,7 @@ public function getAttributes(): array { $attributes = $this->user->getAttributes(); - // @Todo Ugly. Refactor this when JSON format will be available. + /** @Todo Ugly. Refactor this when JSON format will be available. */ $propertyToMangle = [ ['extendedAttributes', 'extendedAttribute'], ['groups', 'group'], @@ -67,22 +67,22 @@ public function getAttributes(): array ['authenticationFactors', 'authenticationFactor'], ]; - foreach ($propertyToMangle as $properties) { - if (!array_key_exists($properties[0], $attributes)) { + foreach ($propertyToMangle as [$parent, $child]) { + if (!array_key_exists($parent, $attributes)) { continue; } - if (!array_key_exists($properties[1], $attributes[$properties[0]])) { + if (!array_key_exists($child, $attributes[$parent])) { continue; } - $attributes[$properties[0]][$properties[1]] = (array) $attributes[$properties[0]][$properties[1]]; + $attributes[$parent][$child] = (array) $attributes[$parent][$child]; - if (array_key_exists(0, $attributes[$properties[0]][$properties[1]])) { + if (array_key_exists(0, $attributes[$parent][$child])) { continue; } - $attributes[$properties[0]][$properties[1]] = [$attributes[$properties[0]][$properties[1]]]; + $attributes[$parent][$child] = [$attributes[$parent][$child]]; } return $attributes;