From 75773b6d2aa7c83053084deeb78acd82c8f210ce Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 10:44:21 -0600
Subject: [PATCH 01/11] Use PHPUnit that is provided by composer instead of the
PHAR
Specify an exact version of PHPUnit. Autoload test classes if developing.
---
build.xml | 18 ++----------------
composer.json | 7 ++++++-
composer.lock | 4 ++--
3 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/build.xml b/build.xml
index 524c721..a74a8f3 100644
--- a/build.xml
+++ b/build.xml
@@ -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/bin/phpunit" passthru="true" checkreturn="true"/>
</target>
<target name="release">
diff --git a/composer.json b/composer.json
index 9a7479c..fc5ac52 100644
--- a/composer.json
+++ b/composer.json
@@ -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"
+ }
}
}
diff --git a/composer.lock b/composer.lock
index 2fffb03..2971b0d 100644
--- a/composer.lock
+++ b/composer.lock
@@ -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": [
{
--
GitLab
From 737771f209923968d9b475ea548fead4734eee12 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 10:47:09 -0600
Subject: [PATCH 02/11] Convert dashes to underscores when converting to HTTP
headers
Changed the signature to `public static` to expose this useful function to other classes. Provided a test.
---
.../wisc/doit/uwphps/UserDetailsProvider.php | 4 +--
.../FederatedPreauthUserDetailsProvider.php | 18 ++++++-------
.../preauth/PreauthUserDetailsProvider.php | 18 ++++++-------
.../doit/uwphps/UserDetailsProviderTest.php | 27 +++++++++++++++++++
4 files changed, 47 insertions(+), 20 deletions(-)
create mode 100644 src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php
diff --git a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
index f211ca7..9ebd735 100644
--- a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
@@ -49,8 +49,8 @@ abstract class UserDetailsProvider
*
* @see https://wiki.shibboleth.net/confluence/display/SHIB2/NativeSPAttributeAccess NativeSPAttributeAccess
*/
- protected function httpHeaderFromAttribute($attribute)
+ public static function httpHeaderFromAttribute($attribute)
{
- return 'HTTP_' . strtoupper($attribute);
+ return 'HTTP_' . strtoupper(str_replace('-', '_', $attribute));
}
}
diff --git a/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
index baafe05..48dfdf1 100644
--- a/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
@@ -34,15 +34,15 @@ class FederatedPreauthUserDetailsProvider extends UserDetailsProvider
if ($this->httpHeaders) {
$userDetails = new UWUserDetails(
- getenv($this->httpHeaderFromAttribute(static::EPPN)),
- getenv($this->httpHeaderFromAttribute(static::SPVI)),
- getenv($this->httpHeaderFromAttribute(static::FULL_NAME)),
- explode(static::DELIMITER, getenv($this->httpHeaderFromAttribute(static::UDDS))),
- getenv($this->httpHeaderFromAttribute(static::EMAIL)),
- getenv($this->httpHeaderFromAttribute(static::SOURCE)),
- getenv($this->httpHeaderFromAttribute(static::ISIS_EMPLID)),
- getenv($this->httpHeaderFromAttribute(static::FIRST_NAME)),
- getenv($this->httpHeaderFromAttribute(static::LAST_NAME))
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EPPN)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::SPVI)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FULL_NAME)),
+ explode(static::DELIMITER, getenv(UserDetailsProvider::httpHeaderFromAttribute(static::UDDS))),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EMAIL)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::SOURCE)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::ISIS_EMPLID)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FIRST_NAME)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::LAST_NAME))
);
} else {
$userDetails = new UWUserDetails(
diff --git a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
index 2406b38..31f8261 100644
--- a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
@@ -36,15 +36,15 @@ class PreauthUserDetailsProvider extends UserDetailsProvider
if ($this->httpHeaders) {
$userDetails = new UWUserDetails(
- getenv($this->httpHeaderFromAttribute(static::EPPN)),
- getenv($this->httpHeaderFromAttribute(static::PVI)),
- getenv($this->httpHeaderFromAttribute(static::FULL_NAME)),
- explode(static::DELIMITER, getenv($this->httpHeaderFromAttribute(static::UDDS))),
- getenv($this->httpHeaderFromAttribute(static::EMAIL)),
- getenv($this->httpHeaderFromAttribute(static::SOURCE)),
- getenv($this->httpHeaderFromAttribute(static::ISIS_EMPLID)),
- getenv($this->httpHeaderFromAttribute(static::FIRST_NAME)),
- getenv($this->httpHeaderFromAttribute(static::LAST_NAME))
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EPPN)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::PVI)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FULL_NAME)),
+ explode(static::DELIMITER, getenv(UserDetailsProvider::httpHeaderFromAttribute(static::UDDS))),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EMAIL)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::SOURCE)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::ISIS_EMPLID)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FIRST_NAME)),
+ getenv(UserDetailsProvider::httpHeaderFromAttribute(static::LAST_NAME))
);
} else {
$userDetails = new UWUserDetails(
diff --git a/src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php b/src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php
new file mode 100644
index 0000000..0ce2e41
--- /dev/null
+++ b/src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php
@@ -0,0 +1,27 @@
+<?php
+namespace edu\wisc\doit\uwphps;
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Tests for {@link UserDetailsProvider} concrete methods.
+ */
+class UserDetailsProviderTest extends TestCase
+{
+
+ /** @var UserDetailsProvider */
+ private $provider;
+
+ public function setUp()
+ {
+ // Must use a mock because it is an abstract class
+ $this->provider = $this->getMockForAbstractClass(UserDetailsProvider::class);
+ }
+
+ /** @test */
+ public function convertsAttributeToHttpHeader()
+ {
+ $this->assertEquals('HTTP_STARFLEETRANK', UserDetailsProvider::httpHeaderFromAttribute('starfleetRank'));
+ $this->assertEquals('HTTP_SHIB_SESSION_ID', UserDetailsProvider::httpHeaderFromAttribute('Shib-Session-Id'));
+ }
+}
--
GitLab
From 08aaa96630e14ecc54e8aab8c57c3c4909708b43 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 10:49:23 -0600
Subject: [PATCH 03/11] Group conditional logic to clarify precedence
It may not be clear to the developer that `&&` takes precedence over `||`. This change was recommended by a code quality scan.
---
.../uwphps/preauth/FederatedPreauthUserDetailsProvider.php | 4 ++--
.../wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
index 48dfdf1..1dd0dd8 100644
--- a/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
@@ -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;
}
diff --git a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
index 31f8261..58b647d 100644
--- a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
@@ -29,8 +29,8 @@ class PreauthUserDetailsProvider 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;
}
--
GitLab
From 1477621c7ae200fae25e6638e0df0192f9e3b668 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 10:51:08 -0600
Subject: [PATCH 04/11] Create missing tests for PreauthUserDetailsProvider
Introduce a `trait` that helps set environment variables when testing
---
.../wisc/doit/uwphps/EnvironmentHelper.php | 44 ++++++++++
.../PreauthUserDetailsProviderTest.php | 85 +++++++++++++++++++
2 files changed, 129 insertions(+)
create mode 100644 src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php
create mode 100644 src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php
diff --git a/src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php b/src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php
new file mode 100644
index 0000000..30cbb1a
--- /dev/null
+++ b/src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php
@@ -0,0 +1,44 @@
+<?php
+namespace edu\wisc\doit\uwphps;
+
+trait EnvironmentHelper
+{
+
+ /**
+ * 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 UserDetailsProvider::httpHeaderFromAttribute()
+ */
+ private function toHttpHeaders(array $environment)
+ {
+ foreach ($environment as $key => $value) {
+ $environment[UserDetailsProvider::httpHeaderFromAttribute($key)] = $value;
+ unset($environment[$key]);
+ }
+ return $environment;
+ }
+}
diff --git a/src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php b/src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php
new file mode 100644
index 0000000..138d9a9
--- /dev/null
+++ b/src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php
@@ -0,0 +1,85 @@
+<?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();
+
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::EPPN], $user->getEppn());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::PVI], $user->getPvi());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::SOURCE], $user->getSource());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::ISIS_EMPLID], $user->getIsisEmplid());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::FIRST_NAME], $user->getFirstName());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
+ }
+
+ /** @test */
+ public function loadsUserWithHttpHeaders()
+ {
+ $this->setEnvironment($this->toHttpHeaders($this->environment));
+ $user = $this->providerWithHttp->loadUser();
+
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::EPPN], $user->getEppn());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::PVI], $user->getPvi());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::SOURCE], $user->getSource());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::ISIS_EMPLID], $user->getIsisEmplid());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::FIRST_NAME], $user->getFirstName());
+ $this->assertEquals($this->environment[PreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
+ }
+}
--
GitLab
From b74fd618b5a885f49440ca15b69d27cb094f1b27 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 12:53:56 -0600
Subject: [PATCH 05/11] Federated preach tests are consistent with regular
preauth tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Renamed to follow class naming conventions. Pulled in new EnvironmentHelper to manage environment variables. Moved default test data from JSON file into source code so it’s easier to find. Reorganized/renamed tests to better describe their assertions.
---
...ederatedPreauthUserDetailsProviderTest.php | 110 ++++++++++++++++++
...ederatedPreauthUserDetailsProviderTest.php | 88 --------------
src/test/resources/testuser_http.json | 13 ---
3 files changed, 110 insertions(+), 101 deletions(-)
create mode 100644 src/test/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProviderTest.php
delete mode 100644 src/test/edu/wisc/doit/uwphps/preauth/HTTPFederatedPreauthUserDetailsProviderTest.php
delete mode 100644 src/test/resources/testuser_http.json
diff --git a/src/test/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProviderTest.php b/src/test/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProviderTest.php
new file mode 100644
index 0000000..5614843
--- /dev/null
+++ b/src/test/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProviderTest.php
@@ -0,0 +1,110 @@
+<?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());
+ }
+}
diff --git a/src/test/edu/wisc/doit/uwphps/preauth/HTTPFederatedPreauthUserDetailsProviderTest.php b/src/test/edu/wisc/doit/uwphps/preauth/HTTPFederatedPreauthUserDetailsProviderTest.php
deleted file mode 100644
index 3d43e21..0000000
--- a/src/test/edu/wisc/doit/uwphps/preauth/HTTPFederatedPreauthUserDetailsProviderTest.php
+++ /dev/null
@@ -1,88 +0,0 @@
-<?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);
- }
-
-}
diff --git a/src/test/resources/testuser_http.json b/src/test/resources/testuser_http.json
deleted file mode 100644
index 0d7f591..0000000
--- a/src/test/resources/testuser_http.json
+++ /dev/null
@@ -1,13 +0,0 @@
-{
- "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
--
GitLab
From 09d36d36f49be98ea934e8f8383a412eb047ba39 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 12:57:30 -0600
Subject: [PATCH 06/11] LocalUserDetailsProvider throws Exception if JSON is
invalid, returns null if JSON is missing
---
.../uwphps/local/JsonDecodingException.php | 9 ++++++
.../uwphps/local/LocalUserDetailsProvider.php | 15 +++++++---
.../local/LocalUserDetailsProviderTest.php | 30 ++++++++++++++-----
src/test/resources/badjson.json | 3 ++
4 files changed, 46 insertions(+), 11 deletions(-)
create mode 100644 src/main/edu/wisc/doit/uwphps/local/JsonDecodingException.php
create mode 100644 src/test/resources/badjson.json
diff --git a/src/main/edu/wisc/doit/uwphps/local/JsonDecodingException.php b/src/main/edu/wisc/doit/uwphps/local/JsonDecodingException.php
new file mode 100644
index 0000000..88fa5ef
--- /dev/null
+++ b/src/main/edu/wisc/doit/uwphps/local/JsonDecodingException.php
@@ -0,0 +1,9 @@
+<?php
+namespace edu\wisc\doit\uwphps\local;
+
+/**
+ * Exception when JSON cannot be decoded
+ */
+class JsonDecodingException extends \RuntimeException
+{
+}
\ No newline at end of file
diff --git a/src/main/edu/wisc/doit/uwphps/local/LocalUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/local/LocalUserDetailsProvider.php
index 7a15d83..9abe0cb 100644
--- a/src/main/edu/wisc/doit/uwphps/local/LocalUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/local/LocalUserDetailsProvider.php
@@ -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],
diff --git a/src/test/edu/wisc/doit/uwphps/local/LocalUserDetailsProviderTest.php b/src/test/edu/wisc/doit/uwphps/local/LocalUserDetailsProviderTest.php
index 3ee4255..a277d06 100644
--- a/src/test/edu/wisc/doit/uwphps/local/LocalUserDetailsProviderTest.php
+++ b/src/test/edu/wisc/doit/uwphps/local/LocalUserDetailsProviderTest.php
@@ -1,6 +1,5 @@
<?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();
+ }
+
}
diff --git a/src/test/resources/badjson.json b/src/test/resources/badjson.json
new file mode 100644
index 0000000..096ced5
--- /dev/null
+++ b/src/test/resources/badjson.json
@@ -0,0 +1,3 @@
+This file should not contain valid JSON.
+
+See: LocalUserDetailsProviderTest
\ No newline at end of file
--
GitLab
From d021fd72444dfa560cd08bdd2795c719a8f662ec Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 12:58:18 -0600
Subject: [PATCH 07/11] Verify missing attributes are set as false
Switch to static calls of PHPUnit assertion methods
---
.../PreauthUserDetailsProviderTest.php | 45 ++++++++++++-------
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php b/src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php
index 138d9a9..7e6d7de 100644
--- a/src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php
+++ b/src/test/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProviderTest.php
@@ -54,17 +54,17 @@ class PreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
public function loadsUser()
{
$this->setEnvironment($this->environment);
-
$user = $this->provider->loadUser();
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::EPPN], $user->getEppn());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::PVI], $user->getPvi());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::SOURCE], $user->getSource());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::ISIS_EMPLID], $user->getIsisEmplid());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::FIRST_NAME], $user->getFirstName());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
+ 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 */
@@ -73,13 +73,24 @@ class PreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
$this->setEnvironment($this->toHttpHeaders($this->environment));
$user = $this->providerWithHttp->loadUser();
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::EPPN], $user->getEppn());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::PVI], $user->getPvi());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::FULL_NAME], $user->getFullName());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::EMAIL], $user->getEmailAddress());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::SOURCE], $user->getSource());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::ISIS_EMPLID], $user->getIsisEmplid());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::FIRST_NAME], $user->getFirstName());
- $this->assertEquals($this->environment[PreauthUserDetailsProvider::LAST_NAME], $user->getLastName());
+ 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());
}
}
--
GitLab
From e29430a9693b8d9bb5426affa54c2e254d054e54 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 12:58:42 -0600
Subject: [PATCH 08/11] Docblock and PSR-2 formatting improvements
---
src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php | 2 +-
.../wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php | 6 +-----
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
index 9ebd735..6a5def3 100644
--- a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
@@ -36,7 +36,7 @@ abstract class UserDetailsProvider
/**
* Map Shibboleth header values to an associative array.
*
- * @return UserDetails
+ * @return UserDetails currently authenticated user, null if not logged in
*/
abstract public function loadUser();
diff --git a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
index 58b647d..e128eee 100644
--- a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
@@ -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,9 +23,6 @@ class PreauthUserDetailsProvider extends UserDetailsProvider
const FIRST_NAME = 'givenName';
const LAST_NAME = 'sn';
- /**
- * {@inheritdoc}
- */
public function loadUser()
{
// Return null if no Shib session is found
@@ -62,5 +59,4 @@ class PreauthUserDetailsProvider extends UserDetailsProvider
return $userDetails;
}
-
}
--
GitLab
From c4f74ce2d998a960c1be554ced27a0b0cdcb14a6 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Tue, 6 Dec 2016 14:34:34 -0600
Subject: [PATCH 09/11] Call the actual PHP script instead of a shell script
This allows interoperability with windows
---
build.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/build.xml b/build.xml
index a74a8f3..fc7d18a 100644
--- a/build.xml
+++ b/build.xml
@@ -34,7 +34,7 @@
<!-- Runs PHPUnit -->
<target name="test">
- <exec command="php vendor/bin/phpunit" passthru="true" checkreturn="true"/>
+ <exec command="php vendor/phpunit/phpunit/phpunit" passthru="true" checkreturn="true"/>
</target>
<target name="release">
--
GitLab
From 5a0656d25defad2e5875fbb148f462025098cee2 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Wed, 7 Dec 2016 11:36:28 -0600
Subject: [PATCH 10/11] Extract HTTP header conversion method to its own trait
for reuse
---
.../edu/wisc/doit/uwphps/HttpHeaderMapper.php | 21 +++++++++++++++
.../wisc/doit/uwphps/UserDetailsProvider.php | 15 +----------
.../FederatedPreauthUserDetailsProvider.php | 18 ++++++-------
.../preauth/PreauthUserDetailsProvider.php | 18 ++++++-------
.../wisc/doit/uwphps/EnvironmentHelper.php | 5 ++--
.../wisc/doit/uwphps/HttpHeaderMapperTest.php | 19 +++++++++++++
.../doit/uwphps/UserDetailsProviderTest.php | 27 -------------------
7 files changed, 62 insertions(+), 61 deletions(-)
create mode 100644 src/main/edu/wisc/doit/uwphps/HttpHeaderMapper.php
create mode 100644 src/test/edu/wisc/doit/uwphps/HttpHeaderMapperTest.php
delete mode 100644 src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php
diff --git a/src/main/edu/wisc/doit/uwphps/HttpHeaderMapper.php b/src/main/edu/wisc/doit/uwphps/HttpHeaderMapper.php
new file mode 100644
index 0000000..6f2f0fd
--- /dev/null
+++ b/src/main/edu/wisc/doit/uwphps/HttpHeaderMapper.php
@@ -0,0 +1,21 @@
+<?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));
+ }
+}
diff --git a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
index 6a5def3..bff1a73 100644
--- a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
@@ -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;
@@ -39,18 +40,4 @@ abstract class UserDetailsProvider
* @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
- */
- public static function httpHeaderFromAttribute($attribute)
- {
- return 'HTTP_' . strtoupper(str_replace('-', '_', $attribute));
- }
}
diff --git a/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
index 1dd0dd8..ed174da 100644
--- a/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/preauth/FederatedPreauthUserDetailsProvider.php
@@ -34,15 +34,15 @@ class FederatedPreauthUserDetailsProvider extends UserDetailsProvider
if ($this->httpHeaders) {
$userDetails = new UWUserDetails(
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EPPN)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::SPVI)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FULL_NAME)),
- explode(static::DELIMITER, getenv(UserDetailsProvider::httpHeaderFromAttribute(static::UDDS))),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EMAIL)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::SOURCE)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::ISIS_EMPLID)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FIRST_NAME)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::LAST_NAME))
+ getenv($this->httpHeaderFromAttribute(static::EPPN)),
+ getenv($this->httpHeaderFromAttribute(static::SPVI)),
+ getenv($this->httpHeaderFromAttribute(static::FULL_NAME)),
+ explode(static::DELIMITER, getenv($this->httpHeaderFromAttribute(static::UDDS))),
+ getenv($this->httpHeaderFromAttribute(static::EMAIL)),
+ getenv($this->httpHeaderFromAttribute(static::SOURCE)),
+ getenv($this->httpHeaderFromAttribute(static::ISIS_EMPLID)),
+ getenv($this->httpHeaderFromAttribute(static::FIRST_NAME)),
+ getenv($this->httpHeaderFromAttribute(static::LAST_NAME))
);
} else {
$userDetails = new UWUserDetails(
diff --git a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
index e128eee..7b2740c 100644
--- a/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/preauth/PreauthUserDetailsProvider.php
@@ -33,15 +33,15 @@ class PreauthUserDetailsProvider extends UserDetailsProvider
if ($this->httpHeaders) {
$userDetails = new UWUserDetails(
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EPPN)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::PVI)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FULL_NAME)),
- explode(static::DELIMITER, getenv(UserDetailsProvider::httpHeaderFromAttribute(static::UDDS))),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::EMAIL)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::SOURCE)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::ISIS_EMPLID)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::FIRST_NAME)),
- getenv(UserDetailsProvider::httpHeaderFromAttribute(static::LAST_NAME))
+ getenv($this->httpHeaderFromAttribute(static::EPPN)),
+ getenv($this->httpHeaderFromAttribute(static::PVI)),
+ getenv($this->httpHeaderFromAttribute(static::FULL_NAME)),
+ explode(static::DELIMITER, getenv($this->httpHeaderFromAttribute(static::UDDS))),
+ getenv($this->httpHeaderFromAttribute(static::EMAIL)),
+ getenv($this->httpHeaderFromAttribute(static::SOURCE)),
+ getenv($this->httpHeaderFromAttribute(static::ISIS_EMPLID)),
+ getenv($this->httpHeaderFromAttribute(static::FIRST_NAME)),
+ getenv($this->httpHeaderFromAttribute(static::LAST_NAME))
);
} else {
$userDetails = new UWUserDetails(
diff --git a/src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php b/src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php
index 30cbb1a..f02f474 100644
--- a/src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php
+++ b/src/test/edu/wisc/doit/uwphps/EnvironmentHelper.php
@@ -3,6 +3,7 @@ namespace edu\wisc\doit\uwphps;
trait EnvironmentHelper
{
+ use HttpHeaderMapper;
/**
* Sets environment variables
@@ -31,12 +32,12 @@ trait EnvironmentHelper
*
* @param array $environment
* @return array environment with variable names converted to HTTP headers
- * @see UserDetailsProvider::httpHeaderFromAttribute()
+ * @see HttpHeaderMapper::httpHeaderFromAttribute()
*/
private function toHttpHeaders(array $environment)
{
foreach ($environment as $key => $value) {
- $environment[UserDetailsProvider::httpHeaderFromAttribute($key)] = $value;
+ $environment[$this->httpHeaderFromAttribute($key)] = $value;
unset($environment[$key]);
}
return $environment;
diff --git a/src/test/edu/wisc/doit/uwphps/HttpHeaderMapperTest.php b/src/test/edu/wisc/doit/uwphps/HttpHeaderMapperTest.php
new file mode 100644
index 0000000..de7c1a6
--- /dev/null
+++ b/src/test/edu/wisc/doit/uwphps/HttpHeaderMapperTest.php
@@ -0,0 +1,19 @@
+<?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'));
+ }
+}
diff --git a/src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php b/src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php
deleted file mode 100644
index 0ce2e41..0000000
--- a/src/test/edu/wisc/doit/uwphps/UserDetailsProviderTest.php
+++ /dev/null
@@ -1,27 +0,0 @@
-<?php
-namespace edu\wisc\doit\uwphps;
-
-use PHPUnit\Framework\TestCase;
-
-/**
- * Tests for {@link UserDetailsProvider} concrete methods.
- */
-class UserDetailsProviderTest extends TestCase
-{
-
- /** @var UserDetailsProvider */
- private $provider;
-
- public function setUp()
- {
- // Must use a mock because it is an abstract class
- $this->provider = $this->getMockForAbstractClass(UserDetailsProvider::class);
- }
-
- /** @test */
- public function convertsAttributeToHttpHeader()
- {
- $this->assertEquals('HTTP_STARFLEETRANK', UserDetailsProvider::httpHeaderFromAttribute('starfleetRank'));
- $this->assertEquals('HTTP_SHIB_SESSION_ID', UserDetailsProvider::httpHeaderFromAttribute('Shib-Session-Id'));
- }
-}
--
GitLab
From d130376983c4a2a3cdffc858857d01a8f9cd6632 Mon Sep 17 00:00:00 2001
From: Andrew Hoffmann <andrew.hoffmann@wisc.edu>
Date: Wed, 7 Dec 2016 11:36:41 -0600
Subject: [PATCH 11/11] Fix documentation for loadUser
---
src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
index bff1a73..b163dbc 100644
--- a/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
+++ b/src/main/edu/wisc/doit/uwphps/UserDetailsProvider.php
@@ -35,7 +35,7 @@ abstract class UserDetailsProvider
}
/**
- * Map Shibboleth header values to an associative array.
+ * Returns the logged-in user
*
* @return UserDetails currently authenticated user, null if not logged in
*/
--
GitLab