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

Allow re-using a UI auth validation for a period of time #8970

Merged
merged 5 commits into from
Dec 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8970.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow re-using an user-interactive authentication session for a period of time.
15 changes: 15 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

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

I debated if this should be separate for each operation? Something like:

session_timeout:
  delete_device: 0
  reset_password: 0

It seemed like added complexity that I didn't want in the first version though. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I'm minded to agree with you.



# Configuration for sending emails from Synapse.
#
Expand Down
4 changes: 2 additions & 2 deletions synapse/config/_base.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ from typing import Any, Iterable, List, Optional
from synapse.config import (
api,
appservice,
auth,
captcha,
cas,
consent_config,
Expand All @@ -14,7 +15,6 @@ from synapse.config import (
logger,
metrics,
oidc_config,
password,
password_auth_providers,
push,
ratelimiting,
Expand Down Expand Up @@ -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
Expand Down
26 changes: 23 additions & 3 deletions synapse/config/password.py → synapse/config/auth.py
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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", {})
Expand All @@ -35,6 +36,10 @@ def read_config(self, config, **kwargs):
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:
Expand Down Expand Up @@ -87,4 +92,19 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
# 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
"""
4 changes: 2 additions & 2 deletions synapse/config/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -30,7 +31,6 @@
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
Expand Down Expand Up @@ -76,7 +76,7 @@ class HomeServerConfig(RootConfig):
CasConfig,
SSOConfig,
JWTConfig,
PasswordConfig,
AuthConfig,
EmailConfig,
PasswordAuthProviderConfig,
PushConfig,
Expand Down
32 changes: 24 additions & 8 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ def __init__(self, hs: "HomeServer"):
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(),
Expand Down Expand Up @@ -283,7 +286,7 @@ async def validate_user_via_ui_auth(
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.

Expand All @@ -310,7 +313,8 @@ async def validate_user_via_ui_auth(
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
Expand All @@ -324,6 +328,16 @@ async def validate_user_via_ui_auth(

"""

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
Expand Down Expand Up @@ -359,6 +373,9 @@ async def validate_user_via_ui_auth(
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]:
Expand Down Expand Up @@ -452,13 +469,10 @@ async def check_ui_auth(
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")
Expand Down Expand Up @@ -563,6 +577,8 @@ async def check_ui_auth(

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
Expand Down
10 changes: 7 additions & 3 deletions synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,18 @@ async def on_POST(self, request):
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)

Expand Down
38 changes: 38 additions & 0 deletions synapse/storage/databases/main/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,42 @@ async def del_user_pending_deactivation(self, user_id: str) -> None:
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"):
Expand Down Expand Up @@ -1150,6 +1186,7 @@ async def add_access_token_to_user(
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",
Expand All @@ -1160,6 +1197,7 @@ async def add_access_token_to_user(
"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",
)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Loading