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

Force user logout option #44

Closed
ser opened this issue Sep 27, 2021 · 13 comments
Closed

Force user logout option #44

ser opened this issue Sep 27, 2021 · 13 comments

Comments

@ser
Copy link

ser commented Sep 27, 2021

There should be an option to invalidate all access tokens for particular user = force logout.

@JOJ0
Copy link
Owner

JOJ0 commented Sep 28, 2021

Hi, thanks for the idea but is there an API available for that? Did you check the Synapse admin API docs yet? https://matrix-org.github.io/synapse/latest/usage/administration/admin_api/index.html

Synadm is actually just a cli frontend for what these API's are offering.

@JOJ0
Copy link
Owner

JOJ0 commented Sep 28, 2021

maybe have a look in the user admin section of the docs and see what would suit your usecase. thank

@ser
Copy link
Author

ser commented Sep 28, 2021

I suppose it requires two steps:

First get all devices belonging to the user:

The API is:

GET /_synapse/admin/v2/users/<user_id>/devices

And second - delete devices:

The API is:

POST /_synapse/admin/v2/users/<user_id>/delete_devices

{
"devices": [
"QBUAZIFURK",
"AUIECTSRND"
],
}

Is it more clear now? :-)

@nemobis
Copy link
Contributor

nemobis commented Oct 3, 2021

I will make a patch for this.

Use case: mass delete unused devices/sessions for all users in a room, e.g. for performance reasons (encryption cost for each message grows with the number of recipient devices) and to reduce the attack surface.

Acceptance criteria:

  • can be chained to a source of user IDs, be it a command line argument or just a previous synadm;
  • allow to delete the devices last seen more than N days ago;
  • allow to keep at least the M most recent devices.

@JOJ0
Copy link
Owner

JOJ0 commented Oct 4, 2021

I will make a patch for this.

@nemobis nice! thanks in advance, that's great news! :-)

Use case: mass delete unused devices/sessions for all users in a room, e.g. for performance reasons (encryption cost for each message grows with the number of recipient devices) and to reduce the attack surface.

Acceptance criteria:

  • can be chained to a source of user IDs, be it a command line argument or just a previous synadm;

My first idea when I read this feature suggestion was that the delete command should simply accept device id's as returned by https://matrix-org.github.io/synapse/latest/admin_api/user_admin_api.html#list-all-devices, but having it accept user id's is probably closer to real world use cases.

Also, at first, I was thinking that actually I am seeing 3 commands here:

  • user device list
  • user device details
  • user device delete
  • allow to delete the devices last seen more than N days ago;
  • allow to keep at least the M most recent devices.

Those two are very nice convenience features, and features that incorporate "days" instead of timestamps would fit user experience of synadm as this concept is already used in media and history subcommands. I am just thinking that it would also be an option to implement this use case into several commands since the user device handling api's are not implemented yet. Above two points could as well be features/options of the user device list and user device details commands. Well, to be honest, in that case it would be necessary to implement one command that uses both api's, the list device and the one showing details of the device. The initial use-case of force-logging out user's could then be scripted with several synadm commands and the help of jq or plain shell magic.

So that's my thoughts on that. What do you think of splitting into several commands? If you'd rather prefer to have all of above proposal into one big multi-functional user force-logout (or something like that) command I'd be fine with it as well.

nemobis added a commit to nemobis/synadm that referenced this issue Oct 5, 2021
@nemobis
Copy link
Contributor

nemobis commented Oct 6, 2021

Thank you @JOJ0 for your comments! For now I've implemented something quick that I needed to do some cleanup on my instance (and seems to work fine), but hopefully it's not too far from what you're describing.

My first idea when I read this feature suggestion was that the delete command should simply accept device id's

That's what I thought initially too, but the deletion API requires a user_id so we'd need to iterate through all users if we're not provided with a user_id, no? I'm not sure there's a real usefulness to that.

So that's my thoughts on that. What do you think of splitting into several commands?

That makes sense to me but a command like user devices details would be quite similar to the existing user whois, and user devices list I'm not sure. That's fine by me (they're just two different versions of the API after all) but I wondered if it might be confusing.

I also lost more time than I care to admit deciding whether to use the words "devices" or "sessions"...

nemobis added a commit to nemobis/synadm that referenced this issue Oct 6, 2021
@JOJ0
Copy link
Owner

JOJ0 commented Nov 13, 2021

Hi @nemobis as you probably noticed, I merged your PR a couple of days ago. I started refactoring and advancing it in this branch: https://github.com/JOJ0/synadm/tree/pr45_refactor. This is the diff so far: master...pr45_refactor
Since you coded the feature initially, you might have a good eye for things could now be broken or work differently as you had in mind ;-)

@ser Please help us test the new feature. Does it do everything you expected when you filed the feature request? Also now would be a good time to request changes in how the output gets formatted and the general workflow of the new command. Do you have synadm installed via pip or via git? If you are unsure how to let it run from the feature branch pr45_refactor, I can assist.

@JOJ0
Copy link
Owner

JOJ0 commented Nov 13, 2021

Also up for discussion: Default values of

  -d, --min-days INTEGER       how many days need to have passed from the last
                               time a device was seen for it to be deleted.
                               [default: 90]
                            
  -s, --min-surviving INTEGER  stop processing devices when only this number
                               of devices is left for this user. Allows to
                               reduce disruption by preserving recently used
                               devices/sessions.  [default: 1]

@JOJ0
Copy link
Owner

JOJ0 commented Nov 21, 2021

@ser Feature finished and in master. Please take a minute to try it out and see if it fits your needs and close the issue. Thanks.
@nemobis still invited to take a quick look :-)
hope to get this released in the next couple of days.
thanks for your ideas and submisssions! Appreciated!

@JOJ0
Copy link
Owner

JOJ0 commented Nov 27, 2021

@JOJ0 JOJ0 closed this as completed Nov 27, 2021
@nemobis
Copy link
Contributor

nemobis commented Nov 28, 2021 via email

@ser
Copy link
Author

ser commented Jun 13, 2022

I've forgotten to acknowledge that everything works amazing and it a function I use every day. Thank you @nemobis <3

@nemobis
Copy link
Contributor

nemobis commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants