-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Delete devices and pushers on logouts etc #2722
Conversation
Whenever an access token is invalidated, we should remove the associated pushers.
Non-functional refactoring to move deactivate_account. This means that we'll be able to properly deactivate devices and access tokens without introducing a dependency loop.
Non-functional refactoring to move set_password. This means that we'll be able to properly deactivate devices and access tokens without introducing a dependency loop.
Make sure that we delete devices whenever a user is logged out due to any of the following situations: * /logout * /logout_all * change password * deactivate account (by the user or by an admin) * invalidate access token from a dynamic module Fixes #2672.
defer.Deferred[list[str, str|None]]: a list of the deleted tokens | ||
and device IDs | ||
defer.Deferred[list[str, int, str|None, int]]: a list of | ||
(token, token id, device id) for each of the deleted tokens |
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.
Why are we changing this? It doesn't seem to be used elsewhere in the code?
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.
we need the token_id so that we can delete the pushers which correspond to the deleted access tokens: 2c6d639#diff-b8464485d36f6f87caee3f4d82524213R743
user_id = user_info["user"].to_string() | ||
if device_id: | ||
# delete the device, which will also delete its access tokens | ||
yield self.hs.get_device_handler().delete_device(user_id, device_id) |
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.
So there's always going to be only one access token per device?
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.
yes, that's what we expect.
This PR is a set of four commits - each is probably worth looking at separately.
The idea is to make sure that we delete devices and pushers whenever a user logs out, to try to avoid accumulating them forever.