Don't no-op logouts

[AndrewFerr]

Disconnect bridge users from their accounts on logout. This does not
actually delete the Signal device being logged out of, though.

[tulir]

There shouldn't be a logout command before logging out is actually supported, but delete-session should probably actually delete the session, not sure why it wasn't made to do that in the first place

[smweber]

Perhaps the name delete-session isn't aligned with other mautrix bridges, but the intent behind that command is to delete the Signal encryption session (by wiping out all the keys) but leave the rest of the state intact to allow relinking a new Signal device to the bridge and get all new encryption sessions without losing group keys, profile data, etc.

Resetting encryption sessions is often necessary with Signal due to bugs in the Signal mobile apps.

[tulir]

Hmm, keeping those things sounds unnecessary since they're resynced from the phone anyway

[smweber]

Maybe, but the phone sync is best effort. I figured why not have a command that retains this data if the user's intent is to immediately reconnect the same account. But if you don't want this command to exist I don't feel strongly about it, a user can achieve the same behaviour by deleting the device in the Signal app (the bridge will wipe encryption keys and keep everything else)

[AndrewFerr]

So is the main purpose of the current delete-session to fix occasional issues with encryption keys, and to typically be followed up immediately with login?

In any case, would it be appropriate for (*User).Logout to also delete the encryption session like delete-session does?

[AndrewFerr]

Expressed differently: when would a user want to log out of the bridge without deleting their Signal encryption session? Intuitively, I'd expect a bridge logout to also delete the session (but now I understand why delete-session should not also trigger a logout, for the reasons you explained).

[smweber]

Yup, you're correct about delete-session. The main reason I didn't make logout actually logout is because I didn't need it to at the time. I was implementing the provisioning API, trying to make it behave identical to the old Signal bridge (to avoid making changes to the Signal provisioning flow in Beeper), and during the provisioning flow Beeper called logout a bunch of times before logging in, and I didn't want to break the reconnection logic, so I just made it no-op.

The ideal would be to change the logout endpoint in the provisioning API to be a more obvious no-op (and eventually fix Beeper to be more sane during Signal provisioning so we don't need the no-op at all), then properly implement the logout command (delete keys and session in bridge, and delete device from Signal). But I don't currently have bandwidth to be a lot of help with this at the moment ;)

[AndrewFerr]

The latest batch of commits tries to delete devices on "hard" logout situations (like logout, or the bridge being unlinked remotely), and to clear only session keys & connections on "soft" logouts (like delete-session). I hope this makes logouts work the way they ought to.

Logging out from the bridge still doesn't unlink the bridge from the primary Signal device, though -- I did some testing and it seems like it's still the case that only the primary device can unlink other devices, and a non-primary device can't unlink itself.

This still has the provisioning API's logout command not as a no-op, but I've given it a delete-session equivalent. If its logout command should still be a no-op, I can move the non-no-op (op?) version to a new endpoint.

[smweber]

I don't know about the bridge being unlinked in the Signal app being a "hard-logout" situation. If a user accidentally unlinks (perhaps not knowing what they're doing) it's nice for them to be able to relink with few consequences. I've also told users to unlink in the app then relink their bridge to fix situations where the Signal apps encryption gets jammed up (happens shockingly often).

[smweber]

This still has the provisioning API's logout command not as a no-op, but I've given it a delete-session equivalent. If its logout command should still be a no-op, I can move the non-no-op (op?) version to a new endpoint.

I think this should be fine :)

[tulir]

Ah right, Signal doesn't allow real logouts, then it should be fine to have logout just delete the session. Should maybe also rename delete-session to something more accurate because it's deleting less than logout

[AndrewFerr]

I don't know about the bridge being unlinked in the Signal app being a "hard-logout" situation. If a user accidentally unlinks (perhaps not knowing what they're doing) it's nice for them to be able to relink with few consequences.

The reason I went for a "hard-logout" on remote unlinking is that changing it to do a "soft-logout" makes future login attempts kick me out right away. i.e. this happens:

• run login, which succeeds
• unlink the bridge device from Signal
• bridge says it got logged out
• run login again, which succeeds
• immediately get 401 responses in logs & bridge says it got logged out

[smweber]

The reason I went for a "hard-logout" on remote unlinking is that changing it to do a "soft-logout" makes future login attempts kick me out right away. i.e. this happens:

* run `login`, which succeeds
* unlink the bridge device from Signal
* bridge says it got logged out
* run `login` again, which succeeds
* immediately get 401 responses in logs & bridge says it got logged out

That's really strange, perhaps there's some difference in how login does provisioning vs the provisioning API? I definitely delete the bridge device in Signal then log back in all the time, it's how I was testing the new prekey stuff I was working on a few weeks ago - but I use the provisioning API S: