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

Update the SSO username picker template to comply with SIWA guidelines #12210

Merged
merged 7 commits into from
Mar 11, 2022
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/12210.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update the SSO username picker template to comply with SIWA guidelines.
9 changes: 7 additions & 2 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1947,8 +1947,13 @@ saml2_config:
#
# localpart_template: Jinja2 template for the localpart of the MXID.
# If this is not set, the user will be prompted to choose their
# own username (see 'sso_auth_account_details.html' in the 'sso'
# section of this file).
# own username (see the documentation for the
# 'sso_auth_account_details.html' template).
#
# confirm_localpart: Whether to prompt the user to validate (or
# change) the generated localpart (see the documentation for the
# 'sso_auth_account_details.html' template), instead of
# registering the account right away.
#
# display_name_template: Jinja2 template for the display name to set
# on first login. If unset, no displayname will be set.
Expand Down
7 changes: 5 additions & 2 deletions docs/templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,11 @@ Below are the templates Synapse will look for when generating pages related to S
for the brand of the IdP
* `user_attributes`: an object containing details about the user that
we received from the IdP. May have the following attributes:
* display_name: the user's display_name
* emails: a list of email addresses
* `display_name`: the user's display name
* `emails`: a list of email addresses
* `localpart`: the local part of the Matrix user ID to register,
if `localpart_template` is set in the mapping provider configuration (empty
string if not)
The template should render a form which submits the following fields:
* `username`: the localpart of the user's chosen user id
* `sso_new_user_consent.html`: HTML page allowing the user to consent to the
Expand Down
9 changes: 7 additions & 2 deletions synapse/config/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,13 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs) -> str
#
# localpart_template: Jinja2 template for the localpart of the MXID.
# If this is not set, the user will be prompted to choose their
# own username (see 'sso_auth_account_details.html' in the 'sso'
# section of this file).
# own username (see the documentation for the
# 'sso_auth_account_details.html' template).
#
# confirm_localpart: Whether to prompt the user to validate (or
# change) the generated localpart (see the documentation for the
# 'sso_auth_account_details.html' template), instead of
# registering the account right away.
#
# display_name_template: Jinja2 template for the display name to set
# on first login. If unset, no displayname will be set.
Expand Down
12 changes: 11 additions & 1 deletion synapse/handlers/oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,7 @@ class OidcSessionData:

class UserAttributeDict(TypedDict):
localpart: Optional[str]
confirm_localpart: bool
display_name: Optional[str]
emails: List[str]

Expand Down Expand Up @@ -1316,6 +1317,7 @@ class JinjaOidcMappingConfig:
display_name_template: Optional[Template]
email_template: Optional[Template]
extra_attributes: Dict[str, Template]
confirm_localpart: bool = False


class JinjaOidcMappingProvider(OidcMappingProvider[JinjaOidcMappingConfig]):
Expand Down Expand Up @@ -1357,12 +1359,17 @@ def parse_template_config(option_name: str) -> Optional[Template]:
"invalid jinja template", path=["extra_attributes", key]
) from e

confirm_localpart = config.get("confirm_localpart") or False
babolivier marked this conversation as resolved.
Show resolved Hide resolved
if not isinstance(confirm_localpart, bool):
raise ConfigError("must be a bool", path=["confirm_localpart"])

return JinjaOidcMappingConfig(
subject_claim=subject_claim,
localpart_template=localpart_template,
display_name_template=display_name_template,
email_template=email_template,
extra_attributes=extra_attributes,
confirm_localpart=confirm_localpart,
)

def get_remote_user_id(self, userinfo: UserInfo) -> str:
Expand Down Expand Up @@ -1398,7 +1405,10 @@ def render_template_field(template: Optional[Template]) -> Optional[str]:
emails.append(email)

return UserAttributeDict(
localpart=localpart, display_name=display_name, emails=emails
localpart=localpart,
display_name=display_name,
emails=emails,
confirm_localpart=self._config.confirm_localpart,
)

async def get_extra_attributes(self, userinfo: UserInfo, token: Token) -> JsonDict:
Expand Down
8 changes: 5 additions & 3 deletions synapse/handlers/sso.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ class UserAttributes:
# if `None`, the mapper has not picked a userid, and the user should be prompted to
# enter one.
localpart: Optional[str]
confirm_localpart: bool = False
display_name: Optional[str] = None
emails: Collection[str] = attr.Factory(list)

Expand Down Expand Up @@ -561,9 +562,10 @@ def _get_url_for_next_new_user_step(
# Must provide either attributes or session, not both
assert (attributes is not None) != (session is not None)

if (attributes and attributes.localpart is None) or (
session and session.chosen_localpart is None
):
if (
attributes
and (attributes.localpart is None or attributes.confirm_localpart is True)
) or (session and session.chosen_localpart is None):
return b"/_synapse/client/pick_username/account_details"
elif self._consent_at_registration and not (
session and session.terms_accepted_version
Expand Down
6 changes: 3 additions & 3 deletions synapse/res/templates/sso_auth_account_details.html
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,15 @@
</head>
<body>
<header>
<h1>Your account is nearly ready</h1>
<p>Check your details before creating an account on {{ server_name }}</p>
<h1>Choose your user name</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

@babolivier Per the issue this should be:

Choose your account name

<p>This is required to create your account on {{ server_name }}, and you can't change this later.</p>
</header>
<main>
<form method="post" class="form__input" id="form">
<div class="username_input" id="username_input">
<label for="field-username">Username</label>
<div class="prefix">@</div>
<input type="text" name="username" id="field-username" autofocus>
<input type="text" name="username" id="field-username" value="{{ user_attributes.localpart }}" autofocus>
<div class="postfix">:{{ server_name }}</div>
</div>
<output for="username_input" id="field-username-output"></output>
Expand Down
8 changes: 8 additions & 0 deletions synapse/rest/synapse/client/pick_username.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,20 @@ async def _async_render_GET(self, request: Request) -> None:
self._sso_handler.render_error(request, "bad_session", e.msg, code=e.code)
return

# The configuration might mandate going through this step to validate an
# automatically generated localpart, so session.chosen_localpart might already
# be set.
localpart = ""
if session.chosen_localpart is not None:
localpart = session.chosen_localpart

idp_id = session.auth_provider_id
template_params = {
"idp": self._sso_handler.get_identity_providers()[idp_id],
"user_attributes": {
"display_name": session.display_name,
"emails": session.emails,
"localpart": localpart,
},
}

Expand Down