feat: delete inactivated users

* organization action tokens remove
* user action tokens remove

Author: pajgo

rfcs/inactive_users_cleanup.md

@@ -22,7 +22,6 @@ 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.
-If Error happens inside hook, it logged into a logfile and not raised.
 
 ## Registration process
 When the user succeeds registration but activation not requested, the new entry added to `inactive-users`.
@@ -34,6 +33,7 @@ When the user succeeds activation `userId`,the entry deleted from `inactive-user
 ## `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.
 
 Other option, is to define new config parameter as object and move `config.deleteInactiveAccounts` into it:
 ```javascript
@@ -45,11 +45,14 @@ const conf = {
 }
 ```
 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?
 
 ## 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.
 
 #### deleteInactivatedUsers `USERS_ACTIVATED` `TTL` as seconds
 ##### Script paramerters:
@@ -71,5 +74,4 @@ Using the username, id, alias and SSO providers fields, script checks and remove
 * User tokens.
 * Private and public id indexes
 * Links and data used in Organization assignment
-* Throttle actions (???). **THROTTLE_PREFIX constant doesn't exist in constants.js, so assuming this left for backward
- compatibility with previous version**
+

scripts/deleteInactivatedUsers.lua.hbs

@@ -1,7 +1,8 @@
 local usersInactiveKey = KEYS[1]
 local exTime = ARGV[1]
-
--- #var defs
+---
+-- var defs
+---
 local delimiter = '{{ KEY_SEPARATOR }}'
 local keyPrefix = '{{ keyPrefix }}'
 
@@ -11,6 +12,7 @@ local ssoProviders = {
     {{/each}}
 };
 
+-- key templates
 local usersDataKeyTemplate = '{{ keyTemplates.USERS_DATA }}'
 local usersMetaKeyTemplate = '{{ keyTemplates.USERS_METADATA }}'
 local usersTokenKeyTemplate = '{{ keyTemplates.USERS_TOKENS }}'
@@ -21,23 +23,23 @@ local organizationsMembersKeyTemplate = '{{ keyTemplates.ORGANIZATIONS_MEMBERS }
 local organizationsMemberKeyTemplate = '{{ keyTemplates.ORGANIZATIONS_MEMBER }}'
 local organizationMemberTemplate = '{{ templates.ORGANIZATIONS_MEMBER }}'
 
+-- simple keys
 local usersAliasToIDKey = '{{ keys.USERS_ALIAS_TO_ID }}'
 local usersUsernameToIDKey = '{{ keys.USERS_USERNAME_TO_ID }}'
 local usersSSOToIDKey = '{{ keys.USERS_SSO_TO_ID }}'
 
+-- indexes
 local organizationsInvitationIndex = '{{ keys.ORGANIZATIONS_INVITATIONS_INDEX }}'
-
 local usersPublicIndex = '{{ keys.USERS_PUBLIC_INDEX }}'
 local usersIndex = '{{ keys.USERS_INDEX }}'
 
-local usersThrottleKeyPrefix = '{{ keys.THROTTLE_PREFIX }}'
-
+-- fields
 local usersUsernameField = '{{ fields.USERS_USERNAME_FIELD }}'
 local usersAliasField = '{{ fields.USERS_ALIAS_FIELD }}'
--- /var defs
+
 
 ---
---- Helper functions
+-- Helper functions
 ---
 local function isempty(s)
   return s == nil or s == '' or s == false;
@@ -52,11 +54,12 @@ local function decode(strval)
   end
 end
 
--- key generators
+-- key generator
 local function key(...)
   return table.concat(arg, delimiter)
 end
 
+-- key from template generator
 local function makeKey(template, templateValues)
   local str = template
   for param, value in pairs(templateValues) do
@@ -65,14 +68,14 @@ local function makeKey(template, templateValues)
   return str
 end
 
-
+-- gets user data
 local function getData(key)
   local fields = { usersUsernameField, usersAliasField, unpack(ssoProviders) }
   local data = redis.call("HMGET", key, unpack(fields))
 
   if #data > 0 then
     local result = {};
-    --convert into table
+    --convert to table
     for i = 1, #data, 1 do
       result[fields[i]] = data[i]
     end
@@ -82,6 +85,11 @@ local function getData(key)
   return nil
 end
 
+---
+-- Script logic functions
+---
+
+-- deletes organization bindings
 local function deleteOrganizationMember(username)
   local userOrganizationsKey = makeKey(usersOrganizationsKeyTemplate, { username = username })
   local organizationIds = redis.call("HKEYS", userOrganizationsKey)
@@ -100,21 +108,28 @@ local function deleteOrganizationMember(username)
 
 end
 
+-- handles emails (usernames)
+local inactiveUserNames = {}
+
 -- delete logic
 local function deleteUser(userID, userData)
   local alias = userData[usersAliasField]
   local username = userData[usersUsernameField]
 
+  -- save username
+  table.insert(inactiveUserNames, username)
+
+  -- delete alias
   if isempty(alias) == false then
     redis.call("HDEL", usersAliasToIDKey, alias, string.lower(alias))
   end
 
-  --almost impossible but
-  if isempty(username) == false then
-    redis.call("HDEL", usersUsernameToIDKey, username)
-    deleteOrganizationMember(username)
-  end
+  redis.call("HDEL", usersUsernameToIDKey, username)
+
+  -- if user assigned to organization
+  deleteOrganizationMember(username)
 
+  -- delete SSO data
   for k, provider in pairs(ssoProviders) do
     local rawData = userData[provider]
     local providerData = decode(rawData)
@@ -131,7 +146,7 @@ local function deleteUser(userID, userData)
   local userDataKey = makeKey(usersDataKeyTemplate, { id = userID })
   redis.call("DEL", userDataKey)
 
-  -- delete meta
+  -- delete meta data
   local usersAudienceKey = makeKey(usersAudienceKeyTemplate, { id = userID })
   local userAudiences = redis.call("SMEMBERS", usersAudienceKey)
 
@@ -144,16 +159,15 @@ local function deleteUser(userID, userData)
   local userTokensKey = makeKey(usersTokenKeyTemplate, { id = userID })
   redis.call("DEL", userTokensKey)
 
+  -- delete user audiences list
   redis.call("DEL", usersAudienceKey)
 
