Merge branch 'rav/macaroon_key_fix' into rav/macaroon_key_fix_0.34
This commit is contained in:
Richard van der Hoff
commit
aa70d24125
|
|
@ -300,20 +300,28 @@ class Auth(object):
|
|||
Raises:
|
||||
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:
|
||||
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)
|
||||
|
||||
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
|
||||
# only be one access token per guest, anyway).
|
||||
#
|
||||
|
|
@ -354,31 +362,15 @@ class Auth(object):
|
|||
"device_id": None,
|
||||
}
|
||||
else:
|
||||
# This codepath exists for several reasons:
|
||||
# * 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
|
||||
)
|
||||
raise RuntimeError("Unknown rights setting %s", rights)
|
||||
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(
|
||||
self.TOKEN_NOT_FOUND_HTTP_STATUS, "Invalid macaroon passed.",
|
||||
errcode=Codes.UNKNOWN_TOKEN
|
||||
|
|
@ -508,11 +500,8 @@ class Auth(object):
|
|||
def _look_up_user_by_access_token(self, token):
|
||||
ret = yield self.store.get_user_by_access_token(token)
|
||||
if not ret:
|
||||
logger.warn("Unrecognised access token - not in store.")
|
||||
raise AuthError(
|
||||
self.TOKEN_NOT_FOUND_HTTP_STATUS, "Unrecognised access token.",
|
||||
errcode=Codes.UNKNOWN_TOKEN
|
||||
)
|
||||
defer.returnValue(None)
|
||||
|
||||
# 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
|
||||
# the fields.
|
||||
|
|
|
|||
|
|
@ -57,8 +57,8 @@ class KeyConfig(Config):
|
|||
# Unfortunately, there are people out there that don't have this
|
||||
# set. Lets just be "nice" and derive one from their secret key.
|
||||
logger.warn("Config is missing missing macaroon_secret_key")
|
||||
seed = self.signing_key[0].seed
|
||||
self.macaroon_secret_key = hashlib.sha256(seed)
|
||||
seed = bytes(self.signing_key[0])
|
||||
self.macaroon_secret_key = hashlib.sha256(seed).digest()
|
||||
|
||||
self.expire_access_token = config.get("expire_access_token", False)
|
||||
|
||||
|
|
@ -71,14 +71,14 @@ class KeyConfig(Config):
|
|||
base_key_name = os.path.join(config_dir_path, server_name)
|
||||
|
||||
if is_generating_file:
|
||||
macaroon_secret_key = random_string_with_symbols(50)
|
||||
macaroon_secret_key = '"%s"' % random_string_with_symbols(50)
|
||||
form_secret = '"%s"' % random_string_with_symbols(50)
|
||||
else:
|
||||
macaroon_secret_key = None
|
||||
macaroon_secret_key = 'null'
|
||||
form_secret = 'null'
|
||||
|
||||
return """\
|
||||
macaroon_secret_key: "%(macaroon_secret_key)s"
|
||||
macaroon_secret_key: %(macaroon_secret_key)s
|
||||
|
||||
# Used to enable access token expiration.
|
||||
expire_access_token: False
|
||||
|
|
|
|||
|
|
@ -192,8 +192,6 @@ class AuthTestCase(unittest.TestCase):
|
|||
|
||||
@defer.inlineCallbacks
|
||||
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(
|
||||
return_value={"name": "@baldrick:matrix.org", "device_id": "device"}
|
||||
)
|
||||
|
|
@ -218,6 +216,7 @@ class AuthTestCase(unittest.TestCase):
|
|||
@defer.inlineCallbacks
|
||||
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_access_token = Mock(return_value=None)
|
||||
|
||||
user_id = "@baldrick:matrix.org"
|
||||
macaroon = pymacaroons.Macaroon(
|
||||
|
|
@ -238,158 +237,6 @@ class AuthTestCase(unittest.TestCase):
|
|||
self.assertTrue(is_guest)
|
||||
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
|
||||
def test_cannot_use_regular_token_as_guest(self):
|
||||
USER_ID = "@percy:matrix.org"
|
||||
|
|
|
|||
Loading…
Reference in New Issue