mirror of
https://mau.dev/maunium/synapse.git
synced 2024-08-27 20:52:30 +00:00
Merge pull request #4374 from matrix-org/rav/macaroon_key_fix_0.34.1
Fix spontaneous logout
This commit is contained in:
commit
3933ce9f13
1
changelog.d/4374.bugfix
Normal file
1
changelog.d/4374.bugfix
Normal file
@ -0,0 +1 @@
|
|||||||
|
Fix spontaneous logout on upgrade
|
@ -300,20 +300,28 @@ class Auth(object):
|
|||||||
Raises:
|
Raises:
|
||||||
AuthError if no user by that token exists or the token is invalid.
|
AuthError if no user by that token exists or the token is invalid.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
if rights == "access":
|
||||||
|
# first look in the database
|
||||||
|
r = yield self._look_up_user_by_access_token(token)
|
||||||
|
if r:
|
||||||
|
defer.returnValue(r)
|
||||||
|
|
||||||
|
# otherwise it needs to be a valid macaroon
|
||||||
try:
|
try:
|
||||||
user_id, guest = self._parse_and_validate_macaroon(token, rights)
|
user_id, guest = self._parse_and_validate_macaroon(token, rights)
|
||||||
except _InvalidMacaroonException:
|
|
||||||
# doesn't look like a macaroon: treat it as an opaque token which
|
|
||||||
# must be in the database.
|
|
||||||
# TODO: it would be nice to get rid of this, but apparently some
|
|
||||||
# people use access tokens which aren't macaroons
|
|
||||||
r = yield self._look_up_user_by_access_token(token)
|
|
||||||
defer.returnValue(r)
|
|
||||||
|
|
||||||
try:
|
|
||||||
user = UserID.from_string(user_id)
|
user = UserID.from_string(user_id)
|
||||||
|
|
||||||
if guest:
|
if rights == "access":
|
||||||
|
if not guest:
|
||||||
|
# non-guest access tokens must be in the database
|
||||||
|
logger.warning("Unrecognised access token - not in store.")
|
||||||
|
raise AuthError(
|
||||||
|
self.TOKEN_NOT_FOUND_HTTP_STATUS,
|
||||||
|
"Unrecognised access token.",
|
||||||
|
errcode=Codes.UNKNOWN_TOKEN,
|
||||||
|
)
|
||||||
|
|
||||||
# Guest access tokens are not stored in the database (there can
|
# Guest access tokens are not stored in the database (there can
|
||||||
# only be one access token per guest, anyway).
|
# only be one access token per guest, anyway).
|
||||||
#
|
#
|
||||||
@ -354,31 +362,15 @@ class Auth(object):
|
|||||||
"device_id": None,
|
"device_id": None,
|
||||||
}
|
}
|
||||||
else:
|
else:
|
||||||
# This codepath exists for several reasons:
|
raise RuntimeError("Unknown rights setting %s", rights)
|
||||||
# * so that we can actually return a token ID, which is used
|
|
||||||
# in some parts of the schema (where we probably ought to
|
|
||||||
# use device IDs instead)
|
|
||||||
# * the only way we currently have to invalidate an
|
|
||||||
# access_token is by removing it from the database, so we
|
|
||||||
# have to check here that it is still in the db
|
|
||||||
# * some attributes (notably device_id) aren't stored in the
|
|
||||||
# macaroon. They probably should be.
|
|
||||||
# TODO: build the dictionary from the macaroon once the
|
|
||||||
# above are fixed
|
|
||||||
ret = yield self._look_up_user_by_access_token(token)
|
|
||||||
if ret["user"] != user:
|
|
||||||
logger.error(
|
|
||||||
"Macaroon user (%s) != DB user (%s)",
|
|
||||||
user,
|
|
||||||
ret["user"]
|
|
||||||
)
|
|
||||||
raise AuthError(
|
|
||||||
self.TOKEN_NOT_FOUND_HTTP_STATUS,
|
|
||||||
"User mismatch in macaroon",
|
|
||||||
errcode=Codes.UNKNOWN_TOKEN
|
|
||||||
)
|
|
||||||
defer.returnValue(ret)
|
defer.returnValue(ret)
|
||||||
except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError):
|
except (
|
||||||
|
_InvalidMacaroonException,
|
||||||
|
pymacaroons.exceptions.MacaroonException,
|
||||||
|
TypeError,
|
||||||
|
ValueError,
|
||||||
|
) as e:
|
||||||
|
logger.warning("Invalid macaroon in auth: %s %s", type(e), e)
|
||||||
raise AuthError(
|
raise AuthError(
|
||||||
self.TOKEN_NOT_FOUND_HTTP_STATUS, "Invalid macaroon passed.",
|
self.TOKEN_NOT_FOUND_HTTP_STATUS, "Invalid macaroon passed.",
|
||||||
errcode=Codes.UNKNOWN_TOKEN
|
errcode=Codes.UNKNOWN_TOKEN
|
||||||
@ -508,11 +500,8 @@ class Auth(object):
|
|||||||
def _look_up_user_by_access_token(self, token):
|
def _look_up_user_by_access_token(self, token):
|
||||||
ret = yield self.store.get_user_by_access_token(token)
|
ret = yield self.store.get_user_by_access_token(token)
|
||||||
if not ret:
|
if not ret:
|
||||||
logger.warn("Unrecognised access token - not in store.")
|
defer.returnValue(None)
|
||||||
raise AuthError(
|
|
||||||
self.TOKEN_NOT_FOUND_HTTP_STATUS, "Unrecognised access token.",
|
|
||||||
errcode=Codes.UNKNOWN_TOKEN
|
|
||||||
)
|
|
||||||
# we use ret.get() below because *lots* of unit tests stub out
|
# we use ret.get() below because *lots* of unit tests stub out
|
||||||
# get_user_by_access_token in a way where it only returns a couple of
|
# get_user_by_access_token in a way where it only returns a couple of
|
||||||
# the fields.
|
# the fields.
|
||||||
|
@ -57,8 +57,8 @@ class KeyConfig(Config):
|
|||||||
# Unfortunately, there are people out there that don't have this
|
# Unfortunately, there are people out there that don't have this
|
||||||
# set. Lets just be "nice" and derive one from their secret key.
|
# set. Lets just be "nice" and derive one from their secret key.
|
||||||
logger.warn("Config is missing missing macaroon_secret_key")
|
logger.warn("Config is missing missing macaroon_secret_key")
|
||||||
seed = self.signing_key[0].seed
|
seed = bytes(self.signing_key[0])
|
||||||
self.macaroon_secret_key = hashlib.sha256(seed)
|
self.macaroon_secret_key = hashlib.sha256(seed).digest()
|
||||||
|
|
||||||
self.expire_access_token = config.get("expire_access_token", False)
|
self.expire_access_token = config.get("expire_access_token", False)
|
||||||
|
|
||||||
|
@ -194,8 +194,6 @@ class AuthTestCase(unittest.TestCase):
|
|||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def test_get_user_from_macaroon(self):
|
def test_get_user_from_macaroon(self):
|
||||||
# TODO(danielwh): Remove this mock when we remove the
|
|
||||||
# get_user_by_access_token fallback.
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
self.store.get_user_by_access_token = Mock(
|
||||||
return_value={"name": "@baldrick:matrix.org", "device_id": "device"}
|
return_value={"name": "@baldrick:matrix.org", "device_id": "device"}
|
||||||
)
|
)
|
||||||
@ -220,6 +218,7 @@ class AuthTestCase(unittest.TestCase):
|
|||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def test_get_guest_user_from_macaroon(self):
|
def test_get_guest_user_from_macaroon(self):
|
||||||
self.store.get_user_by_id = Mock(return_value={"is_guest": True})
|
self.store.get_user_by_id = Mock(return_value={"is_guest": True})
|
||||||
|
self.store.get_user_by_access_token = Mock(return_value=None)
|
||||||
|
|
||||||
user_id = "@baldrick:matrix.org"
|
user_id = "@baldrick:matrix.org"
|
||||||
macaroon = pymacaroons.Macaroon(
|
macaroon = pymacaroons.Macaroon(
|
||||||
@ -240,158 +239,6 @@ class AuthTestCase(unittest.TestCase):
|
|||||||
self.assertTrue(is_guest)
|
self.assertTrue(is_guest)
|
||||||
self.store.get_user_by_id.assert_called_with(user_id)
|
self.store.get_user_by_id.assert_called_with(user_id)
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
|
||||||
def test_get_user_from_macaroon_user_db_mismatch(self):
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@percy:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
user = "@baldrick:matrix.org"
|
|
||||||
macaroon = pymacaroons.Macaroon(
|
|
||||||
location=self.hs.config.server_name,
|
|
||||||
identifier="key",
|
|
||||||
key=self.hs.config.macaroon_secret_key,
|
|
||||||
)
|
|
||||||
macaroon.add_first_party_caveat("gen = 1")
|
|
||||||
macaroon.add_first_party_caveat("type = access")
|
|
||||||
macaroon.add_first_party_caveat("user_id = %s" % (user,))
|
|
||||||
with self.assertRaises(AuthError) as cm:
|
|
||||||
yield self.auth.get_user_by_access_token(macaroon.serialize())
|
|
||||||
self.assertEqual(401, cm.exception.code)
|
|
||||||
self.assertIn("User mismatch", cm.exception.msg)
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
|
||||||
def test_get_user_from_macaroon_missing_caveat(self):
|
|
||||||
# TODO(danielwh): Remove this mock when we remove the
|
|
||||||
# get_user_by_access_token fallback.
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@baldrick:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
macaroon = pymacaroons.Macaroon(
|
|
||||||
location=self.hs.config.server_name,
|
|
||||||
identifier="key",
|
|
||||||
key=self.hs.config.macaroon_secret_key,
|
|
||||||
)
|
|
||||||
macaroon.add_first_party_caveat("gen = 1")
|
|
||||||
macaroon.add_first_party_caveat("type = access")
|
|
||||||
|
|
||||||
with self.assertRaises(AuthError) as cm:
|
|
||||||
yield self.auth.get_user_by_access_token(macaroon.serialize())
|
|
||||||
self.assertEqual(401, cm.exception.code)
|
|
||||||
self.assertIn("No user caveat", cm.exception.msg)
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
|
||||||
def test_get_user_from_macaroon_wrong_key(self):
|
|
||||||
# TODO(danielwh): Remove this mock when we remove the
|
|
||||||
# get_user_by_access_token fallback.
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@baldrick:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
user = "@baldrick:matrix.org"
|
|
||||||
macaroon = pymacaroons.Macaroon(
|
|
||||||
location=self.hs.config.server_name,
|
|
||||||
identifier="key",
|
|
||||||
key=self.hs.config.macaroon_secret_key + "wrong",
|
|
||||||
)
|
|
||||||
macaroon.add_first_party_caveat("gen = 1")
|
|
||||||
macaroon.add_first_party_caveat("type = access")
|
|
||||||
macaroon.add_first_party_caveat("user_id = %s" % (user,))
|
|
||||||
|
|
||||||
with self.assertRaises(AuthError) as cm:
|
|
||||||
yield self.auth.get_user_by_access_token(macaroon.serialize())
|
|
||||||
self.assertEqual(401, cm.exception.code)
|
|
||||||
self.assertIn("Invalid macaroon", cm.exception.msg)
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
|
||||||
def test_get_user_from_macaroon_unknown_caveat(self):
|
|
||||||
# TODO(danielwh): Remove this mock when we remove the
|
|
||||||
# get_user_by_access_token fallback.
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@baldrick:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
user = "@baldrick:matrix.org"
|
|
||||||
macaroon = pymacaroons.Macaroon(
|
|
||||||
location=self.hs.config.server_name,
|
|
||||||
identifier="key",
|
|
||||||
key=self.hs.config.macaroon_secret_key,
|
|
||||||
)
|
|
||||||
macaroon.add_first_party_caveat("gen = 1")
|
|
||||||
macaroon.add_first_party_caveat("type = access")
|
|
||||||
macaroon.add_first_party_caveat("user_id = %s" % (user,))
|
|
||||||
macaroon.add_first_party_caveat("cunning > fox")
|
|
||||||
|
|
||||||
with self.assertRaises(AuthError) as cm:
|
|
||||||
yield self.auth.get_user_by_access_token(macaroon.serialize())
|
|
||||||
self.assertEqual(401, cm.exception.code)
|
|
||||||
self.assertIn("Invalid macaroon", cm.exception.msg)
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
|
||||||
def test_get_user_from_macaroon_expired(self):
|
|
||||||
# TODO(danielwh): Remove this mock when we remove the
|
|
||||||
# get_user_by_access_token fallback.
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@baldrick:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@baldrick:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
user = "@baldrick:matrix.org"
|
|
||||||
macaroon = pymacaroons.Macaroon(
|
|
||||||
location=self.hs.config.server_name,
|
|
||||||
identifier="key",
|
|
||||||
key=self.hs.config.macaroon_secret_key,
|
|
||||||
)
|
|
||||||
macaroon.add_first_party_caveat("gen = 1")
|
|
||||||
macaroon.add_first_party_caveat("type = access")
|
|
||||||
macaroon.add_first_party_caveat("user_id = %s" % (user,))
|
|
||||||
macaroon.add_first_party_caveat("time < -2000") # ms
|
|
||||||
|
|
||||||
self.hs.clock.now = 5000 # seconds
|
|
||||||
self.hs.config.expire_access_token = True
|
|
||||||
# yield self.auth.get_user_by_access_token(macaroon.serialize())
|
|
||||||
# TODO(daniel): Turn on the check that we validate expiration, when we
|
|
||||||
# validate expiration (and remove the above line, which will start
|
|
||||||
# throwing).
|
|
||||||
with self.assertRaises(AuthError) as cm:
|
|
||||||
yield self.auth.get_user_by_access_token(macaroon.serialize())
|
|
||||||
self.assertEqual(401, cm.exception.code)
|
|
||||||
self.assertIn("Invalid macaroon", cm.exception.msg)
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
|
||||||
def test_get_user_from_macaroon_with_valid_duration(self):
|
|
||||||
# TODO(danielwh): Remove this mock when we remove the
|
|
||||||
# get_user_by_access_token fallback.
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@baldrick:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
self.store.get_user_by_access_token = Mock(
|
|
||||||
return_value={"name": "@baldrick:matrix.org"}
|
|
||||||
)
|
|
||||||
|
|
||||||
user_id = "@baldrick:matrix.org"
|
|
||||||
macaroon = pymacaroons.Macaroon(
|
|
||||||
location=self.hs.config.server_name,
|
|
||||||
identifier="key",
|
|
||||||
key=self.hs.config.macaroon_secret_key,
|
|
||||||
)
|
|
||||||
macaroon.add_first_party_caveat("gen = 1")
|
|
||||||
macaroon.add_first_party_caveat("type = access")
|
|
||||||
macaroon.add_first_party_caveat("user_id = %s" % (user_id,))
|
|
||||||
macaroon.add_first_party_caveat("time < 900000000") # ms
|
|
||||||
|
|
||||||
self.hs.clock.now = 5000 # seconds
|
|
||||||
self.hs.config.expire_access_token = True
|
|
||||||
|
|
||||||
user_info = yield self.auth.get_user_by_access_token(macaroon.serialize())
|
|
||||||
user = user_info["user"]
|
|
||||||
self.assertEqual(UserID.from_string(user_id), user)
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def test_cannot_use_regular_token_as_guest(self):
|
def test_cannot_use_regular_token_as_guest(self):
|
||||||
USER_ID = "@percy:matrix.org"
|
USER_ID = "@percy:matrix.org"
|
||||||
|
Loading…
Reference in New Issue
Block a user