[project @ Added store methods cleanupAssociations(), cleanup(), and concrete implementations]

8 files changed, 198 insertions, 87 deletions

diff --git a/Auth/OpenID/FileStore.php b/Auth/OpenID/FileStore.php
index b53b7d2..b312a66 100644
--- a/Auth/OpenID/FileStore.php
+++ b/Auth/OpenID/FileStore.php
@@ -612,6 +612,19 @@ class Auth_OpenID_FileStore extends Auth_OpenID_OpenIDStore {
     {
         return @unlink($filename);
     }
+
+    function cleanupAssociations()
+    {
+        $removed = 0;
+        foreach ($this->_allAssocs() as $pair) {
+            list($assoc_filename, $assoc) = $pair;
+            if ($assoc->getExpiresIn() == 0) {
+                $this->_removeIfPresent($assoc_filename);
+                $removed += 1;
+            }
+        }
+        return $removed;
+    }
 }
 
 ?>
diff --git a/Auth/OpenID/Interface.php b/Auth/OpenID/Interface.php
index ae86637..c7bbe14 100644
--- a/Auth/OpenID/Interface.php
+++ b/Auth/OpenID/Interface.php
@@ -65,6 +65,34 @@ class Auth_OpenID_OpenIDStore {
                       "not implemented", E_USER_ERROR);
     }
 
+    /*
+     * Remove expired associations from the store.
+     *
+     * This method is not called in the normal operation of the
+     * library.  It provides a way for store admins to keep their
+     * storage from filling up with expired data.
+     *
+     * @return the number of associations expired.
+     */
+    function cleanupAssociations()
+    {
+        trigger_error("Auth_OpenID_OpenIDStore::cleanupAssociations ".
+                      "not implemented", E_USER_ERROR);
+    }
+
+    /*
+     * Shortcut for cleanupNonces(), cleanupAssociations().
+     *
+     * This method is not called in the normal operation of the
+     * library.  It provides a way for store admins to keep their
+     * storage from filling up with expired data.
+     */
+    function cleanup()
+    {
+        return array($this->cleanupNonces(),
+                     $this->cleanupAssociations());
+    }
+
     /**
      * This method returns an Association object from storage that
      * matches the server URL and, if specified, handle. It returns
diff --git a/Auth/OpenID/MySQLStore.php b/Auth/OpenID/MySQLStore.php
index 04dded8..30e1f59 100644
--- a/Auth/OpenID/MySQLStore.php
+++ b/Auth/OpenID/MySQLStore.php
@@ -63,6 +63,9 @@ class Auth_OpenID_MySQLStore extends Auth_OpenID_SQLStore {
 
         $this->sql['clean_nonce'] =
             "DELETE FROM %s WHERE timestamp < ?";
+
+        $this->sql['clean_assoc'] =
+            "DELETE FROM %s WHERE issued + lifetime < ?";
     }
 
     /**
diff --git a/Auth/OpenID/PostgreSQLStore.php b/Auth/OpenID/PostgreSQLStore.php
index 8babed4..15c69ce 100644
--- a/Auth/OpenID/PostgreSQLStore.php
+++ b/Auth/OpenID/PostgreSQLStore.php
@@ -64,6 +64,9 @@ class Auth_OpenID_PostgreSQLStore extends Auth_OpenID_SQLStore {
 
         $this->sql['clean_nonce'] =
             "DELETE FROM %s WHERE timestamp < ?";
+
+        $this->sql['clean_assoc'] =
+            "DELETE FROM %s WHERE issued + lifetime < ?";
     }
 
     /**
diff --git a/Auth/OpenID/SQLStore.php b/Auth/OpenID/SQLStore.php
index 4192022..15f7971 100644
--- a/Auth/OpenID/SQLStore.php
+++ b/Auth/OpenID/SQLStore.php
@@ -265,7 +265,8 @@ class Auth_OpenID_SQLStore extends Auth_OpenID_OpenIDStore {
                                                     'get_assoc',
                                                     'get_assocs',
                                                     'remove_assoc',
-                                                    'get_expired')
+                                                    'get_expired',
+                                                    'clean_assoc')
                                     )
                               );
 
@@ -570,6 +571,15 @@ class Auth_OpenID_SQLStore extends Auth_OpenID_OpenIDStore {
         $this->connection->commit();
         return $num;
     }
+
+    function cleanupAssociations()
+    {
+        $this->connection->query($this->sql['clean_assoc'],
+                                 array(time()));
+        $num = $this->connection->affectedRows();
+        $this->connection->commit();
+        return $num;
+    }
 }
 
 ?>
diff --git a/Auth/OpenID/SQLiteStore.php b/Auth/OpenID/SQLiteStore.php
index a242f9b..b5fd48d 100644
--- a/Auth/OpenID/SQLiteStore.php
+++ b/Auth/OpenID/SQLiteStore.php
@@ -50,6 +50,9 @@ class Auth_OpenID_SQLiteStore extends Auth_OpenID_SQLStore {
 
         $this->sql['clean_nonce'] =
             "DELETE FROM %s WHERE timestamp < ?";
+
+        $this->sql['clean_assoc'] =
+            "DELETE FROM %s WHERE issued + lifetime < ?";
     }
 
     /**
diff --git a/Tests/Auth/OpenID/MemStore.php b/Tests/Auth/OpenID/MemStore.php
index 23edfe3..160f0b4 100644
--- a/Tests/Auth/OpenID/MemStore.php
+++ b/Tests/Auth/OpenID/MemStore.php
@@ -4,129 +4,163 @@
  * In-memory OpenID store implementation for testing only
  */
 require_once "Auth/OpenID/Interface.php";
