Check if users exist before inviting them and communicate errors #2317
Conversation
Fixes element-hq/element-web#3283 Fixes element-hq/element-web#3968 Fixes element-hq/element-web#4308 Fixes element-hq/element-web#1597 Fixes element-hq/element-web#6790 This does 3 things: * Makes the `MultiInviter` check for a user profile before attempting an invite. This is to prove the user exists. * Use the `MultiInviter` everywhere to avoid duplicating the logic. Although a couple places only invite one user, it is still worthwhile. * Communicate errors from the `MultiInviter` to the user in all cases. This is done through dialogs, where some existed previously but were not invoked. Specifically to the 403 error not working: What was happening was the `MultiInviter` loop was setting the `fatal` flag, but that didn't resolve the promise it stored. This caused a promise to always be open, therefore never hitting a dialog.
src/RoomInvite.js
Outdated
| const inviter = new MultiInviter(roomId); | ||
| return inviter.invite(addrs); | ||
| return inviter.invite(addrs).then(addrs => Promise.resolve({addrs, inviter})); |
There was a problem hiding this comment.
addrs in the then closure is actually the completion states? Name it accordingly in this file?
There was a problem hiding this comment.
it is, yea. I thought I renamed it too :(
|
This seems like a horrible idea, frankly. This is equating a user existing with having a profile, which #7922 has been the victim of. Can we back this out until we find a way that doesn't brick some peoples bridges? Even the spec states that a missing profile can lead to a 404, even if the user exists. https://matrix.org/docs/spec/client_server/r0.4.0.html#get-matrix-client-r0-profile-userid |
This also breaks features like password providers, mxisd integration. This also goes against the spec which allows users without profile, or the profile endpoint to return different results whenever the HS sees fit. As there is no endpoint which guarantee the validity of a user for a specific domain (which may or may not exist as a synapse account, since synapse is actually not authoritative once you use password providers), please rollback on this which is both breaking existing systems and not respecting the specification. |
|
We're investigating options to fix it, but please direct relevant feedback to the issue instead of on the PR: element-hq/element-web#7922 This PR fixes more than the problem at hand, and the issue is the best place to discuss the problem. |
Fixes element-hq/element-web#3283
Fixes element-hq/element-web#3968
Fixes element-hq/element-web#4308
Fixes element-hq/element-web#1597
Fixes element-hq/element-web#6790
This does 3 things:
MultiInvitercheck for a user profile before attempting an invite. This is to prove the user exists.MultiInvitereverywhere to avoid duplicating the logic. Although a couple places only invite one user, it is still worthwhile.MultiInviterto the user in all cases. This is done through dialogs, where some existed previously but were not invoked.Specifically to the 403 error not working: What was happening was the
MultiInviterloop was setting thefatalflag, but that didn't resolve the promise it stored. This caused a promise to always be open, therefore never hitting a dialog.