Merge branch '33-cache-policy-results' into 'master'

Resolve "Cache Policy Results"

Closes #33

See merge request BeryJu.org/passbook!18

.gitlab-ci.yml

@@ -12,6 +12,7 @@ stages:
 image: python:3.6
 services:
     - postgres:latest
+    - redis:latest
 
 variables:
     POSTGRES_DB: passbook

debian/control

@@ -8,7 +8,7 @@ Standards-Version: 3.9.6
 
 Package: passbook
 Architecture: all
-Recommends: mysql-server, rabbitmq-server
+Recommends: mysql-server, rabbitmq-server, redis-server
 Pre-Depends: adduser, libldap2-dev, libsasl2-dev
 Depends: python3 (>= 3.5) | python3.6 | python3.7, python3-pip, dbconfig-pgsql | dbconfig-no-thanks, ${misc:Depends}
 Description: Authentication Provider/Proxy supporting protocols like SAML, OAuth, LDAP and more.

debian/etc/passbook/config.yml

@@ -11,6 +11,8 @@ debug: false
 secure_proxy_header:
   HTTP_X_FORWARDED_PROTO: https
 rabbitmq: guest:guest@localhost/passbook
+redis: localhost/0
+
 # Error reporting, sends stacktrace to sentry.services.beryju.org
 error_report_enabled: true
 

helm/passbook/requirements.lock

@@ -5,5 +5,8 @@ dependencies:
 - name: postgresql
   repository: https://kubernetes-charts.storage.googleapis.com/
   version: 3.10.1
-digest: sha256:c36e054785f7d706d7d3f525eb1b167dbc89b42f84da7fc167a18bbb6542c999
-generated: 2019-03-11T20:36:35.125079+01:00
+- name: redis
+  repository: https://kubernetes-charts.storage.googleapis.com/
+  version: 5.1.0
+digest: sha256:8bf68bc928a2e3c0f05139635be05fa0840554c7bde4cecd624fac78fb5fa5a3
+generated: 2019-03-21T11:06:51.553379+01:00

helm/passbook/requirements.yaml

@@ -5,3 +5,6 @@ dependencies:
 - name: postgresql
   version: 3.10.1
   repository: https://kubernetes-charts.storage.googleapis.com/
+- name: redis
+  version: 5.1.0
+  repository: https://kubernetes-charts.storage.googleapis.com/

helm/passbook/templates/passbook-configmap.yaml

@@ -37,6 +37,7 @@ data:
     secure_proxy_header:
       HTTP_X_FORWARDED_PROTO: https
     rabbitmq: "user:{{ .Values.rabbitmq.rabbitmq.password }}@{{ .Release.Name }}-rabbitmq"
+    redis: ":{{ .Values.redis.password }}@{{ .Release.Name }}-redis-master/0"
     # Error reporting, sends stacktrace to sentry.services.beryju.org
     error_report_enabled: {{ .Values.config.error_reporting }}
 

passbook/core/policies.py

@@ -2,6 +2,7 @@
 from logging import getLogger
 
 from celery import group
+from django.core.cache import cache
 from ipware import get_client_ip
 
 from passbook.core.celery import CELERY_APP
@@ -9,6 +10,9 @@ from passbook.core.models import Policy, User
 
 LOGGER = getLogger(__name__)
 
+def _cache_key(policy, user):
+    return "%s#%s" % (policy.uuid, user.pk)
+
 @CELERY_APP.task()
 def _policy_engine_task(user_pk, policy_pk, **kwargs):
     """Task wrapper to run policy checking"""
@@ -29,58 +33,76 @@ def _policy_engine_task(user_pk, policy_pk, **kwargs):
     if policy_obj.negate:
         policy_result = not policy_result
     LOGGER.debug("Policy %r#%s got %s", policy_obj.name, policy_obj.pk.hex, policy_result)
+    cache_key = _cache_key(policy_obj, user_obj)
+    cache.set(cache_key, (policy_obj.action, policy_result, message))
+    LOGGER.debug("Cached entry as %s", cache_key)
     return policy_obj.action, policy_result, message
 
 class PolicyEngine:
     """Orchestrate policy checking, launch tasks and return result"""
 
+    __group = None
+    __cached = None
+
     policies = None
-    _group = None
-    _request = None
-    _user = None
+    __request = None
+    __user = None
 
     def __init__(self, policies):
         self.policies = policies
-        self._request = None
-        self._user = None
+        self.__request = None
+        self.__user = None
 
     def for_user(self, user):
         """Check policies for user"""
-        self._user = user
+        self.__user = user
         return self
 
     def with_request(self, request):
         """Set request"""
-        self._request = request
+        self.__request = request
         return self
 
     def build(self):
         """Build task group"""
-        if not self._user:
+        if not self.__user:
             raise ValueError("User not set.")
         signatures = []
+        cached_policies = []
         kwargs = {
-            '__password__': getattr(self._user, '__password__', None),
+            '__password__': getattr(self.__user, '__password__', None),
         }
