Remove client ip address from sessionorigin/remote-clientip

3 files changed, 51 insertions, 94 deletions

diff --git a/examples/remoteaddr_fix.php b/examples/remoteaddr_fix.php
deleted file mode 100644
index 409b4f3..0000000
--- a/examples/remoteaddr_fix.php
+++ /dev/null
@@ -1,9 +0,0 @@
-<?php
-
-// This file set $_SERVER['REMOTE_ADDR'] and should be used when testing a
-// broker on localhost with a remote server.
-//
-// Use this file by adding `-d auto_prepend_file=../remoteaddr_fix.php`.
-
-$_SERVER['REMOTE_ADDR'] = file_get_contents("http://ipecho.net/plain");
-
diff --git a/src/Broker.php b/src/Broker.php
index 4aca418..171b811 100644
--- a/src/Broker.php
+++ b/src/Broker.php
@@ -81,9 +81,9 @@ class Broker
      */
     protected function getSessionId()
     {
-        if (isset($this->token)) return null;
+        if (!isset($this->token)) return null;
 
-        $checksum = hash('sha256', 'session' . $this->token . static::getRemoteAddr() . $this->secret);
+        $checksum = hash('sha256', 'session' . $this->token . $this->secret);
         return "SSO-{$this->broker}-{$this->token}-$checksum";
     }
 
@@ -131,7 +131,7 @@ class Broker
             'command' => 'attach',
             'broker' => $this->broker,
             'token' => $this->token,
-            'checksum' => hash('sha256', 'attach' . $this->token . static::getRemoteAddr() . $this->secret)
+            'checksum' => hash('sha256', 'attach' . $this->token . $this->secret)
         ] + $_GET;
 
         return $this->url . "?" . http_build_query($data + $params);
@@ -287,15 +287,4 @@ class Broker
 
         return $this->request($method, $command, $args);
     }
-
-
-    /**
-     * Get the client IP address
-     *
-     * @return string
-     */
-    protected static function getRemoteAddr()
-    {
-        return $_SERVER['REMOTE_ADDR'];
-    }
 }
diff --git a/src/Server.php b/src/Server.php
index 87b66cc..80864d8 100644
--- a/src/Server.php
+++ b/src/Server.php
@@ -18,7 +18,7 @@ abstract class Server
      * @var array
      */
     protected $options;
-    
+
     /**
      * Cache that stores the special session data for the brokers.
      *
@@ -57,25 +57,25 @@ abstract class Server
     {
         $adapter = new Adapter\File('/tmp');
         $adapter->setOption('ttl', 10 * 3600);
-        
+
         return new Cache($adapter);
     }
-    
+
     /**
      * Start the session for broker requests to the SSO server
      */
     public function startBrokerSession()
     {
         if (isset($this->brokerId)) return;
-    
+
         if (!isset($_GET['sso_session'])) {
-            return $this->fail("No session", 400);
+            return $this->fail("Broker didn't send a session key", 400);
         }
-        
+
         $sid = $_GET['sso_session'];
-        
+
         $linkedId = $this->cache->get($sid);
-    
+
         if (!$linkedId) {
             return $this->fail("The broker session id isn't attached to a user session", 403);
         }
@@ -84,39 +84,34 @@ abstract class Server
             if ($linkedId !== session_id()) throw new \Exception("Session has already started", 400);
             return;
         }
-        
+
         session_id($linkedId);
         session_start();
 
         $this->brokerId = $this->validateBrokerSessionId($sid);
     }
-    
+
     /**
      * Validate the broker session id
-     * 
+     *
+     * @param string $sid session id
      * @return string  the broker id
      */
     protected function validateBrokerSessionId($sid)
     {
         $matches = null;
-        
+
         if (!preg_match('/^SSO-(\w*+)-(\w*+)-([a-z0-9]*+)$/', $_GET['sso_session'], $matches)) {
             return $this->fail("Invalid session id");
         }
-        
+
         $brokerId = $matches[1];
         $token = $matches[2];
-        
-        $clientAddr = $this->getSessionData('client_addr');
-        
-        if (!$clientAddr) {
-            return $this->fail("Unknown client IP address for the attached session", 403);
-        }
 
-        if ($this->generateSessionId($brokerId, $token, $clientAddr) != $sid) {
+        if ($this->generateSessionId($brokerId, $token) != $sid) {
             return $this->fail("Checksum failed: Client IP address may have changed", 403);
         }
-        
+
         return $brokerId;
     }
 
@@ -126,47 +121,40 @@ abstract class Server
     protected function startUserSession()
     {
         if (session_status() !== PHP_SESSION_ACTIVE) session_start();
+    }
 
-        $clientAddr = $this->getSessionData('client_addr');
-        
-        if ($clientAddr && $clientAddr !== $_SERVER['REMOTE_ADDR']) {
-            session_regenerate_id(true);
-        }
-        
-        if (!$clientAddr) {
-            $this->setSessionData('client_addr', static::getRemoteAddr());
-        }
-    }    
-    
     /**
      * Generate session id from session token
      *
+     * @param string $brokerId
+     * @param string $token
      * @return string
      */
