Skip to content
Snippets Groups Projects
Commit 3b1645fa authored by Andrew Hoffmann's avatar Andrew Hoffmann
Browse files

Merge branch 'preauth-test' into 'master'

Test all the things!

This merge request improves the code coverage of the project's unit tests.

I broke up this rather large set of changes into separate commits. See the individual commit messages for details.

* 75773b6d - Use PHPUnit provided by Composer instead of downloading a PHAR
* 737771f2 - Convert dashes to underscores when converting to HTTP headers
* 08aaa966 - Group conditional logic for clarity
* 1477621c - Create missing tests for PreauthUserDetailsProvider, introduce an EnvironmentHelper
* b74fd618 - Federated preauth tests made consistent with new preauth tests, improved coverage, moved test data from JSON to source code
* 09d36d36 - LocalUserDetailsProvider throws Exception if JSON is invalid, returns null if JSON is missing
* d021fd72 - Verify missing attributes are set as false
* e29430a9 - Docblock and PSR-2 formatting improvements

### PHP Trait

In 1477621c, I introduced a lesser-known feature of PHP called a [trait](http://php.net/manual/en/language.oop5.traits.php). It allows one to "inject" common methods into any class without requiring inheritance.

@andrew-summers @weizhong-wang @KJOYNER

See merge request !12
parents 9f009d41 d1303769
No related branches found
No related tags found
1 merge request!12Test all the things!
Showing
with 355 additions and 156 deletions
......@@ -33,22 +33,8 @@
</target>
<!-- Runs PHPUnit -->
<target name="test" depends="get-phpunit">
<exec command="php phpunit.phar" passthru="true" checkreturn="true"/>
</target>
<!-- Downloads PHPUnit -->
<target name="get-phpunit">
<if>
<not>
<available file="phpunit.phar"/>
</not>
<then>
<httpget dir="${project.basedir}"
url="https://phar.phpunit.de/phpunit.phar"
followRedirects="true"/>
</then>
</if>
<target name="test">
<exec command="php vendor/phpunit/phpunit/phpunit" passthru="true" checkreturn="true"/>
</target>
<target name="release">
......
......@@ -15,11 +15,16 @@
"source": "https://git.doit.wisc.edu/adi-ia/uw-php-security"
},
"require-dev": {
"phpunit/phpunit": "^5.4"
"phpunit/phpunit": "5.7.2"
},
"autoload": {
"psr-4": {
"edu\\wisc\\doit\\uwphps\\": "src/main/edu/wisc/doit/uwphps"
}
},
"autoload-dev": {
"psr-4": {
"edu\\wisc\\doit\\uwphps\\": "src/test/edu/wisc/doit/uwphps"
}
}
}
......@@ -4,8 +4,8 @@
"Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file",
"This file is @generated automatically"
],
"hash": "fdb4b179138092bf5b2be9eb78ca9bd2",
"content-hash": "609c5126a4bee0ed8969f6915e8f5ee3",
"hash": "8f6e29a0b7f012c3a62df9e5899d57d3",
"content-hash": "9e664849ddf46e1a2b66fd5185bf57b8",
"packages": [],
"packages-dev": [
{
......
<?php
namespace edu\wisc\doit\uwphps;
/**
* Maps Shibboleth attributes to their HTTP header equivalents
*/
trait HttpHeaderMapper
{
/**
* Map a Shibboleth attribute to its associated HTTP header name.
*
* @param string $attribute attribute to map
* @return string Shibboleth attribute name mapped to its equivalent HTTP header name
*
* @see https://wiki.shibboleth.net/confluence/display/SHIB2/NativeSPAttributeAccess NativeSPAttributeAccess
*/
protected function httpHeaderFromAttribute($attribute)
{
return 'HTTP_' . strtoupper(str_replace('-', '_', $attribute));
}
}
......@@ -8,6 +8,7 @@ namespace edu\wisc\doit\uwphps;
*/
abstract class UserDetailsProvider
{
use HttpHeaderMapper;
/** @var bool Flag indicating if headers are passed prefixed with 'HTTP_' */
protected $httpHeaders;
......@@ -34,23 +35,9 @@ abstract class UserDetailsProvider
}
/**
* Map Shibboleth header values to an associative array.
* Returns the logged-in user
*
* @return UserDetails
* @return UserDetails currently authenticated user, null if not logged in
*/
abstract public function loadUser();
/**
* Map a Shibboleth attribute to its associated HTTP header name.
*
* @param string $attribute attribute to map
* @return string Shibboleth attribute name mapped to its equivalent HTTP header name
*
* @see https://wiki.shibboleth.net/confluence/display/SHIB2/NativeSPAttributeAccess NativeSPAttributeAccess
*/
protected function httpHeaderFromAttribute($attribute)
{
return 'HTTP_' . strtoupper($attribute);
}
}
<?php
namespace edu\wisc\doit\uwphps\local;
/**
* Exception when JSON cannot be decoded
*/
class JsonDecodingException extends \RuntimeException
{
}
\ No newline at end of file
......@@ -26,16 +26,23 @@ class LocalUserDetailsProvider extends PreauthUserDetailsProvider
}
/**
* {@inheritdoc}
* Loads user from configured JSON file.
*
* @return UWUserDetails user, null if file is missing
* @throws JsonDecodingException if unable to decode user JSON file
*/
public function loadUser()
{
$jsonString = file_get_contents($this->filePath);
if ($jsonString === false) {
if (!is_readable($this->filePath)) {
return null;
}
$attributes = json_decode($jsonString, true);
$attributes = json_decode(file_get_contents($this->filePath), true);
// Throw exception if file cannot be decoded
if ($attributes === null) {
throw new JsonDecodingException('Unable to parse JSON in file: ' . realpath($this->filePath));
}
return new UWUserDetails(
$attributes[static::EPPN],
......
......@@ -27,8 +27,8 @@ class FederatedPreauthUserDetailsProvider extends UserDetailsProvider
public function loadUser()
{
// Return null if no Shib session is found
if ($this->httpHeaders && !getenv(static::SHIB_SESSION_ID_HTTP) ||
!$this->httpHeaders && !getenv(static::SHIB_SESSION_ID)) {
if (($this->httpHeaders && !getenv(static::SHIB_SESSION_ID_HTTP)) ||
(!$this->httpHeaders && !getenv(static::SHIB_SESSION_ID))) {
return null;
}
......
......@@ -9,7 +9,7 @@ use edu\wisc\doit\uwphps\UWUserDetails;
* PreauthUserDetailsProvider is an implementation of {@link UserDetailsProvider} for loading users authenticated
* with UW-Madison login.
*
* {@see FederatedPreauthUserDetailsProvider} for loading users authenticated through UW-System Federated login.
* See {@link FederatedPreauthUserDetailsProvider} for loading users authenticated through UW-System Federated login.
*/
class PreauthUserDetailsProvider extends UserDetailsProvider
{
......@@ -23,14 +23,11 @@ class PreauthUserDetailsProvider extends UserDetailsProvider
const FIRST_NAME = 'givenName';
const LAST_NAME = 'sn';
/**
* {@inheritdoc}
*/
public function loadUser()
{
// Return null if no Shib session is found
if ($this->httpHeaders && !getenv(static::SHIB_SESSION_ID_HTTP) ||
!$this->httpHeaders && !getenv(static::SHIB_SESSION_ID)) {
if (($this->httpHeaders && !getenv(static::SHIB_SESSION_ID_HTTP)) ||
(!$this->httpHeaders && !getenv(static::SHIB_SESSION_ID))) {
return null;
}
......@@ -62,5 +59,4 @@ class PreauthUserDetailsProvider extends UserDetailsProvider
return $userDetails;
}
}
<?php
namespace edu\wisc\doit\uwphps;
trait EnvironmentHelper
{
use HttpHeaderMapper;
/**
* Sets environment variables
*
* @param array $environment pairs of environment variable names and values
*/
private function setEnvironment(array $environment)
{
array_walk($environment, function ($value, $key) {
putenv("$key=$value");
});
}
/**
* Removes the given environment variable from the environment
*
* @param string $variable name of environment variable
*/
private function removeEnvironmentVariable($variable)
{
putenv($variable);
}
/**
* Converts environment variable names to their HTTP header equivalents
*
* @param array $environment
* @return array environment with variable names converted to HTTP headers
* @see HttpHeaderMapper::httpHeaderFromAttribute()
*/
private function toHttpHeaders(array $environment)
{
foreach ($environment as $key => $value) {
$environment[$this->httpHeaderFromAttribute($key)] = $value;
unset($environment[$key]);
}
return $environment;
}
}
<?php
namespace edu\wisc\doit\uwphps;
use PHPUnit\Framework\TestCase;
/**
* Tests for {@link HttpHeaderMapper} trait
*/
class HttpHeaderMapperTest extends TestCase
{
use HttpHeaderMapper;
/** @test */
public function convertsAttributeToHttpHeader()
{
$this->assertEquals('HTTP_STARFLEETRANK', $this->httpHeaderFromAttribute('starfleetRank'));
$this->assertEquals('HTTP_SHIB_SESSION_ID', $this->httpHeaderFromAttribute('Shib-Session-Id'));
}
}
<?php
use edu\wisc\doit\uwphps\local\LocalUserDetailsProvider;
namespace edu\wisc\doit\uwphps\local;
/**
* Tests for {@link LocalUserDetailsProvider}.
......@@ -8,12 +7,12 @@ use edu\wisc\doit\uwphps\local\LocalUserDetailsProvider;
class LocalUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
{
/**
* @test
*/
public function loadUser()
private $resourcesDir = __DIR__ . '/../../../../../resources';
/** @test */
public function loadsUser()
{
$userDetailsService = new LocalUserDetailsProvider(__DIR__ . "/../../../../../resources/localuser.json");
$userDetailsService = new LocalUserDetailsProvider("{$this->resourcesDir}/localuser.json");
$user = $userDetailsService->loadUser();
$this->assertEquals("bbadger@wisc.edu", $user->getEppn());
$this->assertEquals("UW123A456", $user->getPvi());
......@@ -25,4 +24,21 @@ class LocalUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
$this->assertEquals("BADGER", $user->getLastName());
}
/** @test */
public function loadsNullUserWhenMissingFile()
{
$provider = new LocalUserDetailsProvider("{$this->resourcesDir}/nobody.json");
$this->assertNull($provider->loadUser());
}
/**
* @test
* @expectedException \edu\wisc\doit\uwphps\local\JsonDecodingException
*/
public function throwsIfInvalidJson()
{
$provider = new LocalUserDetailsProvider("{$this->resourcesDir}/badjson.json");
$provider->loadUser();
}
}
<?php
namespace edu\wisc\doit\uwphps\preauth;
use edu\wisc\doit\uwphps\EnvironmentHelper;
/**
* Tests for {@link FederatedPreauthUserDetailsProvider}.
*/
class FederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
{
use EnvironmentHelper;
/** @var array */
private $attributes;
/** @var FederatedPreauthUserDetailsProvider */
private $userProvider;
/** @var FederatedPreauthUserDetailsProvider configured to use HTTP headers */
private $userProviderHttp;
/** @var array environment variables to be set when testing */
private $environment;
protected function setUp()
{
$this->userProvider = new FederatedPreauthUserDetailsProvider(false);
$this->userProviderHttp = new FederatedPreauthUserDetailsProvider(true);
// Default environment variables
$this->environment = [
FederatedPreauthUserDetailsProvider::FULL_NAME => 'BUCKINGHAM BADGER',
FederatedPreauthUserDetailsProvider::EPPN => 'bbadger@wisc.edu',
FederatedPreauthUserDetailsProvider::FIRST_NAME => 'BUCKINGHAM',
FederatedPreauthUserDetailsProvider::EMAIL => 'bucky.badger@wisc.edu',
FederatedPreauthUserDetailsProvider::SHIB_SESSION_ID => '1234567890',
FederatedPreauthUserDetailsProvider::LAST_NAME => 'BADGER',
FederatedPreauthUserDetailsProvider::SOURCE => 'a_source',
FederatedPreauthUserDetailsProvider::SPVI => 'UW123A456',
FederatedPreauthUserDetailsProvider::ISIS_EMPLID => '123456789'
];
}
/** @test */
public function loadsNullUserWhenNoSession()
{
$this->removeEnvironmentVariable(FederatedPreauthUserDetailsProvider::SHIB_SESSION_ID);
$this->removeEnvironmentVariable(FederatedPreauthUserDetailsProvider::SHIB_SESSION_ID_HTTP);
static::assertNull($this->userProvider->loadUser());
static::assertNull($this->userProviderHttp->loadUser());
}
/** @test */
public function loadsUser()
{
$this->setEnvironment($this->environment);
$user = $this->userProvider->loadUser();
static::assertNotNull($user);
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::EPPN], $user->getEppn());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::SPVI], $user->getPvi());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::SOURCE], $user->getSource());
static::assertEquals(
$this->environment[FederatedPreauthUserDetailsProvider::ISIS_EMPLID],
$user->getIsisEmplid()
);
static::assertEquals(
$this->environment[FederatedPreauthUserDetailsProvider::FIRST_NAME],
$user->getFirstName()
);
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
}
/** @test */
public function loadsUserWithHttpHeaders()
{
$this->setEnvironment($this->toHttpHeaders($this->environment));
$user = $this->userProviderHttp->loadUser();
static::assertNotNull($user);
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::EPPN], $user->getEppn());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::SPVI], $user->getPvi());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::SOURCE], $user->getSource());
static::assertEquals(
$this->environment[FederatedPreauthUserDetailsProvider::ISIS_EMPLID],
$user->getIsisEmplid()
);
static::assertEquals(
$this->environment[FederatedPreauthUserDetailsProvider::FIRST_NAME],
$user->getFirstName()
);
static::assertEquals($this->environment[FederatedPreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
}
/** @test */
public function missingAttributeIsFalse()
{
$this->setEnvironment($this->environment);
$this->removeEnvironmentVariable(FederatedPreauthUserDetailsProvider::EMAIL);
$user = $this->userProvider->loadUser();
static::assertNotNull($user);
static::assertFalse($user->getEmailAddress());
}
}
<?php
use edu\wisc\doit\uwphps\UserDetailsProvider;
use edu\wisc\doit\uwphps\preauth\FederatedPreauthUserDetailsProvider;
/**
* Tests for {@link FederatedPreauthUserDetailsProvider}.
*/
class HTTPFederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
{
/** @var array */
private $attributes;
/** @var UserDetailsProvider */
private $userProvider;
/**
* Populate $_SERVER with Shib attributes to simulate a logged in user
*/
protected function setUp()
{
$jsonString = file_get_contents(__DIR__ . "/../../../../../resources/testuser_http.json");
if ($jsonString === false) {
return null;
}
$this->attributes = json_decode($jsonString, true);
$this->userProvider = new FederatedPreauthUserDetailsProvider(true);
putenv($this->mapAttribute(UserDetailsProvider::EPPN) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::EPPN)]);
putenv($this->mapAttribute(FederatedPreauthUserDetailsProvider::SPVI) . '=' . $this->attributes[$this->mapAttribute(FederatedPreauthUserDetailsProvider::SPVI)]);
putenv($this->mapAttribute(FederatedPreauthUserDetailsProvider::FULL_NAME) . '=' . $this->attributes[$this->mapAttribute(FederatedPreauthUserDetailsProvider::FULL_NAME)]);
putenv($this->mapAttribute(FederatedPreauthUserDetailsProvider::FIRST_NAME) . '=' . $this->attributes[$this->mapAttribute(FederatedPreauthUserDetailsProvider::FIRST_NAME)]);
putenv($this->mapAttribute(FederatedPreauthUserDetailsProvider::LAST_NAME) . '=' . $this->attributes[$this->mapAttribute(FederatedPreauthUserDetailsProvider::LAST_NAME)]);
putenv($this->mapAttribute(UserDetailsProvider::UDDS) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::UDDS)]);
putenv($this->mapAttribute(FederatedPreauthUserDetailsProvider::EMAIL) . '=' . $this->attributes[$this->mapAttribute(FederatedPreauthUserDetailsProvider::EMAIL)]);
putenv($this->mapAttribute(UserDetailsProvider::SOURCE) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::SOURCE)]);
putenv($this->mapAttribute(UserDetailsProvider::ISIS_EMPLID) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::ISIS_EMPLID)]);
putenv(UserDetailsProvider::SHIB_SESSION_ID_HTTP . '=' . $this->attributes[UserDetailsProvider::SHIB_SESSION_ID_HTTP]);
}
/**
* @test
*/
public function loadUser()
{
$user = $this->userProvider->loadUser();
$this->assertNotNull($user);
$this->assertEquals("bbadger@wisc.edu", $user->getEppn());
$this->assertEquals("UW123A456", $user->getPvi());
$this->assertEquals("BUCKINGHAM BADGER", $user->getFullName());
$this->assertEquals("bucky.badger@wisc.edu", $user->getEmailAddress());
$this->assertEquals("a_source", $user->getSource());
$this->assertEquals("123456789", $user->getIsisEmplid());
$this->assertEquals("BUCKINGHAM", $user->getFirstName());
$this->assertEquals("BADGER", $user->getLastName());
$this->assertEquals(["A061234", "A072345"], $user->getUddsMembership());
}
/**
* @test
*/
public function loadUserWithNoEPPN()
{
// Clear Shib session ID to simulate no session
putenv(UserDetailsProvider::SHIB_SESSION_ID_HTTP);
$user = $this->userProvider->loadUser();
$this->assertNull($user);
}
/**
* @test
*/
public function loadUserWithNoEmail()
{
// Clear email to simulate no email
putenv('HTTP_' . strtoupper(FederatedPreauthUserDetailsProvider::EMAIL));
$user = $this->userProvider->loadUser();
$this->assertFalse($user->getEmailAddress());
}
private function mapAttribute($attribute)
{
return 'HTTP_' . strtoupper($attribute);
}
}
<?php
namespace edu\wisc\doit\uwphps\preauth;
use edu\wisc\doit\uwphps\EnvironmentHelper;
use edu\wisc\doit\uwphps\UserDetailsProvider;
/**
* Tests for {@link PreauthUserDetailsProvider}
*/
class PreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
{
use EnvironmentHelper;
/** @var PreauthUserDetailsProvider */
private $provider;
/** @var PreauthUserDetailsProvider */
private $providerWithHttp;
/** @var array environment variables to be set */
private $environment;
protected function setUp()
{
$this->provider = new PreauthUserDetailsProvider(false);
$this->providerWithHttp = new PreauthUserDetailsProvider(true);
// Default environment variables
$this->environment = [
PreauthUserDetailsProvider::FULL_NAME => 'BUCKINGHAM BADGER',
PreauthUserDetailsProvider::EPPN => 'bbadger@wisc.edu',
PreauthUserDetailsProvider::FIRST_NAME => 'BUCKINGHAM',
PreauthUserDetailsProvider::EMAIL => 'bucky.badger@wisc.edu',
PreauthUserDetailsProvider::SHIB_SESSION_ID => '1234567890',
PreauthUserDetailsProvider::LAST_NAME => 'BADGER',
PreauthUserDetailsProvider::SOURCE => 'a_source',
PreauthUserDetailsProvider::PVI => 'UW123A456',
PreauthUserDetailsProvider::ISIS_EMPLID => '123456789'
];
}
/** @test */
public function loadsNullUserWhenNoSession()
{
// Ensure the Shib Session ID environment variables are deleted
$this->removeEnvironmentVariable(UserDetailsProvider::SHIB_SESSION_ID);
$this->removeEnvironmentVariable(UserDetailsProvider::SHIB_SESSION_ID_HTTP);
static::assertNull($this->provider->loadUser());
static::assertNull($this->providerWithHttp->loadUser());
}
/** @test */
public function loadsUser()
{
$this->setEnvironment($this->environment);
$user = $this->provider->loadUser();
static::assertNotNull($user);
static::assertEquals($this->environment[PreauthUserDetailsProvider::EPPN], $user->getEppn());
static::assertEquals($this->environment[PreauthUserDetailsProvider::PVI], $user->getPvi());
static::assertEquals($this->environment[PreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
static::assertEquals($this->environment[PreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
static::assertEquals($this->environment[PreauthUserDetailsProvider::SOURCE], $user->getSource());
static::assertEquals($this->environment[PreauthUserDetailsProvider::ISIS_EMPLID], $user->getIsisEmplid());
static::assertEquals($this->environment[PreauthUserDetailsProvider::FIRST_NAME], $user->getFirstName());
static::assertEquals($this->environment[PreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
}
/** @test */
public function loadsUserWithHttpHeaders()
{
$this->setEnvironment($this->toHttpHeaders($this->environment));
$user = $this->providerWithHttp->loadUser();
static::assertNotNull($user);
static::assertEquals($this->environment[PreauthUserDetailsProvider::EPPN], $user->getEppn());
static::assertEquals($this->environment[PreauthUserDetailsProvider::PVI], $user->getPvi());
static::assertEquals($this->environment[PreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
static::assertEquals($this->environment[PreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
static::assertEquals($this->environment[PreauthUserDetailsProvider::SOURCE], $user->getSource());
static::assertEquals($this->environment[PreauthUserDetailsProvider::ISIS_EMPLID], $user->getIsisEmplid());
static::assertEquals($this->environment[PreauthUserDetailsProvider::FIRST_NAME], $user->getFirstName());
static::assertEquals($this->environment[PreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
}
/** @test */
public function missingAttributeIsFalse()
{
$this->setEnvironment($this->environment);
$this->removeEnvironmentVariable(PreauthUserDetailsProvider::ISIS_EMPLID);
$user = $this->provider->loadUser();
static::assertNotNull($user);
static::assertFalse($user->getIsisEmplid());
}
}
This file should not contain valid JSON.
See: LocalUserDetailsProviderTest
\ No newline at end of file
{
"HTTP_EPPN": "bbadger@wisc.edu",
"HTTP_EDUWISCONSINSPVI": "UW123A456",
"HTTP_CN": "BUCKINGHAM BADGER",
"HTTP_EDUWISCONSINCOMMONNAME": "BUCKINGHAM BADGER",
"HTTP_EDUWISCONSINGIVENNAME": "BUCKINGHAM",
"HTTP_EDUWISCONSINSURNAME": "BADGER",
"HTTP_UDDS": "A061234;A072345",
"HTTP_EDUWISCONSINEMAILADDRESS": "bucky.badger@wisc.edu",
"HTTP_SOURCE": "a_source",
"HTTP_ISISEMPLID": "123456789",
"HTTP_SHIB_SESSION_ID": "1234567890"
}
\ No newline at end of file
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment