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

Commit

Permalink
Implement access token expiry (#5660)
Browse files Browse the repository at this point in the history
  • Loading branch information
anoadragon453 committed Feb 17, 2020
2 parents 2ff6787 + 5f158ec commit 2f450fa
Show file tree
Hide file tree
Showing 16 changed files with 258 additions and 36 deletions.
1 change: 1 addition & 0 deletions changelog.d/5660.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement `session_lifetime` configuration option, after which access tokens will expire.
File renamed without changes.
11 changes: 11 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,17 @@ uploads_path: "DATADIR/uploads"
# # renewal token. Optional.
# invalid_token_html_path: "invalid_token.html"

# Time that a user's session remains valid for, after they log in.
#
# Note that this is not currently compatible with guest logins.
#
# Note also that this is calculated at login time: changes are not applied
# retrospectively to users who have already logged in.
#
# By default, this is infinite.
#
#session_lifetime: 24h

# The user must provide all of the below types of 3PID when registering.
#
#registrations_require_3pid:
Expand Down
12 changes: 12 additions & 0 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,17 @@ def get_user_by_access_token(self, token, rights="access"):
# first look in the database
r = yield self._look_up_user_by_access_token(token)
if r:
valid_until_ms = r["valid_until_ms"]
if (
valid_until_ms is not None
and valid_until_ms < self.clock.time_msec()
):
# there was a valid access token, but it has expired.
# soft-logout the user.
raise InvalidClientTokenError(
msg="Access token has expired", soft_logout=True
)

defer.returnValue(r)

# otherwise it needs to be a valid macaroon
Expand Down Expand Up @@ -510,6 +521,7 @@ def _look_up_user_by_access_token(self, token):
"token_id": ret.get("token_id", None),
"is_guest": False,
"device_id": ret.get("device_id"),
"valid_until_ms": ret.get("valid_until_ms"),
}
defer.returnValue(user_info)

Expand Down
8 changes: 7 additions & 1 deletion synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,14 @@ def __init__(self, msg="Missing access token"):
class InvalidClientTokenError(InvalidClientCredentialsError):
"""Raised when we didn't understand the access token in a request"""

def __init__(self, msg="Unrecognised access token"):
def __init__(self, msg="Unrecognised access token", soft_logout=False):
super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN")
self._soft_logout = soft_logout

def error_dict(self):
d = super().error_dict()
d["soft_logout"] = self._soft_logout
return d


class ResourceLimitError(SynapseError):
Expand Down
16 changes: 16 additions & 0 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ def read_config(self, config, **kwargs):
"disable_msisdn_registration", False
)

session_lifetime = config.get("session_lifetime")
if session_lifetime is not None:
session_lifetime = self.parse_duration(session_lifetime)
self.session_lifetime = session_lifetime

