Skip to content
Snippets Groups Projects
Commit 661a01b1 authored by Andy Summers's avatar Andy Summers
Browse files

Merge branch 'test-fix' into 'master' 

Fix bug with non-HTTP Shib sessions being valid for HTTP sessions

Previously, the `PreauthUserDetailsProvider` was only checking that a
valid Shib session existed by looking for the regular or HTTP Shib
session header. This check is now strengthened by validating the
correct header exists for the correct instance.

Also renamed and changed some things in the `Preauth` test--it's now called `HTTPPreauthUserDetailsProviderTest` to reflect the fact that it's only testing the HTTP version of `Preauth`. Along those same lines, the test user now uses the HTTP headers and has been renamed to `testuser_http.json`.

Please review: @ahoffmann 

See merge request !7
parents 537fb525 f5aa2687
No related branches found
No related tags found
1 merge request!7Fix bug with non-HTTP Shib sessions being valid for HTTP sessions
Hide whitespace changes
Inline Side-by-side
composer.json
+ 1
− 1
View file @ 661a01b1
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
"type": "library", "type": "library",
"homepage": "https://git.doit.wisc.edu/adi-ia/uw-php-security", "homepage": "https://git.doit.wisc.edu/adi-ia/uw-php-security",
"license": "Apache-2.0", "license": "Apache-2.0",
"version": "1.0.2", "version": "1.0.3",
"authors": [{ "authors": [{
"name": "UW-Madison DoIT ADI Integrated Applications", "name": "UW-Madison DoIT ADI Integrated Applications",
"email": "adi-ia@lists.wisc.edu", "email": "adi-ia@lists.wisc.edu",
......
src/main/edu/wisc/doit/FederatedPreauthUserDetailsProvider.php
+ 2
− 2
View file @ 661a01b1
...@@ -28,8 +28,8 @@ class FederatedPreauthUserDetailsProvider implements UserDetailsProvider ...@@ -28,8 +28,8 @@ class FederatedPreauthUserDetailsProvider implements UserDetailsProvider
public function loadUser() public function loadUser()
{ {
// Return null if no Shib session is found // Return null if no Shib session is found
if (empty(getenv(FederatedPreauthUserDetailsProvider::SHIB_SESSION_ID)) && if ($this->httpHeaders && !getenv(FederatedPreauthUserDetailsProvider::SHIB_SESSION_ID_HTTP) ||
empty(getenv(FederatedPreauthUserDetailsProvider::SHIB_SESSION_ID_HTTP))) { !$this->httpHeaders && !getenv(FederatedPreauthUserDetailsProvider::SHIB_SESSION_ID)) {
return null; return null;
} }
......
src/test/edu/wisc/doit/FederatedPreauthUserDetailsProviderTest.php → src/test/edu/wisc/doit/HTTPFederatedPreauthUserDetailsProviderTest.php
+ 79
− 0
View file @ 661a01b1
...@@ -6,7 +6,7 @@ use edu\wisc\doit\FederatedPreauthUserDetailsProvider; ...@@ -6,7 +6,7 @@ use edu\wisc\doit\FederatedPreauthUserDetailsProvider;
/** /**
* Tests for {@link FederatedPreauthUserDetailsProvider}. * Tests for {@link FederatedPreauthUserDetailsProvider}.
*/ */
class FederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCase class HTTPFederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
{ {
/** @var array */ /** @var array */
...@@ -21,18 +21,28 @@ class FederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCas ...@@ -21,18 +21,28 @@ class FederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCas
protected function setUp() protected function setUp()
{ {
parent::setUp(); parent::setUp();
$jsonString = file_get_contents(__DIR__ . "/../../../resources/testuser.json"); $jsonString = file_get_contents(__DIR__ . "/../../../resources/testuser_http.json");
if ($jsonString === false) { if ($jsonString === false) {
return null; return null;
} }
$this->attributes = json_decode($jsonString, true); $this->attributes = json_decode($jsonString, true);
$this->userProvider = new FederatedPreauthUserDetailsProvider(true);
putenv($this->mapAttribute(UserDetailsProvider::FED_EPPN) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::FED_EPPN)]);
putenv($this->mapAttribute(UserDetailsProvider::FED_SPVI) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::FED_SPVI)]);
putenv($this->mapAttribute(UserDetailsProvider::FED_FULLNAME) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::FED_FULLNAME)]);
putenv($this->mapAttribute(UserDetailsProvider::FED_FIRST_NAME) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::FED_FIRST_NAME)]);
putenv($this->mapAttribute(UserDetailsProvider::FED_LAST_NAME) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::FED_LAST_NAME)]);
putenv($this->mapAttribute(UserDetailsProvider::UDDS) . '=' . implode(",", $this->attributes[$this->mapAttribute(UserDetailsProvider::UDDS)]));
putenv($this->mapAttribute(UserDetailsProvider::FED_EMAIL) . '=' . $this->attributes[$this->mapAttribute(UserDetailsProvider::FED_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]);
} }
public function testLoadUser() public function testLoadUser()
{ {
$this->setupHttpPreauthEnvironment();
$this->userProvider = new FederatedPreauthUserDetailsProvider(true);
$user = $this->userProvider->loadUser(); $user = $this->userProvider->loadUser();
$this->assertNotNull($user); $this->assertNotNull($user);
$this->assertEquals("bbadger@wisc.edu", $user->getEppn()); $this->assertEquals("bbadger@wisc.edu", $user->getEppn());
...@@ -47,56 +57,23 @@ class FederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCas ...@@ -47,56 +57,23 @@ class FederatedPreauthUserDetailsProviderTest extends \PHPUnit_Framework_TestCas
public function testLoadUserWithNoEPPN() public function testLoadUserWithNoEPPN()
{ {
$this->setupHttpPreauthEnvironment();
$this->userProvider = new FederatedPreauthUserDetailsProvider(true);
// Clear Shib session ID to simulate no session // Clear Shib session ID to simulate no session
putenv(UserDetailsProvider::SHIB_SESSION_ID); putenv(UserDetailsProvider::SHIB_SESSION_ID_HTTP);
$user = $this->userProvider->loadUser(); $user = $this->userProvider->loadUser();
$this->assertNull($user); $this->assertNull($user);
} }
public function testLoadUserWithNoEmail() public function testLoadUserWithNoEmail()
{ {
$this->setupHttpPreauthEnvironment();
$this->userProvider = new FederatedPreauthUserDetailsProvider(true);
// Clear email to simulate no email // Clear email to simulate no email
putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_EMAIL)); putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_EMAIL));
$user = $this->userProvider->loadUser(); $user = $this->userProvider->loadUser();
$this->assertFalse($user->getEmailAddress()); $this->assertFalse($user->getEmailAddress());
} }
/** private function mapAttribute($attribute)
* Setup environment to simulate a Preauth (Shib) environment.
*/
private function setupPreauthEnvironment()
{
putenv(UserDetailsProvider::FED_EPPN . '=' . $this->attributes[UserDetailsProvider::FED_EPPN]);
putenv(UserDetailsProvider::FED_SPVI . '=' . $this->attributes[UserDetailsProvider::FED_SPVI]);
putenv(UserDetailsProvider::FED_FULLNAME . '=' . $this->attributes[UserDetailsProvider::FED_FULLNAME]);
putenv(UserDetailsProvider::FED_FIRST_NAME . '=' . $this->attributes[UserDetailsProvider::FED_FIRST_NAME]);
putenv(UserDetailsProvider::FED_LAST_NAME . '=' . $this->attributes[UserDetailsProvider::FED_LAST_NAME]);
putenv(UserDetailsProvider::UDDS . '=' . implode(",", $this->attributes[UserDetailsProvider::UDDS]));
putenv(UserDetailsProvider::FED_EMAIL . '=' . $this->attributes[UserDetailsProvider::FED_EMAIL]);
putenv(UserDetailsProvider::SOURCE . '=' . $this->attributes[UserDetailsProvider::SOURCE]);
putenv(UserDetailsProvider::ISIS_EMPLID . '=' . $this->attributes[UserDetailsProvider::ISIS_EMPLID]);
putenv(UserDetailsProvider::SHIB_SESSION_ID . '=' . $this->attributes[UserDetailsProvider::SHIB_SESSION_ID]);
}
/**
* Setup environment to simulate HTTP Preauth.
*/
private function setupHttpPreauthEnvironment()
{ {
putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_EPPN) . '=' . $this->attributes[UserDetailsProvider::FED_EPPN]); return 'HTTP_' . strtoupper($attribute);
putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_SPVI) . '=' . $this->attributes[UserDetailsProvider::FED_SPVI]);
putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_FULLNAME) . '=' . $this->attributes[UserDetailsProvider::FED_FULLNAME]);
putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_FIRST_NAME) . '=' . $this->attributes[UserDetailsProvider::FED_FIRST_NAME]);
putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_LAST_NAME) . '=' . $this->attributes[UserDetailsProvider::FED_LAST_NAME]);
putenv('HTTP_' . strtoupper(UserDetailsProvider::UDDS) . '=' . implode(",", $this->attributes[UserDetailsProvider::UDDS]));
putenv('HTTP_' . strtoupper(UserDetailsProvider::FED_EMAIL) . '=' . $this->attributes[UserDetailsProvider::FED_EMAIL]);
putenv('HTTP_' . strtoupper(UserDetailsProvider::SOURCE) . '=' . $this->attributes[UserDetailsProvider::SOURCE]);
putenv('HTTP_' . strtoupper(UserDetailsProvider::ISIS_EMPLID) . '=' . $this->attributes[UserDetailsProvider::ISIS_EMPLID]);
putenv(UserDetailsProvider::SHIB_SESSION_ID . '=' . $this->attributes[UserDetailsProvider::SHIB_SESSION_ID]);
} }
} }
src/test/edu/wisc/doit/LocalUserDetailsProviderTest.php
+ 1
− 1
View file @ 661a01b1
...@@ -10,7 +10,7 @@ class LocalUserDetailsProviderTest extends \PHPUnit_Framework_TestCase ...@@ -10,7 +10,7 @@ class LocalUserDetailsProviderTest extends \PHPUnit_Framework_TestCase
public function testLoadUser() public function testLoadUser()
{ {
$userDetailsService = new LocalUserDetailsProvider(__DIR__ . "/../../../resources/testuser.json"); $userDetailsService = new LocalUserDetailsProvider(__DIR__ . "/../../../resources/localuser.json");
$user = $userDetailsService->loadUser(); $user = $userDetailsService->loadUser();
$this->assertEquals("bbadger@wisc.edu", $user->getEppn()); $this->assertEquals("bbadger@wisc.edu", $user->getEppn());
$this->assertEquals("UW123A456", $user->getPvi()); $this->assertEquals("UW123A456", $user->getPvi());
......
src/test/resources/testuser.json → src/test/resources/localuser.json
+ 1
− 2
View file @ 661a01b1
...@@ -11,6 +11,5 @@ ...@@ -11,6 +11,5 @@
], ],
"eduWisconsinEmailAddress": "bucky.badger@wisc.edu", "eduWisconsinEmailAddress": "bucky.badger@wisc.edu",
"source": "a_source", "source": "a_source",
"isisEmplid": "123456789", "isisEmplid": "123456789"
"Shib-Session-Id": "1234567890"
} }
\ No newline at end of file
src/test/resources/testuser_http.json 0 → 100644
+ 16
− 0
View file @ 661a01b1
{
"HTTP_EPPN": "bbadger@wisc.edu",
"HTTP_EDUWISCONSINSPVI": "UW123A456",
"HTTP_CN": "BUCKINGHAM BADGER",
"HTTP_EDUWISCONSINCOMMONNAME": "BUCKINGHAM BADGER",
"HTTP_EDUWISCONSINGIVENNAME": "BUCKINGHAM",
"HTTP_EDUWISCONSINSURNAME": "BADGER",
"HTTP_UDDS": [
"UW123A456",
"UW234A567"
],
"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% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment