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

Commit

Permalink
Support configuring the lifetime of non-refreshable access tokens sep…
Browse files Browse the repository at this point in the history
…arately to refreshable access tokens. (#11445)
  • Loading branch information
reivilibre authored Dec 3, 2021
1 parent e5f426c commit 637df95
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.d/11445.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support configuring the lifetime of non-refreshable access tokens separately to refreshable access tokens.
49 changes: 49 additions & 0 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,60 @@ def read_config(self, config, **kwargs):
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")

Expand Down
20 changes: 17 additions & 3 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -116,6 +117,9 @@ def __init__(self, hs: "HomeServer"):
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
)
Expand Down Expand Up @@ -794,13 +798,25 @@ async def register_device_inner(
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
Expand All @@ -818,8 +834,6 @@ class and RegisterDeviceReplicationServlet.
# 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

Expand Down
78 changes: 78 additions & 0 deletions tests/config/test_registration_config.py
Original file line number Diff line number Diff line change
@@ -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}
)
76 changes: 76 additions & 0 deletions tests/rest/client/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,19 @@ def use_refresh_token(self, refresh_token: str) -> FakeChannel:
{"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.
Expand Down Expand Up @@ -671,6 +684,69 @@ def test_refreshable_access_token_expiration(self):
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"}
)
Expand Down

0 comments on commit 637df95

Please sign in to comment.