Allow re-using a UI auth validation for a period of time (#8970)
This commit is contained in:
parent
4136255d3c
commit
5d4c330ed9
|
@ -0,0 +1 @@
|
|||
Allow re-using an user-interactive authentication session for a period of time.
|
|
@ -2068,6 +2068,21 @@ password_config:
|
|||
#
|
||||
#require_uppercase: true
|
||||
|
||||
ui_auth:
|
||||
# The number of milliseconds to allow a user-interactive authentication
|
||||
# session to be active.
|
||||
#
|
||||
# This defaults to 0, meaning the user is queried for their credentials
|
||||
# before every action, but this can be overridden to alow a single
|
||||
# validation to be re-used. This weakens the protections afforded by
|
||||
# the user-interactive authentication process, by allowing for multiple
|
||||
# (and potentially different) operations to use the same validation session.
|
||||
#
|
||||
# Uncomment below to allow for credential validation to last for 15
|
||||
# seconds.
|
||||
#
|
||||
#session_timeout: 15000
|
||||
|
||||
|
||||
# Configuration for sending emails from Synapse.
|
||||
#
|
||||
|
|
|
@ -3,6 +3,7 @@ from typing import Any, Iterable, List, Optional
|
|||
from synapse.config import (
|
||||
api,
|
||||
appservice,
|
||||
auth,
|
||||
captcha,
|
||||
cas,
|
||||
consent_config,
|
||||
|
@ -14,7 +15,6 @@ from synapse.config import (
|
|||
logger,
|
||||
metrics,
|
||||
oidc_config,
|
||||
password,
|
||||
password_auth_providers,
|
||||
push,
|
||||
ratelimiting,
|
||||
|
@ -65,7 +65,7 @@ class RootConfig:
|
|||
sso: sso.SSOConfig
|
||||
oidc: oidc_config.OIDCConfig
|
||||
jwt: jwt_config.JWTConfig
|
||||
password: password.PasswordConfig
|
||||
auth: auth.AuthConfig
|
||||
email: emailconfig.EmailConfig
|
||||
worker: workers.WorkerConfig
|
||||
authproviders: password_auth_providers.PasswordAuthProviderConfig
|
||||
|
|
|
@ -1,5 +1,6 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Copyright 2015, 2016 OpenMarket Ltd
|
||||
# Copyright 2020 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.
|
||||
|
@ -16,11 +17,11 @@
|
|||
from ._base import Config
|
||||
|
||||
|
||||
class PasswordConfig(Config):
|
||||
"""Password login configuration
|
||||
class AuthConfig(Config):
|
||||
"""Password and login configuration
|
||||
"""
|
||||
|
||||
section = "password"
|
||||
section = "auth"
|
||||
|
||||
def read_config(self, config, **kwargs):
|
||||
password_config = config.get("password_config", {})
|
||||
|
@ -35,6 +36,10 @@ class PasswordConfig(Config):
|
|||
self.password_policy = password_config.get("policy") or {}
|
||||
self.password_policy_enabled = self.password_policy.get("enabled", False)
|
||||
|
||||
# User-interactive authentication
|
||||
ui_auth = config.get("ui_auth") or {}
|
||||
self.ui_auth_session_timeout = ui_auth.get("session_timeout", 0)
|
||||
|
||||
def generate_config_section(self, config_dir_path, server_name, **kwargs):
|
||||
return """\
|
||||
password_config:
|
||||
|
@ -87,4 +92,19 @@ class PasswordConfig(Config):
|
|||
# Defaults to 'false'.
|
||||
#
|
||||
#require_uppercase: true
|
||||
|
||||
ui_auth:
|
||||
# The number of milliseconds to allow a user-interactive authentication
|
||||
# session to be active.
|
||||
#
|
||||
# This defaults to 0, meaning the user is queried for their credentials
|
||||
# before every action, but this can be overridden to alow a single
|
||||
# validation to be re-used. This weakens the protections afforded by
|
||||
# the user-interactive authentication process, by allowing for multiple
|
||||
# (and potentially different) operations to use the same validation session.
|
||||
#
|
||||
# Uncomment below to allow for credential validation to last for 15
|
||||
# seconds.
|
||||
#
|
||||
#session_timeout: 15000
|
||||
"""
|
|
@ -17,6 +17,7 @@
|
|||
from ._base import RootConfig
|
||||
from .api import ApiConfig
|
||||
from .appservice import AppServiceConfig
|
||||
from .auth import AuthConfig
|
||||
from .cache import CacheConfig
|
||||
from .captcha import CaptchaConfig
|
||||
from .cas import CasConfig
|
||||
|
@ -30,7 +31,6 @@ from .key import KeyConfig
|
|||
from .logger import LoggingConfig
|
||||
from .metrics import MetricsConfig
|
||||
from .oidc_config import OIDCConfig
|
||||
from .password import PasswordConfig
|
||||
from .password_auth_providers import PasswordAuthProviderConfig
|
||||
from .push import PushConfig
|
||||
from .ratelimiting import RatelimitConfig
|
||||
|
@ -76,7 +76,7 @@ class HomeServerConfig(RootConfig):
|
|||
CasConfig,
|
||||
SSOConfig,
|
||||
JWTConfig,
|
||||
PasswordConfig,
|
||||
AuthConfig,
|
||||
EmailConfig,
|
||||
PasswordAuthProviderConfig,
|
||||
PushConfig,
|
||||
|
|
|
@ -226,6 +226,9 @@ class AuthHandler(BaseHandler):
|
|||
burst_count=self.hs.config.rc_login_failed_attempts.burst_count,
|
||||
)
|
||||
|
||||
# The number of seconds to keep a UI auth session active.
|
||||
self._ui_auth_session_timeout = hs.config.ui_auth_session_timeout
|
||||
|
||||
# Ratelimitier for failed /login attempts
|
||||
self._failed_login_attempts_ratelimiter = Ratelimiter(
|
||||
clock=hs.get_clock(),
|
||||
|
@ -283,7 +286,7 @@ class AuthHandler(BaseHandler):
|
|||
request_body: Dict[str, Any],
|
||||
clientip: str,
|
||||
description: str,
|
||||
) -> Tuple[dict, str]:
|
||||
) -> Tuple[dict, Optional[str]]:
|
||||
"""
|
||||
Checks that the user is who they claim to be, via a UI auth.
|
||||
|
||||
|
@ -310,7 +313,8 @@ class AuthHandler(BaseHandler):
|
|||
have been given only in a previous call).
|
||||
|
||||
'session_id' is the ID of this session, either passed in by the
|
||||
client or assigned by this call
|
||||
client or assigned by this call. This is None if UI auth was
|
||||
skipped (by re-using a previous validation).
|
||||
|
||||
Raises:
|
||||
InteractiveAuthIncompleteError if the client has not yet completed
|
||||
|
@ -324,6 +328,16 @@ class AuthHandler(BaseHandler):
|
|||
|
||||
"""
|
||||
|
||||
if self._ui_auth_session_timeout:
|
||||
last_validated = await self.store.get_access_token_last_validated(
|
||||
requester.access_token_id
|
||||
)
|
||||
if self.clock.time_msec() - last_validated < self._ui_auth_session_timeout:
|
||||
# Return the input parameters, minus the auth key, which matches
|
||||
# the logic in check_ui_auth.
|
||||
request_body.pop("auth", None)
|
||||
return request_body, None
|
||||
|
||||
user_id = requester.user.to_string()
|
||||
|
||||
# Check if we should be ratelimited due to too many previous failed attempts
|
||||
|
@ -359,6 +373,9 @@ class AuthHandler(BaseHandler):
|
|||
if user_id != requester.user.to_string():
|
||||
raise AuthError(403, "Invalid auth")
|
||||
|
||||
# Note that the access token has been validated.
|
||||
await self.store.update_access_token_last_validated(requester.access_token_id)
|
||||
|
||||
return params, session_id
|
||||
|
||||
async def _get_available_ui_auth_types(self, user: UserID) -> Iterable[str]:
|
||||
|
@ -452,13 +469,10 @@ class AuthHandler(BaseHandler):
|
|||
all the stages in any of the permitted flows.
|
||||
"""
|
||||
|
||||
authdict = None
|
||||
sid = None # type: Optional[str]
|
||||
if clientdict and "auth" in clientdict:
|
||||
authdict = clientdict["auth"]
|
||||
del clientdict["auth"]
|
||||
if "session" in authdict:
|
||||
sid = authdict["session"]
|
||||
authdict = clientdict.pop("auth", {})
|
||||
if "session" in authdict:
|
||||
sid = authdict["session"]
|
||||
|
||||
# Convert the URI and method to strings.
|
||||
uri = request.uri.decode("utf-8")
|
||||
|
@ -563,6 +577,8 @@ class AuthHandler(BaseHandler):
|
|||
|
||||
creds = await self.store.get_completed_ui_auth_stages(session.session_id)
|
||||
for f in flows:
|
||||
# If all the required credentials have been supplied, the user has
|
||||
# successfully completed the UI auth process!
|
||||
if len(set(f) - set(creds)) == 0:
|
||||
# it's very useful to know what args are stored, but this can
|
||||
# include the password in the case of registering, so only log
|
||||
|
|
|
@ -254,14 +254,18 @@ class PasswordRestServlet(RestServlet):
|
|||
logger.error("Auth succeeded but no known type! %r", result.keys())
|
||||
raise SynapseError(500, "", Codes.UNKNOWN)
|
||||
|
||||
# If we have a password in this request, prefer it. Otherwise, there
|
||||
# must be a password hash from an earlier request.
|
||||
# If we have a password in this request, prefer it. Otherwise, use the
|
||||
# password hash from an earlier request.
|
||||
if new_password:
|
||||
password_hash = await self.auth_handler.hash(new_password)
|
||||
else:
|
||||
elif session_id is not None:
|
||||
password_hash = await self.auth_handler.get_session_data(
|
||||
session_id, "password_hash", None
|
||||
)
|
||||
else:
|
||||
# UI validation was skipped, but the request did not include a new
|
||||
# password.
|
||||
password_hash = None
|
||||
if not password_hash:
|
||||
raise SynapseError(400, "Missing params: password", Codes.MISSING_PARAM)
|
||||
|
||||
|
|
|
@ -943,6 +943,42 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore):
|
|||
desc="del_user_pending_deactivation",
|
||||
)
|
||||
|
||||
async def get_access_token_last_validated(self, token_id: int) -> int:
|
||||
"""Retrieves the time (in milliseconds) of the last validation of an access token.
|
||||
|
||||
Args:
|
||||
token_id: The ID of the access token to update.
|
||||
Raises:
|
||||
StoreError if the access token was not found.
|
||||
|
||||
Returns:
|
||||
The last validation time.
|
||||
"""
|
||||
result = await self.db_pool.simple_select_one_onecol(
|
||||
"access_tokens", {"id": token_id}, "last_validated"
|
||||
)
|
||||
|
||||
# If this token has not been validated (since starting to track this),
|
||||
# return 0 instead of None.
|
||||
return result or 0
|
||||
|
||||
async def update_access_token_last_validated(self, token_id: int) -> None:
|
||||
"""Updates the last time an access token was validated.
|
||||
|
||||
Args:
|
||||
token_id: The ID of the access token to update.
|
||||
Raises:
|
||||
StoreError if there was a problem updating this.
|
||||
"""
|
||||
now = self._clock.time_msec()
|
||||
|
||||
await self.db_pool.simple_update_one(
|
||||
"access_tokens",
|
||||
{"id": token_id},
|
||||
{"last_validated": now},
|
||||
desc="update_access_token_last_validated",
|
||||
)
|
||||
|
||||
|
||||
class RegistrationBackgroundUpdateStore(RegistrationWorkerStore):
|
||||
def __init__(self, database: DatabasePool, db_conn: Connection, hs: "HomeServer"):
|
||||
|
@ -1150,6 +1186,7 @@ class RegistrationStore(StatsStore, RegistrationBackgroundUpdateStore):
|
|||
The token ID
|
||||
"""
|
||||
next_id = self._access_tokens_id_gen.get_next()
|
||||
now = self._clock.time_msec()
|
||||
|
||||
await self.db_pool.simple_insert(
|
||||
"access_tokens",
|
||||
|
@ -1160,6 +1197,7 @@ class RegistrationStore(StatsStore, RegistrationBackgroundUpdateStore):
|
|||
"device_id": device_id,
|
||||
"valid_until_ms": valid_until_ms,
|
||||
"puppets_user_id": puppets_user_id,
|
||||
"last_validated": now,
|
||||
},
|
||||
desc="add_access_token_to_user",
|
||||
)
|
||||
|
|
|
@ -0,0 +1,18 @@
|
|||
/* Copyright 2020 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.
|
||||
*/
|
||||
|
||||
-- The last time this access token was "validated" (i.e. logged in or succeeded
|
||||
-- at user-interactive authentication).
|
||||
ALTER TABLE access_tokens ADD COLUMN last_validated BIGINT;
|
|
@ -13,7 +13,7 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
from typing import List, Union
|
||||
from typing import Union
|
||||
|
||||
from twisted.internet.defer import succeed
|
||||
|
||||
|
@ -177,13 +177,8 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
def prepare(self, reactor, clock, hs):
|
||||
self.user_pass = "pass"
|
||||
self.user = self.register_user("test", self.user_pass)
|
||||
self.user_tok = self.login("test", self.user_pass)
|
||||
|
||||
def get_device_ids(self, access_token: str) -> List[str]:
|
||||
# Get the list of devices so one can be deleted.
|
||||
channel = self.make_request("GET", "devices", access_token=access_token,)
|
||||
self.assertEqual(channel.code, 200)
|
||||
return [d["device_id"] for d in channel.json_body["devices"]]
|
||||
self.device_id = "dev1"
|
||||
self.user_tok = self.login("test", self.user_pass, self.device_id)
|
||||
|
||||
def delete_device(
|
||||
self,
|
||||
|
@ -219,11 +214,9 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
"""
|
||||
Test user interactive authentication outside of registration.
|
||||
"""
|
||||
device_id = self.get_device_ids(self.user_tok)[0]
|
||||
|
||||
# Attempt to delete this device.
|
||||
# Returns a 401 as per the spec
|
||||
channel = self.delete_device(self.user_tok, device_id, 401)
|
||||
channel = self.delete_device(self.user_tok, self.device_id, 401)
|
||||
|
||||
# Grab the session
|
||||
session = channel.json_body["session"]
|
||||
|
@ -233,7 +226,7 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
# Make another request providing the UI auth flow.
|
||||
self.delete_device(
|
||||
self.user_tok,
|
||||
device_id,
|
||||
self.device_id,
|
||||
200,
|
||||
{
|
||||
"auth": {
|
||||
|
@ -252,14 +245,13 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
UIA - check that still works.
|
||||
"""
|
||||
|
||||
device_id = self.get_device_ids(self.user_tok)[0]
|
||||
channel = self.delete_device(self.user_tok, device_id, 401)
|
||||
channel = self.delete_device(self.user_tok, self.device_id, 401)
|
||||
session = channel.json_body["session"]
|
||||
|
||||
# Make another request providing the UI auth flow.
|
||||
self.delete_device(
|
||||
self.user_tok,
|
||||
device_id,
|
||||
self.device_id,
|
||||
200,
|
||||
{
|
||||
"auth": {
|
||||
|
@ -282,14 +274,11 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
session ID should be rejected.
|
||||
"""
|
||||
# Create a second login.
|
||||
self.login("test", self.user_pass)
|
||||
|
||||
device_ids = self.get_device_ids(self.user_tok)
|
||||
self.assertEqual(len(device_ids), 2)
|
||||
self.login("test", self.user_pass, "dev2")
|
||||
|
||||
# Attempt to delete the first device.
|
||||
# Returns a 401 as per the spec
|
||||
channel = self.delete_devices(401, {"devices": [device_ids[0]]})
|
||||
channel = self.delete_devices(401, {"devices": [self.device_id]})
|
||||
|
||||
# Grab the session
|
||||
session = channel.json_body["session"]
|
||||
|
@ -301,7 +290,7 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
self.delete_devices(
|
||||
200,
|
||||
{
|
||||
"devices": [device_ids[1]],
|
||||
"devices": ["dev2"],
|
||||
"auth": {
|
||||
"type": "m.login.password",
|
||||
"identifier": {"type": "m.id.user", "user": self.user},
|
||||
|
@ -316,14 +305,11 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
The initial requested URI cannot be modified during the user interactive authentication session.
|
||||
"""
|
||||
# Create a second login.
|
||||
self.login("test", self.user_pass)
|
||||
|
||||
device_ids = self.get_device_ids(self.user_tok)
|
||||
self.assertEqual(len(device_ids), 2)
|
||||
self.login("test", self.user_pass, "dev2")
|
||||
|
||||
# Attempt to delete the first device.
|
||||
# Returns a 401 as per the spec
|
||||
channel = self.delete_device(self.user_tok, device_ids[0], 401)
|
||||
channel = self.delete_device(self.user_tok, self.device_id, 401)
|
||||
|
||||
# Grab the session
|
||||
session = channel.json_body["session"]
|
||||
|
@ -332,9 +318,11 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
|
||||
# Make another request providing the UI auth flow, but try to delete the
|
||||
# second device. This results in an error.
|
||||
#
|
||||
# This makes use of the fact that the device ID is embedded into the URL.
|
||||
self.delete_device(
|
||||
self.user_tok,
|
||||
device_ids[1],
|
||||
"dev2",
|
||||
403,
|
||||
{
|
||||
"auth": {
|
||||
|
@ -346,6 +334,52 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
},
|
||||
)
|
||||
|
||||
@unittest.override_config({"ui_auth": {"session_timeout": 5 * 1000}})
|
||||
def test_can_reuse_session(self):
|
||||
"""
|
||||
The session can be reused if configured.
|
||||
|
||||
Compare to test_cannot_change_uri.
|
||||
"""
|
||||
# Create a second and third login.
|
||||
self.login("test", self.user_pass, "dev2")
|
||||
self.login("test", self.user_pass, "dev3")
|
||||
|
||||
# Attempt to delete a device. This works since the user just logged in.
|
||||
self.delete_device(self.user_tok, "dev2", 200)
|
||||
|
||||
# Move the clock forward past the validation timeout.
|
||||
self.reactor.advance(6)
|
||||
|
||||
# Deleting another devices throws the user into UI auth.
|
||||
channel = self.delete_device(self.user_tok, "dev3", 401)
|
||||
|
||||
# Grab the session
|
||||
session = channel.json_body["session"]
|
||||
# Ensure that flows are what is expected.
|
||||
self.assertIn({"stages": ["m.login.password"]}, channel.json_body["flows"])
|
||||
|
||||
# Make another request providing the UI auth flow.
|
||||
self.delete_device(
|
||||
self.user_tok,
|
||||
"dev3",
|
||||
200,
|
||||
{
|
||||
"auth": {
|
||||
"type": "m.login.password",
|
||||
"identifier": {"type": "m.id.user", "user": self.user},
|
||||
"password": self.user_pass,
|
||||
"session": session,
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
# Make another request, but try to delete the first device. This works
|
||||
# due to re-using the previous session.
|
||||
#
|
||||
# Note that *no auth* information is provided, not even a session iD!
|
||||
self.delete_device(self.user_tok, self.device_id, 200)
|
||||
|
||||
def test_does_not_offer_password_for_sso_user(self):
|
||||
login_resp = self.helper.login_via_oidc("username")
|
||||
user_tok = login_resp["access_token"]
|
||||
|
@ -361,8 +395,7 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
def test_does_not_offer_sso_for_password_user(self):
|
||||
# now call the device deletion API: we should get the option to auth with SSO
|
||||
# and not password.
|
||||
device_ids = self.get_device_ids(self.user_tok)
|
||||
channel = self.delete_device(self.user_tok, device_ids[0], 401)
|
||||
channel = self.delete_device(self.user_tok, self.device_id, 401)
|
||||
|
||||
flows = channel.json_body["flows"]
|
||||
self.assertEqual(flows, [{"stages": ["m.login.password"]}])
|
||||
|
@ -373,8 +406,7 @@ class UIAuthTests(unittest.HomeserverTestCase):
|
|||
login_resp = self.helper.login_via_oidc(UserID.from_string(self.user).localpart)
|
||||
self.assertEqual(login_resp["user_id"], self.user)
|
||||
|
||||
device_ids = self.get_device_ids(self.user_tok)
|
||||
channel = self.delete_device(self.user_tok, device_ids[0], 401)
|
||||
channel = self.delete_device(self.user_tok, self.device_id, 401)
|
||||
|
||||
flows = channel.json_body["flows"]
|
||||
# we have no particular expectations of ordering here
|
||||
|
|
Loading…
Reference in New Issue