Stop putting a time caveat on access tokens

The 'time' caveat on the access tokens was something of a lie, since we weren't
enforcing it; more pertinently its presence stops us ever adding useful time
caveats.

Let's move in the right direction by not lying in our caveats.

5 files changed, 12 insertions, 26 deletions

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 1ab27da941..77ff55cddf 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -810,6 +810,10 @@ class Auth(object):
         else:
             v.satisfy_general(lambda c: c.startswith("time < "))
 
+        # access_tokens and refresh_tokens include a nonce for uniqueness: any
+        # value is acceptable
+        v.satisfy_general(lambda c: c.startswith("nonce = "))
+
         v.verify(macaroon, self.hs.config.macaroon_secret_key)
 
     def _verify_expiry(self, caveat):
diff --git a/synapse/config/registration.py b/synapse/config/registration.py
index cc3f879857..87e500c97a 100644
--- a/synapse/config/registration.py
+++ b/synapse/config/registration.py
@@ -32,7 +32,6 @@ class RegistrationConfig(Config):
             )
 
         self.registration_shared_secret = config.get("registration_shared_secret")
-        self.user_creation_max_duration = int(config["user_creation_max_duration"])
 
         self.bcrypt_rounds = config.get("bcrypt_rounds", 12)
         self.trusted_third_party_id_servers = config["trusted_third_party_id_servers"]
@@ -55,11 +54,6 @@ class RegistrationConfig(Config):
         # secret, even if registration is otherwise disabled.
         registration_shared_secret: "%(registration_shared_secret)s"
 
-        # Sets the expiry for the short term user creation in
-        # milliseconds. For instance the bellow duration is two weeks
-        # in milliseconds.
-        user_creation_max_duration: 1209600000
-
         # Set the number of bcrypt rounds used to generate password hash.
         # Larger numbers increase the work factor needed to generate the hash.
         # The default number of rounds is 12.
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index a2866af431..20aaec36a4 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -538,14 +538,15 @@ class AuthHandler(BaseHandler):
                                                    device_id)
         defer.returnValue(refresh_token)
 
-    def generate_access_token(self, user_id, extra_caveats=None,
-                              duration_in_ms=(60 * 60 * 1000)):
+    def generate_access_token(self, user_id, extra_caveats=None):
         extra_caveats = extra_caveats or []
         macaroon = self._generate_base_macaroon(user_id)
         macaroon.add_first_party_caveat("type = access")
-        now = self.hs.get_clock().time_msec()
-        expiry = now + duration_in_ms
-        macaroon.add_first_party_caveat("time < %d" % (expiry,))
+        # Include a nonce, to make sure that each login gets a different
+        # access token.
+        macaroon.add_first_party_caveat("nonce = %s" % (
+            stringutils.random_string_with_symbols(16),
+        ))
         for caveat in extra_caveats:
             macaroon.add_first_party_caveat(caveat)
         return macaroon.serialize()
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 7e119f13b1..886fec8701 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -369,7 +369,7 @@ class RegistrationHandler(BaseHandler):
         defer.returnValue(data)
 
     @defer.inlineCallbacks
-    def get_or_create_user(self, requester, localpart, displayname, duration_in_ms,
+    def get_or_create_user(self, requester, localpart, displayname,
                            password_hash=None):
         """Creates a new user if the user does not exist,
         else revokes all previous access tokens and generates a new one.
@@ -399,8 +399,7 @@ class RegistrationHandler(BaseHandler):
 
         user = UserID(localpart, self.hs.hostname)
         user_id = user.to_string()
-        token = self.auth_handler().generate_access_token(
-            user_id, None, duration_in_ms)
+        token = self.auth_handler().generate_access_token(user_id)
 
         if need_register:
             yield self.store.register(
diff --git a/synapse/rest/client/v1/register.py b/synapse/rest/client/v1/register.py
index b5a76fefac..ecf7e311a9 100644
--- a/synapse/rest/client/v1/register.py
+++ b/synapse/rest/client/v1/register.py
@@ -384,7 +384,6 @@ class CreateUserRestServlet(ClientV1RestServlet):
     def __init__(self, hs):
         super(CreateUserRestServlet, self).__init__(hs)
         self.store = hs.get_datastore()
-        self.direct_user_creation_max_duration = hs.config.user_creation_max_duration
         self.handlers = hs.get_handlers()
 
     @defer.inlineCallbacks
@@ -418,18 +417,8 @@ class CreateUserRestServlet(ClientV1RestServlet):
         if "displayname" not in user_json:
             raise SynapseError(400, "Expected 'displayname' key.")
 
-        if "duration_seconds" not in user_json:
-            raise SynapseError(400, "Expected 'duration_seconds' key.")
-
         localpart = user_json["localpart"].encode("utf-8")
         displayname = user_json["displayname"].encode("utf-8")
-        duration_seconds = 0
-        try:
-            duration_seconds = int(user_json["duration_seconds"])
-        except ValueError:
-            raise SynapseError(400, "Failed to parse 'duration_seconds'")
-        if duration_seconds > self.direct_user_creation_max_duration:
-            duration_seconds = self.direct_user_creation_max_duration
         password_hash = user_json["password_hash"].encode("utf-8") \
             if user_json.get("password_hash") else None
 
@@ -438,7 +427,6 @@ class CreateUserRestServlet(ClientV1RestServlet):
             requester=requester,
             localpart=localpart,
             displayname=displayname,
-            duration_in_ms=(duration_seconds * 1000),
             password_hash=password_hash
         )