feat: delete inactive users and index

Author: pajgo

rfcs/inactive_users/user_cleanup.md

@@ -0,0 +1,80 @@
+# Inactive user cleanup
+
+## Overview and motivation
+For users who didn't pass the activation process, the service using TTL keyspace cleanup and only `Internal Data` deleted.
+A lot of linked keys remain in the database, and this leads to keyspace pollution.
+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.
+
+## 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.
+
+## 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
+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.
+
+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?
+
+## 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.
+
+*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.
+
+##### 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
+

src/actions/activate.js

@@ -5,7 +5,7 @@ const jwt = require('../utils/jwt.js');
 const { getInternalData } = require('../utils/userData');
 const getMetadata = require('../utils/getMetadata');
 const handlePipeline = require('../utils/pipelineError.js');
-const { cleanInactiveUsersIndex } = require('../utils/inactiveUsers');
+const InactiveUser = require('../utils/inactive-user');
 
 const {
   USERS_INDEX,
@@ -183,7 +183,7 @@ async function activateAction({ params }) {
   // TODO: add security logs
   // var remoteip = request.params.remoteip;
   const { token, username } = params;
-  const { log, config } = this;
+  const { log, config, redis, dlock, tokenManager } = this;
   const audience = params.audience || config.defaultAudience;
 
   log.debug('incoming request params %j', params);
@@ -197,8 +197,8 @@ async function activateAction({ params }) {
     erase: config.token.erase,
   };
 
-  // TODO: REMOVEME: Execute `DeleteInactiveUsers` LUA script
-  await cleanInactiveUsersIndex.call(this);
+  const inactiveUsers = new InactiveUser(redis);
+  await inactiveUsers.cleanInactive(config.deleteInactiveAccounts, { dlock, tokenManager });
 
   return Promise
     .bind(context)

src/actions/register.js

@@ -22,13 +22,12 @@ const checkLimits = require('../utils/checkIpLimits');
 const challenge = require('../utils/challenges/challenge');
 const handlePipeline = require('../utils/pipelineError');
 const hashPassword = require('../utils/register/password/hash');
-const { cleanInactiveUsersIndex } = require('../utils/inactiveUsers');
+const InactiveUser = require('../utils/inactive-user');
 
 const {
   USERS_REF,
   USERS_INDEX,
   USERS_SSO_TO_ID,
-  USERS_INACTIVATED,
   USERS_DATA,
   USERS_USERNAME_TO_ID,
   USERS_ACTIVE_FLAG,
@@ -151,6 +150,8 @@ async function performRegistration({ service, params }) {
   const {
     config,
     redis,
+    dlock,
+    tokenManager,
   } = service;
 
   // do verifications of DB state
@@ -175,8 +176,8 @@ async function performRegistration({ service, params }) {
     await verifySSO(service, params);
   }
 
-  // TODO: REMOVEME: Execute `DeleteInactiveUsers` LUA script
-  await cleanInactiveUsersIndex.call(service);
+  const inactiveUsers = new InactiveUser(redis);
+  await inactiveUsers.cleanInactive(config.deleteInactiveAccounts, { dlock, tokenManager });
 
   const [creatorAudience] = audience;
   const defaultAudience = last(audience);
@@ -214,11 +215,7 @@ async function performRegistration({ service, params }) {
   pipeline.hset(USERS_USERNAME_TO_ID, username, userId);
 
   if (activate === false && config.deleteInactiveAccounts >= 0) {
-    /* TODO Remove this line when last task in group merged */
-    pipeline.expire(userDataKey, config.deleteInactiveAccounts);
-
-    /* Add user id to the inactive users index */
-    redis.zadd(USERS_INACTIVATED, created, userId);
+    InactiveUser.add(pipeline, userId, created);
   }
 
   await pipeline.exec().then(handlePipeline);

src/constants.js

@@ -7,9 +7,6 @@ module.exports = exports = {
   USERS_REFERRAL_INDEX: 'users-referral',
   ORGANIZATIONS_INDEX: 'organization-iterator-set',
 
-  /* inactive user ids set */
-  USERS_INACTIVATED: 'users-inactivated',
-
   // id mapping
   USERS_ALIAS_TO_ID: 'users-alias',
   USERS_SSO_TO_ID: 'users-sso-hash',

src/utils/inactive-user.js

@@ -0,0 +1,79 @@
+const Promise = require('bluebird');
+const UserActions = require('./user/actions');
+const DeleteOrgMemberCommand = require('./organization/redis/command/delete-member-from-organizations');
+
+class InactiveUser {
+  static REDIS_KEY = 'users-inactivated';
+
+  /**
+   * @param {ioredis | pipeline}redis
+   */
+  constructor(redis) {
+    this.redis = redis;
+  }
+
+  async cleanInactive(userTTL, deps) {
+    const { dlock, tokenManager } = deps;
+
+    const lock = await dlock
+      .once('delete-inactive-users')
+      .catch({ name: 'LockAcquisitionError' }, () => { return null; });
+
+    if (lock === null) {
+      return null;
+    }
+
+    const users = await this.get(userTTL);
+    const userActions = new UserActions(this.redis);
+    const userEmails = await userActions.getUserEmail(users);
+    
+    const deleteUserWork = [];
+    for (const userId of users) {
+      deleteUserWork.push(userActions.delete(userId));
+    }
+    await Promise.all(deleteUserWork);
+    await lock.release().reflect();
+
+    const deleteOrgMemberCommand = new DeleteOrgMemberCommand(this.redis);
+    const work = [];
+
+    for (const userEmail of userEmails) {
+      work.push(deleteOrgMemberCommand.execute(userEmail));
+      // ignore errors, sometimes tokens empty
+      work.push(UserActions.deleteUserTokens(userEmail, tokenManager).reflect());
+    }
+    await Promise.all(work);
+
+    return users.length;
+  }
+
+  add(userId, createTime) {
+    return this.redis.zadd(InactiveUser.REDIS_KEY, createTime, userId);
+  }
+
+  get(interval) {
+    const expire = Date.now() - (interval * 1000);
+    return this.redis.zrangebyscore(InactiveUser.REDIS_KEY, '-inf', expire);
+  }
+
+  remove(userId) {
+    return this.redis.zrem(InactiveUser.REDIS_KEY, userId);
+  }
+
+  cleanup(interval) {
+    const expire = Date.now() - (interval * 1000);
+    return this.redis.zremrangebyscore(InactiveUser.REDIS_KEY, '-inf', expire);
+  }
+
+  /* Static methods */
+  static add(redis, userId, createTime) {
+    return new InactiveUser(redis).add(userId, createTime);
+  }
+
+  static remove(redis, userId) {
+    return new InactiveUser(redis).add(userId);
+  }
+}
+
+
+module.exports = InactiveUser;

src/utils/inactiveUsers/index.js

@@ -0,0 +1,84 @@
+const { hasOwnProperty } = Object;
+const key = require('../../../key');
+
+const {
+  ORGANIZATIONS_MEMBERS,
+  USERS_ORGANIZATIONS,
+  ORGANIZATIONS_INVITATIONS_INDEX,
+} = require('../../../../constants');
+
+
+const luaScript = `
+local usersOrganizationsKeyTemplate = KEYS[1]
+local orgMembersKeyTemplate = KEYS[2]
+local orgMemberKeyTemplate = KEYS[3]
+
+local orgMemberTemplate = ARGV[1]
+local username = ARGV[2]
+
+-- indexes
+local organizationsInvitationIndex = '{{ keys.ORGANIZATIONS_INVITATIONS_INDEX }}'
+
+-- key from template generator
+local function makeKey(template, templateValues)
+  local str = template
+  for param, value in pairs(templateValues) do
+    str = str:gsub('{'..param..'}', value, 1)
+  end
+  return str
+end
+
+
+local userOrganizationsKey = makeKey(usersOrganizationsKeyTemplate, { username = username })
+local organizationIds = redis.call("HKEYS", userOrganizationsKey)
+
+redis.call('SREM', organizationsInvitationIndex, username)
+
+for _, orgId in pairs(organizationIds) do
+  local organizationMembersKey = makeKey(orgMembersKeyTemplate, { orgid = orgId})
+  local organizationMemberKey = makeKey(orgMemberKeyTemplate, { orgid = orgId, username = username })
+  local organizationMember = makeKey(orgMemberTemplate, {orgid = orgId, username = username })
+
+  redis.call("DEL", organizationMemberKey)
+  redis.call("HDEL", userOrganizationsKey, orgId)
+  redis.call("ZREM", organizationMembersKey, organizationMember )
+end
+`;
+
+
+class DeleteMemberFromOrganizations {
+  static name = 'deleteMemberFromOrganizations';
+
+  constructor(redis) {
+    this.redis = redis;
+    if (!this.isScriptLoaded()) {
+      this.loadScript();
+    }
+  }
+
+  loadScript() {
+    this.redis.defineCommand(DeleteMemberFromOrganizations.name, { lua: luaScript });
+  }
+
+  isScriptLoaded() {
+    return hasOwnProperty.call(this.redis, DeleteMemberFromOrganizations.name);
+  }
+
+  execute(userId) {
+    const scriptKeys = DeleteMemberFromOrganizations.getKeys();
+    const orgMemberTemplate = key('{orgid}', ORGANIZATIONS_MEMBERS, '{username}');
+
+    return this.redis[DeleteMemberFromOrganizations.name](scriptKeys.length, ...scriptKeys, orgMemberTemplate, userId);
+  }
+
+  static getKeys() {
+    return [
+      key('{username}', USERS_ORGANIZATIONS),
+      key('{orgid}', ORGANIZATIONS_MEMBERS),
+      key('{orgid}', ORGANIZATIONS_MEMBERS, '{username}'),
+      ORGANIZATIONS_INVITATIONS_INDEX,
+    ];
+  }
+}
+
+module.exports = DeleteMemberFromOrganizations;