-  -- throttle constants
-  {{#each throttle_actions}}
-    redis.call("DEL", key(userThrottleKeyPrefix, {{this}}, userID))
-  {{/each}}
-
 end
 
--- Command body
+---
+-- Script Logic
+---
+
 redis.replicate_commands();
 
 local inactiveUsers = redis.call("ZRANGEBYSCORE", usersInactiveKey, '-inf', exTime)
@@ -171,5 +185,6 @@ for _ , userID in pairs(inactiveUsers) do
   redis.call('ZREM', usersInactiveKey, userID)
 end
 
--- returns count of deleted users
-return #inactiveUsers
+-- returns emails of deleted users
+-- helps to remove TokenManager tokens
+return inactiveUserNames

src/utils/inactiveUsers/defineCommand.js

@@ -2,7 +2,6 @@ const Promise = require('bluebird');
 const hbs = require('handlebars');
 const path = require('path');
 const fs = require('fs');
-const ld = require('lodash');
 
 const key = require('../key');
 const {
@@ -19,29 +18,24 @@ const {
   USERS_USERNAME_TO_ID,
   USERS_USERNAME_FIELD,
   SSO_PROVIDERS,
-  THROTTLE_PREFIX,
-  USERS_ACTION_ACTIVATE,
-  USERS_ACTION_REGISTER,
-  USERS_ACTION_PASSWORD,
-  USERS_ACTION_RESET,
-
   ORGANIZATIONS_MEMBERS,
   ORGANIZATIONS_INVITATIONS_INDEX,
 } = require('../../constants');
 
 const templateName = 'deleteInactivatedUsers.lua.hbs';
 const KEY_SEPARATOR = '!';
 
+// keys used in script
 const keys = {
   USERS_ALIAS_TO_ID,
   USERS_SSO_TO_ID,
   USERS_USERNAME_TO_ID,
   USERS_INDEX,
   USERS_PUBLIC_INDEX,
-  THROTTLE_PREFIX,
   ORGANIZATIONS_INVITATIONS_INDEX,
 };
 
+// key templates used in script
 const keyTemplates = {
   USERS_DATA: key('{id}', USERS_DATA),
   USERS_METADATA: key('{id}', USERS_METADATA, '{audience}'),
@@ -52,31 +46,28 @@ const keyTemplates = {
   ORGANIZATIONS_MEMBER: key('{orgid}', ORGANIZATIONS_MEMBERS, '{username}'),
 };
 
+// data template, organization stores members without prefix
 const templates = {
   ORGANIZATIONS_MEMBER: key('{orgid}', ORGANIZATIONS_MEMBERS, '{username}'),
 };
 
+// fields used in script
 const fields = {
   USERS_ALIAS_FIELD,
   USERS_USERNAME_FIELD,
 };
-const throttleActions = [
-  USERS_ACTION_ACTIVATE,
-  USERS_ACTION_PASSWORD,
-  USERS_ACTION_REGISTER,
-  USERS_ACTION_RESET,
-];
-
 
 const readFile = f => Promise.fromCallback((cb) => {
   return fs.readFile(f, 'utf-8', cb);
 });
 
 const prefixify = (prefix, obj) => {
   if (prefix !== '') {
-    return ld.mapValues(obj, (value) => {
-      return `${prefix}${value}`;
-    });
+    const objEntries = Object.entries(obj);
+
+    for (const [prop, value] of objEntries) {
+      obj[prop] = `${prefix}${value}`;
+    }
   }
   return obj;
 };
@@ -109,7 +100,6 @@ function templateContext(redisOptions) {
   return {
     KEY_SEPARATOR,
     fields,
-    throttleActions,
     templates,
     keys: prefixify(keyPrefix, keys),
     keyTemplates: prefixify(keyPrefix, keyTemplates),

src/utils/inactiveUsers/index.js

@@ -1,5 +1,14 @@
-const { USERS_INACTIVATED } = require('../../constants');
+const Promise = require('bluebird');
 
+const {
+  USERS_INACTIVATED,
+  USERS_ACTION_ACTIVATE,
+  USERS_ACTION_REGISTER,
+  USERS_ACTION_PASSWORD,
+  USERS_ACTION_RESET,
+  USERS_ACTION_ORGANIZATION_INVITE,
+  USERS_ACTION_ORGANIZATION_REGISTER,
+} = require('../../constants');
 const deleteInactiveUsers = require('./delete');
 const defineCommand = require('./defineCommand');
 
@@ -22,18 +31,44 @@ function removeFromInactiveUsers(redis, userId) {
   redis.zrem(USERS_INACTIVATED, userId);
 }
 
+/**
+ * Deletes invites and action tokens for provided user list
+ * @param userIds
+ * @returns {Promise<[*]>}
+ */
+function cleanTokens(userIds) {
+  const actions = [
+    USERS_ACTION_ACTIVATE, USERS_ACTION_REGISTER,
+    USERS_ACTION_PASSWORD, USERS_ACTION_RESET,
+    USERS_ACTION_ORGANIZATION_INVITE, USERS_ACTION_ORGANIZATION_REGISTER,
+  ];
+
+  const work = userIds.reduce((prev, id) => {
+    const delTokenActions = [];
+    for (const action of actions) {
+      delTokenActions.push(
+        this.tokenManager.remove({ id, action })
+      );
+    }
+    return [...prev, ...delTokenActions];
+  }, []);
+
+  return Promise.all(work);
+}
+
 /**
  * Deletes users that didn't pass activation
  * @param suppressError boolean throw or suppress error
- * @returns {Promise<number>}
+ * @returns {Promise<[]>}
  */
 async function cleanUsers(suppressError = true) {
   const { redis } = this;
   const { deleteInactiveAccounts } = this.config;
-  let deletedUsers = 0;
+  let deletedUsers = [];
 
   try {
     deletedUsers = await deleteInactiveUsers(redis, deleteInactiveAccounts);
+    await cleanTokens.call(this, deletedUsers);
   } catch (e) {
     if (suppressError) {
       this.log.error({ error: e }, e.message);

test/suites/deleteInactivatedUsers.js

@@ -39,9 +39,11 @@ describe('#inactive user', function registerSuite() {
 
   beforeEach(async function pretest() {
     this.users.config.deleteInactiveAccounts = 1;
+    const dispatcher = simpleDispatcher(this.users.router);
+
     await createOrganization.call(this);
-    await simpleDispatcher(this.users.router)('users.register', { ...regUser });
-    return simpleDispatcher(this.users.router)('users.register', { ...regUserNoAlias });
+    await dispatcher('users.register', { ...regUser });
+    return dispatcher('users.register', { ...regUserNoAlias });
   });
 
   describe('throw suppress error', function test() {
@@ -77,7 +79,7 @@ describe('#inactive user', function registerSuite() {
     return delay(() => {
       return cleanUsers.call(this.users)
         .then(async (res) => {
-          expect(res).to.be.eq(2);
+          expect(res.length).to.be.eq(2);
 
           const { username } = regUser;
           const dispatcher = simpleDispatcher(this.users.router);