-require_once 'Auth/OpenID/Nonce.php';
 
-class Tests_Auth_OpenID_MemStore extends Auth_OpenID_OpenIDStore {
-    var $assocs = null;
-    var $nonces = null;
-
-    function Tests_Auth_OpenID_MemStore()
+class ServerAssocs {
+    function ServerAssocs()
     {
         $this->assocs = array();
-        $this->nonces = array();
     }
 
-    function getKey($server_url, $handle)
+    function set($assoc)
+    {
+        $this->assocs[$assoc->handle] = $assoc;
+    }
+
+    function get($handle)
+    {
+        return Auth_OpenID::arrayGet($this->assocs, $handle);
+    }
+
+    function remove($handle)
     {
-        return serialize(array($server_url, $handle));
+        if (array_key_exists($handle, $this->assocs)) {
+            unset($this->assocs[$handle]);
+            return true;
+        } else {
+            return false;
+        }
     }
 
-    function getBest($assoc_list)
+    /*
+     * Returns association with the oldest issued date.
+     *
+     * or null if there are no associations.
+     */
+    function best()
     {
         $best = null;
-        foreach ($assoc_list as $assoc) {
-            if (($best === null) ||
-                ($best->issued < $assoc->issued)) {
+        foreach ($this->assocs as $handle => $assoc) {
+            if (($best === null) || ($best->issued < $assoc->issued)) {
                 $best = $assoc;
             }
         }
         return $best;
     }
 
-    function getExpired()
+    /*
+     * Remove expired associations.
+     *
+     * @return (removed associations, remaining associations)
+     */
+    function cleanup()
     {
-        $expired = array();
-        foreach ($this->assocs as $url => $assocs) {
-            $best = $this->getBest($assocs);
-            if (($best === null) ||
-                ($best->getExpiresIn() == 0)) {
-                $expired[] = $server_url;
+        $remove = array();
+        foreach ($this->assocs as $handle => $assoc) {
+            if ($assoc->getExpiresIn() == 0) {
+                $remove[] = $handle;
             }
         }
 
-        return $expired;
+        foreach ($remove as $handle) {
+            unset($this->assocs[$handle]);
+        }
+
+        return array(count($remove), count($this->assocs));
     }
+}
 
-    function getAssocPairs()
+/*
+ * In-process memory store.
+ *
+ * Use for single long-running processes.  No persistence supplied.
+ */
+class Tests_Auth_OpenID_MemStore {
+    function Tests_Auth_OpenID_MemStore()
     {
-        $pairs = array();
-        foreach ($this->assocs as $key => $assoc) {
-            list($assoc_url, $_) = unserialize($key);
-            $pairs[] = array($assoc_url, $assoc);
-        }
-        return $pairs;
+        $this->server_assocs = array();
+        $this->nonces = array();
     }
 
-    function getServerAssocs($server_url)
+    function &_getServerAssocs($server_url)
     {
-        $matches = array();
-        foreach ($this->getAssocPairs() as $pair) {
-            list($assoc_url, $assoc) = $pair;
-            if ($assoc_url == $server_url) {
-                $matches[] = $assoc;
-            }
+        if (!array_key_exists($server_url, $this->server_assocs)) {
+            $this->server_assocs[$server_url] =& new ServerAssocs();
         }
-        return $matches;
+
+        return $this->server_assocs[$server_url];
+    }
+
+    function storeAssociation($server_url, $assoc)
+    {
+        $assocs =& $this->_getServerAssocs($server_url);
+        $assocs->set($assoc);
     }
 
     function getAssociation($server_url, $handle=null)
     {
-        $assocs = $this->getServerAssocs($server_url);
+        $assocs =& $this->_getServerAssocs($server_url);
         if ($handle === null) {
-            $best = null;
-            foreach ($assocs as $assoc) {
-                if (!isset($best) ||
-                    $best->issued < $assoc->issued) {
-
-                    $best = $assoc;
-                }
-            }
-            return $best;
+            return $assocs->best();
         } else {
-            foreach ($assocs as $assoc) {
-                if ($assoc->handle == $handle) {
-                    return $assoc;
-                }
-            }
-            return null;
+            return $assocs->get($handle);
         }
     }
 
-    function storeAssociation($server_url, &$association)
-    {
-        $key = $this->getKey($server_url, $association->handle);
-        $this->assocs[$key] = $association;
-    }
-
     function removeAssociation($server_url, $handle)
     {
-        $key = $this->getKey($server_url, $handle);
-        $present = isset($this->assocs[$key]);
-        unset($this->assocs[$key]);
-        return $present;
+        $assocs =& $this->_getServerAssocs($server_url);
+        return $assocs->remove($handle);
     }
 
     function useNonce($server_url, $timestamp, $salt)
     {
-        global $Auth_OpenID_SKEW;
-
         $nonce = sprintf("%s%s%s", $server_url, $timestamp, $salt);
-
-        if ( abs($timestamp - gmmktime()) > $Auth_OpenID_SKEW ) {
-            return False;
-        }
-
         if (in_array($nonce, $this->nonces)) {
             return false;
         } else {
-            $this->nonces[] = $nonce;
+            array_push($this->nonces, $anonce);
             return true;
         }
     }
 
-    function reset()
+    function cleanupNonces()
     {
-        $this->assocs = array();
-        $this->nonces = array();
+        global $Auth_OpenID_SKEW;
+
+        $now = time();
+        $expired = array();
+        foreach ($this->nonces as $anonce) {
+            if (abs($anonce[1] - $now) > $Auth_OpenID_SKEW) {
+                // removing items while iterating over the set could
+                // be bad.
+                $expired[] = $anonce;
+            }
+        }
+
+        foreach ($expired as $anonce) {
+            unset($this->nonces[array_search($anonce, $this->nonces)]);
+        }
+
+        return count($expired);
+    }
+
+    function cleanupAssociations()
+    {
+        $remove_urls = array();
+        $removed_assocs = 0;
+        foreach ($this->server_assocs as $server_url => $assocs) {
+            list($removed, $remaining) = $assocs->cleanup();
+            $removed_assocs += $removed;
+            if (!$remaining) {
+                $remove_urls[] = $server_url;
+            }
+        }
+
+        // Remove entries from server_assocs that had none remaining.
+        foreach ($remove_urls as $server_url) {
+            unset($this->server_assocs[$server_url]);
+        }
+
+        return $removed_assocs;
     }
 }
\ No newline at end of file
diff --git a/Tests/Auth/OpenID/StoreTest.php b/Tests/Auth/OpenID/StoreTest.php
index b5b6cb1..877dc22 100644
--- a/Tests/Auth/OpenID/StoreTest.php
+++ b/Tests/Auth/OpenID/StoreTest.php
@@ -122,10 +122,8 @@ class Tests_Auth_OpenID_StoreTest extends PHPUnit_TestCase {
      *
      * OpenIDStore -> NoneType
      */
-    function _testStore($store)
+    function _testStore(&$store)
     {
-        $this->assertTrue($store->getExpired() === array());
-
         // Association functions
         $now = time();
 
@@ -281,14 +279,32 @@ explicitly');
 
         $this->_checkRetrieve($store, $server_url2, null,
                               $assoc2, "(29)");
+
+        // test expired associations
+        // assoc 1: server 1, valid
+        // assoc 2: server 1, expired
+        // assoc 3: server 2, expired
+        // assoc 4: server 3, valid
+        $assocValid1 = $this->genAssoc($now, -3600, 7200);
+        $assocValid2 = $this->genAssoc($now, -5);
+        $assocExpired1 = $this->genAssoc($now, -7200, 3600);
+        $assocExpired2 = $this->genAssoc($now, -7200, 3600);
+
+        $store->cleanupAssociations();
+        $store->storeAssociation($server_url . '1', $assocValid1);
+        $store->storeAssociation($server_url . '1', $assocExpired1);
+        $store->storeAssociation($server_url . '2', $assocExpired2);
+        $store->storeAssociation($server_url . '3', $assocValid2);
+
+        $cleaned = $store->cleanupAssociations();
+        $this->assertEquals(2, $cleaned);
     }
 
     function _checkUseNonce(&$store, $nonce, $expected, $server_url, $msg=null)
     {
         list($stamp, $salt) = Auth_OpenID_splitNonce($nonce);
         $actual = $store->useNonce($server_url, $stamp, $salt);
-        $val = ($actual && $expected) || (!$actual && !$expected);
-        $this->assertTrue($val, "_checkUseNonce failed: $msg");
+        $this->assertEquals(intval($expected), intval($actual), "_checkUseNonce failed: $server_url, $msg");
     }
 
     function _testNonce(&$store)
@@ -340,15 +356,15 @@ explicitly');
 
         $params = Auth_OpenID_splitNonce($old_nonce1);
         array_unshift($params, $server_url);
-        $this->assertTrue(call_user_func_array(array($store, 'useNonce'), $params));
+        $this->assertTrue(call_user_func_array(array(&$store, 'useNonce'), $params));
 
         $params = Auth_OpenID_splitNonce($old_nonce2);
         array_unshift($params, $server_url);
-        $this->assertTrue(call_user_func_array(array($store, 'useNonce'), $params));
+        $this->assertTrue(call_user_func_array(array(&$store, 'useNonce'), $params));
 
         $params = Auth_OpenID_splitNonce($recent_nonce);
         array_unshift($params, $server_url);
-        $this->assertTrue(call_user_func_array(array($store, 'useNonce'), $params));
+        $this->assertTrue(call_user_func_array(array(&$store, 'useNonce'), $params));
 
         $Auth_OpenID_SKEW = 3600;
         $cleaned = $store->cleanupNonces();
@@ -360,15 +376,15 @@ explicitly');
 
         $params = Auth_OpenID_splitNonce($old_nonce1);
         array_unshift($params, $server_url);
-        $this->assertTrue(call_user_func_array(array($store, 'useNonce'), $params));
+        $this->assertTrue(call_user_func_array(array(&$store, 'useNonce'), $params));
         $params = Auth_OpenID_splitNonce($old_nonce2);
         array_unshift($params, $server_url);
-        $this->assertTrue(call_user_func_array(array($store, 'useNonce'), $params));
+        $this->assertTrue(call_user_func_array(array(&$store, 'useNonce'), $params));
 
         // The recent nonce wasn't cleaned, so it should still fail.
         $params = Auth_OpenID_splitNonce($recent_nonce);
         array_unshift($params, $server_url);
-        $this->assertFalse(call_user_func_array(array($store, 'useNonce'), $params));
+        $this->assertFalse(call_user_func_array(array(&$store, 'useNonce'), $params));
 
         $Auth_OpenID_SKEW = $orig_skew;
     }
@@ -377,8 +393,9 @@ explicitly');
     {
         require_once 'Tests/Auth/OpenID/MemStore.php';
         $store = new Tests_Auth_OpenID_MemStore();
-        $this->_testStore(&$store);
-        $this->_testNonce(&$store);
+        $this->_testStore($store);
+        $this->_testNonce($store);
+        $this->_testNonceCleanup($store);
     }
 
     function test_filestore()