Skip to content
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

Clarify UI authentication behavior around resubmitting the original request #732

Open
clokep opened this issue Dec 15, 2020 · 5 comments
Open
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit

Comments

@clokep
Copy link
Member

clokep commented Dec 15, 2020

This has come up a few times between client-server developers and it seems that the UI auth process could use some clarification of the required behavior.

In particular, there's different client behavior about whether they should provide the full request during each stage of UI auth or only the auth information (including the session).

The specification for UIA currently says:

It does this by resubmitting the same request with the addition of an auth key in the object that it submits. [...] It must also contain a session key with the value of the session key given by the homeserver, if one was given.

The example flows then show the full request repeated with the auth dictionary.

Synapse includes some code to allow for only the auth dict to be sent in subsequent requests which is useful for a couple of reasons:

  • During registration (and potentially other operations, e.g. password reset) the credentials only need to be sent a single time instead of in each request, thus the client does not need to worry about how to securely store a password throughout this flow. (In particular, what does a client do if the user gives up halfway through?)
  • It is reasonable for a user to complete registration steps on different devices, e.g. start registration on a device, but do the email verification flow on a different device. The second device wouldn't have access to the rest of the original request.

Synapse supports this behavior, but I believe that Dendrite and Conduit do not. Element Android and Element iOS have some different behavior here (Android assumes that the information only needs to be sent once, while iOS sometimes modifies the information during the flow, which isn't really allowed see matrix-org/synapse#7452 / matrix-org/synapse#7455 / element-hq/element-android#1924.) It was mentioned that Element Web "does keep the password in memory (only), both for the UI state of the form and also to send with each UI auth request". I haven't checked other clients, but since Synapse is currently very flexible here it could be hard to know the correct behavior.

CC @bmarty @richvdh

@richvdh
Copy link
Member

richvdh commented Dec 15, 2020

for clarity, we believe synapse's behaviour here (where you only need to submit a password once during a registration flow) is correct, and the spec should be updated. It's unclear to me if this change requires an MSC.

@bmarty
Copy link

bmarty commented Dec 15, 2020

Also the Client-Server spec for registration does not claim that any parameters are mandatory. This has to be clarified too.

image

@turt2live
Copy link
Member

It's unclear to me if this change requires an MSC.

There's a wide variety of bugs/clarifications to be made for UIA. I'd be inclined to have an MSC that solves all the known problems instead of any particular bits. This could mean replacing the system.

@richvdh
Copy link
Member

richvdh commented Dec 15, 2020

I'd be inclined to have an MSC that solves all the known problems instead of any particular bits. This could mean replacing the system.

that sounds like a recipe for it never getting fixed. Where there are small, known, problems, it's extremely frustrating if they all get stacked up behind a big "fix the whole world!" project.

(In other words, please let's not scope-creep this into "replace UIA".)

@turt2live
Copy link
Member

we don't have to scope it to fixing the world, but I do get increasingly frustrated with MSCs which target extremely narrow parts of the spec - a bit of generalization is good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit
Projects
None yet
Development

No branches or pull requests

4 participants