From 1da78c7d92a98bc88afeccd631af84ba1400bac7 Mon Sep 17 00:00:00 2001 From: Adam Franco Date: Thu, 1 Sep 2016 17:34:35 -0400 Subject: [PATCH 1/4] Add structure to support unit tests. --- Security/CasAuthenticator.php | 12 +++- autoload.php | 7 +++ phpunit.xml.dist | 27 ++++++++ .../Security/CasAuthenticatorTest.php | 63 +++++++++++++++++++ 4 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 autoload.php create mode 100644 phpunit.xml.dist create mode 100644 tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php diff --git a/Security/CasAuthenticator.php b/Security/CasAuthenticator.php index 7a6ff43..3ea6b89 100644 --- a/Security/CasAuthenticator.php +++ b/Security/CasAuthenticator.php @@ -21,12 +21,14 @@ class CasAuthenticator extends AbstractGuardAuthenticator protected $query_ticket_parameter; protected $query_service_parameter; protected $options; + protected $client; /** * Process configuration * @param array $config + * @param optional Client $client */ - public function __construct($config) + public function __construct($config, Client $client = null) { $this->server_login_url = $config['server_login_url']; $this->server_validation_url = $config['server_validation_url']; @@ -35,6 +37,11 @@ public function __construct($config) $this->query_service_parameter = $config['query_service_parameter']; $this->query_ticket_parameter = $config['query_ticket_parameter']; $this->options = $config['options']; + if (is_null($client)) { + $this->client = new Client(); + } else { + $this->client = $client; + } } /** @@ -49,8 +56,7 @@ public function getCredentials(Request $request) $request->get($this->query_ticket_parameter).'&'. $this->query_service_parameter.'='.urlencode($this->removeCasTicket($request->getUri())); - $client = new Client(); - $response = $client->request('GET', $url, $this->options); + $response = $this->client->request('GET', $url, $this->options); $string = $response->getBody()->getContents(); diff --git a/autoload.php b/autoload.php new file mode 100644 index 0000000..c26cd50 --- /dev/null +++ b/autoload.php @@ -0,0 +1,7 @@ + + + + + + + + + + tests + + + + + + diff --git a/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php new file mode 100644 index 0000000..ca69ed0 --- /dev/null +++ b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php @@ -0,0 +1,63 @@ +mockCasServer = new MockHandler(); + $handler = HandlerStack::create($this->mockCasServer); + $client = new Client(['handler' => $handler]); + + $this->authenticator = new CasAuthenticator(array( + 'server_login_url' => 'https://cas.example.com/cas/', + 'server_validation_url' => 'https://cas.example.com/cas/serviceValidate', + 'server_logout_url' => 'https://cas.example.com/cas/logout', + 'xml_namespace' => 'cas', + 'options' => array(), + 'username_attribute' => 'user', + 'query_ticket_parameter' => 'ticket', + 'query_service_parameter' => 'service' + ), + $client); + + $this->provider = new CasUserProvider(); + } + + public function test_get_user_with_name_only() { + // Create a mock response. + $response = new Response( + 200, + array('Content-Type' => 'text/xml'), + ' + + + testuser + +' + ); + $this->mockCasServer->append($response); + + $request = Request::create('http://app.example.com/?ticket=ABC123-1', 'GET'); + + // Get the credentials. + $credentials = $this->authenticator->getCredentials($request); + $this->assertNotEmpty($credentials); + $this->assertEquals('testuser', $credentials['user']); + + // Get the user object for the credentials. + $user = $this->authenticator->getUser($credentials, $this->provider); + $this->assertEquals('testuser', $user->getUsername()); + $this->assertEquals(array('ROLE_USER'), $user->getRoles()); + } + +} From 6fdc867912480f7888912915bc87f079aa73e79b Mon Sep 17 00:00:00 2001 From: Adam Franco Date: Fri, 2 Sep 2016 10:52:57 -0400 Subject: [PATCH 2/4] Populate the user object with attributes from the CAS response if available. --- Security/CasAuthenticator.php | 4 + .../User/CasUserCredentialStoreInterface.php | 13 ++ Security/User/CasUserProvider.php | 140 +++++++++++++++++- .../Security/CasAuthenticatorTest.php | 105 +++++++++++++ 4 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 Security/User/CasUserCredentialStoreInterface.php diff --git a/Security/CasAuthenticator.php b/Security/CasAuthenticator.php index 3ea6b89..3f0d24f 100644 --- a/Security/CasAuthenticator.php +++ b/Security/CasAuthenticator.php @@ -11,6 +11,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\User\UserProviderInterface; +use PRayno\CasAuthBundle\Security\User\CasUserCredentialStoreInterface; class CasAuthenticator extends AbstractGuardAuthenticator { @@ -79,6 +80,9 @@ public function getCredentials(Request $request) public function getUser($credentials, UserProviderInterface $userProvider) { if (isset($credentials[$this->username_attribute])) { + if ($userProvider instanceof CasUserCredentialStoreInterface) { + $userProvider->storeUserCredentials($credentials); + } return $userProvider->loadUserByUsername($credentials[$this->username_attribute]); } else { return null; diff --git a/Security/User/CasUserCredentialStoreInterface.php b/Security/User/CasUserCredentialStoreInterface.php new file mode 100644 index 0000000..fc932c2 --- /dev/null +++ b/Security/User/CasUserCredentialStoreInterface.php @@ -0,0 +1,13 @@ +user_credentials[$credentials['user']] = $credentials; + } else { + throw new \InvalidArgumentException('Credentials must contain a user property'); + } + } + /** * Provides the authenticated user a ROLE_USER * @param $username @@ -22,7 +36,11 @@ public function loadUserByUsername($username) $salt = ""; $roles = ["ROLE_USER"]; - return new CasUser($username, $password, $salt, $roles); + $user = new CasUser($username, $password, $salt, $roles); + if (!empty($this->user_credentials[$username])) { + $this->populateCasAttributes($user, $this->user_credentials[$username]); + } + return $user; } throw new UsernameNotFoundException( @@ -55,4 +73,120 @@ public function supportsClass($class) { return $class === 'PRayno\CasAuthBundle\Security\User\CasUser'; } -} \ No newline at end of file + + /** + * @param UserInterface $user + * @param $credentials + */ + protected function populateCasAttributes(UserInterface $user, $credentials) { + $attras = array(); + + // "Jasig Style" & CAS 3.0 Attributes: + // + // + // + // jsmith + // + // Jasig + // Smith + // John + // CN=Staff,OU=Groups,DC=example,DC=edu + // CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu + // + // PGTIOU-84678-8a9d2sfa23casd + // + // + // + if (isset($credentials['attributes'])) { + foreach ($credentials['attributes'] as $attribute) { + $name = $attribute->getName(); + $value = $attribute->__toString(); + // Make an attribute multi-valued on the second one seen. + if (isset($attras[$name]) && !is_array($attras[$name])) { + $tmp = $attras[$name]; + $attras[$name] = array($tmp); + } + // Add multi-valued attributes. + if (isset($attras[$name])) { + $attras[$name][] = $value; + } + // Single-valued attributes. + else { + $attras[$name] = $value; + } + } + } else { + // "Name-Value" attributes. + // + // Attribute format from these mailing list thread: + // http://jasig.275507.n4.nabble.com/CAS-attributes-and-how-they-appear-in-the-CAS-response-td264272.html + // Note: This is a less widely used format, but in use by at least two institutions. + // + // + // + // jsmith + // + // + // + // + // + // + // + // PGTIOU-84678-8a9d2sfa23casd + // + // + // + if (isset($credentials['attribute']) && isset($credentials['attribute'][0]->attributes()['name']) && isset($credentials['attribute'][0]->attributes()['value'])) { + foreach ($credentials['attribute'] as $attribute) { + $name = (string)$attribute->attributes()['name']; + $value = (string)$attribute->attributes()['value']; + // Make an attribute multi-valued on the second one seen. + if (isset($attras[$name]) && !is_array($attras[$name])) { + $tmp = $attras[$name]; + $attras[$name] = array($tmp); + } + // Add multi-valued attributes. + if (isset($attras[$name])) { + $attras[$name][] = $value; + } + // Single-valued attributes. + else { + $attras[$name] = $value; + } + } + } + // "RubyCAS Style" attributes + // + // + // + // jsmith + // + // RubyCAS + // Smith + // John + // CN=Staff,OU=Groups,DC=example,DC=edu + // CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu + // + // PGTIOU-84678-8a9d2sfa23casd + // + // + // + else { + // Test for elements other than our allowed list know to the protocol. + $skip = array('user', 'proxyGrantingTicket'); + foreach ($credentials as $name => $value) { + if (!in_array($name, $skip)) { + $attras[$name] = $value; + } + } + } + } + + // Add the attributes to the user object. + foreach ($attras as $name => $value) { + if (!isset($user->$name)) { + $user->$name = $value; + } + } + } +} diff --git a/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php index ca69ed0..73c4268 100644 --- a/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php +++ b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php @@ -60,4 +60,109 @@ public function test_get_user_with_name_only() { $this->assertEquals(array('ROLE_USER'), $user->getRoles()); } + public function test_get_user_with_jasig_attributes() { + // Create a mock response. + $response = new Response( + 200, + array('Content-Type' => 'text/xml'), + ' + + + testuser + + Smith + John + CN=Staff,OU=Groups,DC=example,DC=edu + CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu + + +' + ); + $this->mockCasServer->append($response); + + $request = Request::create('http://app.example.com/?ticket=ABC123-1', 'GET'); + + // Get the credentials. + $credentials = $this->authenticator->getCredentials($request); + $this->assertNotEmpty($credentials); + $this->assertEquals('testuser', $credentials['user']); + + // Get the user object for the credentials. + $user = $this->authenticator->getUser($credentials, $this->provider); + $this->assertEquals('testuser', $user->getUsername()); + $this->assertEquals(array('ROLE_USER'), $user->getRoles()); + $this->assertEquals('Smith', $user->surname); + $this->assertEquals('John', $user->givenName); + $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->memberOf); + } + + public function test_get_user_with_name_value_attributes() { + // Create a mock response. + $response = new Response( + 200, + array('Content-Type' => 'text/xml'), + ' + + + testuser + + + + + +' + ); + $this->mockCasServer->append($response); + + $request = Request::create('http://app.example.com/?ticket=ABC123-1', 'GET'); + + // Get the credentials. + $credentials = $this->authenticator->getCredentials($request); + $this->assertNotEmpty($credentials); + $this->assertEquals('testuser', $credentials['user']); + + // Get the user object for the credentials. + $user = $this->authenticator->getUser($credentials, $this->provider); + $this->assertEquals('testuser', $user->getUsername()); + $this->assertEquals(array('ROLE_USER'), $user->getRoles()); + $this->assertEquals('Smith', $user->surname); + $this->assertEquals('John', $user->givenName); + $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->memberOf); + } + + public function test_get_user_with_rubycas_attributes() { + // Create a mock response. + $response = new Response( + 200, + array('Content-Type' => 'text/xml'), + ' + + + testuser + Smith + John + CN=Staff,OU=Groups,DC=example,DC=edu + CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu + +' + ); + $this->mockCasServer->append($response); + + $request = Request::create('http://app.example.com/?ticket=ABC123-1', 'GET'); + + // Get the credentials. + $credentials = $this->authenticator->getCredentials($request); + $this->assertNotEmpty($credentials); + $this->assertEquals('testuser', $credentials['user']); + + // Get the user object for the credentials. + $user = $this->authenticator->getUser($credentials, $this->provider); + $this->assertEquals('testuser', $user->getUsername()); + $this->assertEquals(array('ROLE_USER'), $user->getRoles()); + $this->assertEquals('Smith', $user->surname); + $this->assertEquals('John', $user->givenName); + $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->memberOf); + } + + } From 952baa5f8d2191805241da0fa312f8e28adfae10 Mon Sep 17 00:00:00 2001 From: Adam Franco Date: Fri, 2 Sep 2016 14:04:42 -0400 Subject: [PATCH 3/4] Store the user attributes in an array instead of public parameters. --- Security/User/CasUser.php | 30 +++++++++++++++++-- Security/User/CasUserAttributesInterface.php | 18 +++++++++++ Security/User/CasUserProvider.php | 18 +++++------ .../Security/CasAuthenticatorTest.php | 18 +++++------ 4 files changed, 62 insertions(+), 22 deletions(-) create mode 100644 Security/User/CasUserAttributesInterface.php diff --git a/Security/User/CasUser.php b/Security/User/CasUser.php index 2533f7f..2fbcdef 100644 --- a/Security/User/CasUser.php +++ b/Security/User/CasUser.php @@ -4,13 +4,15 @@ namespace PRayno\CasAuthBundle\Security\User; use Symfony\Component\Security\Core\User\UserInterface; +use PRayno\CasAuthBundle\Security\User\CasUserAttributesInterface; -class CasUser implements UserInterface +class CasUser implements UserInterface, CasUserAttributesInterface { private $username; private $password; private $salt; private $roles; + private $attributes; /** * @param $username @@ -18,12 +20,13 @@ class CasUser implements UserInterface * @param $salt * @param array $roles */ - public function __construct($username, $password, $salt, array $roles) + public function __construct($username, $password, $salt, array $roles, array $attributes = array()) { $this->username = $username; $this->password = $password; $this->salt = $salt; $this->roles = $roles; + $this->attributes = $attributes; } /** @@ -65,4 +68,25 @@ public function eraseCredentials() { } -} \ No newline at end of file + + /** + * @return array + */ + public function getAttributes() + { + return $this->attributes; + } + + /** + * @return mixed string, array, or null if not found. + */ + public function getAttribute($name) + { + if (isset($this->attributes[$name])) { + return $this->attributes[$name]; + } else { + return null; + } + + } +} diff --git a/Security/User/CasUserAttributesInterface.php b/Security/User/CasUserAttributesInterface.php new file mode 100644 index 0000000..e6a6ab0 --- /dev/null +++ b/Security/User/CasUserAttributesInterface.php @@ -0,0 +1,18 @@ +user_credentials[$username])) { - $this->populateCasAttributes($user, $this->user_credentials[$username]); + $attributes = $this->getCasAttributes($this->user_credentials[$username]); + } else { + $attributes = array(); } + $user = new CasUser($username, $password, $salt, $roles, $attributes); + return $user; } @@ -75,10 +78,10 @@ public function supportsClass($class) } /** - * @param UserInterface $user * @param $credentials + * @retun array */ - protected function populateCasAttributes(UserInterface $user, $credentials) { + protected function getCasAttributes($credentials) { $attras = array(); // "Jasig Style" & CAS 3.0 Attributes: @@ -182,11 +185,6 @@ protected function populateCasAttributes(UserInterface $user, $credentials) { } } - // Add the attributes to the user object. - foreach ($attras as $name => $value) { - if (!isset($user->$name)) { - $user->$name = $value; - } - } + return $attras; } } diff --git a/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php index 73c4268..c644b7b 100644 --- a/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php +++ b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php @@ -91,9 +91,9 @@ public function test_get_user_with_jasig_attributes() { $user = $this->authenticator->getUser($credentials, $this->provider); $this->assertEquals('testuser', $user->getUsername()); $this->assertEquals(array('ROLE_USER'), $user->getRoles()); - $this->assertEquals('Smith', $user->surname); - $this->assertEquals('John', $user->givenName); - $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->memberOf); + $this->assertEquals('Smith', $user->getAttribute('surname')); + $this->assertEquals('John', $user->getAttribute('givenName')); + $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->getAttribute('memberOf')); } public function test_get_user_with_name_value_attributes() { @@ -125,9 +125,9 @@ public function test_get_user_with_name_value_attributes() { $user = $this->authenticator->getUser($credentials, $this->provider); $this->assertEquals('testuser', $user->getUsername()); $this->assertEquals(array('ROLE_USER'), $user->getRoles()); - $this->assertEquals('Smith', $user->surname); - $this->assertEquals('John', $user->givenName); - $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->memberOf); + $this->assertEquals('Smith', $user->getAttribute('surname')); + $this->assertEquals('John', $user->getAttribute('givenName')); + $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->getAttribute('memberOf')); } public function test_get_user_with_rubycas_attributes() { @@ -159,9 +159,9 @@ public function test_get_user_with_rubycas_attributes() { $user = $this->authenticator->getUser($credentials, $this->provider); $this->assertEquals('testuser', $user->getUsername()); $this->assertEquals(array('ROLE_USER'), $user->getRoles()); - $this->assertEquals('Smith', $user->surname); - $this->assertEquals('John', $user->givenName); - $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->memberOf); + $this->assertEquals('Smith', $user->getAttribute('surname')); + $this->assertEquals('John', $user->getAttribute('givenName')); + $this->assertEquals(array('CN=Staff,OU=Groups,DC=example,DC=edu', 'CN=Spanish Department,OU=Departments,OU=Groups,DC=example,DC=edu'), $user->getAttribute('memberOf')); } From e00360579c632e8c40cee3ac30f538fe19116400 Mon Sep 17 00:00:00 2001 From: Adam Franco Date: Fri, 2 Sep 2016 14:12:17 -0400 Subject: [PATCH 4/4] Store attributes in the session so that the remain available for multiple requests. I wasn't able to fiture out how to update the `PRaynoCasAuthExtension` to load the session without requiring the `services.yml` to be updated. --- README.md | 8 +++++ Security/User/CasUserProvider.php | 29 ++++++++++++++----- .../Security/CasAuthenticatorTest.php | 4 ++- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 7ef2886..7317cfd 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,14 @@ p_rayno_cas_auth: ``` Note : the xml_namespace and options parameters are optionals +If you want any to access user attributes from the CAS response, inject the session into the user-provider in services.yml : +```yaml +services: + prayno.cas_user_provider: + class: PRayno\CasAuthBundle\Security\User\CasUserProvider + arguments: ['@session'] +``` + Modify your security.xml with the following values (the provider in the following settings should not be used as it's just a very basic example) : ```yaml security: diff --git a/Security/User/CasUserProvider.php b/Security/User/CasUserProvider.php index 0d31f16..49e0d28 100644 --- a/Security/User/CasUserProvider.php +++ b/Security/User/CasUserProvider.php @@ -6,20 +6,35 @@ use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; +use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag; class CasUserProvider implements UserProviderInterface, CasUserCredentialStoreInterface { - protected $user_credentials = array(); + protected $user_attribute_bag; + + /** + * @param SessionInterface $session + */ + public function __construct(SessionInterface $session = null) { + if (!is_null($session)) { + $this->user_attribute_bag = new AttributeBag('PRayno_CasAuthBundle_UserAttributes'); + $session->registerBag($this->user_attribute_bag); + } + } /** * @param array $credentials */ public function storeUserCredentials(array $credentials) { - if ($credentials['user']) { - $this->user_credentials[$credentials['user']] = $credentials; - } else { - throw new \InvalidArgumentException('Credentials must contain a user property'); + if (isset($this->user_attribute_bag)) { + if ($credentials['user']) { + $this->user_attribute_bag->set($credentials['user'], $this->getCasAttributes($credentials)); + } else { + throw new \InvalidArgumentException('Credentials must contain a user property'); + } } } @@ -36,8 +51,8 @@ public function loadUserByUsername($username) $salt = ""; $roles = ["ROLE_USER"]; - if (!empty($this->user_credentials[$username])) { - $attributes = $this->getCasAttributes($this->user_credentials[$username]); + if (isset($this->user_attribute_bag) && $this->user_attribute_bag->has($username)) { + $attributes = $this->user_attribute_bag->get($username); } else { $attributes = array(); } diff --git a/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php index c644b7b..aefd88b 100644 --- a/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php +++ b/tests/PRayno/CasAuthBundle/Security/CasAuthenticatorTest.php @@ -9,6 +9,8 @@ use GuzzleHttp\Client; use GuzzleHttp\Psr7\Response; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; +use Symfony\Component\HttpFoundation\Session\Session; class CasAuthenticatorTest extends \PHPUnit_Framework_TestCase { @@ -30,7 +32,7 @@ public function setUp() { ), $client); - $this->provider = new CasUserProvider(); + $this->provider = new CasUserProvider(new Session(new MockArraySessionStorage())); } public function test_get_user_with_name_only() {