Improve styling and wording of SSO redirect confirm template (#9272)
This commit is contained in:
parent
9c715a5f19
commit
8aed29dc61
|
@ -0,0 +1 @@
|
|||
Improve the user experience of setting up an account via single-sign on.
|
|
@ -1971,7 +1971,8 @@ sso:
|
|||
# * HTML page for a confirmation step before redirecting back to the client
|
||||
# with the login token: 'sso_redirect_confirm.html'.
|
||||
#
|
||||
# When rendering, this template is given three variables:
|
||||
# When rendering, this template is given the following variables:
|
||||
#
|
||||
# * redirect_url: the URL the user is about to be redirected to. Needs
|
||||
# manual escaping (see
|
||||
# https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
|
||||
|
@ -1984,6 +1985,17 @@ sso:
|
|||
#
|
||||
# * server_name: the homeserver's name.
|
||||
#
|
||||
# * new_user: a boolean indicating whether this is the user's first time
|
||||
# logging in.
|
||||
#
|
||||
# * user_id: the user's matrix ID.
|
||||
#
|
||||
# * user_profile.avatar_url: an MXC URI for the user's avatar, if any.
|
||||
# None if the user has not set an avatar.
|
||||
#
|
||||
# * user_profile.display_name: the user's display name. None if the user
|
||||
# has not set a display name.
|
||||
#
|
||||
# * HTML page which notifies the user that they are authenticating to confirm
|
||||
# an operation on their account during the user interactive authentication
|
||||
# process: 'sso_auth_confirm.html'.
|
||||
|
|
|
@ -127,7 +127,8 @@ class SSOConfig(Config):
|
|||
# * HTML page for a confirmation step before redirecting back to the client
|
||||
# with the login token: 'sso_redirect_confirm.html'.
|
||||
#
|
||||
# When rendering, this template is given three variables:
|
||||
# When rendering, this template is given the following variables:
|
||||
#
|
||||
# * redirect_url: the URL the user is about to be redirected to. Needs
|
||||
# manual escaping (see
|
||||
# https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
|
||||
|
@ -140,6 +141,17 @@ class SSOConfig(Config):
|
|||
#
|
||||
# * server_name: the homeserver's name.
|
||||
#
|
||||
# * new_user: a boolean indicating whether this is the user's first time
|
||||
# logging in.
|
||||
#
|
||||
# * user_id: the user's matrix ID.
|
||||
#
|
||||
# * user_profile.avatar_url: an MXC URI for the user's avatar, if any.
|
||||
# None if the user has not set an avatar.
|
||||
#
|
||||
# * user_profile.display_name: the user's display name. None if the user
|
||||
# has not set a display name.
|
||||
#
|
||||
# * HTML page which notifies the user that they are authenticating to confirm
|
||||
# an operation on their account during the user interactive authentication
|
||||
# process: 'sso_auth_confirm.html'.
|
||||
|
|
|
@ -61,6 +61,7 @@ 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.module_api import ModuleApi
|
||||
from synapse.storage.roommember import ProfileInfo
|
||||
from synapse.types import JsonDict, Requester, UserID
|
||||
from synapse.util import stringutils as stringutils
|
||||
from synapse.util.async_helpers import maybe_awaitable
|
||||
|
@ -1396,6 +1397,7 @@ class AuthHandler(BaseHandler):
|
|||
request: Request,
|
||||
client_redirect_url: str,
|
||||
extra_attributes: Optional[JsonDict] = None,
|
||||
new_user: bool = False,
|
||||
):
|
||||
"""Having figured out a mxid for this user, complete the HTTP request
|
||||
|
||||
|
@ -1406,6 +1408,8 @@ class AuthHandler(BaseHandler):
|
|||
process.
|
||||
extra_attributes: Extra attributes which will be passed to the client
|
||||
during successful login. Must be JSON serializable.
|
||||
new_user: True if we should use wording appropriate to a user who has just
|
||||
registered.
|
||||
"""
|
||||
# If the account has been deactivated, do not proceed with the login
|
||||
# flow.
|
||||
|
@ -1414,8 +1418,17 @@ class AuthHandler(BaseHandler):
|
|||
respond_with_html(request, 403, self._sso_account_deactivated_template)
|
||||
return
|
||||
|
||||
profile = await self.store.get_profileinfo(
|
||||
UserID.from_string(registered_user_id).localpart
|
||||
)
|
||||
|
||||
self._complete_sso_login(
|
||||
registered_user_id, request, client_redirect_url, extra_attributes
|
||||
registered_user_id,
|
||||
request,
|
||||
client_redirect_url,
|
||||
extra_attributes,
|
||||
new_user=new_user,
|
||||
user_profile_data=profile,
|
||||
)
|
||||
|
||||
def _complete_sso_login(
|
||||
|
@ -1424,12 +1437,18 @@ class AuthHandler(BaseHandler):
|
|||
request: Request,
|
||||
client_redirect_url: str,
|
||||
extra_attributes: Optional[JsonDict] = None,
|
||||
new_user: bool = False,
|
||||
user_profile_data: Optional[ProfileInfo] = None,
|
||||
):
|
||||
"""
|
||||
The synchronous portion of complete_sso_login.
|
||||
|
||||
This exists purely for backwards compatibility of synapse.module_api.ModuleApi.
|
||||
"""
|
||||
|
||||
if user_profile_data is None:
|
||||
user_profile_data = ProfileInfo(None, None)
|
||||
|
||||
# Store any extra attributes which will be passed in the login response.
|
||||
# Note that this is per-user so it may overwrite a previous value, this
|
||||
# is considered OK since the newest SSO attributes should be most valid.
|
||||
|
@ -1467,6 +1486,9 @@ class AuthHandler(BaseHandler):
|
|||
display_url=redirect_url_no_params,
|
||||
redirect_url=redirect_url,
|
||||
server_name=self._server_name,
|
||||
new_user=new_user,
|
||||
user_id=registered_user_id,
|
||||
user_profile=user_profile_data,
|
||||
)
|
||||
respond_with_html(request, 200, html)
|
||||
|
||||
|
|
|
@ -391,6 +391,8 @@ class SsoHandler:
|
|||
to an additional page. (e.g. to prompt for more information)
|
||||
|
||||
"""
|
||||
new_user = False
|
||||
|
||||
# grab a lock while we try to find a mapping for this user. This seems...
|
||||
# optimistic, especially for implementations that end up redirecting to
|
||||
# interstitial pages.
|
||||
|
@ -431,9 +433,14 @@ class SsoHandler:
|
|||
get_request_user_agent(request),
|
||||
request.getClientIP(),
|
||||
)
|
||||
new_user = True
|
||||
|
||||
await self._auth_handler.complete_sso_login(
|
||||
user_id, request, client_redirect_url, extra_login_attributes
|
||||
user_id,
|
||||
request,
|
||||
client_redirect_url,
|
||||
extra_login_attributes,
|
||||
new_user=new_user,
|
||||
)
|
||||
|
||||
async def _call_attribute_mapper(
|
||||
|
@ -778,6 +785,7 @@ class SsoHandler:
|
|||
request,
|
||||
session.client_redirect_url,
|
||||
session.extra_login_attributes,
|
||||
new_user=True,
|
||||
)
|
||||
|
||||
def _expire_old_sessions(self):
|
||||
|
|
|
@ -279,7 +279,11 @@ class ModuleApi:
|
|||
)
|
||||
|
||||
async def complete_sso_login_async(
|
||||
self, registered_user_id: str, request: SynapseRequest, client_redirect_url: str
|
||||
self,
|
||||
registered_user_id: str,
|
||||
request: SynapseRequest,
|
||||
client_redirect_url: str,
|
||||
new_user: bool = False,
|
||||
):
|
||||
"""Complete a SSO login by redirecting the user to a page to confirm whether they
|
||||
want their access token sent to `client_redirect_url`, or redirect them to that
|
||||
|
@ -291,9 +295,11 @@ class ModuleApi:
|
|||
request: The request to respond to.
|
||||
client_redirect_url: The URL to which to offer to redirect the user (or to
|
||||
redirect them directly if whitelisted).
|
||||
new_user: set to true to use wording for the consent appropriate to a user
|
||||
who has just registered.
|
||||
"""
|
||||
await self._auth_handler.complete_sso_login(
|
||||
registered_user_id, request, client_redirect_url,
|
||||
registered_user_id, request, client_redirect_url, new_user=new_user
|
||||
)
|
||||
|
||||
@defer.inlineCallbacks
|
||||
|
|
|
@ -0,0 +1,83 @@
|
|||
body {
|
||||
font-family: "Inter", "Helvetica", "Arial", sans-serif;
|
||||
font-size: 14px;
|
||||
color: #17191C;
|
||||
}
|
||||
|
||||
header {
|
||||
max-width: 480px;
|
||||
width: 100%;
|
||||
margin: 24px auto;
|
||||
text-align: center;
|
||||
}
|
||||
|
||||
header p {
|
||||
color: #737D8C;
|
||||
line-height: 24px;
|
||||
}
|
||||
|
||||
h1 {
|
||||
font-size: 24px;
|
||||
}
|
||||
|
||||
h2 {
|
||||
font-size: 14px;
|
||||
}
|
||||
|
||||
h2 img {
|
||||
vertical-align: middle;
|
||||
margin-right: 8px;
|
||||
width: 24px;
|
||||
height: 24px;
|
||||
}
|
||||
|
||||
label {
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
main {
|
||||
max-width: 360px;
|
||||
width: 100%;
|
||||
margin: 24px auto;
|
||||
}
|
||||
|
||||
.primary-button {
|
||||
border: none;
|
||||
text-decoration: none;
|
||||
padding: 12px;
|
||||
color: white;
|
||||
background-color: #418DED;
|
||||
font-weight: bold;
|
||||
display: block;
|
||||
border-radius: 12px;
|
||||
width: 100%;
|
||||
margin: 16px 0;
|
||||
cursor: pointer;
|
||||
text-align: center;
|
||||
}
|
||||
|
||||
.profile {
|
||||
display: flex;
|
||||
justify-content: center;
|
||||
margin: 24px 0;
|
||||
}
|
||||
|
||||
.profile .avatar {
|
||||
width: 36px;
|
||||
height: 36px;
|
||||
border-radius: 100%;
|
||||
display: block;
|
||||
margin-right: 8px;
|
||||
}
|
||||
|
||||
.profile .display-name {
|
||||
font-weight: bold;
|
||||
margin-bottom: 4px;
|
||||
}
|
||||
.profile .user-id {
|
||||
color: #737D8C;
|
||||
}
|
||||
|
||||
.profile .display-name, .profile .user-id {
|
||||
line-height: 18px;
|
||||
}
|
|
@ -3,12 +3,34 @@
|
|||
<head>
|
||||
<meta charset="UTF-8">
|
||||
<title>SSO redirect confirmation</title>
|
||||
<meta name="viewport" content="width=device-width, user-scalable=no">
|
||||
<style type="text/css">
|
||||
{% include "sso.css" without context %}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<p>The application at <span style="font-weight:bold">{{ display_url | e }}</span> is requesting full access to your <span style="font-weight:bold">{{ server_name }}</span> Matrix account.</p>
|
||||
<p>If you don't recognise this address, you should ignore this and close this tab.</p>
|
||||
<p>
|
||||
<a href="{{ redirect_url | e }}">I trust this address</a>
|
||||
</p>
|
||||
<header>
|
||||
{% if new_user %}
|
||||
<h1>Your account is now ready</h1>
|
||||
<p>You've made your account on {{ server_name | e }}.</p>
|
||||
{% else %}
|
||||
<h1>Log in</h1>
|
||||
{% endif %}
|
||||
<p>Continue to confirm you trust <strong>{{ display_url | e }}</strong>.</p>
|
||||
</header>
|
||||
<main>
|
||||
{% if user_profile.avatar_url %}
|
||||
<div class="profile">
|
||||
<img src="{{ user_profile.avatar_url | mxc_to_http(64, 64) }}" class="avatar" />
|
||||
<div class="profile-details">
|
||||
{% if user_profile.display_name %}
|
||||
<div class="display-name">{{ user_profile.display_name | e }}</div>
|
||||
{% endif %}
|
||||
<div class="user-id">{{ user_id | e }}</div>
|
||||
</div>
|
||||
</div>
|
||||
{% endif %}
|
||||
<a href="{{ redirect_url | e }}" class="primary-button">Continue</a>
|
||||
</main>
|
||||
</body>
|
||||
</html>
|
||||
</html>
|
||||
|
|
|
@ -62,7 +62,7 @@ class CasHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
# 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
|
||||
"@test_user:test", request, "redirect_uri", None, new_user=True
|
||||
)
|
||||
|
||||
def test_map_cas_user_to_existing_user(self):
|
||||
|
@ -85,7 +85,7 @@ class CasHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
# 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
|
||||
"@test_user:test", request, "redirect_uri", None, new_user=False
|
||||
)
|
||||
|
||||
# Subsequent calls should map to the same mxid.
|
||||
|
@ -94,7 +94,7 @@ class CasHandlerTestCase(HomeserverTestCase):
|
|||
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
|
||||
"@test_user:test", request, "redirect_uri", None, new_user=False
|
||||
)
|
||||
|
||||
def test_map_cas_user_to_invalid_localpart(self):
|
||||
|
@ -112,7 +112,7 @@ class CasHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
# 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
|
||||
"@f=c3=b6=c3=b6:test", request, "redirect_uri", None, new_user=True
|
||||
)
|
||||
|
||||
|
||||
|
|
|
@ -419,7 +419,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
self.get_success(self.handler.handle_oidc_callback(request))
|
||||
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
expected_user_id, request, client_redirect_url, None,
|
||||
expected_user_id, request, client_redirect_url, None, new_user=True
|
||||
)
|
||||
self.provider._exchange_code.assert_called_once_with(code)
|
||||
self.provider._parse_id_token.assert_called_once_with(token, nonce=nonce)
|
||||
|
@ -450,7 +450,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
self.get_success(self.handler.handle_oidc_callback(request))
|
||||
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
expected_user_id, request, client_redirect_url, None,
|
||||
expected_user_id, request, client_redirect_url, None, new_user=False
|
||||
)
|
||||
self.provider._exchange_code.assert_called_once_with(code)
|
||||
self.provider._parse_id_token.assert_not_called()
|
||||
|
@ -623,7 +623,11 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
self.get_success(self.handler.handle_oidc_callback(request))
|
||||
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@foo:test", request, client_redirect_url, {"phone": "1234567"},
|
||||
"@foo:test",
|
||||
request,
|
||||
client_redirect_url,
|
||||
{"phone": "1234567"},
|
||||
new_user=True,
|
||||
)
|
||||
|
||||
def test_map_userinfo_to_user(self):
|
||||
|
@ -637,7 +641,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
}
|
||||
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@test_user:test", ANY, ANY, None,
|
||||
"@test_user:test", ANY, ANY, None, new_user=True
|
||||
)
|
||||
auth_handler.complete_sso_login.reset_mock()
|
||||
|
||||
|
@ -648,7 +652,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
}
|
||||
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@test_user_2:test", ANY, ANY, None,
|
||||
"@test_user_2:test", ANY, ANY, None, new_user=True
|
||||
)
|
||||
auth_handler.complete_sso_login.reset_mock()
|
||||
|
||||
|
@ -685,14 +689,14 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
}
|
||||
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
user.to_string(), ANY, ANY, None,
|
||||
user.to_string(), ANY, ANY, None, new_user=False
|
||||
)
|
||||
auth_handler.complete_sso_login.reset_mock()
|
||||
|
||||
# Subsequent calls should map to the same mxid.
|
||||
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
user.to_string(), ANY, ANY, None,
|
||||
user.to_string(), ANY, ANY, None, new_user=False
|
||||
)
|
||||
auth_handler.complete_sso_login.reset_mock()
|
||||
|
||||
|
@ -707,7 +711,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
}
|
||||
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
user.to_string(), ANY, ANY, None,
|
||||
user.to_string(), ANY, ANY, None, new_user=False
|
||||
)
|
||||
auth_handler.complete_sso_login.reset_mock()
|
||||
|
||||
|
@ -743,7 +747,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
self.get_success(_make_callback_with_userinfo(self.hs, userinfo))
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@TEST_USER_2:test", ANY, ANY, None,
|
||||
"@TEST_USER_2:test", ANY, ANY, None, new_user=False
|
||||
)
|
||||
|
||||
def test_map_userinfo_to_invalid_localpart(self):
|
||||
|
@ -779,7 +783,7 @@ class OidcHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
# test_user is already taken, so test_user1 gets registered instead.
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@test_user1:test", ANY, ANY, None,
|
||||
"@test_user1:test", ANY, ANY, None, new_user=True
|
||||
)
|
||||
auth_handler.complete_sso_login.reset_mock()
|
||||
|
||||
|
|
|
@ -131,7 +131,7 @@ class SamlHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
# 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
|
||||
"@test_user:test", request, "redirect_uri", None, new_user=True
|
||||
)
|
||||
|
||||
@override_config({"saml2_config": {"grandfathered_mxid_source_attribute": "mxid"}})
|
||||
|
@ -157,7 +157,7 @@ class SamlHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
# check that the auth handler got called as expected
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@test_user:test", request, "", None
|
||||
"@test_user:test", request, "", None, new_user=False
|
||||
)
|
||||
|
||||
# Subsequent calls should map to the same mxid.
|
||||
|
@ -166,7 +166,7 @@ class SamlHandlerTestCase(HomeserverTestCase):
|
|||
self.handler._handle_authn_response(request, saml_response, "")
|
||||
)
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@test_user:test", request, "", None
|
||||
"@test_user:test", request, "", None, new_user=False
|
||||
)
|
||||
|
||||
def test_map_saml_response_to_invalid_localpart(self):
|
||||
|
@ -214,7 +214,7 @@ class SamlHandlerTestCase(HomeserverTestCase):
|
|||
|
||||
# test_user is already taken, so test_user1 gets registered instead.
|
||||
auth_handler.complete_sso_login.assert_called_once_with(
|
||||
"@test_user1:test", request, "", None
|
||||
"@test_user1:test", request, "", None, new_user=True
|
||||
)
|
||||
auth_handler.complete_sso_login.reset_mock()
|
||||
|
||||
|
|
Loading…
Reference in New Issue