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] 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