From dd2eb49385f4b7d3bba94597a1fadb04bdeda0a4 Mon Sep 17 00:00:00 2001
From: Steven Hammerton <steven.hammerton@openmarket.com>
Date: Wed, 11 Nov 2015 11:12:35 +0000
Subject: [PATCH] Share more code between macaroon validation

---
 synapse/api/auth.py      | 19 +++++++-------
 synapse/handlers/auth.py | 55 +++++-----------------------------------
 2 files changed, 17 insertions(+), 57 deletions(-)

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 3e891a6193..7fbbd89179 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -587,7 +587,10 @@ class Auth(object):
     def _get_user_from_macaroon(self, macaroon_str):
         try:
             macaroon = pymacaroons.Macaroon.deserialize(macaroon_str)
-            self._validate_macaroon(macaroon)
+            self.validate_macaroon(
+                macaroon, "access",
+                [lambda c: c == "guest = true", lambda c: c.startswith("time < ")]
+            )
 
             user_prefix = "user_id = "
             user = None
@@ -635,26 +638,24 @@ class Auth(object):
                 errcode=Codes.UNKNOWN_TOKEN
             )
 
-    def _validate_macaroon(self, macaroon):
+    def validate_macaroon(self, macaroon, type_string, additional_validation_functions):
         v = pymacaroons.Verifier()
         v.satisfy_exact("gen = 1")
-        v.satisfy_exact("type = access")
+        v.satisfy_exact("type = " + type_string)
         v.satisfy_general(lambda c: c.startswith("user_id = "))
-        v.satisfy_general(self._verify_expiry)
-        v.satisfy_exact("guest = true")
+
+        for validation_function in additional_validation_functions:
+            v.satisfy_general(validation_function)
         v.verify(macaroon, self.hs.config.macaroon_secret_key)
 
         v = pymacaroons.Verifier()
         v.satisfy_general(self._verify_recognizes_caveats)
         v.verify(macaroon, self.hs.config.macaroon_secret_key)
 
-    def _verify_expiry(self, caveat):
+    def verify_expiry(self, caveat):
         prefix = "time < "
         if not caveat.startswith(prefix):
             return False
-        # TODO(daniel): Enable expiry check when clients actually know how to
-        # refresh tokens. (And remember to enable the tests)
-        return True
         expiry = int(caveat[len(prefix):])
         now = self.hs.get_clock().time_msec()
         return now < expiry
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 7a85883aa6..01976a5759 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -47,12 +47,6 @@ class AuthHandler(BaseHandler):
         self.bcrypt_rounds = hs.config.bcrypt_rounds
         self.sessions = {}
         self.INVALID_TOKEN_HTTP_STATUS = 401
-        self._KNOWN_CAVEAT_PREFIXES = set([
-            "gen = ",
-            "type = ",
-            "time < ",
-            "user_id = ",
-        ])
 
     @defer.inlineCallbacks
     def check_auth(self, flows, clientdict, clientip):
@@ -410,7 +404,13 @@ class AuthHandler(BaseHandler):
         return macaroon.serialize()
 
     def validate_short_term_login_token_and_get_user_id(self, login_token):
-        return self._validate_macaroon_and_get_user_id(login_token, "login", True)
+        try:
+            macaroon = pymacaroons.Macaroon.deserialize(login_token)
+            auth_api = self.hs.get_auth()
+            auth_api.validate_macaroon(macaroon, "login", [auth_api.verify_expiry])
+            return self._get_user_from_macaroon(macaroon)
+        except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError):
+            raise AuthError(401, "Invalid token", errcode=Codes.UNKNOWN_TOKEN)
 
     def _generate_base_macaroon(self, user_id):
         macaroon = pymacaroons.Macaroon(
@@ -421,30 +421,6 @@ class AuthHandler(BaseHandler):
         macaroon.add_first_party_caveat("user_id = %s" % (user_id,))
         return macaroon
 
-    def _validate_macaroon_and_get_user_id(self, macaroon_str,
-                                           macaroon_type, validate_expiry):
-        try:
-            macaroon = pymacaroons.Macaroon.deserialize(macaroon_str)
-            user_id = self._get_user_from_macaroon(macaroon)
-            v = pymacaroons.Verifier()
-            v.satisfy_exact("gen = 1")
-            v.satisfy_exact("type = " + macaroon_type)
-            v.satisfy_exact("user_id = " + user_id)
-            if validate_expiry:
-                v.satisfy_general(self._verify_expiry)
-
-            v.verify(macaroon, self.hs.config.macaroon_secret_key)
-
-            v = pymacaroons.Verifier()
-            v.satisfy_general(self._verify_recognizes_caveats)
-            v.verify(macaroon, self.hs.config.macaroon_secret_key)
-            return user_id
-        except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError):
-            raise AuthError(
-                self.INVALID_TOKEN_HTTP_STATUS, "Invalid token",
-                errcode=Codes.UNKNOWN_TOKEN
-            )
-
     def _get_user_from_macaroon(self, macaroon):
         user_prefix = "user_id = "
         for caveat in macaroon.caveats:
@@ -455,23 +431,6 @@ class AuthHandler(BaseHandler):
             errcode=Codes.UNKNOWN_TOKEN
         )
 
-    def _verify_expiry(self, caveat):
-        prefix = "time < "
-        if not caveat.startswith(prefix):
-            return False
-        expiry = int(caveat[len(prefix):])
-        now = self.hs.get_clock().time_msec()
-        return now < expiry
-
-    def _verify_recognizes_caveats(self, caveat):
-        first_space = caveat.find(" ")
-        if first_space < 0:
-            return False
-        second_space = caveat.find(" ", first_space + 1)
-        if second_space < 0:
-            return False
-        return caveat[:second_space + 1] in self._KNOWN_CAVEAT_PREFIXES
-
     @defer.inlineCallbacks
     def set_password(self, user_id, newpassword):
         password_hash = self.hash(newpassword)