feat: delete inactive users

* rework on new MetaData

rfcs/inactive_users/user_cleanup.md

@@ -0,0 +1,35 @@
+# 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` [Described here](#inactive-users-index) 
+  - `user-audiences` [Described here](user_and_organization_meta_update.md)
+
+## `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 process
+When the user succeeds registration but activation not requested, the new entry added to `inactive-users`.
+
+#### Activation process
+When the user succeeds activation the entry deleted from `inactive-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` to be sure that only one process runs.
+* Gets outdated user list and deletes them.
+
+## 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.
+
+* Should We Delete Organization Members?
+
+

src/actions/activate.js

@@ -5,8 +5,12 @@ const jwt = require('../utils/jwt.js');
 const { getInternalData } = require('../utils/userData');
 const getMetadata = require('../utils/get-metadata');
 const handlePipeline = require('../utils/pipeline-error.js');
+const InactiveUser = require('../utils/inactive-user/inactive-user');
+const User = require('../utils/user/user');
+
 const {
   USERS_INDEX,
+  USERS_INACTIVATED,
   USERS_DATA,
   USERS_REFERRAL_INDEX,
   USERS_PUBLIC_INDEX,
@@ -22,7 +26,7 @@ const {
 const Forbidden = new Errors.HttpStatusError(403, 'invalid token');
 const Inactive = new Errors.HttpStatusError(412, 'expired token, please request a new email');
 const Active = new Errors.HttpStatusError(409, 'account is already active, please use sign in form');
-
+const NotFound = new Errors.HttpStatusError(404, 'account not found');
 /**
  * Helper to determine if something is true
  */
@@ -126,6 +130,9 @@ function activateAccount(data, metadata) {
     .persist(userKey)
     .sadd(USERS_INDEX, userId);
 
+  /* delete user id from the inactive users index */
+  pipeline.zrem(USERS_INACTIVATED, userId);
+
   if (alias) {
     pipeline.sadd(USERS_PUBLIC_INDEX, userId);
   }
@@ -152,6 +159,13 @@ function hook(userId) {
   return this.service.hook('users:activate', userId, { audience: this.audience });
 }
 
+async function checkUserDeleting(internalData) {
+  const user = new User(this);
+  if (await user.isUserDeleting(internalData.id)) {
+    throw NotFound;
+  }
+}
+
 /**
  * @api {amqp} <prefix>.activate Activate User
  * @apiVersion 1.0.0
@@ -173,7 +187,7 @@ function hook(userId) {
  * @apiParam (Payload) {String} [audience] - additional metadata will be pushed there from custom hooks
  *
  */
-function activateAction({ params }) {
+async function activateAction({ params }) {
   // TODO: add security logs
   // var remoteip = request.params.remoteip;
   const { token, username } = params;
@@ -191,11 +205,15 @@ function activateAction({ params }) {
     erase: config.token.erase,
   };
 
+  const inactiveUsers = new InactiveUser(this);
+  await inactiveUsers.deleteInactive(config.deleteInactiveAccounts);
+
   return Promise
     .bind(context)
     .then(verifyRequest)
     .bind(this)
     .then((resolvedUsername) => getInternalData.call(this, resolvedUsername))
+    .tap(checkUserDeleting)
     .then((internalData) => Promise.join(
       internalData,
       getMetadata.call(this, internalData[USERS_ID_FIELD], audience).get(audience)

src/actions/register.js

@@ -22,6 +22,8 @@ const checkLimits = require('../utils/check-ip-limits');
 const challenge = require('../utils/challenges/challenge');
 const handlePipeline = require('../utils/pipeline-error');
 const hashPassword = require('../utils/register/password/hash');
+const InactiveUser = require('../utils/inactive-user/inactive-user');
+
 const {
   USERS_REF,
   USERS_INDEX,
@@ -172,6 +174,9 @@ async function performRegistration({ service, params }) {
     await verifySSO(service, params);
   }
 
+  const inactiveUsers = new InactiveUser(service);
+  await inactiveUsers.deleteInactive(config.deleteInactiveAccounts);
+
   const [creatorAudience] = audience;
   const defaultAudience = last(audience);
   const userId = service.flake.next();
@@ -208,7 +213,7 @@ async function performRegistration({ service, params }) {
   pipeline.hset(USERS_USERNAME_TO_ID, username, userId);
 
   if (activate === false && config.deleteInactiveAccounts >= 0) {
-    pipeline.expire(userDataKey, config.deleteInactiveAccounts);
+    inactiveUsers.add(userId, created, pipeline);
   }
 
   await pipeline.exec().then(handlePipeline);

src/actions/remove.js

@@ -2,31 +2,14 @@ const { ActionTransport } = require('@microfleet/core');
 const Promise = require('bluebird');
 const Errors = require('common-errors');
 const intersection = require('lodash/intersection');
-const get = require('../utils/get-value');
-const key = require('../utils/key');
 const { getInternalData } = require('../utils/userData');
 const getMetadata = require('../utils/get-metadata');
-const handlePipeline = require('../utils/pipeline-error');
-const UserMetadata = require('../utils/metadata/user');
+const User = require('../utils/user/user');
+const InactiveUser = require('../utils/inactive-user/inactive-user');
 const {
-  USERS_INDEX,
-  USERS_PUBLIC_INDEX,
-  USERS_ALIAS_TO_ID,
-  USERS_SSO_TO_ID,
-  USERS_USERNAME_TO_ID,
-  USERS_USERNAME_FIELD,
-  USERS_DATA,
-  USERS_TOKENS,
   USERS_ID_FIELD,
-  USERS_ALIAS_FIELD,
   USERS_ADMIN_ROLE,
   USERS_SUPER_ADMIN_ROLE,
-  USERS_ACTION_ACTIVATE,
-  USERS_ACTION_RESET,
-  USERS_ACTION_PASSWORD,
-  USERS_ACTION_REGISTER,
-  THROTTLE_PREFIX,
-  SSO_PROVIDERS,
 } = require('../constants');
 
 // intersection of priority users
@@ -68,55 +51,12 @@ async function removeUser({ params }) {
     throw new Errors.HttpStatusError(400, 'can\'t remove admin user from the system');
   }
 
-  const transaction = redis.pipeline();
-  const alias = internal[USERS_ALIAS_FIELD];
-  const userId = internal[USERS_ID_FIELD];
-  const resolvedUsername = internal[USERS_USERNAME_FIELD];
-  const metaAudiences = await UserMetadata.using(userId, null, redis).getAudience();
-  const userMetadata = UserMetadata.using(userId, null, transaction);
+  const user = new User(this);
+  const inactiveUser = new InactiveUser(this);
 
-  if (alias) {
-    transaction.hdel(USERS_ALIAS_TO_ID, alias.toLowerCase(), alias);
-  }
-
-  transaction.hdel(USERS_USERNAME_TO_ID, resolvedUsername);
-
-  // remove refs to SSO account
-  for (const provider of SSO_PROVIDERS) {
-    const uid = get(internal, `${provider}.uid`, { default: false });
-
-    if (uid) {
-      transaction.hdel(USERS_SSO_TO_ID, uid);
-    }
-  }
-
-  // clean indices
-  transaction.srem(USERS_PUBLIC_INDEX, userId);
-  transaction.srem(USERS_INDEX, userId);
-
-  // remove metadata & internal data
-  transaction.del(key(userId, USERS_DATA));
-  for (const metaAudience of metaAudiences) {
-    userMetadata.deleteMetadata(metaAudience);
-  }
-
-  // remove auth tokens
-  transaction.del(key(userId, USERS_TOKENS));
-
-  // remove throttling on actions
-  transaction.del(key(THROTTLE_PREFIX, USERS_ACTION_ACTIVATE, userId));
-  transaction.del(key(THROTTLE_PREFIX, USERS_ACTION_PASSWORD, userId));
-  transaction.del(key(THROTTLE_PREFIX, USERS_ACTION_REGISTER, userId));
-  transaction.del(key(THROTTLE_PREFIX, USERS_ACTION_RESET, userId));
-
-  // complete it
-  const removeResult = await transaction
-    .exec()
-    .then(handlePipeline);
-
-  // clear cache
-  const now = Date.now();
-  await Promise.all([redis.fsortBust(USERS_INDEX, now), redis.fsortBust(USERS_PUBLIC_INDEX, now)]);
+  await inactiveUser.delete(internal.id);
+  const removeResult = await user.delete(internal);
+  await user.flushCaches();
 
   return removeResult;
 }

src/constants.js

@@ -6,6 +6,7 @@ module.exports = exports = {
   USERS_PUBLIC_INDEX: 'users-public',
   USERS_REFERRAL_INDEX: 'users-referral',
   ORGANIZATIONS_INDEX: 'organization-iterator-set',
+
   // id mapping
   USERS_ALIAS_TO_ID: 'users-alias',
   USERS_SSO_TO_ID: 'users-sso-hash',

src/utils/inactive-user/inactive-user.js

@@ -0,0 +1,52 @@
+const Promise = require('bluebird');
+const RedisInactiveUser = require('./redis/inactive-user');
+const User = require('../user/user');
+
+class InactiveUser {
+  constructor(service) {
+    this.service = service;
+    this.backend = new RedisInactiveUser(service.redis);
+  }
+
+  acquireLock(lockName) {
+    const { dlock } = this.service;
+    return dlock
+      .once(lockName)
+      .disposer((l) => {
+        l.release().reflect();
+      });
+  }
+
+  add(userId, created, pipeline = undefined) {
+    return this.backend.add(userId, created, pipeline);
+  }
+
+  delete(userId) {
+    return this.backend.delete(userId);
+  }
+
+  deleteInactive(userTTL) {
+    return Promise
+      .using(this.acquireLock('delete-inactive-users'), () => this._deleteInactive(userTTL))
+      .catch({ name: 'LockAcquisitionError' }, () => {});
+  }
+
+  async _deleteInactive(userTTL) {
+    const user = new User(this.service);
+    const inactiveUsers = await this.backend.get(userTTL);
+    const work = [];
+
+    for (const userId of inactiveUsers) {
+      work.push(
+        user
+          .delete(userId)
+          .then(() => this.backend.delete(userId))
+      );
+    }
+    await Promise.all(work);
+
+    return inactiveUsers.length;
+  }
+}
+
+module.exports = InactiveUser;

src/utils/inactive-user/redis/inactive-user.js

@@ -0,0 +1,22 @@
+class InactiveUser {
+  static REDIS_KEY = 'users-inactivated';
+
+  constructor(redis) {
+    this.redis = redis;
+  }
+
+  get(interval) {
+    const expire = Date.now() - (interval * 1000);
+    return this.redis.zrangebyscore(InactiveUser.REDIS_KEY, '-inf', expire);
+  }
+
+  add(userId, createTime, redis = this.redis) {
+    return redis.zadd(InactiveUser.REDIS_KEY, createTime, userId);
+  }
+
+  delete(userId, redis = this.redis) {
+    return redis.zrem(InactiveUser.REDIS_KEY, userId);
+  }
+}
+
+module.exports = InactiveUser;

src/utils/user/redis/user.js

@@ -0,0 +1,54 @@
+const Promise = require('bluebird');
+const getInternalData = require('../../userData/get-internal-data');
+
+const {
+  USERS_ALIAS_TO_ID,
+  USERS_SSO_TO_ID,
+  USERS_USERNAME_TO_ID,
+  USERS_INDEX,
+  USERS_PUBLIC_INDEX,
+  USERS_DATA,
+  USERS_TOKENS,
+} = require('../../../constants');
+
+class User {
+  constructor(redis) {
+    this.redis = redis;
+  }
+
+  static getUserDataKey(userId) {
+    return `${userId}!${USERS_DATA}`;
+  }
+
+  flushCaches() {
+    const now = Date.now();
+    return Promise.all([
+      this.redis.fsortBust(USERS_INDEX, now),
+      this.redis.fsortBust(USERS_PUBLIC_INDEX, now),
+    ]);
+  }
+
+  getData(userId) {
+    return getInternalData.call({ redis: this.redis }, userId);
+  }
+
+  delete({ id, username, alias, ssoIds }, redis = this.redis) {
+    const scriptKeys = [
+      USERS_ALIAS_TO_ID, USERS_USERNAME_TO_ID, USERS_SSO_TO_ID,
+      USERS_PUBLIC_INDEX, USERS_INDEX, USERS_TOKENS,
+      User.getUserDataKey(id),
+    ];
+    const luaScript = `
+      ${alias === undefined ? '' : `redis.call("HDEL", KEYS[1], '${alias}', '${alias.toLowerCase()}')`}
+      redis.call("HDEL", KEYS[2], '${username}')
+      ${ssoIds.length > 0 ? `redis.call("HDEL", KEYS[3], ${ssoIds.map((uid) => `'${uid}'`).join(',')})` : ''}
+      redis.call("SREM", KEYS[4], '${id}')
+      redis.call("SREM", KEYS[5], '${id}')
+      redis.call("HDEL", KEYS[6], '${id}')
+      redis.call("DEL", KEYS[7])
+    `;
+    return redis.eval(luaScript, scriptKeys.length, ...scriptKeys);
+  }
+}
+
+module.exports = User;