Rip out more refresh_token code

We might as well treat all refresh_tokens as invalid. Just return a 403 from
/tokenrefresh, so that we don't have a load of dead, untestable code hanging
around.

Still TODO: removing the table from the schema.

7 files changed, 5 insertions, 160 deletions

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index b17025c7ce..ddab210718 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -791,7 +791,7 @@ class Auth(object):
 
         Args:
             macaroon(pymacaroons.Macaroon): The macaroon to validate
-            type_string(str): The kind of token required (e.g. "access", "refresh",
+            type_string(str): The kind of token required (e.g. "access",
                               "delete_pusher")
             verify_expiry(bool): Whether to verify whether the macaroon has expired.
             user_id (str): The user_id required
@@ -820,8 +820,7 @@ 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
+        # access_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)
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 91e7e725b9..9d8e6f19bc 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -539,16 +539,6 @@ class AuthHandler(BaseHandler):
             macaroon.add_first_party_caveat(caveat)
         return macaroon.serialize()
 
-    def generate_refresh_token(self, user_id):
-        m = self._generate_base_macaroon(user_id)
-        m.add_first_party_caveat("type = refresh")
-        # Important to add a nonce, because otherwise every refresh token for a
-        # user will be the same.
-        m.add_first_party_caveat("nonce = %s" % (
-            stringutils.random_string_with_symbols(16),
-        ))
-        return m.serialize()
-
     def generate_short_term_login_token(self, user_id, duration_in_ms=(2 * 60 * 1000)):
         macaroon = self._generate_base_macaroon(user_id)
         macaroon.add_first_party_caveat("type = login")
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index bc2ec95ddd..d5e6ec8b92 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -440,8 +440,6 @@ class RegisterRestServlet(RestServlet):
         access_token = self.auth_handler.generate_access_token(
             user_id, ["guest = true"]
         )
-        # XXX the "guest" caveat is not copied by /tokenrefresh. That's ok
-        # so long as we don't return a refresh_token here.
         defer.returnValue((200, {
             "user_id": user_id,
             "device_id": device_id,
diff --git a/synapse/rest/client/v2_alpha/tokenrefresh.py b/synapse/rest/client/v2_alpha/tokenrefresh.py
index 0d312c91d4..6e76b9e9c2 100644
--- a/synapse/rest/client/v2_alpha/tokenrefresh.py
+++ b/synapse/rest/client/v2_alpha/tokenrefresh.py
@@ -15,8 +15,8 @@
 
 from twisted.internet import defer
 
-from synapse.api.errors import AuthError, StoreError, SynapseError
-from synapse.http.servlet import RestServlet, parse_json_object_from_request
+from synapse.api.errors import AuthError
+from synapse.http.servlet import RestServlet
 
 from ._base import client_v2_patterns
 
@@ -30,30 +30,10 @@ class TokenRefreshRestServlet(RestServlet):
 
     def __init__(self, hs):
         super(TokenRefreshRestServlet, self).__init__()
-        self.hs = hs
-        self.store = hs.get_datastore()
 
     @defer.inlineCallbacks
     def on_POST(self, request):
-        body = parse_json_object_from_request(request)
-        try:
-            old_refresh_token = body["refresh_token"]
-            auth_handler = self.hs.get_auth_handler()
-            refresh_result = yield self.store.exchange_refresh_token(
-                old_refresh_token, auth_handler.generate_refresh_token
-            )
-            (user_id, new_refresh_token, device_id) = refresh_result
-            new_access_token = yield auth_handler.issue_access_token(
-                user_id, device_id
-            )
-            defer.returnValue((200, {
-                "access_token": new_access_token,
-                "refresh_token": new_refresh_token,
-            }))
-        except KeyError:
-            raise SynapseError(400, "Missing required key 'refresh_token'.")
-        except StoreError:
-            raise AuthError(403, "Did not recognize refresh token")
+        raise AuthError(403, "tokenrefresh is no longer supported.")
 
 
 def register_servlets(hs, http_server):
diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py
index 9996f195a0..db146ed348 100644
--- a/synapse/storage/__init__.py
+++ b/synapse/storage/__init__.py
@@ -120,7 +120,6 @@ class DataStore(RoomMemberStore, RoomStore,
         self._transaction_id_gen = IdGenerator(db_conn, "sent_transactions", "id")
         self._state_groups_id_gen = IdGenerator(db_conn, "state_groups", "id")
         self._access_tokens_id_gen = IdGenerator(db_conn, "access_tokens", "id")
-        self._refresh_tokens_id_gen = IdGenerator(db_conn, "refresh_tokens", "id")
         self._event_reports_id_gen = IdGenerator(db_conn, "event_reports", "id")
         self._push_rule_id_gen = IdGenerator(db_conn, "push_rules", "id")
         self._push_rules_enable_id_gen = IdGenerator(db_conn, "push_rules_enable", "id")
diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py
index e404fa72de..983a8ec52b 100644
--- a/synapse/storage/registration.py
+++ b/synapse/storage/registration.py
@@ -68,31 +68,6 @@ class RegistrationStore(background_updates.BackgroundUpdateStore):
             desc="add_access_token_to_user",
         )
 
-    @defer.inlineCallbacks
-    def add_refresh_token_to_user(self, user_id, token, device_id=None):
-        """Adds a refresh token for the given user.
-
-        Args:
-            user_id (str): The user ID.
-            token (str): The new refresh token to add.
-            device_id (str): ID of the device to associate with the access
-               token
-        Raises:
-            StoreError if there was a problem adding this.
-        """
-        next_id = self._refresh_tokens_id_gen.get_next()
-
-        yield self._simple_insert(
-            "refresh_tokens",
-            {
-                "id": next_id,
-                "user_id": user_id,
-                "token": token,
-                "device_id": device_id,
-            },
-            desc="add_refresh_token_to_user",
-        )
-
     def register(self, user_id, token=None, password_hash=None,
                  was_guest=False, make_guest=False, appservice_id=None,
                  create_profile_with_localpart=None, admin=False):
@@ -353,47 +328,6 @@ class RegistrationStore(background_updates.BackgroundUpdateStore):
             token
         )
 
-    def exchange_refresh_token(self, refresh_token, token_generator):
-        """Exchange a refresh token for a new one.
-
-        Doing so invalidates the old refresh token - refresh tokens are single
-        use.
-
-        Args:
-            refresh_token (str): The refresh token of a user.
-            token_generator (fn: str -> str): Function which, when given a
-                user ID, returns a unique refresh token for that user. This
-                function must never return the same value twice.
-        Returns:
-            tuple of (user_id, new_refresh_token, device_id)
-        Raises:
-            StoreError if no user was found with that refresh token.
-        """
-        return self.runInteraction(
-            "exchange_refresh_token",
-            self._exchange_refresh_token,
-            refresh_token,
-            token_generator
-        )
-
-    def _exchange_refresh_token(self, txn, old_token, token_generator):
-        sql = "SELECT user_id, device_id FROM refresh_tokens WHERE token = ?"
-        txn.execute(sql, (old_token,))
-        rows = self.cursor_to_dict(txn)
-        if not rows:
-            raise StoreError(403, "Did not recognize refresh token")
-        user_id = rows[0]["user_id"]
-        device_id = rows[0]["device_id"]
-
-        # TODO(danielwh): Maybe perform a validation on the macaroon that
-        # macaroon.user_id == user_id.
-
-        new_token = token_generator(user_id)
-        sql = "UPDATE refresh_tokens SET token = ? WHERE token = ?"
-        txn.execute(sql, (new_token, old_token,))
-
-        return user_id, new_token, device_id
-
     @defer.inlineCallbacks
     def is_server_admin(self, user):
         res = yield self._simple_select_one_onecol(
diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py
index f7d74dea8e..db0faa7fcb 100644
--- a/tests/storage/test_registration.py
+++ b/tests/storage/test_registration.py
@@ -81,63 +81,11 @@ class RegistrationStoreTestCase(unittest.TestCase):
         self.assertTrue("token_id" in result)
 
     @defer.inlineCallbacks
-    def test_exchange_refresh_token_valid(self):
-        uid = stringutils.random_string(32)
-        device_id = stringutils.random_string(16)
-        generator = TokenGenerator()
-        last_token = generator.generate(uid)
-
-        self.db_pool.runQuery(
-            "INSERT INTO refresh_tokens(user_id, token, device_id) "
-            "VALUES(?,?,?)",
-            (uid, last_token, device_id))
-
-        (found_user_id, refresh_token, device_id) = \
-            yield self.store.exchange_refresh_token(last_token,
-                                                    generator.generate)
-        self.assertEqual(uid, found_user_id)
-
-        rows = yield self.db_pool.runQuery(
-            "SELECT token, device_id FROM refresh_tokens WHERE user_id = ?",
-            (uid, ))
-        self.assertEqual([(refresh_token, device_id)], rows)
-        # We issued token 1, then exchanged it for token 2
-        expected_refresh_token = u"%s-%d" % (uid, 2,)
-        self.assertEqual(expected_refresh_token, refresh_token)
-
-    @defer.inlineCallbacks
-    def test_exchange_refresh_token_none(self):
-        uid = stringutils.random_string(32)
-        generator = TokenGenerator()
-        last_token = generator.generate(uid)
-
-        with self.assertRaises(StoreError):
-            yield self.store.exchange_refresh_token(last_token, generator.generate)
-
-    @defer.inlineCallbacks
-    def test_exchange_refresh_token_invalid(self):
-        uid = stringutils.random_string(32)
-        generator = TokenGenerator()
-        last_token = generator.generate(uid)
-        wrong_token = "%s-wrong" % (last_token,)
-
-        self.db_pool.runQuery(
-            "INSERT INTO refresh_tokens(user_id, token) VALUES(?,?)",
-            (uid, wrong_token,))
-
-        with self.assertRaises(StoreError):
-            yield self.store.exchange_refresh_token(last_token, generator.generate)
-
-    @defer.inlineCallbacks
     def test_user_delete_access_tokens(self):
         # add some tokens
-        generator = TokenGenerator()
-        refresh_token = generator.generate(self.user_id)
         yield self.store.register(self.user_id, self.tokens[0], self.pwhash)
         yield self.store.add_access_token_to_user(self.user_id, self.tokens[1],
                                                   self.device_id)
-        yield self.store.add_refresh_token_to_user(self.user_id, refresh_token,
-                                                   self.device_id)
 
         # now delete some
         yield self.store.user_delete_access_tokens(
@@ -146,9 +94,6 @@ class RegistrationStoreTestCase(unittest.TestCase):
         # check they were deleted
         user = yield self.store.get_user_by_access_token(self.tokens[1])
         self.assertIsNone(user, "access token was not deleted by device_id")
-        with self.assertRaises(StoreError):
-            yield self.store.exchange_refresh_token(refresh_token,
-                                                    generator.generate)
 
         # check the one not associated with the device was not deleted
         user = yield self.store.get_user_by_access_token(self.tokens[0])