Skip to content

Commit

Permalink
feat: delete inactive users and index
Browse files Browse the repository at this point in the history
* rfc remove.
  • Loading branch information
pajgo committed Oct 23, 2019
1 parent c2b4fba commit a08515d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 91 deletions.
82 changes: 19 additions & 63 deletions rfcs/inactive_users/user_cleanup.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,75 +6,31 @@ A lot of linked keys remain in the database, and this leads to keyspace pollutio
For better data handling and clean database structure, I introduce some changes in service logic:

## General defs
- `inactive-users`
Redis database sorted set key. Assigned to `USER_ACTIVATE` constant.
Record contains `userId` as value and `timestamp` as score.
- `user-audiences` [Described here](update_metadata_lua.md#audience-list-update)
- `deleteInactivatedUsers` Redis script, handling all cleanup logic.
- `inactive-users` [Described here](#inactive-users-index)
- `user-audiences` [Described here](user_and_organization_meta_update.md)

## Organization Members
The `organization add member` process doesn't have validation whether the user passed activation and allows
inviting inactive users into an organization. The script checks whether inactivated user assigned to any organization
and deletes users from organization members and user->organization bindings.
## `inactive-users` index
It's an Additional Redis Sorted Set which contains the list of IDs of the `inactive` users.
Each item score is equal to the `timestamp` set on user creation.
To avoid hardcoded SET name new `USERS_INACTIVATED` constant introduced.

## Registration and activation
Every Activation and Registration request event executes `users:cleanup` hook.
The Activation request executes the hook first this strategy saves from inactive
users that hit TTL but tried to pass the activation process.
The Registration request executes the hook after `username` exists check.

## Registration process
#### Registration process
When the user succeeds registration but activation not requested, the new entry added to `inactive-users`.
Record contains `userId` and `current timestamp`.

## Activation process
When the user succeeds activation `userId`,the entry deleted from `inactive-users`.

## `users:cleanup` hook `cleanUsers(suppress?)`
`suppress` parameter defines function error behavior. If parameter set, the function throws errors,
otherwise, function calls `log.error` with `error.message` as message.
Default value is `true`. IMHO User shouldn't know about our problems.
**NOTE:** Old Redis `expire` setting methods left in their place, to save old service behavior.

Other option, is to define new config parameter as object and move `config.deleteInactiveAccounts` into it:
```javascript
const conf = {
deleteInactiveUsers: {
ttl: seconds, // replaces deleteInactiveAccounts
suppressErrors: true || false,
},
}
```
Calls `deleteInactivatedUsers` script with TTL parameter from `service.config.deleteInactiveAccounts`.
When script finished, calls TokenManager to delete Action tokens(`USER_ACTION_*`, ``).
*NOTE*: Should we update TokenManager to add support of pipeline?
#### Activation process
When the user succeeds activation the entry deleted from `inactive-users`.

## Redis Delete Inactive User Script
When the service connects to the Redis server and fires event "plugin:connect:$redisType" `utils/inactive/defineCommand.js` executed.
Function rendering `deleteInactivatedUsers.lua.hbs` and evals resulting script into IORedis.
The Script using a dozen constants, keys, and templates, so all these values rendered inside of the template using template context.
Returns list of deleted users.
#### Index cleanup
On `registration` cleanup method executes before user creation.
On `activation` cleanup method executes before any checks performed by `activation` action.
* Uses `dlock` for itself to be sure that only one process runs.
* Gets outdated user list and deletes them.

*NOTE*: Using experimental `fs.promises.readFile` API function. On `node` 10.x it's an experimental feature,
on `node` >= 11.x function becomes stable without any changes in API.

#### deleteInactivatedUsers `USERS_ACTIVATED` `TTL` as seconds
##### Script paramerters:
1. KEYS[1] Sorted Set name containing the list of users that didn't pass activation.
2. ARGS[1] TTL in seconds.

##### When started:
1. Gets UserId's from ZSET `USERS_ACTIVATED` where score < `now() - TTL * 1000` and iterates over them.
2. Gets dependent userData such as username, alias, and SSO provider information used in delete procedure and calls [Delete process](#delete-process).
3. Deletes processed user ids from `USER_ACTIVATED` key.
## TODO Organization Members Need additional information
The `organization add member` process doesn't have validation whether the user passed activation and allows
inviting inactive users into an organization. The script checks whether inactivated user assigned to any organization
and deletes users from organization members and user->organization bindings.

##### Delete process
The main logic is based on `actions/removeUsers.js`.
Using the username, id, alias and SSO providers fields, script checks and removes dependent data from the database:
* Alias to id binding.
* Username to id binding.
* All assigned metadata. Key names rendered from the template and `user-audiences`.
* SSO provider to id binding. Using `SSO_PROVIDERS` items as the field name decodes and extracts UID's from Internal data.
* User tokens.
* Private and public id indexes
* Links and data used in Organization assignment

28 changes: 0 additions & 28 deletions rfcs/inactive_users/user_index.md

This file was deleted.

0 comments on commit a08515d

Please sign in to comment.