Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Stabilise support for MSC2918 refresh tokens as they have now been merged into the Matrix specification. #11435

Merged
merged 10 commits into from
Dec 6, 2021
1 change: 1 addition & 0 deletions changelog.d/11435.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Stabilise support for [MSC2918](https://github.com/matrix-org/matrix-doc/blob/main/proposals/2918-refreshtokens.md#msc2918-refresh-tokens) refresh tokens as they have now been merged into the Matrix specification.
38 changes: 38 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,44 @@ oembed:
#
#session_lifetime: 24h

# Time that an access token remains valid for, if the session is
# using refresh tokens.
# For more information about refresh tokens, please see the manual.
# Note that this only applies to clients which advertise support for
# refresh tokens.
#
# Note also that this is calculated at login time and refresh time:
# changes are not applied to existing sessions until they are refreshed.
#
# By default, this is 5 minutes.
#
#refreshable_access_token_lifetime: 5m

# Time that a refresh token remains valid for (provided that it is not
# exchanged for another one first).
# This option can be used to automatically log-out inactive sessions.
# Please see the manual for more information.
#
# Note also that this is calculated at login time and refresh time:
# changes are not applied to existing sessions until they are refreshed.
#
# By default, this is infinite.
#
#refresh_token_lifetime: 24h

# Time that an access token remains valid for, if the session is NOT
# using refresh tokens.
# Please note that not all clients support refresh tokens, so setting
# this to a short value may be inconvenient for some users who will
# then be logged out frequently.
#
# Note also that this is calculated at login time: changes are not applied
# retrospectively to existing sessions for users that have already logged in.
#
# By default, this is infinite.
#
#nonrefreshable_access_token_lifetime: 24h

# The user must provide all of the below types of 3PID when registering.
#
#registrations_require_3pid:
Expand Down
38 changes: 38 additions & 0 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,44 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
#
#session_lifetime: 24h

# Time that an access token remains valid for, if the session is
# using refresh tokens.
# For more information about refresh tokens, please see the manual.
# Note that this only applies to clients which advertise support for
# refresh tokens.
#
# Note also that this is calculated at login time and refresh time:
# changes are not applied to existing sessions until they are refreshed.
#
# By default, this is 5 minutes.
#
#refreshable_access_token_lifetime: 5m

# Time that a refresh token remains valid for (provided that it is not
# exchanged for another one first).
# This option can be used to automatically log-out inactive sessions.
# Please see the manual for more information.
#
# Note also that this is calculated at login time and refresh time:
# changes are not applied to existing sessions until they are refreshed.
#
# By default, this is infinite.
#
#refresh_token_lifetime: 24h

# Time that an access token remains valid for, if the session is NOT
# using refresh tokens.
# Please note that not all clients support refresh tokens, so setting
# this to a short value may be inconvenient for some users who will
# then be logged out frequently.
#
# Note also that this is calculated at login time: changes are not applied
# retrospectively to existing sessions for users that have already logged in.
#
# By default, this is infinite.
#
#nonrefreshable_access_token_lifetime: 24h

# The user must provide all of the below types of 3PID when registering.
#
#registrations_require_3pid:
Expand Down
29 changes: 13 additions & 16 deletions synapse/rest/client/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class LoginRestServlet(RestServlet):
JWT_TYPE_DEPRECATED = "m.login.jwt"
APPSERVICE_TYPE = "m.login.application_service"
APPSERVICE_TYPE_UNSTABLE = "uk.half-shot.msc2778.login.application_service"
REFRESH_TOKEN_PARAM = "org.matrix.msc2918.refresh_token"
REFRESH_TOKEN_PARAM = "refresh_token"

def __init__(self, hs: "HomeServer"):
super().__init__()
Expand All @@ -90,7 +90,7 @@ def __init__(self, hs: "HomeServer"):
self.saml2_enabled = hs.config.saml2.saml2_enabled
self.cas_enabled = hs.config.cas.cas_enabled
self.oidc_enabled = hs.config.oidc.oidc_enabled
self._msc2918_enabled = (
self._refresh_tokens_enabled = (
hs.config.registration.refreshable_access_token_lifetime is not None
)

Expand Down Expand Up @@ -163,17 +163,16 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
async def on_POST(self, request: SynapseRequest) -> Tuple[int, LoginResponse]:
login_submission = parse_json_object_from_request(request)

if self._msc2918_enabled:
# Check if this login should also issue a refresh token, as per MSC2918
should_issue_refresh_token = login_submission.get(
"org.matrix.msc2918.refresh_token", False
)
if not isinstance(should_issue_refresh_token, bool):
raise SynapseError(
400, "`org.matrix.msc2918.refresh_token` should be true or false."
)
else:
should_issue_refresh_token = False
# Check to see if the client requested a refresh token.
client_requested_refresh_token = login_submission.get(
LoginRestServlet.REFRESH_TOKEN_PARAM, False
)
if not isinstance(client_requested_refresh_token, bool):
raise SynapseError(400, "`refresh_token` should be true or false.")

should_issue_refresh_token = (
self._refresh_tokens_enabled and client_requested_refresh_token
)
Comment on lines +173 to +175
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they request a refresh token but it's not enabled on this server, are clients required to fall back to the old non-refreshable mechanism?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! That'd be the same as connecting to a server that doesn't support refresh tokens.


try:
if login_submission["type"] in (
Expand Down Expand Up @@ -460,9 +459,7 @@ def _get_auth_flow_dict_for_idp(idp: SsoIdentityProvider) -> JsonDict:


class RefreshTokenServlet(RestServlet):
PATTERNS = client_patterns(
"/org.matrix.msc2918.refresh_token/refresh$", releases=(), unstable=True
)
PATTERNS = (re.compile("^/_matrix/client/v1/refresh$"),)

def __init__(self, hs: "HomeServer"):
self._auth_handler = hs.get_auth_handler()
Expand Down
23 changes: 10 additions & 13 deletions synapse/rest/client/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ def __init__(self, hs: "HomeServer"):
self.password_policy_handler = hs.get_password_policy_handler()
self.clock = hs.get_clock()
self._registration_enabled = self.hs.config.registration.enable_registration
self._msc2918_enabled = (
self._refresh_tokens_enabled = (
hs.config.registration.refreshable_access_token_lifetime is not None
)

Expand All @@ -445,18 +445,15 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
f"Do not understand membership kind: {kind}",
)

if self._msc2918_enabled:
# Check if this registration should also issue a refresh token, as
# per MSC2918
should_issue_refresh_token = body.get(
"org.matrix.msc2918.refresh_token", False
)
if not isinstance(should_issue_refresh_token, bool):
raise SynapseError(
400, "`org.matrix.msc2918.refresh_token` should be true or false."
)
else:
should_issue_refresh_token = False
# Check if the clients wishes for this registration to issue a refresh
# token.
client_requested_refresh_tokens = body.get("refresh_token", False)
if not isinstance(client_requested_refresh_tokens, bool):
raise SynapseError(400, "`refresh_token` should be true or false.")

should_issue_refresh_token = (
self._refresh_tokens_enabled and client_requested_refresh_tokens
)

# Pull out the provided username and do basic sanity checks early since
# the auth layer will store these in sessions.
Expand Down
30 changes: 15 additions & 15 deletions tests/rest/client/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ def use_refresh_token(self, refresh_token: str) -> FakeChannel:
"""
return self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": refresh_token},
)

Expand All @@ -544,7 +544,7 @@ def test_login_issue_refresh_token(self):
login_with_refresh = self.make_request(
"POST",
"/_matrix/client/r0/login",
{"org.matrix.msc2918.refresh_token": True, **body},
{"refresh_token": True, **body},
)
self.assertEqual(login_with_refresh.code, 200, login_with_refresh.result)
self.assertIn("refresh_token", login_with_refresh.json_body)
Expand Down Expand Up @@ -575,7 +575,7 @@ def test_register_issue_refresh_token(self):
"username": "test3",
"password": self.user_pass,
"auth": {"type": LoginType.DUMMY},
"org.matrix.msc2918.refresh_token": True,
"refresh_token": True,
},
)
self.assertEqual(register_with_refresh.code, 200, register_with_refresh.result)
Expand All @@ -590,7 +590,7 @@ def test_token_refresh(self):
"type": "m.login.password",
"user": "test",
"password": self.user_pass,
"org.matrix.msc2918.refresh_token": True,
"refresh_token": True,
}
login_response = self.make_request(
"POST",
Expand All @@ -601,7 +601,7 @@ def test_token_refresh(self):

refresh_response = self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(refresh_response.code, 200, refresh_response.result)
Expand All @@ -628,7 +628,7 @@ def test_refreshable_access_token_expiration(self):
"type": "m.login.password",
"user": "test",
"password": self.user_pass,
"org.matrix.msc2918.refresh_token": True,
"refresh_token": True,
}
login_response = self.make_request(
"POST",
Expand All @@ -642,7 +642,7 @@ def test_refreshable_access_token_expiration(self):

refresh_response = self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(refresh_response.code, 200, refresh_response.result)
Expand Down Expand Up @@ -685,7 +685,7 @@ def test_refresh_token_expiry(self):
"type": "m.login.password",
"user": "test",
"password": self.user_pass,
"org.matrix.msc2918.refresh_token": True,
"refresh_token": True,
}
login_response = self.make_request(
"POST",
Expand Down Expand Up @@ -735,7 +735,7 @@ def test_ultimate_session_expiry(self):
"type": "m.login.password",
"user": "test",
"password": self.user_pass,
"org.matrix.msc2918.refresh_token": True,
"refresh_token": True,
}
login_response = self.make_request(
"POST",
Expand Down Expand Up @@ -792,7 +792,7 @@ def test_refresh_token_invalidation(self):
"type": "m.login.password",
"user": "test",
"password": self.user_pass,
"org.matrix.msc2918.refresh_token": True,
"refresh_token": True,
}
login_response = self.make_request(
"POST",
Expand All @@ -804,7 +804,7 @@ def test_refresh_token_invalidation(self):
# This first refresh should work properly
first_refresh_response = self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand All @@ -814,7 +814,7 @@ def test_refresh_token_invalidation(self):
# This one as well, since the token in the first one was never used
second_refresh_response = self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand All @@ -824,7 +824,7 @@ def test_refresh_token_invalidation(self):
# This one should not, since the token from the first refresh is not valid anymore
third_refresh_response = self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": first_refresh_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand Down Expand Up @@ -852,7 +852,7 @@ def test_refresh_token_invalidation(self):
# Now that the access token from the last valid refresh was used once, refreshing with the N-1 token should fail
fourth_refresh_response = self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": login_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand All @@ -862,7 +862,7 @@ def test_refresh_token_invalidation(self):
# But refreshing from the last valid refresh token still works
fifth_refresh_response = self.make_request(
"POST",
"/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh",
"/_matrix/client/v1/refresh",
{"refresh_token": second_refresh_response.json_body["refresh_token"]},
)
self.assertEqual(
Expand Down