Add logging to processor. Issue #1

Fix some problems with incorrect identity/delegate values. Issue #2

discoverer.inc.php

@@ -63,7 +63,7 @@ class Discoverer {
 
  private $server = null;
  private $servers = array();
- private $delegate = '';
+ private $claimedId = '';
  private $identity = '';
  private $version = 1;
 
@@ -88,7 +88,7 @@ class Discoverer {
  }
 
  public function getDelegate() {
-  return $this->delegate;
+  return $this->claimedId;
  }
 
  public function getIdentity() {
@@ -154,7 +154,7 @@ class Discoverer {
  private function discover($uri) {
   Logger::log('Performing discovery for %s', $uri);
 
-  $this->delegate = $uri;
+  $this->claimedId = $uri;
   $this->server = null;
 
   if (!$this->yadisDiscover($uri)) {
@@ -220,9 +220,9 @@ class Discoverer {
     if ((String) $type == 'http://specs.openid.net/auth/2.0/server') {
      $this->version = 2;
      $this->server = (String) $service->URI;
-     $this->identity = $this->delegate = 'http://specs.openid.net/auth/2.0/identifier_select';
+     $this->identity = 'http://specs.openid.net/auth/2.0/identifier_select';
      $this->servers[] = $server = new Server($this->server, 2);
-     Logger::log('OpenID EP found (server). Server: %s, identity: %s, delegate: %s', $this->server, $this->identity, $this->delegate);
+     Logger::log('OpenID EP found (server). Server: %s, identity: %s, claimed id: %s', $this->server, $this->identity, $this->claimedId);
      $found = true;
     } else if ((String) $type == 'http://specs.openid.net/auth/2.0/signon') {
      $this->version = 2;
@@ -234,9 +234,8 @@ class Discoverer {
      } else {
       $this->identity = 'http://specs.openid.net/auth/2.0/identifier_select';
      }
-     $this->delegate = 'http://specs.openid.net/auth/2.0/identifier_select';
 
-     Logger::log('OpenID EP found (signon). Server: %s, identity: %s, delegate: %s', $this->server, $this->identity, $this->delegate);  
+     Logger::log('OpenID EP found (signon). Server: %s, identity: %s, claimed id: %s', $this->server, $this->identity, $this->claimedId); 
      $found = true;
     } else {
      $services[] = (String) $type;
@@ -344,18 +343,18 @@ class Discoverer {
    $this->servers[] = new Server($this->server, 2);
 
    if (isset($links['openid2.local_id'])) {
-    $this->delegate = $links['openid2.local_id'];
+    $this->identity = $links['openid2.local_id'];
    }
-   Logger::log('OpenID EP found. Server: %s, identity: %s, delegate: %s', $this->server, $this->identity, $this->delegate);
+   Logger::log('OpenID EP found. Server: %s, identity: %s, claimed id: %s', $this->server, $this->identity, $this->claimedId);
   } else if (isset($links['openid.server'])) {
    $this->version = 1;
    $this->server = $links['openid.server'];
    $this->servers[] = new Server($this->server, 2);
 
    if (isset($links['openid.delegate'])) {
-    $this->delegate = $links['openid.delegate'];
+    $this->claimedId = $links['openid.claimedId'];
    }
-   Logger::log('OpenID EP found. Server: %s, identity: %s, delegate: %s', $this->server, $this->identity, $this->delegate);
+   Logger::log('OpenID EP found. Server: %s, identity: %s, claimed id: %s', $this->server, $this->identity, $this->claimedId);
   }
  }
 

processor.php

@@ -22,6 +22,7 @@
  * SOFTWARE.
  */
 
+ require_once(dirname(__FILE__) . '/logging.inc.php');
  require_once(dirname(__FILE__) . '/discoverer.inc.php');
  require_once(dirname(__FILE__) . '/poster.inc.php');
  require_once(dirname(__FILE__) . '/sreg.inc.php');
@@ -60,6 +61,7 @@
  function process() {
   if (defined('OPENID_URL')) {
    // Initial authentication attempt (they just entered their identifier)
+   Logger::log('Processing authentication attempt for %s', OPENID_URL);
 
    $reqs = checkRequests();
    $disc = tryDiscovery(OPENID_URL);
@@ -87,7 +89,7 @@
    $func = 'process' . str_replace(' ', '', ucwords(str_replace('_', ' ',
 			strtolower($_REQUEST['openid_mode']))));
    if (function_exists($func)) {
-  	 call_user_func($func, checkHandleRevocation());
+    call_user_func($func, checkHandleRevocation());
    }
   }
  }
@@ -112,6 +114,8 @@
 
   } else if ($requests['count'] > OPENID_THROTTLE_NUM) {
 
+   Logger::log('Client throttled: %s requests made', $requests['count']);
+
    // More than the legal number of requests
    error('throttled', 'You are trying to authenticate too often');
 
@@ -135,11 +139,13 @@
    $disc = new Discoverer($url);
 
    if ($disc->getServer() == null) {
+    Logger::log('Couldn\'t perform discovery on %s', $url);
     error('notvalid', 'Claimed identity is not a valid identifier');
    }
 
    return $disc;
   } catch (Exception $e) {
+   Logger::log('Error during discovery on %s: %s', $url, $e->getMessage());
    error('discovery', $e->getMessage());
   }
   
@@ -194,7 +200,7 @@
    if ($valid) {
     KeyManager::removeKey($_SESSION['openid']['server'], $_REQUEST['openid_invalidate_handle']);
    } else {
-   	error('noauth', 'Provider didn\'t authenticate message');
+    error('noauth', 'Provider didn\'t authenticate message');
    }
   }
 
@@ -239,8 +245,10 @@
   * @param Boolean $valid True if the request has already been authenticated
   */
  function processPositiveResponse($valid) {
-  if ($_REQUEST['openid_identity'] != $_SESSION['openid']['delegate']) {
-   if ($_SESSION['openid']['delegate'] == 'http://specs.openid.net/auth/2.0/identifier_select') {
+  Logger::log('Positive response: identity = %s, expected = %s', $_REQUEST['openid_identity'], $_SESSION['openid']['identity']);
+
+  if ($_REQUEST['openid_identity'] != $_SESSION['openid']['identity']) {
+   if ($_SESSION['openid']['identity'] == 'http://specs.openid.net/auth/2.0/identifier_select') {
     $disc = new Discoverer($_REQUEST['openid_claimed_id'], false);
  
     if ($disc->hasServer($_SESSION['openid']['server'])) {

urlbuilder.inc.php

@@ -61,8 +61,8 @@
   public static function buildRequest($type, $base, $delegate, $identity, $returnURL, $handle, $version = 1) {
    $args = array(
     'openid.mode' => 'checkid_' . $type,
-    'openid.identity' => $delegate,
-    'openid.claimed_id' => $identity,
+    'openid.identity' => $identity,
+    'openid.claimed_id' => $delegate,
     ($version == 1 ? 'openid.trust_root' : 'openid.realm') => self::getTrustRoot($returnURL),
     'openid.return_to' => self::addArguments($returnURL,
 		array('openid.nonce' => $_SESSION['openid']['nonce']))