From f6acb49677302b6f904b8729a02ebd4841ae224f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 3 Feb 2021 20:31:23 +0000 Subject: [PATCH] Social login UI polish (#9301) --- changelog.d/9301.feature | 1 + synapse/handlers/auth.py | 16 +++- synapse/res/templates/sso.css | 53 +++++++++++-- .../templates/sso_account_deactivated.html | 1 + .../templates/sso_auth_account_details.html | 61 ++++++++++----- synapse/res/templates/sso_auth_bad_user.html | 1 + synapse/res/templates/sso_auth_confirm.html | 3 +- synapse/res/templates/sso_auth_success.html | 1 + synapse/res/templates/sso_error.html | 1 + synapse/res/templates/sso_footer.html | 19 +++++ .../res/templates/sso_login_idp_picker.html | 74 +++++++++++++------ .../res/templates/sso_new_user_consent.html | 13 +--- .../res/templates/sso_partial_profile.html | 19 +++++ .../res/templates/sso_redirect_confirm.html | 42 ++++++----- tests/rest/client/v1/test_login.py | 16 +++- 15 files changed, 240 insertions(+), 81 deletions(-) create mode 100644 changelog.d/9301.feature create mode 100644 synapse/res/templates/sso_footer.html create mode 100644 synapse/res/templates/sso_partial_profile.html diff --git a/changelog.d/9301.feature b/changelog.d/9301.feature new file mode 100644 index 000000000000..a2d0b27da47a --- /dev/null +++ b/changelog.d/9301.feature @@ -0,0 +1 @@ +Further improvements to the user experience of registration via single sign-on. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index a19c5564377a..648fe91f53b5 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -1472,10 +1472,22 @@ def _complete_sso_login( # Remove the query parameters from the redirect URL to get a shorter version of # it. This is only to display a human-readable URL in the template, but not the # URL we redirect users to. - redirect_url_no_params = client_redirect_url.split("?")[0] + url_parts = urllib.parse.urlsplit(client_redirect_url) + + if url_parts.scheme == "https": + # for an https uri, just show the netloc (ie, the hostname. Specifically, + # the bit between "//" and "/"; this includes any potential + # "username:password@" prefix.) + display_url = url_parts.netloc + else: + # for other uris, strip the query-params (including the login token) and + # fragment. + display_url = urllib.parse.urlunsplit( + (url_parts.scheme, url_parts.netloc, url_parts.path, "", "") + ) html = self._sso_redirect_confirm_template.render( - display_url=redirect_url_no_params, + display_url=display_url, redirect_url=redirect_url, server_name=self._server_name, new_user=new_user, diff --git a/synapse/res/templates/sso.css b/synapse/res/templates/sso.css index 46b309ea4e83..338214f5d012 100644 --- a/synapse/res/templates/sso.css +++ b/synapse/res/templates/sso.css @@ -1,16 +1,26 @@ -body { +body, input, select, textarea { font-family: "Inter", "Helvetica", "Arial", sans-serif; font-size: 14px; color: #17191C; } -header { +header, footer { max-width: 480px; width: 100%; margin: 24px auto; text-align: center; } +@media screen and (min-width: 800px) { + header { + margin-top: 90px; + } +} + +header { + min-height: 60px; +} + header p { color: #737D8C; line-height: 24px; @@ -20,6 +30,10 @@ h1 { font-size: 24px; } +a { + color: #418DED; +} + .error_page h1 { color: #FE2928; } @@ -47,6 +61,9 @@ main { .primary-button { border: none; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; text-decoration: none; padding: 12px; color: white; @@ -63,8 +80,17 @@ main { .profile { display: flex; + flex-direction: column; + align-items: center; justify-content: center; - margin: 24px 0; + margin: 24px; + padding: 13px; + border: 1px solid #E9ECF1; + border-radius: 4px; +} + +.profile.with-avatar { + margin-top: 42px; /* (36px / 2) + 24px*/ } .profile .avatar { @@ -72,17 +98,32 @@ main { height: 36px; border-radius: 100%; display: block; - margin-right: 8px; + margin-top: -32px; + margin-bottom: 8px; } .profile .display-name { font-weight: bold; margin-bottom: 4px; + font-size: 15px; + line-height: 18px; } .profile .user-id { color: #737D8C; + font-size: 12px; + line-height: 12px; } -.profile .display-name, .profile .user-id { - line-height: 18px; +footer { + margin-top: 80px; } + +footer svg { + display: block; + width: 46px; + margin: 0px auto 12px auto; +} + +footer p { + color: #737D8C; +} \ No newline at end of file diff --git a/synapse/res/templates/sso_account_deactivated.html b/synapse/res/templates/sso_account_deactivated.html index 50a0979c2f07..c3e4deed933f 100644 --- a/synapse/res/templates/sso_account_deactivated.html +++ b/synapse/res/templates/sso_account_deactivated.html @@ -20,5 +20,6 @@

Your account has been deactivated

administrator.

+ {% include "sso_footer.html" without context %} diff --git a/synapse/res/templates/sso_auth_account_details.html b/synapse/res/templates/sso_auth_account_details.html index 36850a2d6a78..f4fdc40b22d7 100644 --- a/synapse/res/templates/sso_auth_account_details.html +++ b/synapse/res/templates/sso_auth_account_details.html @@ -1,12 +1,29 @@ - Synapse Login + Create your account + -
-

{{ server_name }} Login

-
-

Choose one of the following identity providers:

-
- -
-
+ {% endif %} + {{ p.idp_name }} + + + {% endfor %} + + + {% include "sso_footer.html" without context %} diff --git a/synapse/res/templates/sso_new_user_consent.html b/synapse/res/templates/sso_new_user_consent.html index 8f197007c711..68c8b9f33a30 100644 --- a/synapse/res/templates/sso_new_user_consent.html +++ b/synapse/res/templates/sso_new_user_consent.html @@ -2,7 +2,7 @@ - SSO redirect confirmation + Agree to terms and conditions
- {% if new_user %} -

Your account is now ready

-

You've made your account on {{ server_name }}.

- {% else %} -

Log in

- {% endif %} -

Continue to confirm you trust {{ display_url }}.

+

Continue to your account

- {% if user_profile.avatar_url %} -
- -
- {% if user_profile.display_name %} -
{{ user_profile.display_name }}
- {% endif %} -
{{ user_id }}
-
-
- {% endif %} + {% include "sso_partial_profile.html" %} +

Continuing will grant {{ display_url }} access to your account.

Continue
+ {% include "sso_footer.html" without context %} diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py index bfcb786af8be..49543d9acb78 100644 --- a/tests/rest/client/v1/test_login.py +++ b/tests/rest/client/v1/test_login.py @@ -15,7 +15,7 @@ import time import urllib.parse -from typing import Any, Dict, Union +from typing import Any, Dict, List, Union from urllib.parse import urlencode from mock import Mock @@ -493,13 +493,21 @@ def test_multi_sso_redirect(self): self.assertEqual(channel.code, 200, channel.result) # parse the form to check it has fields assumed elsewhere in this class + html = channel.result["body"].decode("utf-8") p = TestHtmlParser() - p.feed(channel.result["body"].decode("utf-8")) + p.feed(html) p.close() - self.assertCountEqual(p.radios["idp"], ["cas", "oidc", "oidc-idp1", "saml"]) + # there should be a link for each href + returned_idps = [] # type: List[str] + for link in p.links: + path, query = link.split("?", 1) + self.assertEqual(path, "pick_idp") + params = urllib.parse.parse_qs(query) + self.assertEqual(params["redirectUrl"], [TEST_CLIENT_REDIRECT_URL]) + returned_idps.append(params["idp"][0]) - self.assertEqual(p.hiddens["redirectUrl"], TEST_CLIENT_REDIRECT_URL) + self.assertCountEqual(returned_idps, ["cas", "oidc", "oidc-idp1", "saml"]) def test_multi_sso_redirect_to_cas(self): """If CAS is chosen, should redirect to the CAS server"""