Improve check_token validation performance

This patch improves check_token validation performance by only pulling
revocation events based on the token issued_at value, taking advantage
of the table index. In this way, only a subset of relevant events will
be returned for validation.

Benchmarks can be seen at [1], but included here as well:

Time per Request for Old Method
-------------------------------
10 revokes at 7.908
100 revokes at 18.224
1,000 revokes at 110.155
10,000 revokes at 1998.220

Time per Request New Method
---------------------------
10 revokes at 17.636ms,
100 revokes at 17.279ms,
1,000 revokes at 17.370,
10,000 revokes w/all revokes issued before token at 17.263 (best case)
10,000 revokes w/all revokes after token creation 44.934ms (worst case)

[1] https://gist.github.com/csrichard1/4b7b8527ee5a6565a84956cff33cf29b

Change-Id: I9c2f067d870d542ec5909eaf8b24ded07b75f433
Partial-Bug: 1524030

keystone/revoke/backends/base.py

@@ -37,10 +37,13 @@ class RevokeDriverBase(object):
    """Interface for recording and reporting revocation events."""

    @abc.abstractmethod
    def list_events(self, last_fetch=None):
    def list_events(self, last_fetch=None, token=None):
        """return the revocation events, as a list of objects.

        :param last_fetch:   Time of last fetch.  Return all events newer.
        :param token: dictionary of values from a token, normalized for
                      differences between v2 and v3. The checked values are a
                      subset of the attributes of model.TokenEvent
        :returns: A list of keystone.revoke.model.RevokeEvent
                  newer than `last_fetch.`
                  If no last_fetch is specified, returns all events

keystone/revoke/backends/sql.py

@@ -10,6 +10,8 @@
# License for the specific language governing permissions and limitations
# under the License.

import sqlalchemy

from keystone.common import sql
from keystone.models import revoke_model
from keystone.revoke.backends import base
@@ -88,7 +90,30 @@ def _prune_expired_events(self):

            session.flush()

    def list_events(self, last_fetch=None):
    def _list_token_events(self, token):
        with sql.session_for_read() as session:
            query = session.query(RevocationEvent).filter(
                RevocationEvent.issued_before >= token['issued_at'])
            user = [RevocationEvent.user_id.is_(None)]
            proj = [RevocationEvent.project_id.is_(None)]
            audit = [RevocationEvent.audit_id.is_(None)]
            if token['user_id']:
                user.append(RevocationEvent.user_id == token['user_id'])
            if token['trustor_id']:
                user.append(RevocationEvent.user_id == token['trustor_id'])
            if token['trustee_id']:
                user.append(RevocationEvent.user_id == token['trustee_id'])
            if token['project_id']:
                proj.append(RevocationEvent.project_id == token['project_id'])
            if token['audit_id']:
                audit.append(RevocationEvent.audit_id == token['audit_id'])
            query = query.filter(sqlalchemy.and_(sqlalchemy.or_(*user),
                                                 sqlalchemy.or_(*proj),
                                                 sqlalchemy.or_(*audit)))
            events = [revoke_model.RevokeEvent(**e.to_dict()) for e in query]
            return events

    def _list_last_fetch_events(self, last_fetch=None):
        with sql.session_for_read() as session:
            query = session.query(RevocationEvent).order_by(
                RevocationEvent.revoked_at)
@@ -97,9 +122,14 @@ def list_events(self, last_fetch=None):
                query = query.filter(RevocationEvent.revoked_at > last_fetch)

            events = [revoke_model.RevokeEvent(**e.to_dict()) for e in query]

            return events

    def list_events(self, last_fetch=None, token=None):
        if token:
            return self._list_token_events(token)
        else:
            return self._list_last_fetch_events(last_fetch)

    @oslo_db_api.wrap_db_retry(retry_on_deadlock=True)
    def revoke(self, event):
        kwargs = dict()

keystone/revoke/core.py

@@ -190,17 +190,18 @@ def revoke_by_domain_role_assignment(self, domain_id, role_id):
        self.revoke(revoke_model.RevokeEvent(domain_id=domain_id,
                                             role_id=role_id))

    def check_token(self, token_values):
    def check_token(self, token):
        """Check the values from a token against the revocation list.

        :param token_values: dictionary of values from a token, normalized for
        :param token: dictionary of values from a token, normalized for
                             differences between v2 and v3. The checked values
                             are a subset of the attributes of model.TokenEvent

        :raises keystone.exception.TokenNotFound: If the token is invalid.

        """
        if revoke_model.is_revoked(self.list_events(), token_values):
        if revoke_model.is_revoked(self.driver.list_events(token=token),
                                   token):
            raise exception.TokenNotFound(_('Failed to validate token'))

    def revoke(self, event):

keystone/tests/unit/test_revoke.py

@@ -21,6 +21,7 @@
from keystone.common import utils
from keystone import exception
from keystone.models import revoke_model
from keystone.revoke.backends import sql
from keystone.tests import unit
from keystone.tests.unit import test_backend_sql
from keystone.token import provider
@@ -113,6 +114,16 @@ def _matches(event, token_values):

class RevokeTests(object):

    def _assertTokenRevoked(self, events, token_data):
        return self.assertTrue(
            revoke_model.is_revoked(events, token_data),
            'Token should be revoked')

    def _assertTokenNotRevoked(self, events, token_data):
        return self.assertFalse(
            revoke_model.is_revoked(events, token_data),
            'Token should not be revoked')

    def test_list(self):
        self.revoke_api.revoke_by_user(user_id=1)
        self.assertEqual(1, len(self.revoke_api.list_events()))
@@ -129,6 +140,204 @@ def test_list_since(self):
        self.assertEqual(0,
                         len(self.revoke_api.list_events(last_fetch=future)))

    def test_list_revoked_user(self):
        revocation_backend = sql.Revoke()
        events = []

        # This simulates creating a token for a specific user. When we revoke
        # the token we should have a single revocation event in the list. We
        # are going to assert that the token values match the only revocation
        # event in the backend.
        first_token = _sample_blank_token()
        first_token['user_id'] = uuid.uuid4().hex
        add_event(
            events, revoke_model.RevokeEvent(user_id=first_token['user_id'])
        )
        self.revoke_api.revoke_by_user(user_id=first_token['user_id'])
        self._assertTokenRevoked(events, first_token)
        self.assertEqual(
            1, len(revocation_backend.list_events(token=first_token))
        )

        # This simulates creating a separate token for a separate user. We are
        # going to revoke the token just like we did for the previous token.
        # We should have two revocation events stored in the backend but only
        # one should match the values of the second token.
        second_token = _sample_blank_token()
        second_token['user_id'] = uuid.uuid4().hex
        add_event(
            events, revoke_model.RevokeEvent(user_id=second_token['user_id'])
        )
        self.revoke_api.revoke_by_user(user_id=second_token['user_id'])
        self._assertTokenRevoked(events, second_token)
        self.assertEqual(
            1, len(revocation_backend.list_events(token=second_token))
        )
        # This simulates creating another separate token for a separate user,
        # but we're not going to issue a revocation event. Even though we have
        # two revocation events persisted in the backend, neither of them
        # should match the values of the third token. If they did - our
        # revocation event matching would be too heavy handed, which would
        # result in over-generalized revocation patterns.
        third_token = _sample_blank_token()
        third_token['user_id'] = uuid.uuid4().hex
        self._assertTokenNotRevoked(events, third_token)
        self.assertEqual(
            0, len(revocation_backend.list_events(token=third_token))
        )
        # This gets a token but overrides the user_id of the token to be None.
        # Technically this should never happen because tokens must belong to
        # a user. What we're testing here is that the two revocation events
        # we've created won't match None values for the user_id.
        fourth_token = _sample_blank_token()
        fourth_token['user_id'] = None
        self._assertTokenNotRevoked(events, fourth_token)
        self.assertEqual(
            0, len(revocation_backend.list_events(token=fourth_token))
        )

    def test_list_revoked_project(self):
        revocation_backend = sql.Revoke()
        events = []
        token = _sample_blank_token()

        # Create a token for a project, revoke token, check the token we
        # created has been revoked, and check the list returned a match for
        # the token when passed in.
        first_token = _sample_blank_token()
        first_token['project_id'] = uuid.uuid4().hex
        add_event(events, revoke_model.RevokeEvent(
            project_id=first_token['project_id']))
        revocation_backend.revoke(revoke_model.RevokeEvent(
            project_id=first_token['project_id']))
        self._assertTokenRevoked(events, first_token)
        self.assertEqual(1, len(revocation_backend.list_events(
            token=first_token)))

        # Create a second token, revoke it, check the token has been revoked,
        # and check the list to make sure that even though we now have 2
        # revoked events in the revocation list, it will only return 1 because
        # only one match for our second_token should exist
        second_token = _sample_blank_token()
        second_token['project_id'] = uuid.uuid4().hex
        add_event(events, revoke_model.RevokeEvent(
            project_id=second_token['project_id']))
        revocation_backend.revoke(revoke_model.RevokeEvent(
            project_id=second_token['project_id']))
        self._assertTokenRevoked(events, second_token)
        self.assertEqual(
            1, len(revocation_backend.list_events(token=second_token)))

        # This gets a token but overrides project_id of the token to be None.
        # We expect that since there are two events which both have populated
        # project_ids, this should not match this third_token with any other
        # event in the list so we should recieve 0
        third_token = _sample_blank_token()
        third_token['project_id'] = None
        self._assertTokenNotRevoked(events, token)
        self.assertEqual(0, len(revocation_backend.list_events(token=token)))

    def test_list_revoked_audit(self):
        revocation_backend = sql.Revoke()
        events = []

        # Create a token with audit_id set, revoke it, check it is revoked,
        # check to make sure that list_events matches the token to the event we
        # just revoked.
        first_token = _sample_blank_token()
        first_token['audit_id'] = provider.random_urlsafe_str()
        add_event(events, revoke_model.RevokeEvent(
            audit_id=first_token['audit_id']))
        self.revoke_api.revoke_by_audit_id(
            audit_id=first_token['audit_id'])
        self._assertTokenRevoked(events, first_token)
        self.assertEqual(
            1, len(revocation_backend.list_events(token=first_token)))

        # Create a second token, revoke it, check it is revoked, check to make
        # sure that list events only finds 1 match since there are 2 and they
        # dont both have different populated audit_id fields
        second_token = _sample_blank_token()
        second_token['audit_id'] = provider.random_urlsafe_str()
        add_event(events, revoke_model.RevokeEvent(
            audit_id=second_token['audit_id']))
        self.revoke_api.revoke_by_audit_id(
            audit_id=second_token['audit_id'])
        self._assertTokenRevoked(events, second_token)
        self.assertEqual(
            1, len(revocation_backend.list_events(token=second_token)))

        # Create a third token with audit_id set to None to make sure that
        # since there are no events currently revoked with audit_id None this
        # finds no matches
        third_token = _sample_blank_token()
        third_token['audit_id'] = None
        self._assertTokenNotRevoked(events, third_token)
        self.assertEqual(
            0, len(revocation_backend.list_events(token=third_token)))

    def test_list_revoked_since(self):
        revocation_backend = sql.Revoke()
        token = _sample_blank_token()
        self.revoke_api.revoke_by_user(user_id=None)
        self.revoke_api.revoke_by_user(user_id=None)
        self.assertEqual(2, len(revocation_backend.list_events(token=token)))
        future = timeutils.utcnow() + datetime.timedelta(seconds=1000)
        token['issued_at'] = future
        self.assertEqual(0, len(revocation_backend.list_events(token=token)))

    def test_list_revoked_multiple_filters(self):
        revocation_backend = sql.Revoke()
        events = []

        # create token that sets key/value filters in list_revoked
        first_token = _sample_blank_token()
        first_token['user_id'] = uuid.uuid4().hex
        first_token['project_id'] = uuid.uuid4().hex
        first_token['audit_id'] = provider.random_urlsafe_str()
        # revoke event and then verify that that there is only one revocation
        # and verify the only revoked event is the token
        add_event(events, revoke_model.RevokeEvent(
            user_id=first_token['user_id'],
            project_id=first_token['project_id'],
            audit_id=first_token['audit_id']))
        self.revoke_api.revoke(revoke_model.RevokeEvent(
            user_id=first_token['user_id'],
            project_id=first_token['project_id'],
            audit_id=first_token['audit_id']))
        self._assertTokenRevoked(events, first_token)
        self.assertEqual(
            1, len(revocation_backend.list_events(token=first_token)))
        # If a token has None values which the event contains it shouldn't
        # match and not be revoked
        second_token = _sample_blank_token()
        self._assertTokenNotRevoked(events, second_token)
        self.assertEqual(
            0, len(revocation_backend.list_events(token=second_token)))
        # If an event column and corresponding dict value don't match, Then
        # it should not add the event in the list. Demonstrate for project
        third_token = _sample_blank_token()
        third_token['project_id'] = uuid.uuid4().hex
        self._assertTokenNotRevoked(events, third_token)
        self.assertEqual(
            0, len(revocation_backend.list_events(token=third_token)))
        # A revoked event with user_id as null and token user_id non null
        # should still be return an event and be revoked if other non null
        # event fields match non null token fields
        fourth_token = _sample_blank_token()
        fourth_token['user_id'] = uuid.uuid4().hex
        fourth_token['project_id'] = uuid.uuid4().hex
        fourth_token['audit_id'] = provider.random_urlsafe_str()
        add_event(events, revoke_model.RevokeEvent(
            project_id=fourth_token['project_id'],
            audit_id=fourth_token['audit_id']))
        self.revoke_api.revoke(revoke_model.RevokeEvent(
            project_id=fourth_token['project_id'],
            audit_id=fourth_token['audit_id']))
        self._assertTokenRevoked(events, fourth_token)
        self.assertEqual(
            1, len(revocation_backend.list_events(token=fourth_token)))

    @mock.patch.object(timeutils, 'utcnow')
    def test_expired_events_are_removed(self, mock_utcnow):
        def _sample_token_values():

releasenotes/notes/bug-1524030-ccff6b0ec9d1cbf2.yaml

@@ -0,0 +1,12 @@
---
fixes:
  - >
    [`bug 1524030 <https://bugs.launchpad.net/keystone/+bug/1524030>`_]
    During token validation we've reduced the number of revocation events
    returned, only returning a subset of events relevant to the token. Thus,
    improving overall token validation performance.
other:
  - >
    The revoke backend driver interface has changed. We've added a token
    parameter to the ``list_events`` method in order to improve performance by
    reducing the number of revocation events returned during token validation.