launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02893
[Merge] lp:~stub/launchpad/session-prune into lp:launchpad
Stuart Bishop has proposed merging lp:~stub/launchpad/session-prune into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#357516 Session cookies are being held for too long - from a users perspective the old ones are now invalid
https://bugs.launchpad.net/bugs/357516
#729019 session pruner should be in tree
https://bugs.launchpad.net/bugs/729019
For more details, see:
https://code.launchpad.net/~stub/launchpad/session-prune/+merge/52522
If a user multiple authenticated sessions, remove all but the most recent 6.
Also, pull out extra_prune_clause from the BulkPruner, as it is currently only useful for pathalogical cases that don't really matter and it broke our end of loop detection (if all items of a batch are skipped due to the extra_prune_clause, then no rows would be deleted and the task would terminate). It would be possible to support this, but it will require refactoring and perform worse.
--
https://code.launchpad.net/~stub/launchpad/session-prune/+merge/52522
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/session-prune into lp:launchpad.
=== modified file 'database/schema/launchpad_session.sql'
--- database/schema/launchpad_session.sql 2010-09-10 09:45:45 +0000
+++ database/schema/launchpad_session.sql 2011-03-08 07:56:23 +0000
@@ -29,3 +29,28 @@
GRANT SELECT, INSERT, UPDATE, DELETE ON TimeLimitedToken TO session;
-- And the garbo needs to run on it too.
GRANT SELECT, DELETE ON TimeLimitedToken TO session;
+
+
+-- This helper needs to exist in the session database so the BulkPruner
+-- can clean up unwanted sessions.
+CREATE OR REPLACE FUNCTION cursor_fetch(cur refcursor, n integer)
+RETURNS SETOF record LANGUAGE plpgsql AS
+$$
+DECLARE
+ r record;
+ count integer;
+BEGIN
+ FOR count IN 1..n LOOP
+ FETCH FORWARD FROM cur INTO r;
+ IF NOT FOUND THEN
+ RETURN;
+ END IF;
+ RETURN NEXT r;
+ END LOOP;
+END;
+$$;
+
+COMMENT ON FUNCTION cursor_fetch(refcursor, integer) IS
+'Fetch the next n items from a cursor. Work around for not being able to use FETCH inside a SELECT statement.';
+
+GRANT EXECUTE ON FUNCTION cursor_fetch(refcursor, integer) TO session;
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2011-02-23 10:28:53 +0000
+++ lib/lp/scripts/garbo.py 2011-03-08 07:56:23 +0000
@@ -74,6 +74,7 @@
LaunchpadCronScript,
SilentLaunchpadScriptFailure,
)
+from lp.services.session.model import SessionData
from lp.translations.interfaces.potemplate import IPOTemplateSet
from lp.translations.model.potranslation import POTranslation
@@ -107,10 +108,14 @@
# from. Must be overridden.
target_table_class = None
- # The column name in target_table we use as the integer key. May be
- # overridden.
+ # The column name in target_table we use as the key. The type must
+ # match that returned by the ids_to_prune_query and the
+ # target_table_key_type. May be overridden.
target_table_key = 'id'
+ # SQL type of the target_table_key. May be overridden.
+ target_table_key_type = 'integer'
+
# An SQL query returning a list of ids to remove from target_table.
# The query must return a single column named 'id' and should not
# contain duplicates. Must be overridden.
@@ -119,10 +124,17 @@
# See `TunableLoop`. May be overridden.
maximum_chunk_size = 10000
+ def getStore(self):
+ """The master Store for the table we are pruning.
+
+ May be overridden.
+ """
+ return IMasterStore(self.target_table_class)
+
def __init__(self, log, abort_time=None):
super(BulkPruner, self).__init__(log, abort_time)
- self.store = IMasterStore(self.target_table_class)
+ self.store = self.getStore()
self.target_table_name = self.target_table_class.__storm_table__
# Open the cursor.
@@ -139,11 +151,14 @@
def __call__(self, chunk_size):
"""See `ITunableLoop`."""
result = self.store.execute("""
- DELETE FROM %s WHERE %s IN (
+ DELETE FROM %s
+ WHERE %s IN (
SELECT id FROM
- cursor_fetch('bulkprunerid', %d) AS f(id integer))
+ cursor_fetch('bulkprunerid', %d) AS f(id %s))
"""
- % (self.target_table_name, self.target_table_key, chunk_size))
+ % (
+ self.target_table_name, self.target_table_key,
+ chunk_size, self.target_table_key_type))
self._num_removed = result.rowcount
transaction.commit()
@@ -157,9 +172,7 @@
XXX bug=723596 StuartBishop: This job only needs to run once per month.
"""
-
target_table_class = POTranslation
-
ids_to_prune_query = """
SELECT POTranslation.id AS id FROM POTranslation
EXCEPT (
@@ -186,6 +199,68 @@
"""
+class SessionPruner(BulkPruner):
+ """Base class for session removal."""
+
+ target_table_class = SessionData
+ target_table_key = 'client_id'
+ target_table_key_type = 'text'
+
+
+class AntiqueSessionPruner(SessionPruner):
+ """Remove sessions not accessed for 60 days"""
+
+ ids_to_prune_query = """
+ SELECT client_id AS id FROM SessionData
+ WHERE last_accessed < CURRENT_TIMESTAMP - CAST('60 days' AS interval)
+ """
+
+
+class UnusedSessionPruner(SessionPruner):
+ """Remove sessions older than 1 day with no authentication credentials."""
+
+ ids_to_prune_query = """
+ SELECT client_id AS id FROM SessionData
+ WHERE
+ last_accessed < CURRENT_TIMESTAMP - CAST('1 day' AS interval)
+ AND client_id NOT IN (
+ SELECT client_id
+ FROM SessionPkgData
+ WHERE
+ product_id = 'launchpad.authenticateduser'
+ AND key='logintime')
+ """
+
+
+class DuplicateSessionPruner(SessionPruner):
+ """Remove all but the most recent 6 authenticated sessions for a user.
+
+ We sometimes see users with dozens or thousands of authenticated
+ sessions. To limit exposure to replay attacks, we remove all but
+ the most recent 6 of them for a given user.
+ """
+
+ ids_to_prune_query = """
+ SELECT client_id AS id
+ FROM (
+ SELECT
+ sessiondata.client_id,
+ last_accessed,
+ rank() OVER pickle AS rank
+ FROM SessionData, SessionPkgData
+ WHERE
+ SessionData.client_id = SessionPkgData.client_id
+ AND product_id = 'launchpad.authenticateduser'
+ AND key='accountid'
+ WINDOW pickle AS (PARTITION BY pickle ORDER BY last_accessed DESC)
+ ) AS whatever
+ WHERE
+ rank > 6
+ AND last_accessed < CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
+ - CAST('1 hour' AS interval)
+ """
+
+
class OAuthNoncePruner(TunableLoop):
"""An ITunableLoop to prune old OAuthNonce records.
@@ -977,6 +1052,9 @@
RevisionCachePruner,
BugHeatUpdater,
BugWatchScheduler,
+ AntiqueSessionPruner,
+ UnusedSessionPruner,
+ DuplicateSessionPruner,
]
experimental_tunable_loops = []
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2011-02-28 11:50:34 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2011-03-08 07:56:23 +0000
@@ -10,7 +10,6 @@
datetime,
timedelta,
)
-import operator
import time
from pytz import UTC
@@ -18,7 +17,10 @@
Min,
SQL,
)
-from storm.locals import Storm, Int
+from storm.locals import (
+ Int,
+ Storm,
+ )
from storm.store import Store
import transaction
from zope.component import getUtility
@@ -69,13 +71,20 @@
PersonCreationRationale,
)
from lp.scripts.garbo import (
+ AntiqueSessionPruner,
BulkPruner,
DailyDatabaseGarbageCollector,
+ DuplicateSessionPruner,
HourlyDatabaseGarbageCollector,
OpenIDConsumerAssociationPruner,
+ UnusedSessionPruner,
)
from lp.services.job.model.job import Job
from lp.services.log.logger import BufferLogger
+from lp.services.session.model import (
+ SessionData,
+ SessionPkgData,
+ )
from lp.testing import (
TestCase,
TestCaseWithFactory,
@@ -181,6 +190,153 @@
pruner(chunk_size)
+class TestSessionPruner(TestCase):
+ layer = ZopelessDatabaseLayer
+
+ def setUp(self):
+ super(TestCase, self).setUp()
+
+ # Session database isn't reset between tests. We need to do this
+ # manually.
+ nuke_all_sessions = IMasterStore(SessionData).find(SessionData).remove
+ nuke_all_sessions()
+ self.addCleanup(nuke_all_sessions)
+
+ recent = datetime.now(UTC)
+ yesterday = recent - timedelta(days=1)
+ ancient = recent - timedelta(days=61)
+
+ self.make_session(u'recent_auth', recent, 'auth1')
+ self.make_session(u'recent_unauth', recent, False)
+ self.make_session(u'yesterday_auth', yesterday, 'auth2')
+ self.make_session(u'yesterday_unauth', yesterday, False)
+ self.make_session(u'ancient_auth', ancient, 'auth3')
+ self.make_session(u'ancient_unauth', ancient, False)
+
+ def make_session(self, client_id, accessed, authenticated=None):
+ session_data = SessionData()
+ session_data.client_id = client_id
+ session_data.last_accessed = accessed
+ IMasterStore(SessionData).add(session_data)
+
+ if authenticated:
+ # Add login time information.
+ session_pkg_data = SessionPkgData()
+ session_pkg_data.client_id = client_id
+ session_pkg_data.product_id = u'launchpad.authenticateduser'
+ session_pkg_data.key = u'logintime'
+ session_pkg_data.pickle = 'value is ignored'
+ IMasterStore(SessionPkgData).add(session_pkg_data)
+
+ # Add authenticated as information.
+ session_pkg_data = SessionPkgData()
+ session_pkg_data.client_id = client_id
+ session_pkg_data.product_id = u'launchpad.authenticateduser'
+ session_pkg_data.key = u'accountid'
+ # Normally Account.id, but the session pruning works
+ # at the SQL level and doesn't unpickle anything.
+ session_pkg_data.pickle = authenticated
+ IMasterStore(SessionPkgData).add(session_pkg_data)
+
+ def sessionExists(self, client_id):
+ store = IMasterStore(SessionData)
+ return not store.find(
+ SessionData, SessionData.client_id == client_id).is_empty()
+
+ def test_antique_session_pruner(self):
+ chunk_size = 2
+ log = BufferLogger()
+ pruner = AntiqueSessionPruner(log)
+ try:
+ while not pruner.isDone():
+ pruner(chunk_size)
+ finally:
+ pruner.cleanUp()
+
+ expected_sessions = set([
+ u'recent_auth',
+ u'recent_unauth',
+ u'yesterday_auth',
+ u'yesterday_unauth',
+ # u'ancient_auth',
+ # u'ancient_unauth',
+ ])
+
+ found_sessions = set(
+ IMasterStore(SessionData).find(SessionData.client_id))
+
+ self.assertEqual(expected_sessions, found_sessions)
+
+ def test_unused_session_pruner(self):
+ chunk_size = 2
+ log = BufferLogger()
+ pruner = UnusedSessionPruner(log)
+ try:
+ while not pruner.isDone():
+ pruner(chunk_size)
+ finally:
+ pruner.cleanUp()
+
+ expected_sessions = set([
+ u'recent_auth',
+ u'recent_unauth',
+ u'yesterday_auth',
+ # u'yesterday_unauth',
+ u'ancient_auth',
+ # u'ancient_unauth',
+ ])
+
+ found_sessions = set(
+ IMasterStore(SessionData).find(SessionData.client_id))
+
+ self.assertEqual(expected_sessions, found_sessions)
+
+ def test_duplicate_session_pruner(self):
+ # None of the sessions created in setUp() are duplicates, so
+ # they will all survive the pruning.
+ expected_sessions = set([
+ u'recent_auth',
+ u'recent_unauth',
+ u'yesterday_auth',
+ u'yesterday_unauth',
+ u'ancient_auth',
+ u'ancient_unauth',
+ ])
+
+ now = datetime.now(UTC)
+
+ # Make some duplicate logins from a few days ago.
+ # Only the most recent 6 will be kept. Oldest is 'old dupe 9',
+ # most recent 'old dupe 1'.
+ for count in range(1, 10):
+ self.make_session(
+ u'old dupe %d' % count,
+ now - timedelta(days=2, seconds=count),
+ 'old dupe')
+ for count in range(1, 7):
+ expected_sessions.add(u'old dupe %d' % count)
+
+ # Make some other duplicate logins less than an hour old.
+ # All of these will be kept.
+ for count in range(1, 10):
+ self.make_session(u'new dupe %d' % count, now, 'new dupe')
+ expected_sessions.add(u'new dupe %d' % count)
+
+ chunk_size = 2
+ log = BufferLogger()
+ pruner = DuplicateSessionPruner(log)
+ try:
+ while not pruner.isDone():
+ pruner(chunk_size)
+ finally:
+ pruner.cleanUp()
+
+ found_sessions = set(
+ IMasterStore(SessionData).find(SessionData.client_id))
+
+ self.assertEqual(expected_sessions, found_sessions)
+
+
class TestGarbo(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
@@ -209,7 +365,7 @@
return collector
def test_OAuthNoncePruner(self):
- now = datetime.utcnow().replace(tzinfo=UTC)
+ now = datetime.now(UTC)
timestamps = [
now - timedelta(days=2), # Garbage
now - timedelta(days=1) - timedelta(seconds=60), # Garbage
@@ -287,7 +443,7 @@
self.failUnless(earliest >= now - 24*60*60, 'Still have old nonces')
def test_CodeImportResultPruner(self):
- now = datetime.utcnow().replace(tzinfo=UTC)
+ now = datetime.now(UTC)
store = IMasterStore(CodeImportResult)
results_to_keep_count = (
@@ -344,7 +500,7 @@
>= now - timedelta(days=30))
def test_CodeImportEventPruner(self):
- now = datetime.utcnow().replace(tzinfo=UTC)
+ now = datetime.now(UTC)
store = IMasterStore(CodeImportResult)
LaunchpadZopelessLayer.switchDbUser('testadmin')
=== modified file 'lib/lp/services/configure.zcml'
--- lib/lp/services/configure.zcml 2011-03-07 16:32:12 +0000
+++ lib/lp/services/configure.zcml 2011-03-08 07:56:23 +0000
@@ -16,5 +16,6 @@
<include package=".profile" />
<include package=".salesforce" />
<include package=".scripts" />
+ <include package=".session" />
<include package=".worlddata" />
</configure>
=== added directory 'lib/lp/services/session'
=== added file 'lib/lp/services/session/__init__.py'
=== added file 'lib/lp/services/session/adapters.py'
--- lib/lp/services/session/adapters.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/session/adapters.py 2011-03-08 07:56:23 +0000
@@ -0,0 +1,40 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Session adapters."""
+
+__metaclass__ = type
+__all__ = []
+
+
+from zope.component import adapter
+from zope.interface import implementer
+
+from canonical.database.sqlbase import session_store
+from canonical.launchpad.interfaces.lpstorm import (
+ IMasterStore,
+ ISlaveStore,
+ IStore,
+ )
+from lp.services.session.interfaces import IUseSessionStore
+
+
+@adapter(IUseSessionStore)
+@implementer(IMasterStore)
+def session_master_store(cls):
+ """Adapt a Session database object to an `IMasterStore`."""
+ return session_store()
+
+
+@adapter(IUseSessionStore)
+@implementer(ISlaveStore)
+def session_slave_store(cls):
+ """Adapt a Session database object to an `ISlaveStore`."""
+ return session_store()
+
+
+@adapter(IUseSessionStore)
+@implementer(IStore)
+def session_default_store(cls):
+ """Adapt an Session database object to an `IStore`."""
+ return session_store()
=== added file 'lib/lp/services/session/configure.zcml'
--- lib/lp/services/session/configure.zcml 1970-01-01 00:00:00 +0000
+++ lib/lp/services/session/configure.zcml 2011-03-08 07:56:23 +0000
@@ -0,0 +1,12 @@
+<!-- Copyright 2011 Canonical Ltd. This software is licensed under the
+ GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+<configure
+ xmlns="http://namespaces.zope.org/zope";
+ xmlns:browser="http://namespaces.zope.org/browser";
+ xmlns:i18n="http://namespaces.zope.org/i18n";
+ i18n_domain="launchpad">
+ <adapter factory=".adapters.session_master_store" />
+ <adapter factory=".adapters.session_slave_store" />
+ <adapter factory=".adapters.session_default_store" />
+</configure>
=== added file 'lib/lp/services/session/interfaces.py'
--- lib/lp/services/session/interfaces.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/session/interfaces.py 2011-03-08 07:56:23 +0000
@@ -0,0 +1,15 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Session interfaces."""
+
+__metaclass__ = type
+__all__ = ['IUseSessionStore']
+
+
+from zope.interface import Interface
+
+
+class IUseSessionStore(Interface):
+ """Marker interface for Session Storm database classes and instances."""
+ pass
=== added file 'lib/lp/services/session/model.py'
--- lib/lp/services/session/model.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/session/model.py 2011-03-08 07:56:23 +0000
@@ -0,0 +1,47 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Session Storm database classes"""
+
+__metaclass__ = type
+__all__ = ['SessionData', 'SessionPkgData']
+
+from storm.locals import (
+ Pickle,
+ Storm,
+ Unicode,
+ )
+from zope.interface import (
+ classProvides,
+ implements,
+ )
+
+from canonical.database.datetimecol import UtcDateTimeCol
+from lp.services.session.interfaces import IUseSessionStore
+
+
+class SessionData(Storm):
+ """A user's Session."""
+
+ classProvides(IUseSessionStore)
+ implements(IUseSessionStore)
+
+ __storm_table__ = 'SessionData'
+ client_id = Unicode(primary=True)
+ created = UtcDateTimeCol()
+ last_accessed = UtcDateTimeCol()
+
+
+class SessionPkgData(Storm):
+ """Data storage for a Session."""
+
+ classProvides(IUseSessionStore)
+ implements(IUseSessionStore)
+
+ __storm_table__ = 'SessionPkgData'
+ __storm_primary__ = 'client_id', 'product_id', 'key'
+
+ client_id = Unicode()
+ product_id = Unicode()
+ key = Unicode()
+ pickle = Pickle()
=== added directory 'lib/lp/services/session/tests'
=== added file 'lib/lp/services/session/tests/__init__.py'
=== added file 'lib/lp/services/session/tests/test_session.py'
--- lib/lp/services/session/tests/test_session.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/session/tests/test_session.py 2011-03-08 07:56:23 +0000
@@ -0,0 +1,32 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Session tests."""
+
+__metaclass__ = type
+
+from canonical.launchpad.interfaces.lpstorm import (
+ IMasterStore,
+ ISlaveStore,
+ IStore,
+ )
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.services.session.model import (
+ SessionData,
+ SessionPkgData,
+ )
+from lp.testing import TestCase
+
+
+class TestSessionModelAdapters(TestCase):
+ layer = DatabaseFunctionalLayer
+
+ def test_adapters(self):
+ for adapter in [IMasterStore, ISlaveStore, IStore]:
+ for cls in [SessionData, SessionPkgData]:
+ for obj in [cls, cls()]:
+ store = adapter(obj)
+ self.assert_(
+ 'session' in store.get_database()._dsn,
+ 'Unknown store returned adapting %r to %r'
+ % (obj, adapter))
=== modified file 'lib/lp/testing/tests/test_standard_test_template.py'
--- lib/lp/testing/tests/test_standard_test_template.py 2011-02-02 13:19:02 +0000
+++ lib/lp/testing/tests/test_standard_test_template.py 2011-03-08 07:56:23 +0000
@@ -6,15 +6,16 @@
__metaclass__ = type
from canonical.testing.layers import DatabaseFunctionalLayer
-from lp.testing import TestCase # or TestCaseWithFactory
+# or TestCaseWithFactory
+from lp.testing import TestCase
class TestSomething(TestCase):
# XXX: Sample test class. Replace with your own test class(es).
- # XXX: Optional layer--see lib/canonical/testing/layers.py
- # Get the simplest layer that your test will work on, or if you
- # don't even use the database, don't set it at all.
+ # XXX: layer--see lib/canonical/testing/layers.py
+ # Get the simplest layer that your test will work on. For unit tests
+ # requiring no resources, this is BaseLayer.
layer = DatabaseFunctionalLayer
# XXX: Sample test. Replace with your own test methods.