def generate_config_section(self, generate_secrets=False, **kwargs):
if generate_secrets:
registration_shared_secret = 'registration_shared_secret: "%s"' % (
Expand Down Expand Up @@ -205,6 +210,17 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
# # renewal token. Optional.
# invalid_token_html_path: "invalid_token.html"
# Time that a user's session remains valid for, after they log in.
#
# Note that this is not currently compatible with guest logins.
#
# Note also that this is calculated at login time: changes are not applied
# retrospectively to users who have already logged in.
#
# By default, this is infinite.
#
#session_lifetime: 24h
# The user must provide all of the below types of 3PID when registering.
#
#registrations_require_3pid:
Expand Down
17 changes: 14 additions & 3 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# limitations under the License.

import logging
import time
import unicodedata

import attr
Expand Down Expand Up @@ -558,7 +559,7 @@ def _get_session_info(self, session_id):
return self.sessions[session_id]

@defer.inlineCallbacks
def get_access_token_for_user_id(self, user_id, device_id=None):
def get_access_token_for_user_id(self, user_id, device_id, valid_until_ms):
"""
Creates a new access token for the user with the given user ID.
Expand All @@ -572,16 +573,26 @@ def get_access_token_for_user_id(self, user_id, device_id=None):
device_id (str|None): the device ID to associate with the tokens.
None to leave the tokens unassociated with a device (deprecated:
we should always have a device ID)
valid_until_ms (int|None): when the token is valid until. None for
no expiry.
Returns:
The access token for the user's session.
Raises:
StoreError if there was a problem storing the token.
"""
logger.info("Logging in user %s on device %s", user_id, device_id)
fmt_expiry = ""
if valid_until_ms is not None:
fmt_expiry = time.strftime(
" until %Y-%m-%d %H:%M:%S", time.localtime(valid_until_ms / 1000.0)
)
logger.info("Logging in user %s on device %s%s", user_id, device_id, fmt_expiry)

yield self.auth.check_auth_blocking(user_id)

access_token = self.macaroon_gen.generate_access_token(user_id)
yield self.store.add_access_token_to_user(user_id, access_token, device_id)
yield self.store.add_access_token_to_user(
user_id, access_token, device_id, valid_until_ms
)

# the device *should* have been registered before we got here; however,
# it's possible we raced against a DELETE operation. The thing we
Expand Down
35 changes: 24 additions & 11 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ def __init__(self, hs):
self.device_handler = hs.get_device_handler()
self.pusher_pool = hs.get_pusherpool()

self.session_lifetime = hs.config.session_lifetime

@defer.inlineCallbacks
def check_username(self, localpart, guest_access_token=None, assigned_user_id=None):
if types.contains_invalid_mxid_characters(localpart):
Expand Down Expand Up @@ -753,6 +755,8 @@ def register_with_store(
def register_device(self, user_id, device_id, initial_display_name, is_guest=False):
"""Register a device for a user and generate an access token.
The access token will be limited by the homeserver's session_lifetime config.
Args:
user_id (str): full canonical @user:id
device_id (str|None): The device ID to check, or None to generate
Expand All @@ -773,20 +777,29 @@ def register_device(self, user_id, device_id, initial_display_name, is_guest=Fal
is_guest=is_guest,
)
defer.returnValue((r["device_id"], r["access_token"]))
else:
device_id = yield self.device_handler.check_device_registered(
user_id, device_id, initial_display_name
)

valid_until_ms = None
if self.session_lifetime is not None:
if is_guest:
access_token = self.macaroon_gen.generate_access_token(
user_id, ["guest = true"]
)
else:
access_token = yield self._auth_handler.get_access_token_for_user_id(
user_id, device_id=device_id
raise Exception(
"session_lifetime is not currently implemented for guest access"
)
valid_until_ms = self.clock.time_msec() + self.session_lifetime

device_id = yield self.device_handler.check_device_registered(
user_id, device_id, initial_display_name
)
if is_guest:
assert valid_until_ms is None
access_token = self.macaroon_gen.generate_access_token(
user_id, ["guest = true"]
)
else:
access_token = yield self._auth_handler.get_access_token_for_user_id(
user_id, device_id=device_id, valid_until_ms=valid_until_ms
)

defer.returnValue((device_id, access_token))
defer.returnValue((device_id, access_token))

@defer.inlineCallbacks
def post_registration_actions(
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/storage_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class StorageProviderWrapper(StorageProvider):
backend (StorageProvider)
store_local (bool): Whether to store new local files or not.
store_synchronous (bool): Whether to wait for file to be successfully
uploaded, or todo the upload in the backgroud.
uploaded, or todo the upload in the background.
store_remote (bool): Whether remote media should be uploaded
"""

Expand Down
27 changes: 18 additions & 9 deletions synapse/storage/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ def get_user_by_access_token(self, token):
token (str): The access token of a user.
Returns:
defer.Deferred: None, if the token did not match, otherwise dict
including the keys `name`, `is_guest`, `device_id`, `token_id`.
including the keys `name`, `is_guest`, `device_id`, `token_id`,
`valid_until_ms`.
"""
return self.runInteraction(
"get_user_by_access_token", self._query_for_auth, token
Expand Down Expand Up @@ -306,7 +307,7 @@ def is_server_admin(self, user):
def _query_for_auth(self, txn, token):
sql = (
"SELECT users.name, users.is_guest, access_tokens.id as token_id,"
" access_tokens.device_id"
" access_tokens.device_id, access_tokens.valid_until_ms"
" FROM users"
" INNER JOIN access_tokens on users.name = access_tokens.user_id"
" WHERE token = ?"
Expand Down Expand Up @@ -625,7 +626,7 @@ def __init__(self, db_conn, hs):
)

self.register_background_update_handler(
"users_set_deactivated_flag", self._backgroud_update_set_deactivated_flag
"users_set_deactivated_flag", self._background_update_set_deactivated_flag
)

# Create a background job for culling expired 3PID validity tokens
Expand All @@ -640,14 +641,14 @@ def start_cull():
hs.get_clock().looping_call(start_cull, THIRTY_MINUTES_IN_MS)

@defer.inlineCallbacks
def _backgroud_update_set_deactivated_flag(self, progress, batch_size):
def _background_update_set_deactivated_flag(self, progress, batch_size):
"""Retrieves a list of all deactivated users and sets the 'deactivated' flag to 1
for each of them.
"""

last_user = progress.get("user_id", "")

def _backgroud_update_set_deactivated_flag_txn(txn):
def _background_update_set_deactivated_flag_txn(txn):
txn.execute(
"""
SELECT
Expand Down Expand Up @@ -692,7 +693,7 @@ def _backgroud_update_set_deactivated_flag_txn(txn):
return False

end = yield self.runInteraction(
"users_set_deactivated_flag", _backgroud_update_set_deactivated_flag_txn
"users_set_deactivated_flag", _background_update_set_deactivated_flag_txn
)

if end:
Expand All @@ -701,22 +702,30 @@ def _backgroud_update_set_deactivated_flag_txn(txn):
defer.returnValue(batch_size)

@defer.inlineCallbacks
def add_access_token_to_user(self, user_id, token, device_id=None):
def add_access_token_to_user(self, user_id, token, device_id, valid_until_ms):
"""Adds an access token for the given user.
Args:
user_id (str): The user ID.
token (str): The new access token to add.
device_id (str): ID of the device to associate with the access
token
token
valid_until_ms (int|None): when the token is valid until. None for
no expiry.
Raises:
StoreError if there was a problem adding this.
"""
next_id = self._access_tokens_id_gen.get_next()

yield self._simple_insert(
"access_tokens",
{"id": next_id, "user_id": user_id, "token": token, "device_id": device_id},
{
"id": next_id,
"user_id": user_id,
"token": token,
"device_id": device_id,
"valid_until_ms": valid_until_ms,
},
desc="add_access_token_to_user",
)

Expand Down
18 changes: 18 additions & 0 deletions synapse/storage/schema/delta/55/access_token_expiry.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* Copyright 2019 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.
*/

-- when this access token can be used until, in ms since the epoch. NULL means the token
-- never expires.
ALTER TABLE access_tokens ADD COLUMN valid_until_ms BIGINT;
6 changes: 4 additions & 2 deletions tests/api/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,11 @@ def test_cannot_use_regular_token_as_guest(self):
self.store.add_access_token_to_user = Mock()

token = yield self.hs.handlers.auth_handler.get_access_token_for_user_id(
USER_ID, "DEVICE"
USER_ID, "DEVICE", valid_until_ms=None
)
self.store.add_access_token_to_user.assert_called_with(
USER_ID, token, "DEVICE", None
)
self.store.add_access_token_to_user.assert_called_with(USER_ID, token, "DEVICE")

def get_user(tok):
if token != tok:
Expand Down
20 changes: 15 additions & 5 deletions tests/handlers/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ def test_short_term_login_token_cannot_replace_user_id(self):
def test_mau_limits_disabled(self):
self.hs.config.limit_usage_by_mau = False
# Ensure does not throw exception
yield self.auth_handler.get_access_token_for_user_id("user_a")
yield self.auth_handler.get_access_token_for_user_id(
"user_a", device_id=None, valid_until_ms=None
)

yield self.auth_handler.validate_short_term_login_token_and_get_user_id(
self._get_macaroon().serialize()
Expand All @@ -131,7 +133,9 @@ def test_mau_limits_exceeded_large(self):
)

with self.assertRaises(ResourceLimitError):
yield self.auth_handler.get_access_token_for_user_id("user_a")
yield self.auth_handler.get_access_token_for_user_id(
"user_a", device_id=None, valid_until_ms=None
)

self.hs.get_datastore().get_monthly_active_count = Mock(
return_value=defer.succeed(self.large_number_of_users)
Expand All @@ -150,7 +154,9 @@ def test_mau_limits_parity(self):
return_value=defer.succeed(self.hs.config.max_mau_value)
)
with self.assertRaises(ResourceLimitError):
yield self.auth_handler.get_access_token_for_user_id("user_a")
yield self.auth_handler.get_access_token_for_user_id(
"user_a", device_id=None, valid_until_ms=None
)

self.hs.get_datastore().get_monthly_active_count = Mock(
return_value=defer.succeed(self.hs.config.max_mau_value)
Expand All @@ -166,7 +172,9 @@ def test_mau_limits_parity(self):
self.hs.get_datastore().get_monthly_active_count = Mock(
return_value=defer.succeed(self.hs.config.max_mau_value)
)
yield self.auth_handler.get_access_token_for_user_id("user_a")
yield self.auth_handler.get_access_token_for_user_id(
"user_a", device_id=None, valid_until_ms=None
)
self.hs.get_datastore().user_last_seen_monthly_active = Mock(
return_value=defer.succeed(self.hs.get_clock().time_msec())
)
Expand All @@ -185,7 +193,9 @@ def test_mau_limits_not_exceeded(self):
return_value=defer.succeed(self.small_number_of_users)
)
# Ensure does not raise exception
yield self.auth_handler.get_access_token_for_user_id("user_a")
yield self.auth_handler.get_access_token_for_user_id(
"user_a", device_id=None, valid_until_ms=None
)

self.hs.get_datastore().get_monthly_active_count = Mock(
return_value=defer.succeed(self.small_number_of_users)
Expand Down
5 changes: 4 additions & 1 deletion tests/handlers/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@ def get_or_create_user(self, requester, localpart, displayname, password_hash=No
)
else:
yield self.hs.get_auth_handler().delete_access_tokens_for_user(user_id)
yield self.store.add_access_token_to_user(user_id=user_id, token=token)

yield self.store.add_access_token_to_user(
user_id=user_id, token=token, device_id=None, valid_until_ms=None
)

if displayname is not None:
# logger.info("setting user display name: %s -> %s", user_id, displayname)
Expand Down
Loading

0 comments on commit 2f450fa

Please sign in to comment.