-    protected function generateSessionId($brokerId, $token, $client_addr = null)
+    protected function generateSessionId($brokerId, $token)
     {
         $broker = $this->getBrokerInfo($brokerId);
 
         if (!isset($broker)) return null;
-        if (!isset($client_addr)) $client_addr = static::getRemoteAddr();
 
-        return "SSO-{$brokerId}-{$token}-" . hash('sha256', 'session' . $token . $client_addr . $broker['secret']);
+        return "SSO-{$brokerId}-{$token}-" . hash('sha256', 'session' . $token . $broker['secret']);
     }
 
     /**
      * Generate session id from session token
      *
+     * @param string $brokerId
+     * @param string $token
      * @return string
      */
     protected function generateAttachChecksum($brokerId, $token)
     {
         $broker = $this->getBrokerInfo($brokerId);
-        
+
         if (!isset($broker)) return null;
 
-        return hash('sha256', 'attach' . $token . static::getRemoteAddr() . $broker['secret']);
+        return hash('sha256', 'attach' . $token . $broker['secret']);
     }
-    
+
 
     /**
      * Detect the type for the HTTP response.
@@ -191,21 +179,21 @@ abstract class Server
     public function attach()
     {
         $this->detectReturnType();
-        
+
         if (empty($_REQUEST['broker'])) return $this->fail("No broker specified", 400);
         if (empty($_REQUEST['token'])) return $this->fail("No token specified", 400);
 
         if (!$this->returnType) return $this->fail("No return url specified", 400);
 
         $checksum = $this->generateAttachChecksum($_REQUEST['broker'], $_REQUEST['token']);
-        
+
         if (empty($_REQUEST['checksum']) || $checksum != $_REQUEST['checksum']) {
             return $this->fail("Invalid checksum", 400);
         }
 
         $this->startUserSession();
         $sid = $this->generateSessionId($_REQUEST['broker'], $_REQUEST['token']);
-        
+
         $this->cache->set($sid, $this->getSessionData('id'));
         $this->outputAttachSuccess();
     }
@@ -218,17 +206,17 @@ abstract class Server
         if ($this->returnType === 'image') {
             $this->outputImage();
         }
-        
+
         if ($this->returnType === 'json') {
-            header('Content-type: application/json; charset=UTF-8');        
+            header('Content-type: application/json; charset=UTF-8');
             echo json_encode(['success' => 'attached']);
         }
-        
+
         if ($this->returnType === 'jsonp') {
             $data = json_encode(['success' => 'attached']);
             echo $_REQUEST['callback'] . "($data, 200);";
         }
-        
+
         if ($this->returnType === 'redirect') {
             $url = $_REQUEST['return_url'];
             header("Location: $url", true, 307);
@@ -287,9 +275,9 @@ abstract class Server
     {
         $this->startBrokerSession();
         $user = null;
-        
+
         $username = $this->getSessionData('sso_user');
-        
+
         if ($username) {
             $user = $this->getUserInfo($username);
             if (!$user) return $this->fail("User not found", 500); // Shouldn't happen
@@ -299,10 +287,10 @@ abstract class Server
         echo json_encode($user);
     }
 
-    
+
     /**
      * Set session data
-     * 
+     *
      * @param string $key
      * @param string $value
      */
@@ -312,22 +300,22 @@ abstract class Server
             unset($_SESSION[$key]);
             return;
         }
-        
+
         $_SESSION[$key] = $value;
     }
-    
+
     /**
      * Get session data
-     * 
+     *
      * @param type $key
      */
     protected function getSessionData($key)
     {
         if ($key === 'id') return session_id();
-        
+
         return isset($_SESSION[$key]) ? $_SESSION[$key] : null;
     }
-    
+
 
     /**
      * An error occured.
@@ -340,9 +328,9 @@ abstract class Server
         if (!empty($this->options['fail_exception'])) {
             throw new Exception($message, $http_status);
         }
-    
+
         if ($http_status === 500) trigger_error($message, E_USER_WARNING);
-        
+
         if ($this->returnType === 'jsonp') {
             echo $_REQUEST['callback'] . "(" . json_encode(['error' => $message]) . ", $http_status);";
             exit();
@@ -354,7 +342,7 @@ abstract class Server
             echo "You're being redirected to <a href='{$url}'>$url</a>";
             exit();
         }
-        
+
         http_response_code($http_status);
         header('Content-type: application/json; charset=UTF-8');
 
@@ -362,7 +350,7 @@ abstract class Server
         exit();
     }
 
-    
+
     /**
      * Authenticate using user credentials
      *
@@ -371,7 +359,7 @@ abstract class Server
      * @return \Jasny\ValidationResult
      */
     abstract protected function authenticate($username, $password);
-    
+
     /**
      * Get the secret key and other info of a broker
      *
@@ -379,7 +367,7 @@ abstract class Server
      * @return array
      */
     abstract protected function getBrokerInfo($brokerId);
-    
+
     /**
      * Get the information about a user
      *
@@ -387,16 +375,5 @@ abstract class Server
      * @return array|object
      */
     abstract protected function getUserInfo($username);
-    
-    
-    /**
-     * Get the client IP address
-     *
-     * @return string
-     */
-    protected static function getRemoteAddr()
-    {
-        return $_SERVER['REMOTE_ADDR'];
-    }
 }