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

Save login tokens in database #13844

Merged
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/13844.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Save login tokens in database and prevent login token reuse.
9 changes: 9 additions & 0 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ process, for example:
dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
```

# Upgrading to v1.71.0

## Removal of the `generate_short_term_login_token` module API method

As announced with the release of [Synapse 1.69.0](#deprecation-of-the-generate_short_term_login_token-module-api-method), the deprecated `generate_short_term_login_token` module method has been removed.

Modules relying on it can instead use the `create_login_token` method.


# Upgrading to v1.69.0

## Changes to the receipts replication streams
Expand Down
64 changes: 54 additions & 10 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import attr
import bcrypt
import unpaddedbase64
from prometheus_client import Counter

from twisted.internet.defer import CancelledError
from twisted.web.server import Request
Expand All @@ -48,6 +49,7 @@
Codes,
InteractiveAuthIncompleteError,
LoginError,
NotFoundError,
StoreError,
SynapseError,
UserDeactivatedError,
Expand All @@ -63,10 +65,14 @@
from synapse.http.site import SynapseRequest
from synapse.logging.context import defer_to_thread
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.storage.databases.main.registration import (
LoginTokenExpired,
LoginTokenLookupResult,
LoginTokenReused,
)
from synapse.types import JsonDict, Requester, UserID
from synapse.util import stringutils as stringutils
from synapse.util.async_helpers import delay_cancellation, maybe_awaitable
from synapse.util.macaroons import LoginTokenAttributes
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.stringutils import base62_encode
from synapse.util.threepids import canonicalise_email
Expand All @@ -80,6 +86,12 @@

INVALID_USERNAME_OR_PASSWORD = "Invalid username or password"

invalid_login_token_counter = Counter(
"synapse_user_login_invalid_login_tokens",
"Counts the number of rejected m.login.token on /login",
["reason"],
)


def convert_client_dict_legacy_fields_to_identifier(
submission: JsonDict,
Expand Down Expand Up @@ -883,6 +895,25 @@ def _verify_refresh_token(self, token: str) -> bool:

return True

async def create_login_token_for_user_id(
self,
user_id: str,
duration_ms: int = (2 * 60 * 1000),
auth_provider_id: Optional[str] = None,
auth_provider_session_id: Optional[str] = None,
) -> str:
login_token = self.generate_login_token()
now = self._clock.time_msec()
expiry_ts = now + duration_ms
await self.store.add_login_token_to_user(
user_id=user_id,
token=login_token,
expiry_ts=expiry_ts,
auth_provider_id=auth_provider_id,
auth_provider_session_id=auth_provider_session_id,
)
return login_token

async def create_refresh_token_for_user_id(
self,
user_id: str,
Expand Down Expand Up @@ -1401,6 +1432,18 @@ async def _check_local_password(self, user_id: str, password: str) -> Optional[s
return None
return user_id

def generate_login_token(self) -> str:
"""Generates an opaque string, for use as an short-term login token"""

# we use the following format for access tokens:
# syl_<random string>_<base62 crc check>

random_string = stringutils.random_string(20)
base = f"syl_{random_string}"

crc = base62_encode(crc32(base.encode("ascii")), minwidth=6)
return f"{base}_{crc}"

def generate_access_token(self, for_user: UserID) -> str:
"""Generates an opaque string, for use as an access token"""

Expand All @@ -1427,16 +1470,17 @@ def generate_refresh_token(self, for_user: UserID) -> str:
crc = base62_encode(crc32(base.encode("ascii")), minwidth=6)
return f"{base}_{crc}"

async def validate_short_term_login_token(
self, login_token: str
) -> LoginTokenAttributes:
async def consume_login_token(self, login_token: str) -> LoginTokenLookupResult:
try:
res = self.macaroon_gen.verify_short_term_login_token(login_token)
except Exception:
raise AuthError(403, "Invalid login token", errcode=Codes.FORBIDDEN)
return await self.store.consume_login_token(login_token)
except LoginTokenExpired:
invalid_login_token_counter.labels("expired").inc()
except LoginTokenReused:
invalid_login_token_counter.labels("reused").inc()
except NotFoundError:
invalid_login_token_counter.labels("not found").inc()

await self.auth_blocking.check_auth_blocking(res.user_id)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not needed, because the check_auth_blocking is called by the caller. So before we effectively checked that twice.

return res
raise AuthError(403, "Invalid login token", errcode=Codes.FORBIDDEN)

async def delete_access_token(self, access_token: str) -> None:
"""Invalidate a single access token
Expand Down Expand Up @@ -1711,7 +1755,7 @@ async def complete_sso_login(
)

# Create a login token
login_token = self.macaroon_gen.generate_short_term_login_token(
login_token = await self.create_login_token_for_user_id(
registered_user_id,
auth_provider_id=auth_provider_id,
auth_provider_session_id=auth_provider_session_id,
Expand Down
41 changes: 1 addition & 40 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,50 +771,11 @@ async def create_login_token(
auth_provider_session_id: The session ID got during login from the SSO IdP,
if any.
"""
# The deprecated `generate_short_term_login_token` method defaulted to an empty
# string for the `auth_provider_id` because of how the underlying macaroon was
# generated. This will change to a proper NULL-able field when the tokens get
# moved to the database.
return self._hs.get_macaroon_generator().generate_short_term_login_token(
return await self._hs.get_auth_handler().create_login_token_for_user_id(
user_id,
auth_provider_id or "",
auth_provider_session_id,
duration_in_ms,
)

def generate_short_term_login_token(
self,
user_id: str,
duration_in_ms: int = (2 * 60 * 1000),
auth_provider_id: str = "",
auth_provider_session_id: Optional[str] = None,
) -> str:
"""Generate a login token suitable for m.login.token authentication

Added in Synapse v1.9.0.

This was deprecated in Synapse v1.69.0 in favor of create_login_token, and will
be removed in Synapse 1.71.0.

Args:
user_id: gives the ID of the user that the token is for

duration_in_ms: the time that the token will be valid for

auth_provider_id: the ID of the SSO IdP that the user used to authenticate
to get this token, if any. This is encoded in the token so that
/login can report stats on number of successful logins by IdP.
"""
logger.warn(
"A module configured on this server uses ModuleApi.generate_short_term_login_token(), "
"which is deprecated in favor of ModuleApi.create_login_token(), and will be removed in "
"Synapse 1.71.0",
)
return self._hs.get_macaroon_generator().generate_short_term_login_token(
user_id,
auth_provider_id,
auth_provider_session_id,
duration_in_ms,
)

@defer.inlineCallbacks
Expand Down
3 changes: 1 addition & 2 deletions synapse/rest/client/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,7 @@ async def _do_token_login(
The body of the JSON response.
"""
token = login_submission["token"]
auth_handler = self.auth_handler
res = await auth_handler.validate_short_term_login_token(token)
res = await self.auth_handler.consume_login_token(token)

return await self._complete_login(
res.user_id,
Expand Down
5 changes: 2 additions & 3 deletions synapse/rest/client/login_token_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ def __init__(self, hs: "HomeServer"):
self.store = hs.get_datastores().main
self.clock = hs.get_clock()
self.server_name = hs.config.server.server_name
self.macaroon_gen = hs.get_macaroon_generator()
self.auth_handler = hs.get_auth_handler()
self.token_timeout = hs.config.experimental.msc3882_token_timeout
self.ui_auth = hs.config.experimental.msc3882_ui_auth
Expand All @@ -76,10 +75,10 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
can_skip_ui_auth=False, # Don't allow skipping of UI auth
)

login_token = self.macaroon_gen.generate_short_term_login_token(
login_token = await self.auth_handler.create_login_token_for_user_id(
user_id=requester.user.to_string(),
auth_provider_id="org.matrix.msc3882.login_token_request",
duration_in_ms=self.token_timeout,
duration_ms=self.token_timeout,
)

return (
Expand Down
Loading