-        if self._request:
-            kwargs['remote_ip'], _ = get_client_ip(self._request)
+        if self.__request:
+            kwargs['remote_ip'], _ = get_client_ip(self.__request)
             if not kwargs['remote_ip']:
                 kwargs['remote_ip'] = '255.255.255.255'
         for policy in self.policies:
-            signatures.append(_policy_engine_task.s(self._user.pk, policy.pk.hex, **kwargs))
-        self._group = group(signatures)()
+            cached_policy = cache.get(_cache_key(policy, self.__user), None)
+            if cached_policy:
+                LOGGER.debug("Taking result from cache for %s", policy.pk.hex)
+                cached_policies.append(cached_policy)
+            else:
+                LOGGER.debug("Evaluating policy %s", policy.pk.hex)
+                signatures.append(_policy_engine_task.s(self.__user.pk, policy.pk.hex, **kwargs))
+        # If all policies are cached, we have an empty list here.
+        if signatures:
+            self.__group = group(signatures)()
+        self.__cached = cached_policies
         return self
 
     @property
     def result(self):
         """Get policy-checking result"""
         messages = []
+        result = []
         try:
+            if self.__group:
                 # ValueError can be thrown from _policy_engine_task when user is None
-            group_result = self._group.get()
+                result += self.__group.get()
+            result += self.__cached
         except ValueError as exc:
             return False, str(exc)
-        for policy_action, policy_result, policy_message in group_result:
+        for policy_action, policy_result, policy_message in result:
             passing = (policy_action == Policy.ACTION_ALLOW and policy_result) or \
                       (policy_action == Policy.ACTION_DENY and not policy_result)
             LOGGER.debug('Action=%s, Result=%r => %r', policy_action, policy_result, passing)

passbook/core/requirements.txt

@@ -1,12 +1,13 @@
-django>=2.0
-django-model-utils
+celery
+cherrypy
+colorlog
 django-ipware
+django-model-utils
+django-redis
+django>=2.0
 djangorestframework
+idna<2.8,>=2.5
+markdown
+psycopg2
 PyYAML
 raven
-markdown
-colorlog
-celery
-psycopg2
-idna<2.8,>=2.5
-cherrypy

passbook/core/settings.py

@@ -45,6 +45,8 @@ AUTH_USER_MODEL = 'passbook_core.User'
 
 CSRF_COOKIE_NAME = 'passbook_csrf'
 SESSION_COOKIE_NAME = 'passbook_session'
+SESSION_ENGINE = "django.contrib.sessions.backends.cache"
+SESSION_CACHE_ALIAS = "default"
 LANGUAGE_COOKIE_NAME = 'passbook_language'
 
 AUTHENTICATION_BACKENDS = [
@@ -99,6 +101,16 @@ REST_FRAMEWORK = {
     ]
 }
 
+CACHES = {
+    "default": {
+        "BACKEND": "django_redis.cache.RedisCache",
+        "LOCATION": "redis://%s" % CONFIG.get('redis'),
+        "OPTIONS": {
+            "CLIENT_CLASS": "django_redis.client.DefaultClient",
+        }
+    }
+}
+
 MIDDLEWARE = [
     'django.middleware.security.SecurityMiddleware',
     'django.contrib.sessions.middleware.SessionMiddleware',

passbook/core/signals.py

@@ -1,10 +1,15 @@
 """passbook core signals"""
+from logging import getLogger
 
+from django.core.cache import cache
 from django.core.signals import Signal
+from django.db.models.signals import post_save
 from django.dispatch import receiver
 
 from passbook.core.exceptions import PasswordPolicyInvalid
 
+LOGGER = getLogger(__name__)
+
 user_signed_up = Signal(providing_args=['request', 'user'])
 invitation_created = Signal(providing_args=['request', 'invitation'])
 invitation_used = Signal(providing_args=['request', 'invitation', 'user'])
@@ -24,3 +29,14 @@ def password_policy_checker(sender, password, **kwargs):
         passing, messages = policy_engine.result
         if not passing:
             raise PasswordPolicyInvalid(*messages)
+
+@receiver(post_save)
+# pylint: disable=unused-argument
+def invalidate_policy_cache(sender, instance, **kwargs):
+    """Invalidate Policy cache when policy is updated"""
+    from passbook.core.models import Policy
+    if isinstance(instance, Policy):
+        LOGGER.debug("Invalidating cache for %s", instance.pk)
+        keys = cache.keys("%s#*" % instance.pk)
+        cache.delete_many(keys)
+        LOGGER.debug("Deleted %d keys", len(keys))

passbook/lib/default.yml

@@ -30,6 +30,7 @@ debug: false
 secure_proxy_header:
   HTTP_X_FORWARDED_PROTO: https
 rabbitmq: guest:guest@localhost/passbook
+redis: localhost/0
 # Error reporting, sends stacktrace to sentry.services.beryju.org
 error_report_enabled: true
 secret_key: 9$@r!d^1^jrn#fk#1#@ks#9&i$^s#1)_13%$rwjrhd=e8jfi_s