From 1d8b80b3346b31a297668e093fb813d9ce7a1b48 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Fri, 26 Nov 2021 14:27:14 +0000 Subject: Support expiry of refresh tokens and expiry of the overall session when refresh tokens are in use. (#11425) --- tests/rest/client/test_auth.py | 125 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 124 insertions(+), 1 deletion(-) (limited to 'tests/rest/client/test_auth.py') diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index 8552671431..8045b7f76a 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from http import HTTPStatus from typing import Optional, Union from twisted.internet.defer import succeed @@ -513,6 +514,16 @@ class RefreshAuthTests(unittest.HomeserverTestCase): self.user_pass = "pass" self.user = self.register_user("test", self.user_pass) + def use_refresh_token(self, refresh_token: str) -> FakeChannel: + """ + Helper that makes a request to use a refresh token. + """ + return self.make_request( + "POST", + "/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh", + {"refresh_token": refresh_token}, + ) + def test_login_issue_refresh_token(self): """ A login response should include a refresh_token only if asked. @@ -599,7 +610,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): ) @override_config({"refreshable_access_token_lifetime": "1m"}) - def test_refresh_token_expiration(self): + def test_refreshable_access_token_expiration(self): """ The access token should have some time as specified in the config. """ @@ -623,6 +634,118 @@ class RefreshAuthTests(unittest.HomeserverTestCase): self.assertApproximates( refresh_response.json_body["expires_in_ms"], 60 * 1000, 100 ) + access_token = refresh_response.json_body["access_token"] + + # Advance 59 seconds in the future (just shy of 1 minute, the time of expiry) + self.reactor.advance(59.0) + # Check that our token is valid + self.assertEqual( + self.make_request( + "GET", "/_matrix/client/v3/account/whoami", access_token=access_token + ).code, + HTTPStatus.OK, + ) + + # Advance 2 more seconds (just past the time of expiry) + self.reactor.advance(2.0) + # Check that our token is invalid + self.assertEqual( + self.make_request( + "GET", "/_matrix/client/v3/account/whoami", access_token=access_token + ).code, + HTTPStatus.UNAUTHORIZED, + ) + + @override_config( + {"refreshable_access_token_lifetime": "1m", "refresh_token_lifetime": "2m"} + ) + def test_refresh_token_expiry(self): + """ + The refresh token can be configured to have a limited lifetime. + When that lifetime has ended, the refresh token can no longer be used to + refresh the session. + """ + + body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + login_response = self.make_request( + "POST", + "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", + body, + ) + self.assertEqual(login_response.code, HTTPStatus.OK, login_response.result) + refresh_token1 = login_response.json_body["refresh_token"] + + # Advance 119 seconds in the future (just shy of 2 minutes) + self.reactor.advance(119.0) + + # Refresh our session. The refresh token should still JUST be valid right now. + # By doing so, we get a new access token and a new refresh token. + refresh_response = self.use_refresh_token(refresh_token1) + self.assertEqual(refresh_response.code, HTTPStatus.OK, refresh_response.result) + self.assertIn( + "refresh_token", + refresh_response.json_body, + "No new refresh token returned after refresh.", + ) + refresh_token2 = refresh_response.json_body["refresh_token"] + + # Advance 121 seconds in the future (just a bit more than 2 minutes) + self.reactor.advance(121.0) + + # Try to refresh our session, but instead notice that the refresh token is + # not valid (it just expired). + refresh_response = self.use_refresh_token(refresh_token2) + self.assertEqual( + refresh_response.code, HTTPStatus.FORBIDDEN, refresh_response.result + ) + + @override_config( + { + "refreshable_access_token_lifetime": "2m", + "refresh_token_lifetime": "2m", + "session_lifetime": "3m", + } + ) + def test_ultimate_session_expiry(self): + """ + The session can be configured to have an ultimate, limited lifetime. + """ + + body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + login_response = self.make_request( + "POST", + "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", + body, + ) + self.assertEqual(login_response.code, 200, login_response.result) + refresh_token = login_response.json_body["refresh_token"] + + # Advance shy of 2 minutes into the future + self.reactor.advance(119.0) + + # Refresh our session. The refresh token should still be valid right now. + refresh_response = self.use_refresh_token(refresh_token) + self.assertEqual(refresh_response.code, 200, refresh_response.result) + self.assertIn( + "refresh_token", + refresh_response.json_body, + "No new refresh token returned after refresh.", + ) + # Notice that our access token lifetime has been diminished to match the + # session lifetime. + # 3 minutes - 119 seconds = 61 seconds. + self.assertEqual(refresh_response.json_body["expires_in_ms"], 61_000) + refresh_token = refresh_response.json_body["refresh_token"] + + # Advance 61 seconds into the future. Our session should have expired + # now, because we've had our 3 minutes. + self.reactor.advance(61.0) + + # Try to issue a new, refreshed, access token. + # This should fail because the refresh token's lifetime has also been + # diminished as our session expired. + refresh_response = self.use_refresh_token(refresh_token) + self.assertEqual(refresh_response.code, 403, refresh_response.result) def test_refresh_token_invalidation(self): """Refresh tokens are invalidated after first use of the next token. -- cgit 1.5.1 From 1b6691dce4ce232a7863ebc4b419e1b91ce72ed9 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Fri, 26 Nov 2021 19:06:16 +0000 Subject: Update MSC2918 refresh token support to confirm with the latest revision: accept the `refresh_tokens` parameter in the request body rather than in the URL parameters. (#11430) --- changelog.d/11430.misc | 1 + synapse/rest/client/login.py | 12 +++++---- synapse/rest/client/register.py | 9 ++++--- tests/rest/client/test_auth.py | 58 +++++++++++++++++++++++++++++++---------- 4 files changed, 58 insertions(+), 22 deletions(-) create mode 100644 changelog.d/11430.misc (limited to 'tests/rest/client/test_auth.py') diff --git a/changelog.d/11430.misc b/changelog.d/11430.misc new file mode 100644 index 0000000000..28f06f4c4e --- /dev/null +++ b/changelog.d/11430.misc @@ -0,0 +1 @@ +Update [MSC2918 refresh token](https://github.com/matrix-org/matrix-doc/blob/main/proposals/2918-refreshtokens.md#msc2918-refresh-tokens) support to confirm with the latest revision: accept the `refresh_tokens` parameter in the request body rather than in the URL parameters. \ No newline at end of file diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py index c982e54156..09f378f919 100644 --- a/synapse/rest/client/login.py +++ b/synapse/rest/client/login.py @@ -38,7 +38,6 @@ from synapse.http.server import HttpServer, finish_request from synapse.http.servlet import ( RestServlet, assert_params_in_dict, - parse_boolean, parse_bytes_from_args, parse_json_object_from_request, parse_string, @@ -165,11 +164,14 @@ class LoginRestServlet(RestServlet): 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 = parse_boolean( - request, name=LoginRestServlet.REFRESH_TOKEN_PARAM, default=False + # 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 diff --git a/synapse/rest/client/register.py b/synapse/rest/client/register.py index d2b11e39d9..11fd6cd24d 100644 --- a/synapse/rest/client/register.py +++ b/synapse/rest/client/register.py @@ -41,7 +41,6 @@ from synapse.http.server import HttpServer, finish_request, respond_with_html from synapse.http.servlet import ( RestServlet, assert_params_in_dict, - parse_boolean, parse_json_object_from_request, parse_string, ) @@ -449,9 +448,13 @@ class RegisterRestServlet(RestServlet): if self._msc2918_enabled: # Check if this registration should also issue a refresh token, as # per MSC2918 - should_issue_refresh_token = parse_boolean( - request, name="org.matrix.msc2918.refresh_token", default=False + 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 diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index 8045b7f76a..d8a94f4c12 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -529,7 +529,11 @@ class RefreshAuthTests(unittest.HomeserverTestCase): A login response should include a refresh_token only if asked. """ # Test login - body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + body = { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + } login_without_refresh = self.make_request( "POST", "/_matrix/client/r0/login", body @@ -539,8 +543,8 @@ class RefreshAuthTests(unittest.HomeserverTestCase): login_with_refresh = self.make_request( "POST", - "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", - body, + "/_matrix/client/r0/login", + {"org.matrix.msc2918.refresh_token": True, **body}, ) self.assertEqual(login_with_refresh.code, 200, login_with_refresh.result) self.assertIn("refresh_token", login_with_refresh.json_body) @@ -566,11 +570,12 @@ class RefreshAuthTests(unittest.HomeserverTestCase): register_with_refresh = self.make_request( "POST", - "/_matrix/client/r0/register?org.matrix.msc2918.refresh_token=true", + "/_matrix/client/r0/register", { "username": "test3", "password": self.user_pass, "auth": {"type": LoginType.DUMMY}, + "org.matrix.msc2918.refresh_token": True, }, ) self.assertEqual(register_with_refresh.code, 200, register_with_refresh.result) @@ -581,10 +586,15 @@ class RefreshAuthTests(unittest.HomeserverTestCase): """ A refresh token can be used to issue a new access token. """ - body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + body = { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + "org.matrix.msc2918.refresh_token": True, + } login_response = self.make_request( "POST", - "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", + "/_matrix/client/r0/login", body, ) self.assertEqual(login_response.code, 200, login_response.result) @@ -614,10 +624,15 @@ class RefreshAuthTests(unittest.HomeserverTestCase): """ The access token should have some time as specified in the config. """ - body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + body = { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + "org.matrix.msc2918.refresh_token": True, + } login_response = self.make_request( "POST", - "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", + "/_matrix/client/r0/login", body, ) self.assertEqual(login_response.code, 200, login_response.result) @@ -666,10 +681,15 @@ class RefreshAuthTests(unittest.HomeserverTestCase): refresh the session. """ - body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + body = { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + "org.matrix.msc2918.refresh_token": True, + } login_response = self.make_request( "POST", - "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", + "/_matrix/client/r0/login", body, ) self.assertEqual(login_response.code, HTTPStatus.OK, login_response.result) @@ -711,10 +731,15 @@ class RefreshAuthTests(unittest.HomeserverTestCase): The session can be configured to have an ultimate, limited lifetime. """ - body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + body = { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + "org.matrix.msc2918.refresh_token": True, + } login_response = self.make_request( "POST", - "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", + "/_matrix/client/r0/login", body, ) self.assertEqual(login_response.code, 200, login_response.result) @@ -763,10 +788,15 @@ class RefreshAuthTests(unittest.HomeserverTestCase): |-> fourth_refresh (fails) """ - body = {"type": "m.login.password", "user": "test", "password": self.user_pass} + body = { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + "org.matrix.msc2918.refresh_token": True, + } login_response = self.make_request( "POST", - "/_matrix/client/r0/login?org.matrix.msc2918.refresh_token=true", + "/_matrix/client/r0/login", body, ) self.assertEqual(login_response.code, 200, login_response.result) -- cgit 1.5.1 From 637df95de63196033a6da4a6e286e1d58ea517b6 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Fri, 3 Dec 2021 16:42:44 +0000 Subject: Support configuring the lifetime of non-refreshable access tokens separately to refreshable access tokens. (#11445) --- changelog.d/11445.feature | 1 + synapse/config/registration.py | 49 ++++++++++++++++++++ synapse/handlers/register.py | 20 ++++++-- tests/config/test_registration_config.py | 78 ++++++++++++++++++++++++++++++++ tests/rest/client/test_auth.py | 76 +++++++++++++++++++++++++++++++ 5 files changed, 221 insertions(+), 3 deletions(-) create mode 100644 changelog.d/11445.feature create mode 100644 tests/config/test_registration_config.py (limited to 'tests/rest/client/test_auth.py') diff --git a/changelog.d/11445.feature b/changelog.d/11445.feature new file mode 100644 index 0000000000..211a722b65 --- /dev/null +++ b/changelog.d/11445.feature @@ -0,0 +1 @@ +Support configuring the lifetime of non-refreshable access tokens separately to refreshable access tokens. \ No newline at end of file diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 47853199f4..68a4985398 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -130,11 +130,60 @@ class RegistrationConfig(Config): int ] = refreshable_access_token_lifetime + if ( + self.session_lifetime is not None + and "refreshable_access_token_lifetime" in config + ): + if self.session_lifetime < self.refreshable_access_token_lifetime: + raise ConfigError( + "Both `session_lifetime` and `refreshable_access_token_lifetime` " + "configuration options have been set, but `refreshable_access_token_lifetime` " + " exceeds `session_lifetime`!" + ) + + # The `nonrefreshable_access_token_lifetime` applies for tokens that can NOT be + # refreshed using a refresh token. + # If it is None, then these tokens last for the entire length of the session, + # which is infinite by default. + # The intention behind this configuration option is to help with requiring + # all clients to use refresh tokens, if the homeserver administrator requires. + nonrefreshable_access_token_lifetime = config.get( + "nonrefreshable_access_token_lifetime", + None, + ) + if nonrefreshable_access_token_lifetime is not None: + nonrefreshable_access_token_lifetime = self.parse_duration( + nonrefreshable_access_token_lifetime + ) + self.nonrefreshable_access_token_lifetime = nonrefreshable_access_token_lifetime + + if ( + self.session_lifetime is not None + and self.nonrefreshable_access_token_lifetime is not None + ): + if self.session_lifetime < self.nonrefreshable_access_token_lifetime: + raise ConfigError( + "Both `session_lifetime` and `nonrefreshable_access_token_lifetime` " + "configuration options have been set, but `nonrefreshable_access_token_lifetime` " + " exceeds `session_lifetime`!" + ) + refresh_token_lifetime = config.get("refresh_token_lifetime") if refresh_token_lifetime is not None: refresh_token_lifetime = self.parse_duration(refresh_token_lifetime) self.refresh_token_lifetime: Optional[int] = refresh_token_lifetime + if ( + self.session_lifetime is not None + and self.refresh_token_lifetime is not None + ): + if self.session_lifetime < self.refresh_token_lifetime: + raise ConfigError( + "Both `session_lifetime` and `refresh_token_lifetime` " + "configuration options have been set, but `refresh_token_lifetime` " + " exceeds `session_lifetime`!" + ) + # The fallback template used for authenticating using a registration token self.registration_token_template = self.read_template("registration_token.html") diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 24ca11b924..b14ddd8267 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -1,4 +1,5 @@ # Copyright 2014 - 2016 OpenMarket Ltd +# Copyright 2021 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -116,6 +117,9 @@ class RegistrationHandler: self.pusher_pool = hs.get_pusherpool() self.session_lifetime = hs.config.registration.session_lifetime + self.nonrefreshable_access_token_lifetime = ( + hs.config.registration.nonrefreshable_access_token_lifetime + ) self.refreshable_access_token_lifetime = ( hs.config.registration.refreshable_access_token_lifetime ) @@ -794,13 +798,25 @@ class RegistrationHandler: class and RegisterDeviceReplicationServlet. """ assert not self.hs.config.worker.worker_app + now_ms = self.clock.time_msec() access_token_expiry = None if self.session_lifetime is not None: if is_guest: raise Exception( "session_lifetime is not currently implemented for guest access" ) - access_token_expiry = self.clock.time_msec() + self.session_lifetime + access_token_expiry = now_ms + self.session_lifetime + + if self.nonrefreshable_access_token_lifetime is not None: + if access_token_expiry is not None: + # Don't allow the non-refreshable access token to outlive the + # session. + access_token_expiry = min( + now_ms + self.nonrefreshable_access_token_lifetime, + access_token_expiry, + ) + else: + access_token_expiry = now_ms + self.nonrefreshable_access_token_lifetime refresh_token = None refresh_token_id = None @@ -818,8 +834,6 @@ class RegistrationHandler: # that this value is set before setting this flag). assert self.refreshable_access_token_lifetime is not None - now_ms = self.clock.time_msec() - # Set the expiry time of the refreshable access token access_token_expiry = now_ms + self.refreshable_access_token_lifetime diff --git a/tests/config/test_registration_config.py b/tests/config/test_registration_config.py new file mode 100644 index 0000000000..17a84d20d8 --- /dev/null +++ b/tests/config/test_registration_config.py @@ -0,0 +1,78 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from synapse.config import ConfigError +from synapse.config.homeserver import HomeServerConfig + +from tests.unittest import TestCase +from tests.utils import default_config + + +class RegistrationConfigTestCase(TestCase): + def test_session_lifetime_must_not_be_exceeded_by_smaller_lifetimes(self): + """ + session_lifetime should logically be larger than, or at least as large as, + all the different token lifetimes. + Test that the user is faced with configuration errors if they make it + smaller, as that configuration doesn't make sense. + """ + config_dict = default_config("test") + + # First test all the error conditions + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "session_lifetime": "30m", + "nonrefreshable_access_token_lifetime": "31m", + **config_dict, + } + ) + + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "session_lifetime": "30m", + "refreshable_access_token_lifetime": "31m", + **config_dict, + } + ) + + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "session_lifetime": "30m", + "refresh_token_lifetime": "31m", + **config_dict, + } + ) + + # Then test all the fine conditions + HomeServerConfig().parse_config_dict( + { + "session_lifetime": "31m", + "nonrefreshable_access_token_lifetime": "31m", + **config_dict, + } + ) + + HomeServerConfig().parse_config_dict( + { + "session_lifetime": "31m", + "refreshable_access_token_lifetime": "31m", + **config_dict, + } + ) + + HomeServerConfig().parse_config_dict( + {"session_lifetime": "31m", "refresh_token_lifetime": "31m", **config_dict} + ) diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index d8a94f4c12..7239e1a1b5 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -524,6 +524,19 @@ class RefreshAuthTests(unittest.HomeserverTestCase): {"refresh_token": refresh_token}, ) + def is_access_token_valid(self, access_token) -> bool: + """ + Checks whether an access token is valid, returning whether it is or not. + """ + code = self.make_request( + "GET", "/_matrix/client/v3/account/whoami", access_token=access_token + ).code + + # Either 200 or 401 is what we get back; anything else is a bug. + assert code in {HTTPStatus.OK, HTTPStatus.UNAUTHORIZED} + + return code == HTTPStatus.OK + def test_login_issue_refresh_token(self): """ A login response should include a refresh_token only if asked. @@ -671,6 +684,69 @@ class RefreshAuthTests(unittest.HomeserverTestCase): HTTPStatus.UNAUTHORIZED, ) + @override_config( + { + "refreshable_access_token_lifetime": "1m", + "nonrefreshable_access_token_lifetime": "10m", + } + ) + def test_different_expiry_for_refreshable_and_nonrefreshable_access_tokens(self): + """ + Tests that the expiry times for refreshable and non-refreshable access + tokens can be different. + """ + body = { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + } + login_response1 = self.make_request( + "POST", + "/_matrix/client/r0/login", + {"org.matrix.msc2918.refresh_token": True, **body}, + ) + self.assertEqual(login_response1.code, 200, login_response1.result) + self.assertApproximates( + login_response1.json_body["expires_in_ms"], 60 * 1000, 100 + ) + refreshable_access_token = login_response1.json_body["access_token"] + + login_response2 = self.make_request( + "POST", + "/_matrix/client/r0/login", + body, + ) + self.assertEqual(login_response2.code, 200, login_response2.result) + nonrefreshable_access_token = login_response2.json_body["access_token"] + + # Advance 59 seconds in the future (just shy of 1 minute, the time of expiry) + self.reactor.advance(59.0) + + # Both tokens should still be valid. + self.assertTrue(self.is_access_token_valid(refreshable_access_token)) + self.assertTrue(self.is_access_token_valid(nonrefreshable_access_token)) + + # Advance to 61 s (just past 1 minute, the time of expiry) + self.reactor.advance(2.0) + + # Only the non-refreshable token is still valid. + self.assertFalse(self.is_access_token_valid(refreshable_access_token)) + self.assertTrue(self.is_access_token_valid(nonrefreshable_access_token)) + + # Advance to 599 s (just shy of 10 minutes, the time of expiry) + self.reactor.advance(599.0 - 61.0) + + # It's still the case that only the non-refreshable token is still valid. + self.assertFalse(self.is_access_token_valid(refreshable_access_token)) + self.assertTrue(self.is_access_token_valid(nonrefreshable_access_token)) + + # Advance to 601 s (just past 10 minutes, the time of expiry) + self.reactor.advance(2.0) + + # Now neither token is valid. + self.assertFalse(self.is_access_token_valid(refreshable_access_token)) + self.assertFalse(self.is_access_token_valid(nonrefreshable_access_token)) + @override_config( {"refreshable_access_token_lifetime": "1m", "refresh_token_lifetime": "2m"} ) -- cgit 1.5.1 From 2f053f3f82ca174cc1c858c75afffae51af8ce0d Mon Sep 17 00:00:00 2001 From: reivilibre Date: Mon, 6 Dec 2021 19:11:43 +0000 Subject: Stabilise support for MSC2918 refresh tokens as they have now been merged into the Matrix specification. (#11435) --- changelog.d/11435.feature | 1 + docs/sample_config.yaml | 38 ++++++++++++++++++++++++++++++++++++++ synapse/config/registration.py | 38 ++++++++++++++++++++++++++++++++++++++ synapse/rest/client/login.py | 29 +++++++++++++---------------- synapse/rest/client/register.py | 23 ++++++++++------------- tests/rest/client/test_auth.py | 30 +++++++++++++++--------------- 6 files changed, 115 insertions(+), 44 deletions(-) create mode 100644 changelog.d/11435.feature (limited to 'tests/rest/client/test_auth.py') diff --git a/changelog.d/11435.feature b/changelog.d/11435.feature new file mode 100644 index 0000000000..9e127fae3c --- /dev/null +++ b/changelog.d/11435.feature @@ -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. \ No newline at end of file diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index ae476d19ac..6696ed5d1e 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -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: diff --git a/synapse/config/registration.py b/synapse/config/registration.py index 68a4985398..7a059c6dec 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -220,6 +220,44 @@ class RegistrationConfig(Config): # #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: diff --git a/synapse/rest/client/login.py b/synapse/rest/client/login.py index 1b23fa18cf..f9994658c4 100644 --- a/synapse/rest/client/login.py +++ b/synapse/rest/client/login.py @@ -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__() @@ -90,7 +90,7 @@ class LoginRestServlet(RestServlet): 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 ) @@ -163,17 +163,16 @@ class LoginRestServlet(RestServlet): 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 + ) try: if login_submission["type"] in ( @@ -463,9 +462,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() diff --git a/synapse/rest/client/register.py b/synapse/rest/client/register.py index 11fd6cd24d..8b56c76aed 100644 --- a/synapse/rest/client/register.py +++ b/synapse/rest/client/register.py @@ -419,7 +419,7 @@ class RegisterRestServlet(RestServlet): 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 ) @@ -445,18 +445,15 @@ class RegisterRestServlet(RestServlet): 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. diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index 7239e1a1b5..aa8ad6d2e1 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -520,7 +520,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): """ return self.make_request( "POST", - "/_matrix/client/unstable/org.matrix.msc2918.refresh_token/refresh", + "/_matrix/client/v1/refresh", {"refresh_token": refresh_token}, ) @@ -557,7 +557,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): 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) @@ -588,7 +588,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): "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) @@ -603,7 +603,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): "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", @@ -614,7 +614,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): 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) @@ -641,7 +641,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): "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", @@ -655,7 +655,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): 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) @@ -761,7 +761,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): "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", @@ -811,7 +811,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): "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", @@ -868,7 +868,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): "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", @@ -880,7 +880,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): # 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( @@ -890,7 +890,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): # 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( @@ -900,7 +900,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): # 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( @@ -928,7 +928,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): # 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( @@ -938,7 +938,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): # 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( -- cgit 1.5.1 From 2d42e586a8c54be1a83643148358b1651c1ca666 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Tue, 7 Dec 2021 10:49:39 +0000 Subject: Fix the test breakage introduced by #11435 as a result of concurrent PRs (#11522) --- changelog.d/11522.feature | 1 + tests/rest/client/test_auth.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/11522.feature (limited to 'tests/rest/client/test_auth.py') diff --git a/changelog.d/11522.feature b/changelog.d/11522.feature new file mode 100644 index 0000000000..9e127fae3c --- /dev/null +++ b/changelog.d/11522.feature @@ -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. \ No newline at end of file diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index aa8ad6d2e1..72bbc87b4a 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -703,7 +703,7 @@ class RefreshAuthTests(unittest.HomeserverTestCase): login_response1 = self.make_request( "POST", "/_matrix/client/r0/login", - {"org.matrix.msc2918.refresh_token": True, **body}, + {"refresh_token": True, **body}, ) self.assertEqual(login_response1.code, 200, login_response1.result) self.assertApproximates( -- cgit 1.5.1