From bfb939a91b1e199059d288b6f0b821f199c6231b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 26 Mar 2020 17:41:03 +0100 Subject: [PATCH 1/8] Improve the UX of the login fallback when using SSO * Don't show the login forms if we're currently logging in with a password or a token. * Submit directly the SSO login form, showing only a spinner to the user, in order to eliminate from the clunkiness of SSO through this fallback. --- synapse/static/client/login/index.html | 2 +- synapse/static/client/login/js/login.js | 42 ++++++++++++++++++------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/synapse/static/client/login/index.html b/synapse/static/client/login/index.html index bcb6bc6bb743..712b0e398094 100644 --- a/synapse/static/client/login/index.html +++ b/synapse/static/client/login/index.html @@ -9,7 +9,7 @@

-

Log in with one of the following methods

+

diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index 276c271bbeed..b2419c1da52a 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -5,6 +5,8 @@ window.matrixLogin = { serverAcceptsSso: false, }; +var title_pre_auth = "Log in with one of the following methods" + var submitPassword = function(user, pwd) { console.log("Logging in with password..."); var data = { @@ -13,7 +15,6 @@ var submitPassword = function(user, pwd) { password: pwd, }; $.post(matrixLogin.endpoint, JSON.stringify(data), function(response) { - show_login(); matrixLogin.onLogin(response); }).error(errorFunc); }; @@ -25,7 +26,6 @@ var submitToken = function(loginToken) { token: loginToken }; $.post(matrixLogin.endpoint, JSON.stringify(data), function(response) { - show_login(); matrixLogin.onLogin(response); }).error(errorFunc); }; @@ -46,20 +46,37 @@ var setFeedbackString = function(text) { }; var show_login = function() { - $("#loading").hide(); - var this_page = window.location.origin + window.location.pathname; $("#sso_redirect_url").val(this_page); - if (matrixLogin.serverAcceptsPassword) { - $("#password_flow").show(); + if (matrixLogin.serverAcceptsCas) { + $("#sso_form").attr("action", "/_matrix/client/r0/login/cas/redirect"); } - if (matrixLogin.serverAcceptsSso) { - $("#sso_flow").show(); - } else if (matrixLogin.serverAcceptsCas) { - $("#sso_form").attr("action", "/_matrix/client/r0/login/cas/redirect"); - $("#sso_flow").show(); + if ((matrixLogin.serverAcceptsSso || matrixLogin.serverAcceptsCas)) { + if (try_token()) { + // Only show the SSO form if there's a login token in the query. That's + // because, if there is a token, and this function is run, it means an error + // happened, and in this case it's nicer to show the form with an error + // rather than redirect immediately to the SSO portal. + $("#sso_form").show(); + } else { + // Submit the SSO form instead of displaying it. The reason behind this + // behaviour is that the user will likely arrive here after clicking on a + // button, in the client, with a label such as "Continue with SSO". And even + // if that's not the case, it kind of makes sense to direct the user directly + // to the SSO portal and skip the password login form. + $("#sso_form").submit(); + return; + } + } + + set_title(title_pre_auth); + + $("#loading").hide(); + + if (matrixLogin.serverAcceptsPassword) { + $("#password_flow").show(); } if (!matrixLogin.serverAcceptsPassword && !matrixLogin.serverAcceptsCas && !matrixLogin.serverAcceptsSso) { @@ -74,6 +91,9 @@ var show_spinner = function() { $("#loading").show(); }; +var set_title = function(title) { + $("#title").innerText = title; +}; var fetch_info = function(cb) { $.get(matrixLogin.endpoint, function(response) { From 1c02f20f028e6794374c8f7b0b382879a41e6809 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 26 Mar 2020 18:35:13 +0100 Subject: [PATCH 2/8] Changelog --- changelog.d/7152.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7152.feature diff --git a/changelog.d/7152.feature b/changelog.d/7152.feature new file mode 100644 index 000000000000..fafa79c7e7f5 --- /dev/null +++ b/changelog.d/7152.feature @@ -0,0 +1 @@ +Improve the support for SSO authentication on the login fallback page. From adb38c0aa97f829a4c810c6713e34ca71e6f50ba Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 27 Mar 2020 10:53:07 +0100 Subject: [PATCH 3/8] Fix title updates --- synapse/static/client/login/js/login.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index b2419c1da52a..79c98f262e33 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -5,10 +5,12 @@ window.matrixLogin = { serverAcceptsSso: false, }; -var title_pre_auth = "Log in with one of the following methods" +var title_pre_auth = "Log in with one of the following methods"; +var title_post_auth = "Logging in..."; var submitPassword = function(user, pwd) { console.log("Logging in with password..."); + set_title(title_post_auth); var data = { type: "m.login.password", user: user, @@ -21,6 +23,7 @@ var submitPassword = function(user, pwd) { var submitToken = function(loginToken) { console.log("Logging in with login token..."); + set_title(title_post_auth); var data = { type: "m.login.token", token: loginToken @@ -92,7 +95,7 @@ var show_spinner = function() { }; var set_title = function(title) { - $("#title").innerText = title; + $("#title").text(title); }; var fetch_info = function(cb) { From 45409c2ef3aa42a08ab18246038c735a40b5d394 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 27 Mar 2020 12:15:46 +0100 Subject: [PATCH 4/8] Incorporate review --- synapse/static/client/login/js/login.js | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index 79c98f262e33..8206c94407b4 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -52,23 +52,19 @@ var show_login = function() { var this_page = window.location.origin + window.location.pathname; $("#sso_redirect_url").val(this_page); - if (matrixLogin.serverAcceptsCas) { - $("#sso_form").attr("action", "/_matrix/client/r0/login/cas/redirect"); - } - - if ((matrixLogin.serverAcceptsSso || matrixLogin.serverAcceptsCas)) { - if (try_token()) { - // Only show the SSO form if there's a login token in the query. That's - // because, if there is a token, and this function is run, it means an error - // happened, and in this case it's nicer to show the form with an error - // rather than redirect immediately to the SSO portal. + if (matrixLogin.serverAcceptsSso) { + if (try_token() || matrixLogin.serverAcceptsPassword) { + // Show the SSO form if there's a login token in the query. That's because, + // if there is a token, and this function is run, it means an error happened, + // and in this case it's nicer to show the form with an error rather than + // redirect immediately to the SSO portal. + // Also show the form if the server is accepting password login as well. $("#sso_form").show(); } else { // Submit the SSO form instead of displaying it. The reason behind this // behaviour is that the user will likely arrive here after clicking on a - // button, in the client, with a label such as "Continue with SSO". And even - // if that's not the case, it kind of makes sense to direct the user directly - // to the SSO portal and skip the password login form. + // button, in the client, with a label such as "Continue with SSO", so + // clicking on another button with the same semantics is a bit pointless. $("#sso_form").submit(); return; } From 706e4261b1d9d279bf26310c77807ff6ad3e3115 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 27 Mar 2020 12:20:25 +0100 Subject: [PATCH 5/8] Drop CAS entirely --- synapse/static/client/login/js/login.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index 8206c94407b4..d5a915ad2830 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -1,7 +1,6 @@ window.matrixLogin = { endpoint: location.origin + "/_matrix/client/r0/login", serverAcceptsPassword: false, - serverAcceptsCas: false, serverAcceptsSso: false, }; @@ -78,7 +77,7 @@ var show_login = function() { $("#password_flow").show(); } - if (!matrixLogin.serverAcceptsPassword && !matrixLogin.serverAcceptsCas && !matrixLogin.serverAcceptsSso) { + if (!matrixLogin.serverAcceptsPassword && !matrixLogin.serverAcceptsSso) { $("#no_login_types").show(); } }; @@ -97,13 +96,8 @@ var set_title = function(title) { var fetch_info = function(cb) { $.get(matrixLogin.endpoint, function(response) { var serverAcceptsPassword = false; - var serverAcceptsCas = false; for (var i=0; i Date: Fri, 27 Mar 2020 14:37:57 +0100 Subject: [PATCH 6/8] Incorporate review --- synapse/static/client/login/js/login.js | 28 ++++++++++--------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index d5a915ad2830..b8d570553c65 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -33,7 +33,7 @@ var submitToken = function(loginToken) { }; var errorFunc = function(err) { - show_login(); + show_login(true); if (err.responseJSON && err.responseJSON.error) { setFeedbackString(err.responseJSON.error + " (" + err.responseJSON.errcode + ")"); @@ -47,32 +47,24 @@ var setFeedbackString = function(text) { $("#feedback").text(text); }; -var show_login = function() { +var show_login = function(inhibit_redirect) { var this_page = window.location.origin + window.location.pathname; $("#sso_redirect_url").val(this_page); + // If inhibit_redirect is false, and SSO is the only supported login method, we can + // redirect straight to the SSO page if (matrixLogin.serverAcceptsSso) { - if (try_token() || matrixLogin.serverAcceptsPassword) { - // Show the SSO form if there's a login token in the query. That's because, - // if there is a token, and this function is run, it means an error happened, - // and in this case it's nicer to show the form with an error rather than - // redirect immediately to the SSO portal. - // Also show the form if the server is accepting password login as well. - $("#sso_form").show(); - } else { - // Submit the SSO form instead of displaying it. The reason behind this - // behaviour is that the user will likely arrive here after clicking on a - // button, in the client, with a label such as "Continue with SSO", so - // clicking on another button with the same semantics is a bit pointless. + if (!inhibit_redirect && !matrixLogin.serverAcceptsPassword) { $("#sso_form").submit(); return; } + + // Otherwise, show the SSO form the form + $("#sso_form").show(); } set_title(title_pre_auth); - $("#loading").hide(); - if (matrixLogin.serverAcceptsPassword) { $("#password_flow").show(); } @@ -80,6 +72,8 @@ var show_login = function() { if (!matrixLogin.serverAcceptsPassword && !matrixLogin.serverAcceptsSso) { $("#no_login_types").show(); } + + $("#loading").hide(); }; var show_spinner = function() { @@ -115,7 +109,7 @@ var fetch_info = function(cb) { matrixLogin.onLoad = function() { fetch_info(function() { if (!try_token()) { - show_login(); + show_login(false); } }); }; From 71f21c2376a4e8754a49cb2c9e78f195e576cec0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 27 Mar 2020 14:56:23 +0100 Subject: [PATCH 7/8] Incorporate review --- synapse/static/client/login/js/login.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index b8d570553c65..be46b6449630 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -33,6 +33,8 @@ var submitToken = function(loginToken) { }; var errorFunc = function(err) { + // We want to show the error to the user rather than redirecting immediately to the + // SSO portal (if SSO is the only login option), so we inhibit the redirect. show_login(true); if (err.responseJSON && err.responseJSON.error) { @@ -63,8 +65,6 @@ var show_login = function(inhibit_redirect) { $("#sso_form").show(); } - set_title(title_pre_auth); - if (matrixLogin.serverAcceptsPassword) { $("#password_flow").show(); } @@ -73,6 +73,8 @@ var show_login = function(inhibit_redirect) { $("#no_login_types").show(); } + set_title(title_pre_auth); + $("#loading").hide(); }; From 781f104d8825d08cad579efd750d87d553b6e44e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 27 Mar 2020 15:00:19 +0100 Subject: [PATCH 8/8] Typo --- synapse/static/client/login/js/login.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/static/client/login/js/login.js b/synapse/static/client/login/js/login.js index be46b6449630..debe46437134 100644 --- a/synapse/static/client/login/js/login.js +++ b/synapse/static/client/login/js/login.js @@ -61,7 +61,7 @@ var show_login = function(inhibit_redirect) { return; } - // Otherwise, show the SSO form the form + // Otherwise, show the SSO form $("#sso_form").show(); }