-
Notifications
You must be signed in to change notification settings - Fork 385
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
MSC2284: Making the identity server optional during discovery #2284
Conversation
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.
This seems reasonable to me from a client perspective... The IS should (soon) be optional for all authentication flows, so it's only needed lazily for lookups and binding.
Clients can end up being configured with an invalid or inoperable identity server. This is | ||
considered a feature by this proposal to allow for less intelligent clients to have their | ||
identity server disabled. Intelligent clients could interpret the lack of identity server | ||
as the homeserver/user asking that identity server functionality be disabled in the client. |
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.
I feel like in the interest of infrastructure debuggability and general client UX, there should be a flag that indicates whether the identity server is intentionally disabled, so that intelligent clients can distinguish between this and an erroneous misconfiguration (or transient unavailability due to infrastructural issues).
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.
I'm not sure I agree. In terms of transient unavailability, users won't be able to log in in the first place because the homeserver information will also be missing. Spelling errors, structural problems, etc are easily debugged by a community member in a support channel - the problem should be obvious. For instance, if you end up with a user experience that doesn't involve the identity server you were expecting, you can probably guess that it has to do with your config. Hopefully people these days are still testing their configuration changes before going live.
The other point for defaulting to no identity server instead of wanting an explicit flag is to protect user privacy. Where possible, the spec should be heavily suggesting to clients that a default identity server not be used. When the spec says that lack of useful identity server information means that none is selected, the client can't reasonably default to an identity server which might be risky to the user. Not to mention a flag saying "I absolutely don't want to use an identity server" is more fiddly to implement, particularly when the interaction between users and homeservers is already insanely complicated during login.
This feels very sane indeed to me - we've been bitten multiple times by inexplicable client failures just because the IS was unavailable, and given an IS isn't even compulsory, an invalid IS shouldn't block anything. |
As discussed in MSC2284, this relaxes the identity server discovery to a `FAIL_PROMPT` state so that clients can choose to warn and continue. Part of element-hq/element-web#11102 Implements matrix-org/matrix-spec-proposals#2284
As discussed in MSC2284, this relaxes the identity server discovery to a `FAIL_PROMPT` state so that clients can choose to warn and continue. Part of element-hq/element-web#11102 Implements matrix-org/matrix-spec-proposals#2284
This seems largely accepted by the spec core team and surrounding teams, so to try and push it forward/get more feedback: @mscbot fcp merge |
Team member @turt2live has proposed to merge this. The next step is review by the rest of the tagged people: No concerns currently listed. Once at least 75% of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
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.
I'm generally not a fan of adding prompts to UX but I agree that FAIL_ERROR
is a bit too harsh and that client applications can do some further intelligence on whether/how they should interact with the user on the matter.
🔔 This is now entering its final comment period, as per the review above. 🔔 |
hello @mscbot, it is time. |
The final comment period, with a disposition to merge, as per the review above, is now complete. |
As discussed in MSC2284, this relaxes the identity server discovery to a `FAIL_PROMPT` state so that clients can choose to warn and continue. Part of element-hq/element-web#11102 Implements matrix-org/matrix-spec-proposals#2284
As discussed in MSC2284, this relaxes the identity server discovery to a `FAIL_PROMPT` state so that clients can choose to warn and continue. Part of element-hq/element-web#11102 Implements matrix-org/matrix-spec-proposals#2284
Historical implementation on this is here: matrix-org/matrix-js-sdk#1062 |
Merged 🎉 |
Rendered