From 1c4f05db41eab20f8be4ac2dac0f7e86b0b7e1fd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 28 Nov 2016 09:55:21 +0000 Subject: 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. --- synapse/api/auth.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'synapse/api/auth.py') 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): -- cgit 1.4.1 From 4febfe47f03a97578e186fa6cae28c29ad8327cb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 30 Nov 2016 07:36:32 +0000 Subject: Comments Update comments in verify_macaroon --- synapse/api/auth.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'synapse/api/auth.py') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 77ff55cddf..b8c2917f21 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -790,9 +790,6 @@ class Auth(object): type_string(str): The kind of token required (e.g. "access", "refresh", "delete_pusher") verify_expiry(bool): Whether to verify whether the macaroon has expired. - This should really always be True, but there exist access tokens - in the wild which expire when they should not, so we can't - enforce expiry yet. user_id (str): The user_id required """ v = pymacaroons.Verifier() @@ -805,6 +802,15 @@ class Auth(object): v.satisfy_exact("type = " + type_string) v.satisfy_exact("user_id = %s" % user_id) v.satisfy_exact("guest = true") + + # verify_expiry should really always be True, but there exist access + # tokens in the wild which expire when they should not, so we can't + # enforce expiry yet (so we have to allow any caveat starting with + # 'time < ' in access tokens). + # + # On the other hand, short-term login tokens (as used by CAS login, for + # example) have an expiry time which we do want to enforce. + if verify_expiry: v.satisfy_general(self._verify_expiry) else: -- cgit 1.4.1