-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow serverName
to be specified in URL
#17824
base: develop
Are you sure you want to change the base?
Conversation
Signed-off-by: Aaron Raimist <[email protected]>
Signed-off-by: Aaron Raimist <[email protected]>
@vector-im/product this could potentially be abused where someone sends you a link including a serverName they control and if you don't notice the server isn't one you expect then your username/password may be sent to the wrong server. |
Definitely a valid concern but personally I don't see it as being a significantly different risk from someone sending you a link to app.el3ment.io which is configured to use to evil.com's server. Or the risk of sending matrix.org your username and password when you meant to use your personal server. The ultimate solution is to stop people from ever being able to log in with just a localpart and always requiring the full username to be entered. (And doing .well-known lookup on registration) |
Thanks a lot! This would enable us to use a single element-web instance for our homeservers. see also : element-hq/element-meta#1580 |
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.
Code seems fine, though the product team's review will be a lot more interesting.
Co-authored-by: Travis Ralston <[email protected]>
Co-authored-by: Travis Ralston <[email protected]>
defaultUsername was added in matrix-org/matrix-react-sdk#5674. I'm not aware of any others but they may exist. Signed-off-by: Aaron Raimist <[email protected]>
fwiw this would be a major usability step forward for new signups - I'm rolling this out to some non-tech friends and without this, its a lot of screenshots, careful settings, and .. mess. This will be a huge improvement! |
Co-authored-by: Stanislav Motylkov <[email protected]>
Signed-off-by: Aaron Raimist <[email protected]>
For a client-agnostic approach this could be an enhancement of the Matrix-URI-scheme. Such a link would tell a Matrix client to start the registration or login flow, optionally prefill the username/localpart, optionally include the registration token and do that on the specified server. |
@turt2live Any chance you could give an update here? Any chance this PR can get reviewed by the product team? |
Are there any plans to get this branch merged in? It's been sitting here for two years, and the original issue is almost 6 years old. If there are any blockers, could they be discussed & run by the community so perhaps someone could contribute the remaining requirements to get this pushed through? |
@vector-im/security how do you feel about being able to specify homeserver URL in the Element URL in the context of phishing and other such attacks |
I can't build your fork with the latest develop branches of matrix-js-sdk and matrix-react-sdk. Maybe I'm missing something in my dev VM. As an extra guard rail, we could force the user to type their MXID when using this parameter. This will be an extra layer if an attacker tries to exploit a legitimate element-web installation for phishing. I imagine this also voids the quality of life improvement of this PR, right? |
@davidegirardi
At which point, given if you type your MXID your server is auto-discovered, is there any point to this?
Quite |
I agree and there are concrete possibilities of quite a few users of password managers to autotype only the local part of the MXID. We would be basically creating a variation of an open redirect but for credential submission. I think the QoL does not balance the risk for the people who actually need that QoL improvement. I'm voting no to this. |
I think the only way this can work safely is if any url-suggested homeserver comes with a chunky warning interstitial. |
Isn't that going to:
I am also thinking about the role of the move to OIDC in this. I don't know how the flow will look like but since the authentication will happen in the OpenID provider, this QoL improvement starts reducing its return value. |
I guess this problem gets easier with OIDC, as your password manager will be keyed to the server's auth domain rather than the client domain.
I disagree, I think the QoL stays the same (no need to input server manually) and password manager has all the odds to protect you by not suggesting you enter your creds into a phishing server |
How exactly does this differ to registers the actual Additionally, if one is using a password manager, there's A) little chance that the credentials transfer to more personal platforms, and B) a great chance that the password manager itself has warned the user about the possibility of credential theft with auto-logins Additionally additionally, the responsibility of the server owner is handling / dealing with this sort of thing. My server uses SSO (via Keycloak, Authelia, etc) as its only login method, because the built-in password protocol in Matrix is simply not suited for a large userbase deployment (I believe there are discussions open for this, moving to an open authentication standard as the default for the protocol). Overall, what I'm seeing with this argument is a simple misunderstanding of how phishing is executed and combated; and a harmful one, because simply defaulting to |
Point of fact, this is from the last TWIM:
The expectation is on the application UI to make the URL very clear, and the expectation is on the user to verify if that URL is the one they are expecting. |
Your password manager is smart enough to not suggest a password for
Definitely, but common security sense also has to apply, users aren't all the most security cautious and can get very easily desensitised to small differences like |
Dropping in because I also expected this to work, given that room links worked once similarly. I don't see any security issues with this being enabled for homeservers that require oidc, like mine. It moves all credential entry elsewhere. |
|
1 similar comment
|
This comment was marked as spam.
This comment was marked as spam.
Thanks for your contribution @aaronraimist . I have discussed this with both Element Product and Security teams and they would be happy to accept this feature if validation checks were added to the implementation to ensure the target homeserver requires OIDC. If this is something you could add to this PR(along with the required tests and checks) we could accept it. Would you like to pursue this? If not, we will close the PR and somebody else is welcome to open another with such an implementation. |
Fixes #5469
This may not be exactly how you want to implement it (since it follows the same path as
default_server_name
which is deprecated) but it works. I'm not really sure whydefault_server_name
is deprecated.Try it:
https://localhost:8080/#/login?serverName=kde.org
https://localhost:8080/#/register?serverName=kde.org
Also works when the homeserver URL (without https://) is used instead of the server name:
https://localhost:8080/#/login?serverName=kde.modular.im
Also works with the defaultUsername query param
https://localhost:8080/#/login?defaultUsername=alice&serverName=raim.ist
This PR currently has none of the required changelog labels.
A reviewer can add one of:
T-Deprecation
,T-Enhancement
,T-Defect
,T-Task
to indicate what type of change this is, or addType: [enhancement/defect/task]
to the description and I'll add them for you.