-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
SAML2 Improvements and redirect stuff #5316
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -727,6 +727,9 @@ def validate_login(self, username, login_submission): | |||||||||
if canonical_user_id: | ||||||||||
defer.returnValue((canonical_user_id, None)) | ||||||||||
|
||||||||||
if login_type == LoginType.SSO: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you share the thinking behind this change? it looks like it's turning a 400 into a 403, but which endpoint is this for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I remember correctly, because of this synapse/synapse/rest/client/v1/login.py Lines 251 to 254 in dc3e586
Due to no if condition being met it would fail because SSO is not a valid login type. |
||||||||||
known_login_type = True | ||||||||||
|
||||||||||
if not known_login_type: | ||||||||||
raise SynapseError(400, "Unknown login type %s" % login_type) | ||||||||||
|
||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,7 @@ var show_login = function() { | |
} | ||
|
||
if (matrixLogin.serverAcceptsSso) { | ||
$("#sso_form").attr("action", "/_matrix/client/r0/login/sso/redirect"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't this the default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In which point the default? This change was needed as otherwise no SSO redirect would have happened. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default action for #sso_form is |
||
$("#sso_flow").show(); | ||
} else if (matrixLogin.serverAcceptsCas) { | ||
$("#sso_form").attr("action", "/_matrix/client/r0/login/cas/redirect"); | ||
|
@@ -79,7 +80,7 @@ var fetch_info = function(cb) { | |
$.get(matrixLogin.endpoint, function(response) { | ||
var serverAcceptsPassword = false; | ||
var serverAcceptsCas = false; | ||
for (var i=0; i<response.flows.length; i++) { | ||
for (var i = 0; i < response.flows.length; i++) { | ||
var flow = response.flows[i]; | ||
if ("m.login.cas" === flow.type) { | ||
matrixLogin.serverAcceptsCas = true; | ||
|
@@ -121,6 +122,7 @@ matrixLogin.onLogin = function(response) { | |
// clobber this function | ||
console.log("onLogin - This function should be replaced to proceed."); | ||
console.log(response); | ||
alert("Login successful!"); | ||
}; | ||
|
||
var parseQsFromUrl = function(query) { | ||
|
@@ -143,7 +145,7 @@ var try_token = function() { | |
if (pos == -1) { | ||
return false; | ||
} | ||
var qs = parseQsFromUrl(window.location.href.substr(pos+1)); | ||
var qs = parseQsFromUrl(window.location.href.substr(pos + 1)); | ||
|
||
var loginToken = qs.loginToken; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh I think this is redundant. The default is for 'enabled' to be considered
True
: the only reason that the config parser checks it is to avoid breaking people who haveenabled: false
in their config files.