Recent release broke login using mxid #27

Closed famedly-bot closed 1 year ago

In GitLab by @nico-famedly on Aug 15, 2022, 13:10

Our SDK now regularly fails to login when trying to sign in using only the mxid. This is explicitly supported by the spec, so this should work.

This might affect more stuff than our tests, our tests are just the first one we noticed.

CI job for reference: https://gitlab.com/famedly/company/frontend/famedlysdk/-/jobs/2884344937

In GitLab by @nico-famedly on Aug 15, 2022, 13:39

mentioned in merge request famedly/company/frontend/famedlysdk!1099

In GitLab by @jdreichmann on Aug 15, 2022, 13:52

For reference, where in the spec is this allowed? If /login does not advertise m.login.password, simple password login is not available, and in CS-API v1.3 Section 4.6 the payload for /login only talks about user+password, 3PID password login and a token login...

In GitLab by @krille-chan on Aug 15, 2022, 13:56

https://integration-tests-stable.famedly.de/_matrix/client/v3/login clearly says that m.login.password is allowed and https://spec.matrix.org/v1.3/client-server-api/#login says:

"identifier": {
  "type": "m.id.user",
  "user": "<user_id or user localpart>"
}

and user_id here is mxid

In GitLab by @agraven on Aug 15, 2022, 14:04

The password is still expected in that case, the spec page you linked shows that the full object is

{
  "type": "m.login.password",
  "identifier": {
    "type": "m.id.user",
    "user": "<user_id or user localpart>"
  },
  "password": "<password>"
}

I'm concerned we're talking past each other. Just to confirm, is your expectation that it should be possible to login with type "m.login.password" without putting a password in the json object?

In GitLab by @krille-chan on Aug 15, 2022, 14:14

I have just only copied the json part which was important for this discussion. Of course we are also sending the password and the type and maybe other keys like device display name

In GitLab by @agraven on Aug 15, 2022, 14:23

Okay, good to have that cleared up. The phrasing in issue description that only the mxid and not the password and not the mxid had been sent, hence my confusion. In that case some clarification would be appreciated. Are you just reporting that password login fails when it shouldn't, or is there something particular about the way login was attempted that you wish to highlight?

In GitLab by @nico-famedly on Aug 15, 2022, 14:39

No, from what I can tell it is just that normal password login now fails on the integration server, if you use an mxid instead of just the username. No fany login, no UIA, etc (unless I am missing something. It could totally be that our tests fall back to doing UIA now).

In GitLab by @agraven on Aug 15, 2022, 14:40

an mxid instead of just the username

What do you mean by this? When you write username, do you mean localpart?

In GitLab by @nico-famedly on Aug 15, 2022, 14:40

Yes

In GitLab by @nico-famedly on Aug 15, 2022, 14:42

Well, actually no, username can be something else, that gets mapped to the localpart then, i.e. your username can be "Nico", but the mxid then "@nico:matrix.org", because of lowercase mapping, I think. But I mean something that is not the full mxid in any case.

In GitLab by @agraven on Aug 15, 2022, 14:43

Okay, now I'm no longer confused.

In GitLab by @agraven on Aug 17, 2022, 13:10

Do you know what version UIA Proxy was upgraded to when this issue started occuring?

In GitLab by @agraven on Aug 17, 2022, 14:39

For posterity, UIA proxy strips the user identifier down to the localpart here, using the logic implemented here. This behavior hasn't changed for most of UIA Proxy's existence, so I don't have a good guess for what's going wrong here.

In GitLab by @agraven on Aug 18, 2022, 08:37

added #28 as child task

In GitLab by @agraven on Aug 18, 2022, 08:37

added #29 as child task

In GitLab by @agraven on Aug 18, 2022, 08:38

removed child task #29

In GitLab by @agraven on Aug 18, 2022, 08:38

removed child task #28

In GitLab by @herr-rodrigo on Sep 8, 2022, 11:29

JC to follow up with an explanation around why this won't go ahead on its current form.

In GitLab by @jdreichmann on Sep 8, 2022, 11:34

FYI when JC is not part of the participants, tag him ^^ @jcgruenhage

In GitLab by @herr-rodrigo on Sep 8, 2022, 11:44

Hi @jdreichmann , we discussed this on our IM squad daily earlier today and @jcgruenhage is aware, thanks for tagging him anyways.

In GitLab by @jdreichmann on Sep 8, 2022, 11:59

Discussion on squad dailys is not documented though ^^ tagging people when they are specifically adressed is always preferrable, especially considering not everyone can know the context of private discussions

In GitLab by @kate-shine on Sep 8, 2022, 12:04

JC was part of that discussion, but tagging is still good - it puts that mention into user's Gitlab To-Do list and makes it harder to forget.

In GitLab by @kate-shine on Sep 9, 2022, 11:42

Failure was due to loss of cache that maps mxids to users. Issue could be fixed by UIA priming the cache from LDAP on each launch, but we won't pursue that path because UIA is being phased out anyway :)

In GitLab by @nico-famedly on Sep 29, 2022, 13:04

I think this also sometimes breaks UIA prompts until you login again, is that possible?

(I reopened that because we got the same error while testing resetting the device signing keys, which was stuck on 401 and required a new login by that user to fix)

famedly / uia-proxy

Recent release broke login using mxid #27