From e1b8e37f936b115e2164d272333c9b15342e6f88 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 16 Dec 2020 20:01:53 +0000
Subject: Push login completion down into SsoHandler (#8941)
This is another part of my work towards fixing #8876. It moves some of the logic currently in the SAML and OIDC handlers - in particular the call to `AuthHandler.complete_sso_login` down into the `SsoHandler`.
---
synapse/handlers/sso.py | 58 ++++++++++++++++++++++++++++++-------------------
1 file changed, 36 insertions(+), 22 deletions(-)
(limited to 'synapse/handlers/sso.py')
diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py
index 112a7d5b2c..f054b66a53 100644
--- a/synapse/handlers/sso.py
+++ b/synapse/handlers/sso.py
@@ -21,7 +21,8 @@ from twisted.web.http import Request
from synapse.api.errors import RedirectException
from synapse.http.server import respond_with_html
-from synapse.types import UserID, contains_invalid_mxid_characters
+from synapse.http.site import SynapseRequest
+from synapse.types import JsonDict, UserID, contains_invalid_mxid_characters
from synapse.util.async_helpers import Linearizer
if TYPE_CHECKING:
@@ -119,15 +120,16 @@ class SsoHandler:
# No match.
return None
- async def get_mxid_from_sso(
+ async def complete_sso_login_request(
self,
auth_provider_id: str,
remote_user_id: str,
- user_agent: str,
- ip_address: str,
+ request: SynapseRequest,
+ client_redirect_url: str,
sso_to_matrix_id_mapper: Callable[[int], Awaitable[UserAttributes]],
grandfather_existing_users: Optional[Callable[[], Awaitable[Optional[str]]]],
- ) -> str:
+ extra_login_attributes: Optional[JsonDict] = None,
+ ) -> None:
"""
Given an SSO ID, retrieve the user ID for it and possibly register the user.
@@ -146,12 +148,18 @@ class SsoHandler:
given user-agent and IP address and the SSO ID is linked to this matrix
ID for subsequent calls.
+ Finally, we generate a redirect to the supplied redirect uri, with a login token
+
Args:
auth_provider_id: A unique identifier for this SSO provider, e.g.
"oidc" or "saml".
+
remote_user_id: The unique identifier from the SSO provider.
- user_agent: The user agent of the client making the request.
- ip_address: The IP address of the client making the request.
+
+ request: The request to respond to
+
+ client_redirect_url: The redirect URL passed in by the client.
+
sso_to_matrix_id_mapper: A callable to generate the user attributes.
The only parameter is an integer which represents the amount of
times the returned mxid localpart mapping has failed.
@@ -163,12 +171,13 @@ class SsoHandler:
to the user.
RedirectException to redirect to an additional page (e.g.
to prompt the user for more information).
+
grandfather_existing_users: A callable which can return an previously
existing matrix ID. The SSO ID is then linked to the returned
matrix ID.
- Returns:
- The user ID associated with the SSO response.
+ extra_login_attributes: An optional dictionary of extra
+ attributes to be provided to the client in the login response.
Raises:
MappingException if there was a problem mapping the response to a user.
@@ -181,28 +190,33 @@ class SsoHandler:
# interstitial pages.
with await self._mapping_lock.queue(auth_provider_id):
# first of all, check if we already have a mapping for this user
- previously_registered_user_id = await self.get_sso_user_by_remote_user_id(
+ user_id = await self.get_sso_user_by_remote_user_id(
auth_provider_id, remote_user_id,
)
- if previously_registered_user_id:
- return previously_registered_user_id
# Check for grandfathering of users.
- if grandfather_existing_users:
- previously_registered_user_id = await grandfather_existing_users()
- if previously_registered_user_id:
+ if not user_id and grandfather_existing_users:
+ user_id = await grandfather_existing_users()
+ if user_id:
# Future logins should also match this user ID.
await self._store.record_user_external_id(
- auth_provider_id, remote_user_id, previously_registered_user_id
+ auth_provider_id, remote_user_id, user_id
)
- return previously_registered_user_id
# Otherwise, generate a new user.
- attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper)
- user_id = await self._register_mapped_user(
- attributes, auth_provider_id, remote_user_id, user_agent, ip_address,
- )
- return user_id
+ if not user_id:
+ attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper)
+ user_id = await self._register_mapped_user(
+ attributes,
+ auth_provider_id,
+ remote_user_id,
+ request.get_user_agent(""),
+ request.getClientIP(),
+ )
+
+ await self._auth_handler.complete_sso_login(
+ user_id, request, client_redirect_url, extra_login_attributes
+ )
async def _call_attribute_mapper(
self, sso_to_matrix_id_mapper: Callable[[int], Awaitable[UserAttributes]],
--
cgit 1.5.1
From 28877fade90a5cfb3457c9e6c70924dbbe8af715 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Fri, 18 Dec 2020 14:19:46 +0000
Subject: Implement a username picker for synapse (#8942)
The final part (for now) of my work to implement a username picker in synapse itself. The idea is that we allow
`UsernameMappingProvider`s to return `localpart=None`, in which case, rather than redirecting the browser
back to the client, we redirect to a username-picker resource, which allows the user to enter a username.
We *then* complete the SSO flow (including doing the client permission checks).
The static resources for the username picker itself (in
https://github.com/matrix-org/synapse/tree/rav/username_picker/synapse/res/username_picker)
are essentially lifted wholesale from
https://github.com/matrix-org/matrix-synapse-saml-mozilla/tree/master/matrix_synapse_saml_mozilla/res.
As the comment says, we might want to think about making them customisable, but that can be a follow-up.
Fixes #8876.
---
changelog.d/8942.feature | 1 +
docs/sample_config.yaml | 5 +-
docs/sso_mapping_providers.md | 28 +--
synapse/app/homeserver.py | 2 +
synapse/config/oidc_config.py | 5 +-
synapse/handlers/oidc_handler.py | 59 +++----
synapse/handlers/sso.py | 254 ++++++++++++++++++++++++++-
synapse/res/username_picker/index.html | 19 ++
synapse/res/username_picker/script.js | 95 ++++++++++
synapse/res/username_picker/style.css | 27 +++
synapse/rest/synapse/client/pick_username.py | 88 ++++++++++
synapse/types.py | 8 +-
tests/handlers/test_oidc.py | 143 ++++++++++++++-
tests/unittest.py | 8 +-
14 files changed, 683 insertions(+), 59 deletions(-)
create mode 100644 changelog.d/8942.feature
create mode 100644 synapse/res/username_picker/index.html
create mode 100644 synapse/res/username_picker/script.js
create mode 100644 synapse/res/username_picker/style.css
create mode 100644 synapse/rest/synapse/client/pick_username.py
(limited to 'synapse/handlers/sso.py')
diff --git a/changelog.d/8942.feature b/changelog.d/8942.feature
new file mode 100644
index 0000000000..d450ef4998
--- /dev/null
+++ b/changelog.d/8942.feature
@@ -0,0 +1 @@
+Add support for allowing users to pick their own user ID during a single-sign-on login.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 549c581a97..077cb619c7 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -1825,9 +1825,10 @@ oidc_config:
# * user: The claims returned by the UserInfo Endpoint and/or in the ID
# Token
#
- # This must be configured if using the default mapping provider.
+ # If this is not set, the user will be prompted to choose their
+ # own username.
#
- localpart_template: "{{ user.preferred_username }}"
+ #localpart_template: "{{ user.preferred_username }}"
# Jinja2 template for the display name to set on first login.
#
diff --git a/docs/sso_mapping_providers.md b/docs/sso_mapping_providers.md
index 7714b1d844..e1d6ede7ba 100644
--- a/docs/sso_mapping_providers.md
+++ b/docs/sso_mapping_providers.md
@@ -15,12 +15,18 @@ where SAML mapping providers come into play.
SSO mapping providers are currently supported for OpenID and SAML SSO
configurations. Please see the details below for how to implement your own.
-It is the responsibility of the mapping provider to normalise the SSO attributes
-and map them to a valid Matrix ID. The
-[specification for Matrix IDs](https://matrix.org/docs/spec/appendices#user-identifiers)
-has some information about what is considered valid. Alternately an easy way to
-ensure it is valid is to use a Synapse utility function:
-`synapse.types.map_username_to_mxid_localpart`.
+It is up to the mapping provider whether the user should be assigned a predefined
+Matrix ID based on the SSO attributes, or if the user should be allowed to
+choose their own username.
+
+In the first case - where users are automatically allocated a Matrix ID - it is
+the responsibility of the mapping provider to normalise the SSO attributes and
+map them to a valid Matrix ID. The [specification for Matrix
+IDs](https://matrix.org/docs/spec/appendices#user-identifiers) has some
+information about what is considered valid.
+
+If the mapping provider does not assign a Matrix ID, then Synapse will
+automatically serve an HTML page allowing the user to pick their own username.
External mapping providers are provided to Synapse in the form of an external
Python module. You can retrieve this module from [PyPI](https://pypi.org) or elsewhere,
@@ -80,8 +86,9 @@ A custom mapping provider must specify the following methods:
with failures=1. The method should then return a different
`localpart` value, such as `john.doe1`.
- Returns a dictionary with two keys:
- - localpart: A required string, used to generate the Matrix ID.
- - displayname: An optional string, the display name for the user.
+ - `localpart`: A string, used to generate the Matrix ID. If this is
+ `None`, the user is prompted to pick their own username.
+ - `displayname`: An optional string, the display name for the user.
* `get_extra_attributes(self, userinfo, token)`
- This method must be async.
- Arguments:
@@ -165,12 +172,13 @@ A custom mapping provider must specify the following methods:
redirected to.
- This method must return a dictionary, which will then be used by Synapse
to build a new user. The following keys are allowed:
- * `mxid_localpart` - Required. The mxid localpart of the new user.
+ * `mxid_localpart` - The mxid localpart of the new user. If this is
+ `None`, the user is prompted to pick their own username.
* `displayname` - The displayname of the new user. If not provided, will default to
the value of `mxid_localpart`.
* `emails` - A list of emails for the new user. If not provided, will
default to an empty list.
-
+
Alternatively it can raise a `synapse.api.errors.RedirectException` to
redirect the user to another page. This is useful to prompt the user for
additional information, e.g. if you want them to provide their own username.
diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py
index bbb7407838..8d9b53be53 100644
--- a/synapse/app/homeserver.py
+++ b/synapse/app/homeserver.py
@@ -63,6 +63,7 @@ from synapse.rest import ClientRestResource
from synapse.rest.admin import AdminRestResource
from synapse.rest.health import HealthResource
from synapse.rest.key.v2 import KeyApiV2Resource
+from synapse.rest.synapse.client.pick_username import pick_username_resource
from synapse.rest.well_known import WellKnownResource
from synapse.server import HomeServer
from synapse.storage import DataStore
@@ -192,6 +193,7 @@ class SynapseHomeServer(HomeServer):
"/_matrix/client/versions": client_resource,
"/.well-known/matrix/client": WellKnownResource(self),
"/_synapse/admin": AdminRestResource(self),
+ "/_synapse/client/pick_username": pick_username_resource(self),
}
)
diff --git a/synapse/config/oidc_config.py b/synapse/config/oidc_config.py
index 1abf8ed405..4e3055282d 100644
--- a/synapse/config/oidc_config.py
+++ b/synapse/config/oidc_config.py
@@ -203,9 +203,10 @@ class OIDCConfig(Config):
# * user: The claims returned by the UserInfo Endpoint and/or in the ID
# Token
#
- # This must be configured if using the default mapping provider.
+ # If this is not set, the user will be prompted to choose their
+ # own username.
#
- localpart_template: "{{{{ user.preferred_username }}}}"
+ #localpart_template: "{{{{ user.preferred_username }}}}"
# Jinja2 template for the display name to set on first login.
#
diff --git a/synapse/handlers/oidc_handler.py b/synapse/handlers/oidc_handler.py
index cbd11a1382..709f8dfc13 100644
--- a/synapse/handlers/oidc_handler.py
+++ b/synapse/handlers/oidc_handler.py
@@ -947,7 +947,7 @@ class OidcHandler(BaseHandler):
UserAttributeDict = TypedDict(
- "UserAttributeDict", {"localpart": str, "display_name": Optional[str]}
+ "UserAttributeDict", {"localpart": Optional[str], "display_name": Optional[str]}
)
C = TypeVar("C")
@@ -1028,10 +1028,10 @@ env = Environment(finalize=jinja_finalize)
@attr.s
class JinjaOidcMappingConfig:
- subject_claim = attr.ib() # type: str
- localpart_template = attr.ib() # type: Template
- display_name_template = attr.ib() # type: Optional[Template]
- extra_attributes = attr.ib() # type: Dict[str, Template]
+ subject_claim = attr.ib(type=str)
+ localpart_template = attr.ib(type=Optional[Template])
+ display_name_template = attr.ib(type=Optional[Template])
+ extra_attributes = attr.ib(type=Dict[str, Template])
class JinjaOidcMappingProvider(OidcMappingProvider[JinjaOidcMappingConfig]):
@@ -1047,18 +1047,14 @@ class JinjaOidcMappingProvider(OidcMappingProvider[JinjaOidcMappingConfig]):
def parse_config(config: dict) -> JinjaOidcMappingConfig:
subject_claim = config.get("subject_claim", "sub")
- if "localpart_template" not in config:
- raise ConfigError(
- "missing key: oidc_config.user_mapping_provider.config.localpart_template"
- )
-
- try:
- localpart_template = env.from_string(config["localpart_template"])
- except Exception as e:
- raise ConfigError(
- "invalid jinja template for oidc_config.user_mapping_provider.config.localpart_template: %r"
- % (e,)
- )
+ localpart_template = None # type: Optional[Template]
+ if "localpart_template" in config:
+ try:
+ localpart_template = env.from_string(config["localpart_template"])
+ except Exception as e:
+ raise ConfigError(
+ "invalid jinja template", path=["localpart_template"]
+ ) from e
display_name_template = None # type: Optional[Template]
if "display_name_template" in config:
@@ -1066,26 +1062,22 @@ class JinjaOidcMappingProvider(OidcMappingProvider[JinjaOidcMappingConfig]):
display_name_template = env.from_string(config["display_name_template"])
except Exception as e:
raise ConfigError(
- "invalid jinja template for oidc_config.user_mapping_provider.config.display_name_template: %r"
- % (e,)
- )
+ "invalid jinja template", path=["display_name_template"]
+ ) from e
extra_attributes = {} # type Dict[str, Template]
if "extra_attributes" in config:
extra_attributes_config = config.get("extra_attributes") or {}
if not isinstance(extra_attributes_config, dict):
- raise ConfigError(
- "oidc_config.user_mapping_provider.config.extra_attributes must be a dict"
- )
+ raise ConfigError("must be a dict", path=["extra_attributes"])
for key, value in extra_attributes_config.items():
try:
extra_attributes[key] = env.from_string(value)
except Exception as e:
raise ConfigError(
- "invalid jinja template for oidc_config.user_mapping_provider.config.extra_attributes.%s: %r"
- % (key, e)
- )
+ "invalid jinja template", path=["extra_attributes", key]
+ ) from e
return JinjaOidcMappingConfig(
subject_claim=subject_claim,
@@ -1100,14 +1092,17 @@ class JinjaOidcMappingProvider(OidcMappingProvider[JinjaOidcMappingConfig]):
async def map_user_attributes(
self, userinfo: UserInfo, token: Token, failures: int
) -> UserAttributeDict:
- localpart = self._config.localpart_template.render(user=userinfo).strip()
+ localpart = None
+
+ if self._config.localpart_template:
+ localpart = self._config.localpart_template.render(user=userinfo).strip()
- # Ensure only valid characters are included in the MXID.
- localpart = map_username_to_mxid_localpart(localpart)
+ # Ensure only valid characters are included in the MXID.
+ localpart = map_username_to_mxid_localpart(localpart)
- # Append suffix integer if last call to this function failed to produce
- # a usable mxid.
- localpart += str(failures) if failures else ""
+ # Append suffix integer if last call to this function failed to produce
+ # a usable mxid.
+ localpart += str(failures) if failures else ""
display_name = None # type: Optional[str]
if self._config.display_name_template is not None:
diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py
index f054b66a53..548b02211b 100644
--- a/synapse/handlers/sso.py
+++ b/synapse/handlers/sso.py
@@ -13,17 +13,19 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
-from typing import TYPE_CHECKING, Awaitable, Callable, List, Optional
+from typing import TYPE_CHECKING, Awaitable, Callable, Dict, List, Optional
import attr
+from typing_extensions import NoReturn
from twisted.web.http import Request
-from synapse.api.errors import RedirectException
+from synapse.api.errors import RedirectException, SynapseError
from synapse.http.server import respond_with_html
from synapse.http.site import SynapseRequest
from synapse.types import JsonDict, UserID, contains_invalid_mxid_characters
from synapse.util.async_helpers import Linearizer
+from synapse.util.stringutils import random_string
if TYPE_CHECKING:
from synapse.server import HomeServer
@@ -40,16 +42,52 @@ class MappingException(Exception):
@attr.s
class UserAttributes:
- localpart = attr.ib(type=str)
+ # the localpart of the mxid that the mapper has assigned to the user.
+ # if `None`, the mapper has not picked a userid, and the user should be prompted to
+ # enter one.
+ localpart = attr.ib(type=Optional[str])
display_name = attr.ib(type=Optional[str], default=None)
emails = attr.ib(type=List[str], default=attr.Factory(list))
+@attr.s(slots=True)
+class UsernameMappingSession:
+ """Data we track about SSO sessions"""
+
+ # A unique identifier for this SSO provider, e.g. "oidc" or "saml".
+ auth_provider_id = attr.ib(type=str)
+
+ # user ID on the IdP server
+ remote_user_id = attr.ib(type=str)
+
+ # attributes returned by the ID mapper
+ display_name = attr.ib(type=Optional[str])
+ emails = attr.ib(type=List[str])
+
+ # An optional dictionary of extra attributes to be provided to the client in the
+ # login response.
+ extra_login_attributes = attr.ib(type=Optional[JsonDict])
+
+ # where to redirect the client back to
+ client_redirect_url = attr.ib(type=str)
+
+ # expiry time for the session, in milliseconds
+ expiry_time_ms = attr.ib(type=int)
+
+
+# the HTTP cookie used to track the mapping session id
+USERNAME_MAPPING_SESSION_COOKIE_NAME = b"username_mapping_session"
+
+
class SsoHandler:
# The number of attempts to ask the mapping provider for when generating an MXID.
_MAP_USERNAME_RETRIES = 1000
+ # the time a UsernameMappingSession remains valid for
+ _MAPPING_SESSION_VALIDITY_PERIOD_MS = 15 * 60 * 1000
+
def __init__(self, hs: "HomeServer"):
+ self._clock = hs.get_clock()
self._store = hs.get_datastore()
self._server_name = hs.hostname
self._registration_handler = hs.get_registration_handler()
@@ -59,6 +97,9 @@ class SsoHandler:
# a lock on the mappings
self._mapping_lock = Linearizer(name="sso_user_mapping", clock=hs.get_clock())
+ # a map from session id to session data
+ self._username_mapping_sessions = {} # type: Dict[str, UsernameMappingSession]
+
def render_error(
self, request, error: str, error_description: Optional[str] = None
) -> None:
@@ -206,6 +247,18 @@ class SsoHandler:
# Otherwise, generate a new user.
if not user_id:
attributes = await self._call_attribute_mapper(sso_to_matrix_id_mapper)
+
+ if attributes.localpart is None:
+ # the mapper doesn't return a username. bail out with a redirect to
+ # the username picker.
+ await self._redirect_to_username_picker(
+ auth_provider_id,
+ remote_user_id,
+ attributes,
+ client_redirect_url,
+ extra_login_attributes,
+ )
+
user_id = await self._register_mapped_user(
attributes,
auth_provider_id,
@@ -243,10 +296,8 @@ class SsoHandler:
)
if not attributes.localpart:
- raise MappingException(
- "Error parsing SSO response: SSO mapping provider plugin "
- "did not return a localpart value"
- )
+ # the mapper has not picked a localpart
+ return attributes
# Check if this mxid already exists
user_id = UserID(attributes.localpart, self._server_name).to_string()
@@ -261,6 +312,59 @@ class SsoHandler:
)
return attributes
+ async def _redirect_to_username_picker(
+ self,
+ auth_provider_id: str,
+ remote_user_id: str,
+ attributes: UserAttributes,
+ client_redirect_url: str,
+ extra_login_attributes: Optional[JsonDict],
+ ) -> NoReturn:
+ """Creates a UsernameMappingSession and redirects the browser
+
+ Called if the user mapping provider doesn't return a localpart for a new user.
+ Raises a RedirectException which redirects the browser to the username picker.
+
+ Args:
+ auth_provider_id: A unique identifier for this SSO provider, e.g.
+ "oidc" or "saml".
+
+ remote_user_id: The unique identifier from the SSO provider.
+
+ attributes: the user attributes returned by the user mapping provider.
+
+ client_redirect_url: The redirect URL passed in by the client, which we
+ will eventually redirect back to.
+
+ extra_login_attributes: An optional dictionary of extra
+ attributes to be provided to the client in the login response.
+
+ Raises:
+ RedirectException
+ """
+ session_id = random_string(16)
+ now = self._clock.time_msec()
+ session = UsernameMappingSession(
+ auth_provider_id=auth_provider_id,
+ remote_user_id=remote_user_id,
+ display_name=attributes.display_name,
+ emails=attributes.emails,
+ client_redirect_url=client_redirect_url,
+ expiry_time_ms=now + self._MAPPING_SESSION_VALIDITY_PERIOD_MS,
+ extra_login_attributes=extra_login_attributes,
+ )
+
+ self._username_mapping_sessions[session_id] = session
+ logger.info("Recorded registration session id %s", session_id)
+
+ # Set the cookie and redirect to the username picker
+ e = RedirectException(b"/_synapse/client/pick_username")
+ e.cookies.append(
+ b"%s=%s; path=/"
+ % (USERNAME_MAPPING_SESSION_COOKIE_NAME, session_id.encode("ascii"))
+ )
+ raise e
+
async def _register_mapped_user(
self,
attributes: UserAttributes,
@@ -269,9 +373,38 @@ class SsoHandler:
user_agent: str,
ip_address: str,
) -> str:
+ """Register a new SSO user.
+
+ This is called once we have successfully mapped the remote user id onto a local
+ user id, one way or another.
+
+ Args:
+ attributes: user attributes returned by the user mapping provider,
+ including a non-empty localpart.
+
+ auth_provider_id: A unique identifier for this SSO provider, e.g.
+ "oidc" or "saml".
+
+ remote_user_id: The unique identifier from the SSO provider.
+
+ user_agent: The user-agent in the HTTP request (used for potential
+ shadow-banning.)
+
+ ip_address: The IP address of the requester (used for potential
+ shadow-banning.)
+
+ Raises:
+ a MappingException if the localpart is invalid.
+
+ a SynapseError with code 400 and errcode Codes.USER_IN_USE if the localpart
+ is already taken.
+ """
+
# Since the localpart is provided via a potentially untrusted module,
# ensure the MXID is valid before registering.
- if contains_invalid_mxid_characters(attributes.localpart):
+ if not attributes.localpart or contains_invalid_mxid_characters(
+ attributes.localpart
+ ):
raise MappingException("localpart is invalid: %s" % (attributes.localpart,))
logger.debug("Mapped SSO user to local part %s", attributes.localpart)
@@ -326,3 +459,108 @@ class SsoHandler:
await self._auth_handler.complete_sso_ui_auth(
user_id, ui_auth_session_id, request
)
+
+ async def check_username_availability(
+ self, localpart: str, session_id: str,
+ ) -> bool:
+ """Handle an "is username available" callback check
+
+ Args:
+ localpart: desired localpart
+ session_id: the session id for the username picker
+ Returns:
+ True if the username is available
+ Raises:
+ SynapseError if the localpart is invalid or the session is unknown
+ """
+
+ # make sure that there is a valid mapping session, to stop people dictionary-
+ # scanning for accounts
+
+ self._expire_old_sessions()
+ session = self._username_mapping_sessions.get(session_id)
+ if not session:
+ logger.info("Couldn't find session id %s", session_id)
+ raise SynapseError(400, "unknown session")
+
+ logger.info(
+ "[session %s] Checking for availability of username %s",
+ session_id,
+ localpart,
+ )
+
+ if contains_invalid_mxid_characters(localpart):
+ raise SynapseError(400, "localpart is invalid: %s" % (localpart,))
+ user_id = UserID(localpart, self._server_name).to_string()
+ user_infos = await self._store.get_users_by_id_case_insensitive(user_id)
+
+ logger.info("[session %s] users: %s", session_id, user_infos)
+ return not user_infos
+
+ async def handle_submit_username_request(
+ self, request: SynapseRequest, localpart: str, session_id: str
+ ) -> None:
+ """Handle a request to the username-picker 'submit' endpoint
+
+ Will serve an HTTP response to the request.
+
+ Args:
+ request: HTTP request
+ localpart: localpart requested by the user
+ session_id: ID of the username mapping session, extracted from a cookie
+ """
+ self._expire_old_sessions()
+ session = self._username_mapping_sessions.get(session_id)
+ if not session:
+ logger.info("Couldn't find session id %s", session_id)
+ raise SynapseError(400, "unknown session")
+
+ logger.info("[session %s] Registering localpart %s", session_id, localpart)
+
+ attributes = UserAttributes(
+ localpart=localpart,
+ display_name=session.display_name,
+ emails=session.emails,
+ )
+
+ # the following will raise a 400 error if the username has been taken in the
+ # meantime.
+ user_id = await self._register_mapped_user(
+ attributes,
+ session.auth_provider_id,
+ session.remote_user_id,
+ request.get_user_agent(""),
+ request.getClientIP(),
+ )
+
+ logger.info("[session %s] Registered userid %s", session_id, user_id)
+
+ # delete the mapping session and the cookie
+ del self._username_mapping_sessions[session_id]
+
+ # delete the cookie
+ request.addCookie(
+ USERNAME_MAPPING_SESSION_COOKIE_NAME,
+ b"",
+ expires=b"Thu, 01 Jan 1970 00:00:00 GMT",
+ path=b"/",
+ )
+
+ await self._auth_handler.complete_sso_login(
+ user_id,
+ request,
+ session.client_redirect_url,
+ session.extra_login_attributes,
+ )
+
+ def _expire_old_sessions(self):
+ to_expire = []
+ now = int(self._clock.time_msec())
+
+ for session_id, session in self._username_mapping_sessions.items():
+ if session.expiry_time_ms <= now:
+ to_expire.append(session_id)
+
+ for session_id in to_expire:
+ logger.info("Expiring mapping session %s", session_id)
+ del self._username_mapping_sessions[session_id]
diff --git a/synapse/res/username_picker/index.html b/synapse/res/username_picker/index.html
new file mode 100644
index 0000000000..37ea8bb6d8
--- /dev/null
+++ b/synapse/res/username_picker/index.html
@@ -0,0 +1,19 @@
+
+
+
+ Synapse Login
+
+
+
+
+
+
+
+
+
+
+
diff --git a/synapse/res/username_picker/script.js b/synapse/res/username_picker/script.js
new file mode 100644
index 0000000000..416a7c6f41
--- /dev/null
+++ b/synapse/res/username_picker/script.js
@@ -0,0 +1,95 @@
+let inputField = document.getElementById("field-username");
+let inputForm = document.getElementById("form");
+let submitButton = document.getElementById("button-submit");
+let message = document.getElementById("message");
+
+// Submit username and receive response
+function showMessage(messageText) {
+ // Unhide the message text
+ message.classList.remove("hidden");
+
+ message.textContent = messageText;
+};
+
+function doSubmit() {
+ showMessage("Success. Please wait a moment for your browser to redirect.");
+
+ // remove the event handler before re-submitting the form.
+ delete inputForm.onsubmit;
+ inputForm.submit();
+}
+
+function onResponse(response) {
+ // Display message
+ showMessage(response);
+
+ // Enable submit button and input field
+ submitButton.classList.remove('button--disabled');
+ submitButton.value = "Submit";
+};
+
+let allowedUsernameCharacters = RegExp("[^a-z0-9\\.\\_\\=\\-\\/]");
+function usernameIsValid(username) {
+ return !allowedUsernameCharacters.test(username);
+}
+let allowedCharactersString = "lowercase letters, digits, ., _, -, /, =";
+
+function buildQueryString(params) {
+ return Object.keys(params)
+ .map(k => encodeURIComponent(k) + '=' + encodeURIComponent(params[k]))
+ .join('&');
+}
+
+function submitUsername(username) {
+ if(username.length == 0) {
+ onResponse("Please enter a username.");
+ return;
+ }
+ if(!usernameIsValid(username)) {
+ onResponse("Invalid username. Only the following characters are allowed: " + allowedCharactersString);
+ return;
+ }
+
+ // if this browser doesn't support fetch, skip the availability check.
+ if(!window.fetch) {
+ doSubmit();
+ return;
+ }
+
+ let check_uri = 'check?' + buildQueryString({"username": username});
+ fetch(check_uri, {
+ // include the cookie
+ "credentials": "same-origin",
+ }).then((response) => {
+ if(!response.ok) {
+ // for non-200 responses, raise the body of the response as an exception
+ return response.text().then((text) => { throw text; });
+ } else {
+ return response.json();
+ }
+ }).then((json) => {
+ if(json.error) {
+ throw json.error;
+ } else if(json.available) {
+ doSubmit();
+ } else {
+ onResponse("This username is not available, please choose another.");
+ }
+ }).catch((err) => {
+ onResponse("Error checking username availability: " + err);
+ });
+}
+
+function clickSubmit() {
+ event.preventDefault();
+ if(submitButton.classList.contains('button--disabled')) { return; }
+
+ // Disable submit button and input field
+ submitButton.classList.add('button--disabled');
+
+ // Submit username
+ submitButton.value = "Checking...";
+ submitUsername(inputField.value);
+};
+
+inputForm.onsubmit = clickSubmit;
diff --git a/synapse/res/username_picker/style.css b/synapse/res/username_picker/style.css
new file mode 100644
index 0000000000..745bd4c684
--- /dev/null
+++ b/synapse/res/username_picker/style.css
@@ -0,0 +1,27 @@
+input[type="text"] {
+ font-size: 100%;
+ background-color: #ededf0;
+ border: 1px solid #fff;
+ border-radius: .2em;
+ padding: .5em .9em;
+ display: block;
+ width: 26em;
+}
+
+.button--disabled {
+ border-color: #fff;
+ background-color: transparent;
+ color: #000;
+ text-transform: none;
+}
+
+.hidden {
+ display: none;
+}
+
+.tooltip {
+ background-color: #f9f9fa;
+ padding: 1em;
+ margin: 1em 0;
+}
+
diff --git a/synapse/rest/synapse/client/pick_username.py b/synapse/rest/synapse/client/pick_username.py
new file mode 100644
index 0000000000..d3b6803e65
--- /dev/null
+++ b/synapse/rest/synapse/client/pick_username.py
@@ -0,0 +1,88 @@
+# -*- coding: utf-8 -*-
+# 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.
+from typing import TYPE_CHECKING
+
+import pkg_resources
+
+from twisted.web.http import Request
+from twisted.web.resource import Resource
+from twisted.web.static import File
+
+from synapse.api.errors import SynapseError
+from synapse.handlers.sso import USERNAME_MAPPING_SESSION_COOKIE_NAME
+from synapse.http.server import DirectServeHtmlResource, DirectServeJsonResource
+from synapse.http.servlet import parse_string
+from synapse.http.site import SynapseRequest
+
+if TYPE_CHECKING:
+ from synapse.server import HomeServer
+
+
+def pick_username_resource(hs: "HomeServer") -> Resource:
+ """Factory method to generate the username picker resource.
+
+ This resource gets mounted under /_synapse/client/pick_username. The top-level
+ resource is just a File resource which serves up the static files in the resources
+ "res" directory, but it has a couple of children:
+
+ * "submit", which does the mechanics of registering the new user, and redirects the
+ browser back to the client URL
+
+ * "check": checks if a userid is free.
+ """
+
+ # XXX should we make this path customisable so that admins can restyle it?
+ base_path = pkg_resources.resource_filename("synapse", "res/username_picker")
+
+ res = File(base_path)
+ res.putChild(b"submit", SubmitResource(hs))
+ res.putChild(b"check", AvailabilityCheckResource(hs))
+
+ return res
+
+
+class AvailabilityCheckResource(DirectServeJsonResource):
+ def __init__(self, hs: "HomeServer"):
+ super().__init__()
+ self._sso_handler = hs.get_sso_handler()
+
+ async def _async_render_GET(self, request: Request):
+ localpart = parse_string(request, "username", required=True)
+
+ session_id = request.getCookie(USERNAME_MAPPING_SESSION_COOKIE_NAME)
+ if not session_id:
+ raise SynapseError(code=400, msg="missing session_id")
+
+ is_available = await self._sso_handler.check_username_availability(
+ localpart, session_id.decode("ascii", errors="replace")
+ )
+ return 200, {"available": is_available}
+
+
+class SubmitResource(DirectServeHtmlResource):
+ def __init__(self, hs: "HomeServer"):
+ super().__init__()
+ self._sso_handler = hs.get_sso_handler()
+
+ async def _async_render_POST(self, request: SynapseRequest):
+ localpart = parse_string(request, "username", required=True)
+
+ session_id = request.getCookie(USERNAME_MAPPING_SESSION_COOKIE_NAME)
+ if not session_id:
+ raise SynapseError(code=400, msg="missing session_id")
+
+ await self._sso_handler.handle_submit_username_request(
+ request, localpart, session_id.decode("ascii", errors="replace")
+ )
diff --git a/synapse/types.py b/synapse/types.py
index 3ab6bdbe06..c7d4e95809 100644
--- a/synapse/types.py
+++ b/synapse/types.py
@@ -349,15 +349,17 @@ NON_MXID_CHARACTER_PATTERN = re.compile(
)
-def map_username_to_mxid_localpart(username, case_sensitive=False):
+def map_username_to_mxid_localpart(
+ username: Union[str, bytes], case_sensitive: bool = False
+) -> str:
"""Map a username onto a string suitable for a MXID
This follows the algorithm laid out at
https://matrix.org/docs/spec/appendices.html#mapping-from-other-character-sets.
Args:
- username (unicode|bytes): username to be mapped
- case_sensitive (bool): true if TEST and test should be mapped
+ username: username to be mapped
+ case_sensitive: true if TEST and test should be mapped
onto different mxids
Returns:
diff --git a/tests/handlers/test_oidc.py b/tests/handlers/test_oidc.py
index c54f1c5797..368d600b33 100644
--- a/tests/handlers/test_oidc.py
+++ b/tests/handlers/test_oidc.py
@@ -13,14 +13,21 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import json
-from urllib.parse import parse_qs, urlparse
+import re
+from typing import Dict
+from urllib.parse import parse_qs, urlencode, urlparse
from mock import ANY, Mock, patch
import pymacaroons
+from twisted.web.resource import Resource
+
+from synapse.api.errors import RedirectException
from synapse.handlers.oidc_handler import OidcError
from synapse.handlers.sso import MappingException
+from synapse.rest.client.v1 import login
+from synapse.rest.synapse.client.pick_username import pick_username_resource
from synapse.server import HomeServer
from synapse.types import UserID
@@ -793,6 +800,140 @@ class OidcHandlerTestCase(HomeserverTestCase):
"mapping_error", "Unable to generate a Matrix ID from the SSO response"
)
+ def test_empty_localpart(self):
+ """Attempts to map onto an empty localpart should be rejected."""
+ userinfo = {
+ "sub": "tester",
+ "username": "",
+ }
+ self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
+ self.assertRenderedError("mapping_error", "localpart is invalid: ")
+
+ @override_config(
+ {
+ "oidc_config": {
+ "user_mapping_provider": {
+ "config": {"localpart_template": "{{ user.username }}"}
+ }
+ }
+ }
+ )
+ def test_null_localpart(self):
+ """Mapping onto a null localpart via an empty OIDC attribute should be rejected"""
+ userinfo = {
+ "sub": "tester",
+ "username": None,
+ }
+ self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
+ self.assertRenderedError("mapping_error", "localpart is invalid: ")
+
+
+class UsernamePickerTestCase(HomeserverTestCase):
+ servlets = [login.register_servlets]
+
+ def default_config(self):
+ config = super().default_config()
+ config["public_baseurl"] = BASE_URL
+ oidc_config = {
+ "enabled": True,
+ "client_id": CLIENT_ID,
+ "client_secret": CLIENT_SECRET,
+ "issuer": ISSUER,
+ "scopes": SCOPES,
+ "user_mapping_provider": {
+ "config": {"display_name_template": "{{ user.displayname }}"}
+ },
+ }
+
+ # Update this config with what's in the default config so that
+ # override_config works as expected.
+ oidc_config.update(config.get("oidc_config", {}))
+ config["oidc_config"] = oidc_config
+
+ # whitelist this client URI so we redirect straight to it rather than
+ # serving a confirmation page
+ config["sso"] = {"client_whitelist": ["https://whitelisted.client"]}
+ return config
+
+ def create_resource_dict(self) -> Dict[str, Resource]:
+ d = super().create_resource_dict()
+ d["/_synapse/client/pick_username"] = pick_username_resource(self.hs)
+ return d
+
+ def test_username_picker(self):
+ """Test the happy path of a username picker flow."""
+ client_redirect_url = "https://whitelisted.client"
+
+ # first of all, mock up an OIDC callback to the OidcHandler, which should
+ # raise a RedirectException
+ userinfo = {"sub": "tester", "displayname": "Jonny"}
+ f = self.get_failure(
+ _make_callback_with_userinfo(
+ self.hs, userinfo, client_redirect_url=client_redirect_url
+ ),
+ RedirectException,
+ )
+
+ # check the Location and cookies returned by the RedirectException
+ self.assertEqual(f.value.location, b"/_synapse/client/pick_username")
+ cookieheader = f.value.cookies[0]
+ regex = re.compile(b"^username_mapping_session=([a-zA-Z]+);")
+ m = regex.search(cookieheader)
+ if not m:
+ self.fail("cookie header %s does not match %s" % (cookieheader, regex))
+
+ # introspect the sso handler a bit to check that the username mapping session
+ # looks ok.
+ session_id = m.group(1).decode("ascii")
+ username_mapping_sessions = self.hs.get_sso_handler()._username_mapping_sessions
+ self.assertIn(
+ session_id, username_mapping_sessions, "session id not found in map"
+ )
+ session = username_mapping_sessions[session_id]
+ self.assertEqual(session.remote_user_id, "tester")
+ self.assertEqual(session.display_name, "Jonny")
+ self.assertEqual(session.client_redirect_url, client_redirect_url)
+
+ # the expiry time should be about 15 minutes away
+ expected_expiry = self.clock.time_msec() + (15 * 60 * 1000)
+ self.assertApproximates(session.expiry_time_ms, expected_expiry, tolerance=1000)
+
+ # Now, submit a username to the username picker, which should serve a redirect
+ # back to the client
+ submit_path = f.value.location + b"/submit"
+ content = urlencode({b"username": b"bobby"}).encode("utf8")
+ chan = self.make_request(
+ "POST",
+ path=submit_path,
+ content=content,
+ content_is_form=True,
+ custom_headers=[
+ ("Cookie", cookieheader),
+ # old versions of twisted don't do form-parsing without a valid
+ # content-length header.
+ ("Content-Length", str(len(content))),
+ ],
+ )
+ self.assertEqual(chan.code, 302, chan.result)
+ location_headers = chan.headers.getRawHeaders("Location")
+ # ensure that the returned location starts with the requested redirect URL
+ self.assertEqual(
+ location_headers[0][: len(client_redirect_url)], client_redirect_url
+ )
+
+ # fish the login token out of the returned redirect uri
+ parts = urlparse(location_headers[0])
+ query = parse_qs(parts.query)
+ login_token = query["loginToken"][0]
+
+ # finally, submit the matrix login token to the login API, which gives us our
+ # matrix access token, mxid, and device id.
+ chan = self.make_request(
+ "POST", "/login", content={"type": "m.login.token", "token": login_token},
+ )
+ self.assertEqual(chan.code, 200, chan.result)
+ self.assertEqual(chan.json_body["user_id"], "@bobby:test")
+
async def _make_callback_with_userinfo(
hs: HomeServer, userinfo: dict, client_redirect_url: str = "http://client/redirect"
diff --git a/tests/unittest.py b/tests/unittest.py
index 39e5e7b85c..af7f752c5a 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -20,7 +20,7 @@ import hmac
import inspect
import logging
import time
-from typing import Dict, Optional, Type, TypeVar, Union
+from typing import Dict, Iterable, Optional, Tuple, Type, TypeVar, Union
from mock import Mock, patch
@@ -383,6 +383,9 @@ class HomeserverTestCase(TestCase):
federation_auth_origin: str = None,
content_is_form: bool = False,
await_result: bool = True,
+ custom_headers: Optional[
+ Iterable[Tuple[Union[bytes, str], Union[bytes, str]]]
+ ] = None,
) -> FakeChannel:
"""
Create a SynapseRequest at the path using the method and containing the
@@ -405,6 +408,8 @@ class HomeserverTestCase(TestCase):
true (the default), will pump the test reactor until the the renderer
tells the channel the request is finished.
+ custom_headers: (name, value) pairs to add as request headers
+
Returns:
The FakeChannel object which stores the result of the request.
"""
@@ -420,6 +425,7 @@ class HomeserverTestCase(TestCase):
federation_auth_origin,
content_is_form,
await_result,
+ custom_headers,
)
def setup_test_homeserver(self, *args, **kwargs):
--
cgit 1.5.1
From 4218473f9ea6a2680c21e96368dfe9c06271c8a4 Mon Sep 17 00:00:00 2001
From: Patrick Cloke
Date: Fri, 18 Dec 2020 13:09:45 -0500
Subject: Refactor the CAS handler in prep for using the abstracted SSO code.
(#8958)
This makes the CAS handler look more like the SAML/OIDC handlers:
* Render errors to users instead of throwing JSON errors.
* Internal reorganization.
---
changelog.d/8958.misc | 1 +
docs/dev/cas.md | 6 +-
synapse/handlers/cas_handler.py | 215 ++++++++++++++++++++++++++++------------
synapse/handlers/sso.py | 9 +-
4 files changed, 162 insertions(+), 69 deletions(-)
create mode 100644 changelog.d/8958.misc
(limited to 'synapse/handlers/sso.py')
diff --git a/changelog.d/8958.misc b/changelog.d/8958.misc
new file mode 100644
index 0000000000..1507073e4f
--- /dev/null
+++ b/changelog.d/8958.misc
@@ -0,0 +1 @@
+Properly store the mapping of external ID to Matrix ID for CAS users.
diff --git a/docs/dev/cas.md b/docs/dev/cas.md
index f8d02cc82c..592b2d8d4f 100644
--- a/docs/dev/cas.md
+++ b/docs/dev/cas.md
@@ -31,7 +31,7 @@ easy to run CAS implementation built on top of Django.
You should now have a Django project configured to serve CAS authentication with
a single user created.
-## Configure Synapse (and Riot) to use CAS
+## Configure Synapse (and Element) to use CAS
1. Modify your `homeserver.yaml` to enable CAS and point it to your locally
running Django test server:
@@ -51,9 +51,9 @@ and that the CAS server is on port 8000, both on localhost.
## Testing the configuration
-Then in Riot:
+Then in Element:
-1. Visit the login page with a Riot pointing at your homeserver.
+1. Visit the login page with a Element pointing at your homeserver.
2. Click the Single Sign-On button.
3. Login using the credentials created with `createsuperuser`.
4. You should be logged in.
diff --git a/synapse/handlers/cas_handler.py b/synapse/handlers/cas_handler.py
index f4ea0a9767..e9891e1316 100644
--- a/synapse/handlers/cas_handler.py
+++ b/synapse/handlers/cas_handler.py
@@ -13,13 +13,15 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
-import urllib
-from typing import TYPE_CHECKING, Dict, Optional, Tuple
+import urllib.parse
+from typing import TYPE_CHECKING, Dict, Optional
from xml.etree import ElementTree as ET
+import attr
+
from twisted.web.client import PartialDownloadError
-from synapse.api.errors import Codes, LoginError
+from synapse.api.errors import HttpResponseException
from synapse.http.site import SynapseRequest
from synapse.types import UserID, map_username_to_mxid_localpart
@@ -29,6 +31,26 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
+class CasError(Exception):
+ """Used to catch errors when validating the CAS ticket.
+ """
+
+ def __init__(self, error, error_description=None):
+ self.error = error
+ self.error_description = error_description
+
+ def __str__(self):
+ if self.error_description:
+ return "{}: {}".format(self.error, self.error_description)
+ return self.error
+
+
+@attr.s(slots=True, frozen=True)
+class CasResponse:
+ username = attr.ib(type=str)
+ attributes = attr.ib(type=Dict[str, Optional[str]])
+
+
class CasHandler:
"""
Utility class for to handle the response from a CAS SSO service.
@@ -50,6 +72,8 @@ class CasHandler:
self._http_client = hs.get_proxied_http_client()
+ self._sso_handler = hs.get_sso_handler()
+
def _build_service_param(self, args: Dict[str, str]) -> str:
"""
Generates a value to use as the "service" parameter when redirecting or
@@ -69,14 +93,20 @@ class CasHandler:
async def _validate_ticket(
self, ticket: str, service_args: Dict[str, str]
- ) -> Tuple[str, Optional[str]]:
+ ) -> CasResponse:
"""
- Validate a CAS ticket with the server, parse the response, and return the user and display name.
+ Validate a CAS ticket with the server, and return the parsed the response.
Args:
ticket: The CAS ticket from the client.
service_args: Additional arguments to include in the service URL.
Should be the same as those passed to `get_redirect_url`.
+
+ Raises:
+ CasError: If there's an error parsing the CAS response.
+
+ Returns:
+ The parsed CAS response.
"""
uri = self._cas_server_url + "/proxyValidate"
args = {
@@ -89,66 +119,65 @@ class CasHandler:
# Twisted raises this error if the connection is closed,
# even if that's being used old-http style to signal end-of-data
body = pde.response
+ except HttpResponseException as e:
+ description = (
+ (
+ 'Authorization server responded with a "{status}" error '
+ "while exchanging the authorization code."
+ ).format(status=e.code),
+ )
+ raise CasError("server_error", description) from e
- user, attributes = self._parse_cas_response(body)
- displayname = attributes.pop(self._cas_displayname_attribute, None)
-
- for required_attribute, required_value in self._cas_required_attributes.items():
- # If required attribute was not in CAS Response - Forbidden
- if required_attribute not in attributes:
- raise LoginError(401, "Unauthorized", errcode=Codes.UNAUTHORIZED)
-
- # Also need to check value
- if required_value is not None:
- actual_value = attributes[required_attribute]
- # If required attribute value does not match expected - Forbidden
- if required_value != actual_value:
- raise LoginError(401, "Unauthorized", errcode=Codes.UNAUTHORIZED)
-
- return user, displayname
+ return self._parse_cas_response(body)
- def _parse_cas_response(
- self, cas_response_body: bytes
- ) -> Tuple[str, Dict[str, Optional[str]]]:
+ def _parse_cas_response(self, cas_response_body: bytes) -> CasResponse:
"""
Retrieve the user and other parameters from the CAS response.
Args:
cas_response_body: The response from the CAS query.
+ Raises:
+ CasError: If there's an error parsing the CAS response.
+
Returns:
- A tuple of the user and a mapping of other attributes.
+ The parsed CAS response.
"""
+
+ # Ensure the response is valid.
+ root = ET.fromstring(cas_response_body)
+ if not root.tag.endswith("serviceResponse"):
+ raise CasError(
+ "missing_service_response",
+ "root of CAS response is not serviceResponse",
+ )
+
+ success = root[0].tag.endswith("authenticationSuccess")
+ if not success:
+ raise CasError("unsucessful_response", "Unsuccessful CAS response")
+
+ # Iterate through the nodes and pull out the user and any extra attributes.
user = None
attributes = {}
- try:
- root = ET.fromstring(cas_response_body)
- if not root.tag.endswith("serviceResponse"):
- raise Exception("root of CAS response is not serviceResponse")
- success = root[0].tag.endswith("authenticationSuccess")
- for child in root[0]:
- if child.tag.endswith("user"):
- user = child.text
- if child.tag.endswith("attributes"):
- for attribute in child:
- # ElementTree library expands the namespace in
- # attribute tags to the full URL of the namespace.
- # We don't care about namespace here and it will always
- # be encased in curly braces, so we remove them.
- tag = attribute.tag
- if "}" in tag:
- tag = tag.split("}")[1]
- attributes[tag] = attribute.text
- if user is None:
- raise Exception("CAS response does not contain user")
- except Exception:
- logger.exception("Error parsing CAS response")
- raise LoginError(401, "Invalid CAS response", errcode=Codes.UNAUTHORIZED)
- if not success:
- raise LoginError(
- 401, "Unsuccessful CAS response", errcode=Codes.UNAUTHORIZED
- )
- return user, attributes
+ for child in root[0]:
+ if child.tag.endswith("user"):
+ user = child.text
+ if child.tag.endswith("attributes"):
+ for attribute in child:
+ # ElementTree library expands the namespace in
+ # attribute tags to the full URL of the namespace.
+ # We don't care about namespace here and it will always
+ # be encased in curly braces, so we remove them.
+ tag = attribute.tag
+ if "}" in tag:
+ tag = tag.split("}")[1]
+ attributes[tag] = attribute.text
+
+ # Ensure a user was found.
+ if user is None:
+ raise CasError("no_user", "CAS response does not contain user")
+
+ return CasResponse(user, attributes)
def get_redirect_url(self, service_args: Dict[str, str]) -> str:
"""
@@ -201,7 +230,68 @@ class CasHandler:
args["redirectUrl"] = client_redirect_url
if session:
args["session"] = session
- username, user_display_name = await self._validate_ticket(ticket, args)
+
+ try:
+ cas_response = await self._validate_ticket(ticket, args)
+ except CasError as e:
+ logger.exception("Could not validate ticket")
+ self._sso_handler.render_error(request, e.error, e.error_description, 401)
+ return
+
+ await self._handle_cas_response(
+ request, cas_response, client_redirect_url, session
+ )
+
+ async def _handle_cas_response(
+ self,
+ request: SynapseRequest,
+ cas_response: CasResponse,
+ client_redirect_url: Optional[str],
+ session: Optional[str],
+ ) -> None:
+ """Handle a CAS response to a ticket request.
+
+ Assumes that the response has been validated. Maps the user onto an MXID,
+ registering them if necessary, and returns a response to the browser.
+
+ Args:
+ request: the incoming request from the browser. We'll respond to it with an
+ HTML page or a redirect
+
+ cas_response: The parsed CAS response.
+
+ client_redirect_url: the redirectUrl parameter from the `/cas/ticket` HTTP request, if given.
+ This should be the same as the redirectUrl from the original `/login/sso/redirect` request.
+
+ session: The session parameter from the `/cas/ticket` HTTP request, if given.
+ This should be the UI Auth session id.
+ """
+
+ # Ensure that the attributes of the logged in user meet the required
+ # attributes.
+ for required_attribute, required_value in self._cas_required_attributes.items():
+ # If required attribute was not in CAS Response - Forbidden
+ if required_attribute not in cas_response.attributes:
+ self._sso_handler.render_error(
+ request,
+ "unauthorised",
+ "You are not authorised to log in here.",
+ 401,
+ )
+ return
+
+ # Also need to check value
+ if required_value is not None:
+ actual_value = cas_response.attributes[required_attribute]
+ # If required attribute value does not match expected - Forbidden
+ if required_value != actual_value:
+ self._sso_handler.render_error(
+ request,
+ "unauthorised",
+ "You are not authorised to log in here.",
+ 401,
+ )
+ return
# Pull out the user-agent and IP from the request.
user_agent = request.get_user_agent("")
@@ -209,7 +299,7 @@ class CasHandler:
# Get the matrix ID from the CAS username.
user_id = await self._map_cas_user_to_matrix_user(
- username, user_display_name, user_agent, ip_address
+ cas_response, user_agent, ip_address
)
if session:
@@ -225,18 +315,13 @@ class CasHandler:
)
async def _map_cas_user_to_matrix_user(
- self,
- remote_user_id: str,
- display_name: Optional[str],
- user_agent: str,
- ip_address: str,
+ self, cas_response: CasResponse, user_agent: str, ip_address: str,
) -> str:
"""
Given a CAS username, retrieve the user ID for it and possibly register the user.
Args:
- remote_user_id: The username from the CAS response.
- display_name: The display name from the CAS response.
+ cas_response: The parsed CAS response.
user_agent: The user agent of the client making the request.
ip_address: The IP address of the client making the request.
@@ -244,15 +329,17 @@ class CasHandler:
The user ID associated with this response.
"""
- localpart = map_username_to_mxid_localpart(remote_user_id)
+ localpart = map_username_to_mxid_localpart(cas_response.username)
user_id = UserID(localpart, self._hostname).to_string()
registered_user_id = await self._auth_handler.check_user_exists(user_id)
+ displayname = cas_response.attributes.get(self._cas_displayname_attribute, None)
+
# If the user does not exist, register it.
if not registered_user_id:
registered_user_id = await self._registration_handler.register_user(
localpart=localpart,
- default_display_name=display_name,
+ default_display_name=displayname,
user_agent_ips=[(user_agent, ip_address)],
)
diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py
index 548b02211b..b0a8c8c7d2 100644
--- a/synapse/handlers/sso.py
+++ b/synapse/handlers/sso.py
@@ -101,7 +101,11 @@ class SsoHandler:
self._username_mapping_sessions = {} # type: Dict[str, UsernameMappingSession]
def render_error(
- self, request, error: str, error_description: Optional[str] = None
+ self,
+ request: Request,
+ error: str,
+ error_description: Optional[str] = None,
+ code: int = 400,
) -> None:
"""Renders the error template and responds with it.
@@ -113,11 +117,12 @@ class SsoHandler:
We'll respond with an HTML page describing the error.
error: A technical identifier for this error.
error_description: A human-readable description of the error.
+ code: The integer error code (an HTTP response code)
"""
html = self._error_template.render(
error=error, error_description=error_description
)
- respond_with_html(request, 400, html)
+ respond_with_html(request, code, html)
async def get_sso_user_by_remote_user_id(
self, auth_provider_id: str, remote_user_id: str
--
cgit 1.5.1
From 0eccf531466d762ede0dd365284a8465bfb18d0f Mon Sep 17 00:00:00 2001
From: Patrick Cloke
Date: Sun, 3 Jan 2021 11:25:44 -0500
Subject: Use the SSO handler helpers for CAS registration/login. (#8856)
---
changelog.d/8856.misc | 1 +
synapse/handlers/cas_handler.py | 112 +++++++++++++++++++++++++------------
synapse/handlers/sso.py | 4 +-
tests/handlers/test_cas.py | 121 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 199 insertions(+), 39 deletions(-)
create mode 100644 changelog.d/8856.misc
create mode 100644 tests/handlers/test_cas.py
(limited to 'synapse/handlers/sso.py')
diff --git a/changelog.d/8856.misc b/changelog.d/8856.misc
new file mode 100644
index 0000000000..1507073e4f
--- /dev/null
+++ b/changelog.d/8856.misc
@@ -0,0 +1 @@
+Properly store the mapping of external ID to Matrix ID for CAS users.
diff --git a/synapse/handlers/cas_handler.py b/synapse/handlers/cas_handler.py
index e9891e1316..fca210a5a6 100644
--- a/synapse/handlers/cas_handler.py
+++ b/synapse/handlers/cas_handler.py
@@ -22,6 +22,7 @@ import attr
from twisted.web.client import PartialDownloadError
from synapse.api.errors import HttpResponseException
+from synapse.handlers.sso import MappingException, UserAttributes
from synapse.http.site import SynapseRequest
from synapse.types import UserID, map_username_to_mxid_localpart
@@ -62,6 +63,7 @@ class CasHandler:
def __init__(self, hs: "HomeServer"):
self.hs = hs
self._hostname = hs.hostname
+ self._store = hs.get_datastore()
self._auth_handler = hs.get_auth_handler()
self._registration_handler = hs.get_registration_handler()
@@ -72,6 +74,9 @@ class CasHandler:
self._http_client = hs.get_proxied_http_client()
+ # identifier for the external_ids table
+ self._auth_provider_id = "cas"
+
self._sso_handler = hs.get_sso_handler()
def _build_service_param(self, args: Dict[str, str]) -> str:
@@ -267,6 +272,14 @@ class CasHandler:
This should be the UI Auth session id.
"""
+ # first check if we're doing a UIA
+ if session:
+ return await self._sso_handler.complete_sso_ui_auth_request(
+ self._auth_provider_id, cas_response.username, session, request,
+ )
+
+ # otherwise, we're handling a login request.
+
# Ensure that the attributes of the logged in user meet the required
# attributes.
for required_attribute, required_value in self._cas_required_attributes.items():
@@ -293,54 +306,79 @@ class CasHandler:
)
return
- # Pull out the user-agent and IP from the request.
- user_agent = request.get_user_agent("")
- ip_address = self.hs.get_ip_from_request(request)
-
- # Get the matrix ID from the CAS username.
- user_id = await self._map_cas_user_to_matrix_user(
- cas_response, user_agent, ip_address
- )
+ # Call the mapper to register/login the user
- if session:
- await self._auth_handler.complete_sso_ui_auth(
- user_id, session, request,
- )
- else:
- # If this not a UI auth request than there must be a redirect URL.
- assert client_redirect_url
+ # If this not a UI auth request than there must be a redirect URL.
+ assert client_redirect_url is not None
- await self._auth_handler.complete_sso_login(
- user_id, request, client_redirect_url
- )
+ try:
+ await self._complete_cas_login(cas_response, request, client_redirect_url)
+ except MappingException as e:
+ logger.exception("Could not map user")
+ self._sso_handler.render_error(request, "mapping_error", str(e))
- async def _map_cas_user_to_matrix_user(
- self, cas_response: CasResponse, user_agent: str, ip_address: str,
- ) -> str:
+ async def _complete_cas_login(
+ self,
+ cas_response: CasResponse,
+ request: SynapseRequest,
+ client_redirect_url: str,
+ ) -> None:
"""
- Given a CAS username, retrieve the user ID for it and possibly register the user.
+ Given a CAS response, complete the login flow
+
+ Retrieves the remote user ID, registers the user if necessary, and serves
+ a redirect back to the client with a login-token.
Args:
cas_response: The parsed CAS response.
- user_agent: The user agent of the client making the request.
- ip_address: The IP address of the client making the request.
+ request: The request to respond to
+ client_redirect_url: The redirect URL passed in by the client.
- Returns:
- The user ID associated with this response.
+ Raises:
+ MappingException if there was a problem mapping the response to a user.
+ RedirectException: some mapping providers may raise this if they need
+ to redirect to an interstitial page.
"""
-
+ # Note that CAS does not support a mapping provider, so the logic is hard-coded.
localpart = map_username_to_mxid_localpart(cas_response.username)
- user_id = UserID(localpart, self._hostname).to_string()
- registered_user_id = await self._auth_handler.check_user_exists(user_id)
- displayname = cas_response.attributes.get(self._cas_displayname_attribute, None)
+ async def cas_response_to_user_attributes(failures: int) -> UserAttributes:
+ """
+ Map from CAS attributes to user attributes.
+ """
+ # Due to the grandfathering logic matching any previously registered
+ # mxids it isn't expected for there to be any failures.
+ if failures:
+ raise RuntimeError("CAS is not expected to de-duplicate Matrix IDs")
+
+ display_name = cas_response.attributes.get(
+ self._cas_displayname_attribute, None
+ )
+
+ return UserAttributes(localpart=localpart, display_name=display_name)
- # If the user does not exist, register it.
- if not registered_user_id:
- registered_user_id = await self._registration_handler.register_user(
- localpart=localpart,
- default_display_name=displayname,
- user_agent_ips=[(user_agent, ip_address)],
+ async def grandfather_existing_users() -> Optional[str]:
+ # Since CAS did not always use the user_external_ids table, always
+ # to attempt to map to existing users.
+ user_id = UserID(localpart, self._hostname).to_string()
+
+ logger.debug(
+ "Looking for existing account based on mapped %s", user_id,
)
- return registered_user_id
+ users = await self._store.get_users_by_id_case_insensitive(user_id)
+ if users:
+ registered_user_id = list(users.keys())[0]
+ logger.info("Grandfathering mapping to %s", registered_user_id)
+ return registered_user_id
+
+ return None
+
+ await self._sso_handler.complete_sso_login_request(
+ self._auth_provider_id,
+ cas_response.username,
+ request,
+ client_redirect_url,
+ cas_response_to_user_attributes,
+ grandfather_existing_users,
+ )
diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py
index b0a8c8c7d2..33cd6bc178 100644
--- a/synapse/handlers/sso.py
+++ b/synapse/handlers/sso.py
@@ -173,7 +173,7 @@ class SsoHandler:
request: SynapseRequest,
client_redirect_url: str,
sso_to_matrix_id_mapper: Callable[[int], Awaitable[UserAttributes]],
- grandfather_existing_users: Optional[Callable[[], Awaitable[Optional[str]]]],
+ grandfather_existing_users: Callable[[], Awaitable[Optional[str]]],
extra_login_attributes: Optional[JsonDict] = None,
) -> None:
"""
@@ -241,7 +241,7 @@ class SsoHandler:
)
# Check for grandfathering of users.
- if not user_id and grandfather_existing_users:
+ if not user_id:
user_id = await grandfather_existing_users()
if user_id:
# Future logins should also match this user ID.
diff --git a/tests/handlers/test_cas.py b/tests/handlers/test_cas.py
new file mode 100644
index 0000000000..bd7a1b6891
--- /dev/null
+++ b/tests/handlers/test_cas.py
@@ -0,0 +1,121 @@
+# 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.
+from mock import Mock
+
+from synapse.handlers.cas_handler import CasResponse
+
+from tests.test_utils import simple_async_mock
+from tests.unittest import HomeserverTestCase
+
+# These are a few constants that are used as config parameters in the tests.
+BASE_URL = "https://synapse/"
+SERVER_URL = "https://issuer/"
+
+
+class CasHandlerTestCase(HomeserverTestCase):
+ def default_config(self):
+ config = super().default_config()
+ config["public_baseurl"] = BASE_URL
+ cas_config = {
+ "enabled": True,
+ "server_url": SERVER_URL,
+ "service_url": BASE_URL,
+ }
+ config["cas_config"] = cas_config
+
+ return config
+
+ def make_homeserver(self, reactor, clock):
+ hs = self.setup_test_homeserver()
+
+ self.handler = hs.get_cas_handler()
+
+ # Reduce the number of attempts when generating MXIDs.
+ sso_handler = hs.get_sso_handler()
+ sso_handler._MAP_USERNAME_RETRIES = 3
+
+ return hs
+
+ def test_map_cas_user_to_user(self):
+ """Ensure that mapping the CAS user returned from a provider to an MXID works properly."""
+
+ # stub out the auth handler
+ auth_handler = self.hs.get_auth_handler()
+ auth_handler.complete_sso_login = simple_async_mock()
+
+ cas_response = CasResponse("test_user", {})
+ request = _mock_request()
+ self.get_success(
+ self.handler._handle_cas_response(request, cas_response, "redirect_uri", "")
+ )
+
+ # check that the auth handler got called as expected
+ auth_handler.complete_sso_login.assert_called_once_with(
+ "@test_user:test", request, "redirect_uri", None
+ )
+
+ def test_map_cas_user_to_existing_user(self):
+ """Existing users can log in with CAS account."""
+ store = self.hs.get_datastore()
+ self.get_success(
+ store.register_user(user_id="@test_user:test", password_hash=None)
+ )
+
+ # stub out the auth handler
+ auth_handler = self.hs.get_auth_handler()
+ auth_handler.complete_sso_login = simple_async_mock()
+
+ # Map a user via SSO.
+ cas_response = CasResponse("test_user", {})
+ request = _mock_request()
+ self.get_success(
+ self.handler._handle_cas_response(request, cas_response, "redirect_uri", "")
+ )
+
+ # check that the auth handler got called as expected
+ auth_handler.complete_sso_login.assert_called_once_with(
+ "@test_user:test", request, "redirect_uri", None
+ )
+
+ # Subsequent calls should map to the same mxid.
+ auth_handler.complete_sso_login.reset_mock()
+ self.get_success(
+ self.handler._handle_cas_response(request, cas_response, "redirect_uri", "")
+ )
+ auth_handler.complete_sso_login.assert_called_once_with(
+ "@test_user:test", request, "redirect_uri", None
+ )
+
+ def test_map_cas_user_to_invalid_localpart(self):
+ """CAS automaps invalid characters to base-64 encoding."""
+
+ # stub out the auth handler
+ auth_handler = self.hs.get_auth_handler()
+ auth_handler.complete_sso_login = simple_async_mock()
+
+ cas_response = CasResponse("föö", {})
+ request = _mock_request()
+ self.get_success(
+ self.handler._handle_cas_response(request, cas_response, "redirect_uri", "")
+ )
+
+ # check that the auth handler got called as expected
+ auth_handler.complete_sso_login.assert_called_once_with(
+ "@f=c3=b6=c3=b6:test", request, "redirect_uri", None
+ )
+
+
+def _mock_request():
+ """Returns a mock which will stand in as a SynapseRequest"""
+ return Mock(spec=["getClientIP", "get_user_agent"])
--
cgit 1.5.1