#306 Create OpenID nonce table
Opened 9 months ago by dbnicholson. Modified 4 months ago
dbnicholson/ipsilon openid-nonce-table  into  master

@@ -76,19 +76,21 @@ 

          return

  

      def _initialize_schema(self):

-         q = self._query(self._db, 'association', UNIQUE_DATA_TABLE,

-                         trans=False)

-         q.create()

+         for tablename in ['association', 'nonce']:

Why the nonce is here?

+             q = self._query(self._db, tablename, UNIQUE_DATA_TABLE,

+                             trans=False)

+             q.create()

  

      def _upgrade_schema(self, old_version):

          if old_version == 1:

              # In schema version 2, we added indexes and primary keys

              # pylint: disable=protected-access

-             table = self._query(self._db, 'association', UNIQUE_DATA_TABLE,

-                                 trans=False)._table

-             self._db.add_constraint(table.primary_key)

-             for index in table.indexes:

-                 self._db.add_index(index)

+             for tablename in ['association', 'nonce']:

+                 table = self._query(self._db, tablename, UNIQUE_DATA_TABLE,

+                                     trans=False)._table

+                 self._db.add_constraint(table.primary_key)

+                 for index in table.indexes:

+                     self._db.add_index(index)

              table = self._query(self._db, 'openid_extensions', OPTIONS_TABLE,

                                  trans=False)._table

              self._db.add_constraint(table.primary_key)

OpenIDStore registers the nonce table for automatic cleanup, but it's
never created. Even though the table isn't required, a SQL error is
logged every time automatic cleanup is triggered if it doesn't exist.

Fixes: #240

Signed-off-by: Dan Nicholson nicholson@endlessm.com

Sorry, didn't checked the issue.

Otherwise it looks good to me.

Hi,

Thanks for your patch, and I'm sorry for the delay in responding to this!
Unfortunately, since the "nonce" table wasn't created previously, we shouldn't assume it exists on database upgrades (the "dbupgrades" test shows that this fails).
Instead, this should bump the schema version for OpenID and create the nonce table on upgrade.
The creation on initialization does look fine to me.

Given the delay in my reply, I'm willing to fix the patch up if you'd prefer, just let me know.

Unfortunately, since the "nonce" table wasn't created previously, we shouldn't assume it exists on database upgrades (the "dbupgrades" test shows that this fails).
Instead, this should bump the schema version for OpenID and create the nonce table on upgrade.

Makes sense. I only see one schema version and not one specifically for OpenID. Or are you saying that I should modify the 2 to 3 upgrade so that it creates the "nonce" table?

Or are you saying that I should modify the 2 to 3 upgrade so that it creates the "nonce" table?

Oh, that wouldn't work either since schema version 2 didn't have it. So, should I bump CURRENT_SCHEMA_VERSION to 4 and add no-op 3 to 4 upgrades everywhere except in openid where it creates the nonce table?