[WIP] delinactive user cleanup and index

.mdeprc

@@ -1,4 +1,4 @@
 {
-  "node": "10.16.3",
-  "rebuild": ["ms-flakeless", "scrypt"]
+  "node": "12.13.0",
+  "rebuild": ["ms-flakeless"]
 }

CHANGELOG.md

@@ -1,3 +1,15 @@
+# [12.0.0](https://github.com/makeomatic/ms-users/compare/v11.4.0...v12.0.0) (2019-10-31)
+
+
+### Features
+
+* upgrade deps for node 12.13.0 ([#432](https://github.com/makeomatic/ms-users/issues/432)) ([d092478](https://github.com/makeomatic/ms-users/commit/d0924786a9274d655f19393033af26c369bf4115))
+
+
+### BREAKING CHANGES
+
+* removes scrypt for built-in crypto implementation, while API remains the same the underlaying library is slightly different. Node has changed a major version, too
+
 # [11.4.0](https://github.com/makeomatic/ms-users/compare/v11.3.1...v11.4.0) (2019-10-03)
 
 

package.json

@@ -2,7 +2,7 @@
   "name": "ms-users",
   "description": "Core of the microservice for handling users",
   "main": "./lib/index.js",
-  "version": "11.4.0",
+  "version": "12.0.0",
   "scripts": {
     "compile": "rimraf ./lib; babel -d ./lib --copy-files ./src",
     "pretest": "yarn compile",
@@ -31,27 +31,27 @@
     "@hapi/bell": "^11.1.0",
     "@hapi/hapi": "^18.4.0",
     "@hapi/vision": "^5.5.4",
-    "@microfleet/core": "^15.0.1",
+    "@microfleet/core": "^15.3.1",
     "@microfleet/transport-amqp": "^15.0.0",
     "@microfleet/validation": "^8.1.2",
-    "bluebird": "^3.7.0",
+    "bluebird": "^3.7.1",
     "bytes": "^3.0.0",
     "common-errors": "^1.0.5",
     "csv-write-stream": "^2.0.0",
-    "disposable-email-domains": "^1.0.48",
-    "dlock": "^10.0.0",
+    "disposable-email-domains": "^1.0.49",
+    "dlock": "^11.0.0",
     "flake-idgen": "^1.1.0",
     "get-stdin": "^7.0.0",
     "get-value": "^3.0.1",
-    "handlebars": "^4.4.2",
+    "handlebars": "^4.5.1",
     "ioredis": "^4.14.1",
     "is": "^3.3.0",
     "jsonwebtoken": "^8.5.1",
     "jwa": "^1.4.1",
     "lodash": "^4.17.15",
     "moment": "^2.23.0",
     "ms-conf": "^5.0.2",
-    "ms-flakeless": "^4.2.0",
+    "ms-flakeless": "^4.3.0",
     "ms-mailer-client": "^8.0.1",
     "ms-mailer-templates": "^1.14.1",
     "ms-token": "^3.1.0",
@@ -62,26 +62,26 @@
     "redis-filtered-sort": "^2.3.0",
     "request": "^2.88.0",
     "request-promise": "^4.2.4",
-    "scrypt": "^6.0.1",
+    "scrypt-kdf": "^2.0.1",
     "serialize-error": "^5.0.0",
     "serialize-javascript": "^2.1.0",
     "stdout-stream": "^1.4.1",
     "tough-cookie": "^3.0.0",
     "uuid": "^3.3.3",
-    "yargs": "^14.0.0",
+    "yargs": "^14.2.0",
     "zxcvbn": "^4.4.2"
   },
   "devDependencies": {
-    "@babel/cli": "^7.6.2",
-    "@babel/core": "^7.6.2",
+    "@babel/cli": "^7.6.4",
+    "@babel/core": "^7.6.4",
     "@babel/plugin-proposal-class-properties": "^7.5.5",
     "@babel/plugin-proposal-object-rest-spread": "^7.6.2",
     "@babel/plugin-transform-strict-mode": "^7.2.0",
     "@babel/register": "^7.6.2",
-    "@makeomatic/deploy": "^9.1.1",
-    "@semantic-release/changelog": "^3.0.4",
-    "@semantic-release/exec": "^3.3.7",
-    "@semantic-release/git": "^7.0.16",
+    "@makeomatic/deploy": "^9.3.2",
+    "@semantic-release/changelog": "^3.0.5",
+    "@semantic-release/exec": "^3.3.8",
+    "@semantic-release/git": "^7.0.17",
     "apidoc": "^0.17.7",
     "apidoc-plugin-schema": "^0.1.8",
     "babel-eslint": "^10.0.3",
@@ -90,18 +90,19 @@
     "cheerio": "^1.0.0-rc.3",
     "codecov": "^3.6.1",
     "cross-env": "^6.0.3",
-    "eslint": "^6.5.1",
-    "eslint-config-makeomatic": "^3.1.0",
+    "eslint": "^6.6.0",
+    "eslint-config-makeomatic": "^4.0.0",
     "eslint-plugin-import": "^2.18.2",
-    "eslint-plugin-mocha": "^6.1.1",
+    "eslint-plugin-mocha": "^6.2.1",
     "eslint-plugin-promise": "^4.2.1",
+    "eslint-plugin-unicorn": "^12.1.0",
     "faker": "^4.1.0",
-    "glob": "^7.1.4",
+    "glob": "^7.1.5",
     "json": "^9.0.6",
     "md5": "^2.2.1",
-    "mocha": "^6.2.1",
+    "mocha": "^6.2.2",
     "nyc": "^14.1.1",
-    "puppeteer": "1.20.0",
+    "puppeteer": "2.0.0",
     "rimraf": "^3.0.0",
     "sinon": "^7.5.0"
   },

rfcs/inactive_users/user_and_organization_meta_update.md

@@ -0,0 +1,24 @@
+# User/Organization metadata update
+## Overview and Motivation
+When user or organization metadata is updated, the Service should track audiences with assigned metadata.
+For each assigned meta hash always exists a single `audience`, but there is no list of `audiences` assigned to the user or organization.
+
+To achieve this ability, I advise these updates: 
+
+## Audience lists
+Audiences stored in sets with names created from `USERS_AUDIENCE` or `ORGANISATION_AUDIENCE` constants and `Id`
+(e.g.: `{ms-users}10110110111!audiences`). Both keys contain `audience` names that are currently have assigned values.
+
+The `audience` list will be updated on each update of the metadata.
+
+## Metadata Handling classes
+Service logic is updated to use 2 specific classes that will perform all CRUD operations on User or Organization metadata.
+
+* Classes located in: `utils/metadata/{user|organization}.js`.
+* Both classes use same [Redis backend](#redis-metadata-backend-class).
+
+## Redis Metadata Backend class
+The class performs all work on metadata using Redis DB as a backend.
+
+## Notice
+* All User or Organization metadata operations should be performed using Provided classes otherwise, audiences won't be tracked.

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/.eslintrc

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "unicorn/filename-case": 0
+  }
+}

src/actions/activate.js

@@ -3,10 +3,14 @@ const Promise = require('bluebird');
 const redisKey = require('../utils/key.js');
 const jwt = require('../utils/jwt.js');
 const { getInternalData } = require('../utils/userData');
-const getMetadata = require('../utils/getMetadata');
-const handlePipeline = require('../utils/pipelineError.js');
+const getMetadata = require('../utils/get-metadata');
+const handlePipeline = require('../utils/pipeline-error.js');
+const InactiveUser = require('../utils/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.cleanUsersOnce(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/alias.js

@@ -2,13 +2,14 @@ const Promise = require('bluebird');
 const Errors = require('common-errors');
 const { ActionTransport } = require('@microfleet/core');
 const { getInternalData } = require('../utils/userData');
-const isActive = require('../utils/isActive');
-const isBanned = require('../utils/isBanned');
+const isActive = require('../utils/is-active');
+const isBanned = require('../utils/is-banned');
 const key = require('../utils/key');
-const handlePipeline = require('../utils/pipelineError');
+const handlePipeline = require('../utils/pipeline-error');
+const UserMetadata = require('../utils/metadata/user');
+
 const {
   USERS_DATA,
-  USERS_METADATA,
   USERS_ALIAS_TO_ID,
   USERS_ID_FIELD,
   USERS_ALIAS_FIELD,
@@ -69,10 +70,12 @@ async function assignAlias({ params }) {
       return Promise.reject(err);
     }
 
-    const pipeline = redis.pipeline([
-      ['hset', key(userId, USERS_DATA), USERS_ALIAS_FIELD, alias],
-      ['hset', key(userId, USERS_METADATA, defaultAudience), USERS_ALIAS_FIELD, JSON.stringify(alias)],
-    ]);
+    const pipeline = redis.pipeline();
+
+    pipeline.hset(key(userId, USERS_DATA), USERS_ALIAS_FIELD, alias);
+    UserMetadata
+      .using(userId, defaultAudience, pipeline)
+      .update(USERS_ALIAS_FIELD, JSON.stringify(alias));
 
     if (activeUser) {
       pipeline.sadd(USERS_PUBLIC_INDEX, username);

src/actions/ban.js

@@ -3,10 +3,11 @@ const { ActionTransport } = require('@microfleet/core');
 const mapValues = require('lodash/mapValues');
 const redisKey = require('../utils/key.js');
 const { getInternalData } = require('../utils/userData');
-const handlePipeline = require('../utils/pipelineError.js');
+const handlePipeline = require('../utils/pipeline-error');
+const UserMetadata = require('../utils/metadata/user');
+
 const {
-  USERS_DATA, USERS_METADATA,
-  USERS_BANNED_FLAG, USERS_TOKENS, USERS_BANNED_DATA,
+  USERS_DATA, USERS_BANNED_FLAG, USERS_TOKENS, USERS_BANNED_DATA,
 } = require('../constants.js');
 
 // helper
@@ -25,26 +26,32 @@ function lockUser({
       remoteip: remoteip || '',
     },
   };
+  const pipeline = redis.pipeline();
+
+  pipeline.hset(redisKey(id, USERS_DATA), USERS_BANNED_FLAG, 'true');
+  // set .banned on metadata for filtering & sorting users by that field
+  UserMetadata
+    .using(id, defaultAudience, pipeline)
+    .updateMulti(mapValues(data, stringify));
+  pipeline.del(redisKey(id, USERS_TOKENS));
 
-  return redis
-    .pipeline()
-    .hset(redisKey(id, USERS_DATA), USERS_BANNED_FLAG, 'true')
-    // set .banned on metadata for filtering & sorting users by that field
-    .hmset(redisKey(id, USERS_METADATA, defaultAudience), mapValues(data, stringify))
-    .del(redisKey(id, USERS_TOKENS))
-    .exec();
+  return pipeline.exec();
 }
 
 function unlockUser({ id }) {
   const { redis, config } = this;
   const { jwt: { defaultAudience } } = config;
+  const pipeline = redis.pipeline();
 
-  return redis
-    .pipeline()
-    .hdel(redisKey(id, USERS_DATA), USERS_BANNED_FLAG)
-    // remove .banned on metadata for filtering & sorting users by that field
-    .hdel(redisKey(id, USERS_METADATA, defaultAudience), 'banned', USERS_BANNED_DATA)
-    .exec();
+  pipeline.hdel(redisKey(id, USERS_DATA), USERS_BANNED_FLAG);
+  // remove .banned on metadata for filtering & sorting users by that field
+  UserMetadata
+    .using(id, defaultAudience, pipeline)
+    .delete([
+      'banned',
+      USERS_BANNED_DATA,
+    ]);
+  return pipeline.exec();
 }
 
 /**

src/actions/challenge.js

@@ -2,8 +2,8 @@ const Promise = require('bluebird');
 const passThrough = require('lodash/identity');
 const { ActionTransport } = require('@microfleet/core');
 const { getInternalData } = require('../utils/userData');
-const getMetadata = require('../utils/getMetadata');
-const isActive = require('../utils/isActive');
+const getMetadata = require('../utils/get-metadata');
+const isActive = require('../utils/is-active');
 const challenge = require('../utils/challenges/challenge');
 const {
   USERS_ACTION_ACTIVATE,

src/actions/disposable-password.js

@@ -2,9 +2,9 @@ const Promise = require('bluebird');
 const { ActionTransport } = require('@microfleet/core');
 const challenge = require('../utils/challenges/challenge');
 const { getInternalData } = require('../utils/userData');
-const isActive = require('../utils/isActive');
-const isBanned = require('../utils/isBanned');
-const hasNotPassword = require('../utils/hasNotPassword');
+const isActive = require('../utils/is-active');
+const isBanned = require('../utils/is-banned');
+const hasNotPassword = require('../utils/has-no-password');
 const { USERS_ACTION_DISPOSABLE_PASSWORD, USERS_USERNAME_FIELD } = require('../constants');
 
 /**

src/actions/getMetadata.js

@@ -4,7 +4,7 @@ const Errors = require('common-errors');
 const noop = require('lodash/noop');
 const identity = require('lodash/identity');
 const get = require('../utils/get-value');
-const getMetadata = require('../utils/getMetadata');
+const getMetadata = require('../utils/get-metadata');
 const { getUserId } = require('../utils/userData');
 const { USERS_ALIAS_FIELD } = require('../constants');