Remove non-functional 'expire_access_token' setting (#5782)

The `expire_access_token` didn't do what it sounded like it should do. What it
actually did was make Synapse enforce the 'time' caveat on macaroons used as
access tokens, but since our access token macaroons never contained such a
caveat, it was always a no-op.

(The code to add 'time' caveats was removed back in v0.18.5, in #1656)
This commit is contained in:
Richard van der Hoff 2019-07-30 08:25:02 +01:00 committed by GitHub
parent 865077f1d1
commit 8c97f6414c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 9 additions and 37 deletions

1
changelog.d/5782.removal Normal file
View file

@ -0,0 +1 @@
Remove non-functional 'expire_access_token' setting.

View file

@ -942,10 +942,6 @@ uploads_path: "DATADIR/uploads"
# #
# macaroon_secret_key: <PRIVATE STRING> # macaroon_secret_key: <PRIVATE STRING>
# Used to enable access token expiration.
#
#expire_access_token: False
# a secret which is used to calculate HMACs for form values, to stop # a secret which is used to calculate HMACs for form values, to stop
# falsification of values. Must be specified for the User Consent # falsification of values. Must be specified for the User Consent
# forms to work. # forms to work.

View file

@ -410,21 +410,16 @@ class Auth(object):
try: try:
user_id = self.get_user_id_from_macaroon(macaroon) user_id = self.get_user_id_from_macaroon(macaroon)
has_expiry = False
guest = False guest = False
for caveat in macaroon.caveats: for caveat in macaroon.caveats:
if caveat.caveat_id.startswith("time "): if caveat.caveat_id == "guest = true":
has_expiry = True
elif caveat.caveat_id == "guest = true":
guest = True guest = True
self.validate_macaroon( self.validate_macaroon(macaroon, rights, user_id=user_id)
macaroon, rights, self.hs.config.expire_access_token, user_id=user_id
)
except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError): except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError):
raise InvalidClientTokenError("Invalid macaroon passed.") raise InvalidClientTokenError("Invalid macaroon passed.")
if not has_expiry and rights == "access": if rights == "access":
self.token_cache[token] = (user_id, guest) self.token_cache[token] = (user_id, guest)
return user_id, guest return user_id, guest
@ -450,7 +445,7 @@ class Auth(object):
return caveat.caveat_id[len(user_prefix) :] return caveat.caveat_id[len(user_prefix) :]
raise InvalidClientTokenError("No user caveat in macaroon") raise InvalidClientTokenError("No user caveat in macaroon")
def validate_macaroon(self, macaroon, type_string, verify_expiry, user_id): def validate_macaroon(self, macaroon, type_string, user_id):
""" """
validate that a Macaroon is understood by and was signed by this server. validate that a Macaroon is understood by and was signed by this server.
@ -458,7 +453,6 @@ class Auth(object):
macaroon(pymacaroons.Macaroon): The macaroon to validate macaroon(pymacaroons.Macaroon): The macaroon to validate
type_string(str): The kind of token required (e.g. "access", type_string(str): The kind of token required (e.g. "access",
"delete_pusher") "delete_pusher")
verify_expiry(bool): Whether to verify whether the macaroon has expired.
user_id (str): The user_id required user_id (str): The user_id required
""" """
v = pymacaroons.Verifier() v = pymacaroons.Verifier()
@ -471,19 +465,7 @@ class Auth(object):
v.satisfy_exact("type = " + type_string) v.satisfy_exact("type = " + type_string)
v.satisfy_exact("user_id = %s" % user_id) v.satisfy_exact("user_id = %s" % user_id)
v.satisfy_exact("guest = true") 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) v.satisfy_general(self._verify_expiry)
else:
v.satisfy_general(lambda c: c.startswith("time < "))
# access_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.satisfy_general(lambda c: c.startswith("nonce = "))

View file

@ -116,8 +116,6 @@ class KeyConfig(Config):
seed = bytes(self.signing_key[0]) seed = bytes(self.signing_key[0])
self.macaroon_secret_key = hashlib.sha256(seed).digest() self.macaroon_secret_key = hashlib.sha256(seed).digest()
self.expire_access_token = config.get("expire_access_token", False)
# a secret which is used to calculate HMACs for form values, to stop # a secret which is used to calculate HMACs for form values, to stop
# falsification of values # falsification of values
self.form_secret = config.get("form_secret", None) self.form_secret = config.get("form_secret", None)
@ -144,10 +142,6 @@ class KeyConfig(Config):
# #
%(macaroon_secret_key)s %(macaroon_secret_key)s
# Used to enable access token expiration.
#
#expire_access_token: False
# a secret which is used to calculate HMACs for form values, to stop # a secret which is used to calculate HMACs for form values, to stop
# falsification of values. Must be specified for the User Consent # falsification of values. Must be specified for the User Consent
# forms to work. # forms to work.

View file

@ -860,7 +860,7 @@ class AuthHandler(BaseHandler):
try: try:
macaroon = pymacaroons.Macaroon.deserialize(login_token) macaroon = pymacaroons.Macaroon.deserialize(login_token)
user_id = auth_api.get_user_id_from_macaroon(macaroon) user_id = auth_api.get_user_id_from_macaroon(macaroon)
auth_api.validate_macaroon(macaroon, "login", True, user_id) auth_api.validate_macaroon(macaroon, "login", user_id)
except Exception: except Exception:
raise AuthError(403, "Invalid token", errcode=Codes.FORBIDDEN) raise AuthError(403, "Invalid token", errcode=Codes.FORBIDDEN)
self.ratelimit_login_per_account(user_id) self.ratelimit_login_per_account(user_id)

View file

@ -44,7 +44,7 @@ class RegistrationTestCase(unittest.HomeserverTestCase):
hs_config["max_mau_value"] = 50 hs_config["max_mau_value"] = 50
hs_config["limit_usage_by_mau"] = True hs_config["limit_usage_by_mau"] = True
hs = self.setup_test_homeserver(config=hs_config, expire_access_token=True) hs = self.setup_test_homeserver(config=hs_config)
return hs return hs
def prepare(self, reactor, clock, hs): def prepare(self, reactor, clock, hs):

View file

@ -36,7 +36,7 @@ class TestResourceLimitsServerNotices(unittest.HomeserverTestCase):
"room_name": "Server Notices", "room_name": "Server Notices",
} }
hs = self.setup_test_homeserver(config=hs_config, expire_access_token=True) hs = self.setup_test_homeserver(config=hs_config)
return hs return hs
def prepare(self, reactor, clock, hs): def prepare(self, reactor, clock, hs):

View file

@ -126,7 +126,6 @@ def default_config(name, parse=False):
"enable_registration": True, "enable_registration": True,
"enable_registration_captcha": False, "enable_registration_captcha": False,
"macaroon_secret_key": "not even a little secret", "macaroon_secret_key": "not even a little secret",
"expire_access_token": False,
"trusted_third_party_id_servers": [], "trusted_third_party_id_servers": [],
"room_invite_state_types": [], "room_invite_state_types": [],
"password_providers": [], "password_providers": [],