From ab7664c9ba5fe8afe4c5a430c943ad6713d9cc63 Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 23 Aug 2019 17:59:21 +0600 Subject: [PATCH 1/9] feat: delete inactivated users * rfcs --- rfcs/inactive_users_cleanup.md | 64 +++++++++++++++++++++++++++++ rfcs/update_metadata_lua.md | 73 ++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 rfcs/inactive_users_cleanup.md create mode 100644 rfcs/update_metadata_lua.md diff --git a/rfcs/inactive_users_cleanup.md b/rfcs/inactive_users_cleanup.md new file mode 100644 index 000000000..1f0d1e36c --- /dev/null +++ b/rfcs/inactive_users_cleanup.md @@ -0,0 +1,64 @@ +# Inactive user cleanup + +## Overview and motivation +Currently `ms-users` service using TTL based cleanup for users who didn't pass the activation process. +In this case, there are lots of data keys staying in the database. That brings additional 'mess' into DB data. +To provide generally better data handling and clean database structure, I introduce some changes in service logic. + +## General defs + - `inactive-users` + Redis database sorted set key. Bound 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. This case 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 +On Activation and Registration request event `users:cleanup` is emitted, and executed as a hook. +When Activation action executes, the hook starts before all actions. This strategy saves from inactive users +that hit TTL but tried to pass the activation process. +When Registration action executed, hook executed after `username` exists check. +Handler not breaking general logic and not throwing errors, logs error into a logfile. + +## Registration process +When the user succeeds registration but activation not requested, the new record added to `inactive-users`. +Record contains `userId` and `currenttimestamp` + +## Activation process +When the user succeeds activation `userId` record deleted from `inactive-users`. + +## `users:cleanup` hook +Calls `deleteInactivatedUsers` script with TTL parameter from `service.config.deleteInactiveAccounts` + +## Redis Delete Inactive User Script +When service connects to Redis server and fires event "plugin:connect:$redisType" `utils/inactive/defineCommand.js` executed. +Function processes `deleteInactivatedUsers.lua.hbs` and loads resulting script into IORedis. +Script depends on lots of constants and key templates, so all these values rendered inside of the template. + +#### 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` where score < `now() - TTL * 1000` +2. Gets dependent userData such username, alias, and SSO provider information, used in delete procedure. +3. Deletes processed user ids from `USER_ACTIVATED` key. + +##### Delete process +The main logic adopted from `actions/removeUsers.js`. +Using provided username, id, alias and SSO providers fields, script checks and deletes dependent data from the database: +* Alias to id binding +* Username to id binding +* All assigned metadata. Key names created from the provided template and `user-audiences`. +* SSO provider to id binding. Script decodes Data stored in JSON string and iterates over assigned `uid's`. +UID's extracted from Internal data by `SSO_PROVIDERS` name as the field name. +* 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** diff --git a/rfcs/update_metadata_lua.md b/rfcs/update_metadata_lua.md new file mode 100644 index 000000000..a6b2c9c0a --- /dev/null +++ b/rfcs/update_metadata_lua.md @@ -0,0 +1,73 @@ +# User/Organization metadata update rework +## Overview and Motivation +The `ms-users` Script that updates user or organization metadata records using the Redis pipeline javascript code. + +## Audience lists +Audiences stored in sets formed from `USERS_AUDIENCE` or `ORGANISATION_AUDIENCE` constants and `Id` +(eg: `{ms-users}10110110111!audiences`). Both keys contain `audience` names that are currently have assigned values. + +## utils/updateMetadata.js +Almost all logic in this file removed and ported into LUA Script. +This Function checks the consistency of the provided `opts`. If `opts.metadata` and `opts.audiences` are objects, script transforming them to an array containing these objects. Checks count of meta operations and audiences to equal each other. +Organization meta update request `utils/setOrganizationMetadata.js` uses the same functionality, so the same changes applied to it. + +After commands execution result returned from the script, decoded from JSON string. + +## script/updateMetadata.lua +Script repeats all logic including custom scripts support. + +### Script parameters: +1. KEYS[1] Audiences key template +2. KEYS[2] used as metadata key template, eg: "{ms-users}{id}!metadata!{audience}" +3. ARGV[1] Id - organization or user-id +4. ARGV[2] JSON encoded opts parameter opts.{script, metadata, audiences} + +### Depending on metadata or script set: +If `opt.metadata` set: + * Script starts iterating audiences. + * On each audience, creates metadata key from provided template + * Iterates over provided in `opt.metadata` operations, based on index of `opts.audiences`. + ```javascript + const opts = { + audiences: ['first', 'second'], + metadata: [{ + // first audience commands, + }, { + // second audience commands + }], + } + ``` + Commands execute in order: `audiences[0]` => `metadata[0]`,`audiences[1]` => `metadata[1]`, + +If `opt.script` set: + * Script starts iterating audiences and creates metadata keys from provided template + * Iterates over `opt.script`: + * EVAL's script from `script.lua` and executes with params generated from: metadata keys(generated in previous step) + and passed `script.argv` + * If script evaluation fails, script returns redis.error witch description. + +When operations/scripts processed, the script forms JSON object like +```javascript +const metaResponse = [ + //forEach audience + { + '$incr': { + field: 'result', // result returned from HINCRBY command + }, + '$remove': intCount, // count of deleted fields + '$set': "OK", // or cmd hset result. + }, +]; + +const scriptResponse = { + 'scriptName': [ + // values returned from script + ], +}; +``` + +### Audience list update +When all update operations succeeded, script get's current list of user's or organization's audiences from HSET `KEYS[1]`, +unions them with `opts.audiences` and generates full list metadata keys. Iterates over them to check whether some data exists. +If no data exists, the script deletes the corresponding audience from HSET `KEYS[1]` + From 3d66715afbbb076fc90528118cb0ab76b3e02805 Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 23 Aug 2019 18:06:38 +0600 Subject: [PATCH 2/9] feat: delete inactivated users * code --- scripts/deleteInactivatedUsers.lua.hbs | 175 +++++++++++++++++++++++ scripts/updateMetadata.lua | 141 ++++++++++++++++++ src/actions/activate.js | 5 + src/actions/register.js | 5 +- src/actions/remove.js | 6 + src/constants.js | 4 + src/users.js | 5 + src/utils/inactiveUsers/defineCommand.js | 140 ++++++++++++++++++ src/utils/inactiveUsers/delete.js | 17 +++ src/utils/inactiveUsers/index.js | 44 ++++++ src/utils/setOrganizationMetadata.js | 30 ++-- src/utils/updateMetadata.js | 148 +++++++------------ 12 files changed, 610 insertions(+), 110 deletions(-) create mode 100644 scripts/deleteInactivatedUsers.lua.hbs create mode 100644 scripts/updateMetadata.lua create mode 100644 src/utils/inactiveUsers/defineCommand.js create mode 100644 src/utils/inactiveUsers/delete.js create mode 100644 src/utils/inactiveUsers/index.js diff --git a/scripts/deleteInactivatedUsers.lua.hbs b/scripts/deleteInactivatedUsers.lua.hbs new file mode 100644 index 000000000..ee8768120 --- /dev/null +++ b/scripts/deleteInactivatedUsers.lua.hbs @@ -0,0 +1,175 @@ +local usersInactiveKey = KEYS[1] +local exTime = ARGV[1] + +-- #var defs +local delimiter = '{{ KEY_SEPARATOR }}' +local keyPrefix = '{{ keyPrefix }}' + +local ssoProviders = { + {{#each sso}} + "{{ this }}", + {{/each}} +}; + +local usersDataKeyTemplate = '{{ keyTemplates.USERS_DATA }}' +local usersMetaKeyTemplate = '{{ keyTemplates.USERS_METADATA }}' +local usersTokenKeyTemplate = '{{ keyTemplates.USERS_TOKENS }}' +local usersAudienceKeyTemplate = '{{ keyTemplates.USERS_AUDIENCE }}' + +local usersOrganizationsKeyTemplate = '{{ keyTemplates.USERS_ORGANIZATIONS }}' +local organizationsMembersKeyTemplate = '{{ keyTemplates.ORGANIZATIONS_MEMBERS }}' +local organizationsMemberKeyTemplate = '{{ keyTemplates.ORGANIZATIONS_MEMBER }}' +local organizationMemberTemplate = '{{ templates.ORGANIZATIONS_MEMBER }}' + +local usersAliasToIDKey = '{{ keys.USERS_ALIAS_TO_ID }}' +local usersUsernameToIDKey = '{{ keys.USERS_USERNAME_TO_ID }}' +local usersSSOToIDKey = '{{ keys.USERS_SSO_TO_ID }}' + +local organizationsInvitationIndex = '{{ keys.ORGANIZATIONS_INVITATIONS_INDEX }}' + +local usersPublicIndex = '{{ keys.USERS_PUBLIC_INDEX }}' +local usersIndex = '{{ keys.USERS_INDEX }}' + +local usersThrottleKeyPrefix = '{{ keys.THROTTLE_PREFIX }}' + +local usersUsernameField = '{{ fields.USERS_USERNAME_FIELD }}' +local usersAliasField = '{{ fields.USERS_ALIAS_FIELD }}' +-- /var defs + +--- +--- Helper functions +--- +local function isempty(s) + return s == nil or s == '' or s == false; +end + +--decodes json +local function decode(strval) + if type(strval) == "string" then + return cjson.decode(strval) + else + return {} + end +end + +-- key generators +local function key(...) + return table.concat(arg, delimiter) +end + +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 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 + for i = 1, #data, 1 do + result[fields[i]] = data[i] + end + return result; + end + + return nil +end + +local function deleteOrganizationMember(username) + 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(organizationsMembersKeyTemplate, { orgid = orgId}) + local organizationMemberKey = makeKey(organizationsMemberKeyTemplate, { orgid = orgId, username = username }) + local organizationMember = makeKey(organizationMemberTemplate, {orgid = orgId, username = username }) + + redis.call("DEL", organizationMemberKey) + redis.call("HDEL", userOrganizationsKey, orgId) + redis.call("ZREM", organizationMembersKey, organizationMember ) + end + +end + +-- delete logic +local function deleteUser(userID, userData) + local alias = userData[usersAliasField] + local username = userData[usersUsernameField] + + 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 + + for k, provider in pairs(ssoProviders) do + local rawData = userData[provider] + local providerData = decode(rawData) + if isempty(providerData['uid']) == false then + redis.call("HDEL", usersSSOToIDKey, providerData['uid']) + end + end + + -- clean indicies + redis.call("HDEL", usersPublicIndex, userID ) + redis.call("SREM", usersIndex, userID) + + -- delete user data + local userDataKey = makeKey(usersDataKeyTemplate, { id = userID }) + redis.call("DEL", userDataKey) + + -- delete meta + local usersAudienceKey = makeKey(usersAudienceKeyTemplate, { id = userID }) + local userAudiences = redis.call("SMEMBERS", usersAudienceKey) + + for k, audience in pairs(userAudiences) do + local metaKey = makeKey(usersMetaKeyTemplate, { id = userID, audience = audience }) + redis.call("DEL", metaKey) + end + + -- delete USERS_TOKENS + local userTokensKey = makeKey(usersTokenKeyTemplate, { id = userID }) + redis.call("DEL", userTokensKey) + + redis.call("DEL", usersAudienceKey, userID) + + -- throttle constants + {{#each throttle_actions}} + redis.call("DEL", key(userThrottleKeyPrefix, {{this}}, userID)) + {{/each}} + +end + +-- Command body +redis.replicate_commands(); + +local inactiveUsers = redis.call("ZRANGEBYSCORE", usersInactiveKey, '-inf', exTime) + +for key, userID in pairs(inactiveUsers) do + local internalDatakey = makeKey(usersDataKeyTemplate, { id = userID }) + local userData = getData(internalDatakey) + + if userData ~= nil then + deleteUser(userID, userData) + end +end + +for _ , userID in pairs(inactiveUsers) do + redis.call('ZREM', usersInactiveKey, userID) +end + +-- returns count of deleted users +return #inactiveUsers diff --git a/scripts/updateMetadata.lua b/scripts/updateMetadata.lua new file mode 100644 index 000000000..a34846cdb --- /dev/null +++ b/scripts/updateMetadata.lua @@ -0,0 +1,141 @@ +local audienceKeyTemplate = KEYS[1] +local metaDataTemplate = KEYS[2] +local Id = ARGV[1] +local updateOptsJson = ARGV[2] + +redis.replicate_commands() + +local updateOpts = cjson.decode(updateOptsJson) + +local function loadScript(code, environment) + if setfenv and loadstring then + local f = assert(loadstring(code)) + setfenv(f,environment) + return f + else + return assert(load(code, nil,"t",environment)) + end +end + +local function tablesUniqueItems(...) + local args = {...} + local tableWithUniqueItems = {} + for _, passedTable in pairs(args) do + for __, keyName in pairs(passedTable) do + tableWithUniqueItems[keyName] = keyName + end + end + return tableWithUniqueItems +end + +local function makeKey (template, id, audience) + local str = template:gsub('{id}', id, 1) + if audience ~= nil then + str = str:gsub('{audience}', audience, 1) + end + return str +end + +-- +-- available ops definition +-- +local function opSet(metaKey, args) + local setArgs = {} + local result = {} + + for field, value in pairs(args) do + table.insert(setArgs, field) + table.insert(setArgs, value) + end + + local callResult = redis.call("HMSET", metaKey, unpack(setArgs)) + result[1] = callResult.ok + return result +end + +local function opRemove(metaKey, args) + local result = 0; + for i, field in pairs(args) do + result = result + redis.call("HDEL", metaKey, field) + end + return result +end + +local function opIncr(metaKey, args) + local result = {} + for field, incrVal in pairs(args) do + result[field] = redis.call("HINCRBY", metaKey, field, incrVal) + end + return result +end + +-- operations index +local metaOps = { + ['$set'] = opSet, + ['$remove'] = opRemove, + ['$incr'] = opIncr +} + +-- +-- Script body +-- +local scriptResult = {} + +local keysToProcess = {}; +for i, audience in ipairs(updateOpts.audiences) do + local key = makeKey(metaDataTemplate, Id, audience) + table.insert(keysToProcess, i, key); +end + +if updateOpts.metaOps then + for i, op in ipairs(updateOpts.metaOps) do + local targetOpKey = keysToProcess[i] + local metaProcessResult = {}; + + for opName, opArg in pairs(op) do + local processFn = metaOps[opName]; + + if processFn == nil then + return redis.error_reply("Unsupported command:" .. opName) + end + if type(opArg) ~= "table" then + return redis.error_reply("Args for ".. opName .." must be and array") + end + + metaProcessResult[opName] = processFn(targetOpKey, opArg) + end + table.insert(scriptResult, metaProcessResult) + end + +elseif updateOpts.scripts then + local env = {}; + -- allow read access to this script scope + setmetatable(env,{__index=_G}) + + for i, script in pairs(updateOpts.scripts) do + env.ARGV = script.argv + env.KEYS = keysToProcess + local fn = loadScript(script.lua, env) + scriptResult[script.name] = fn() + end + +end + +local audienceKey = makeKey(audienceKeyTemplate, Id) +local audiences = redis.call("SMEMBERS", audienceKey) +local processedAudiences = updateOpts.audiences +local uniqueAudiences = tablesUniqueItems(audiences, processedAudiences) + +for _, audience in pairs(uniqueAudiences) do + local metaKey = makeKey(metaDataTemplate, Id, audience) + local dataLen = redis.call("HLEN", metaKey) + + if (dataLen > 0) then + redis.call("SADD", audienceKey, audience) + else + redis.call("SREM", audienceKey, audience) + end +end + + +return cjson.encode(scriptResult) diff --git a/src/actions/activate.js b/src/actions/activate.js index 783f13029..72edbdbc9 100644 --- a/src/actions/activate.js +++ b/src/actions/activate.js @@ -5,6 +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 { removeFromInactiveUsers } = require('../utils/inactiveUsers'); const { USERS_INDEX, USERS_DATA, @@ -126,6 +127,8 @@ function activateAccount(data, metadata) { .persist(userKey) .sadd(USERS_INDEX, userId); + removeFromInactiveUsers(pipeline, userId); + if (alias) { pipeline.sadd(USERS_PUBLIC_INDEX, userId); } @@ -192,6 +195,8 @@ function activateAction({ params }) { }; return Promise + .bind(this, ['users:cleanup']) + .spread(this.hook) .bind(context) .then(verifyRequest) .bind(this) diff --git a/src/actions/register.js b/src/actions/register.js index ba0413630..4c7cf1101 100644 --- a/src/actions/register.js +++ b/src/actions/register.js @@ -22,6 +22,7 @@ const checkLimits = require('../utils/checkIpLimits'); const challenge = require('../utils/challenges/challenge'); const handlePipeline = require('../utils/pipelineError'); const hashPassword = require('../utils/register/password/hash'); +const { addToInactiveUsers } = require('../utils/inactiveUsers'); const { USERS_REF, USERS_INDEX, @@ -172,6 +173,8 @@ async function performRegistration({ service, params }) { await verifySSO(service, params); } + await service.hook('users:cleanup'); + const [creatorAudience] = audience; const defaultAudience = last(audience); const userId = service.flake.next(); @@ -208,7 +211,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); + addToInactiveUsers(pipeline, userId, audience); } await pipeline.exec().then(handlePipeline); diff --git a/src/actions/remove.js b/src/actions/remove.js index ce08906cf..fc3315834 100644 --- a/src/actions/remove.js +++ b/src/actions/remove.js @@ -12,6 +12,8 @@ const { USERS_PUBLIC_INDEX, USERS_ALIAS_TO_ID, USERS_SSO_TO_ID, + USERS_ACTIVATE, + USERS_AUDIENCE, USERS_USERNAME_TO_ID, USERS_USERNAME_FIELD, USERS_DATA, @@ -92,6 +94,10 @@ async function removeUser({ params }) { transaction.srem(USERS_PUBLIC_INDEX, userId); transaction.srem(USERS_INDEX, userId); + // clean audiences and delete from inactive list + transaction.zrem(USERS_ACTIVATE, userId); + transaction.hdel(USERS_AUDIENCE, userId); + // remove metadata & internal data transaction.del(key(userId, USERS_DATA)); transaction.del(key(userId, USERS_METADATA, audience)); diff --git a/src/constants.js b/src/constants.js index 525284ecc..fa11207a8 100644 --- a/src/constants.js +++ b/src/constants.js @@ -6,6 +6,8 @@ module.exports = exports = { USERS_PUBLIC_INDEX: 'users-public', USERS_REFERRAL_INDEX: 'users-referral', ORGANIZATIONS_INDEX: 'organization-iterator-set', + // inactive user id's list + USERS_INACTIVATED: 'users-inactivated', // id mapping USERS_ALIAS_TO_ID: 'users-alias', USERS_SSO_TO_ID: 'users-sso-hash', @@ -18,6 +20,7 @@ module.exports = exports = { // hashes USERS_DATA: 'data', USERS_METADATA: 'metadata', + USERS_AUDIENCE: 'users-audiences', USERS_TOKENS: 'tokens', USERS_API_TOKENS: 'api-tokens', USERS_API_TOKENS_ZSET: 'api-tokens-set', @@ -26,6 +29,7 @@ module.exports = exports = { USERS_ORGANIZATIONS: 'user-organizations', ORGANIZATIONS_DATA: 'data', ORGANIZATIONS_METADATA: 'metadata', + ORGANIZATIONS_AUDIENCE: 'organization-audiences', ORGANIZATIONS_MEMBERS: 'members', // standard JWT with TTL diff --git a/src/users.js b/src/users.js index 481dfcd46..8f428d0b4 100644 --- a/src/users.js +++ b/src/users.js @@ -71,11 +71,15 @@ module.exports = class Users extends Microfleet { }); this.on(`plugin:connect:${this.redisType}`, (redis) => { + const inactiveUsers = require('./utils/inactiveUsers'); fsort.attach(redis, 'fsort'); // init token manager const tokenManagerOpts = { backend: { connection: redis } }; this.tokenManager = new TokenManager(merge({}, config.tokenManager, tokenManagerOpts)); + + inactiveUsers.defineCommand(redis, config.redis); + this.on('users:cleanup', inactiveUsers.cleanUsers); }); this.on('plugin:start:http', (server) => { @@ -96,6 +100,7 @@ module.exports = class Users extends Microfleet { this.on(`plugin:close:${this.redisType}`, () => { this.dlock = null; this.tokenManager = null; + this.removeListener('users:cleanup'); }); // add migration connector diff --git a/src/utils/inactiveUsers/defineCommand.js b/src/utils/inactiveUsers/defineCommand.js new file mode 100644 index 000000000..9c503d0ca --- /dev/null +++ b/src/utils/inactiveUsers/defineCommand.js @@ -0,0 +1,140 @@ +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 { + USERS_AUDIENCE, + USERS_ALIAS_TO_ID, + USERS_SSO_TO_ID, + USERS_DATA, + USERS_METADATA, + USERS_TOKENS, + USERS_INDEX, + USERS_PUBLIC_INDEX, + USERS_ORGANIZATIONS, + USERS_ALIAS_FIELD, + 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 = '!'; + +const keys = { + USERS_ALIAS_TO_ID, + USERS_SSO_TO_ID, + USERS_USERNAME_TO_ID, + USERS_INDEX, + USERS_PUBLIC_INDEX, + THROTTLE_PREFIX, + ORGANIZATIONS_INVITATIONS_INDEX, +}; + +const keyTemplates = { + USERS_DATA: key('{id}', USERS_DATA), + USERS_METADATA: key('{id}', USERS_METADATA, '{audience}'), + USERS_TOKENS: key('{id}', USERS_TOKENS), + USERS_AUDIENCE: key('{id}', USERS_AUDIENCE), + USERS_ORGANIZATIONS: key('{username}', USERS_ORGANIZATIONS), + ORGANIZATIONS_MEMBERS: key('{orgid}', ORGANIZATIONS_MEMBERS), + ORGANIZATIONS_MEMBER: key('{orgid}', ORGANIZATIONS_MEMBERS, '{username}'), +}; + +const templates = { + ORGANIZATIONS_MEMBER: key('{orgid}', ORGANIZATIONS_MEMBERS, '{username}'), +}; + +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}`; + }); + } + return obj; +}; + +/** + * Compiles script from template including partials + * @param file - string path script template file + * @param templateCtx - context passed into template + * @returns {Promise} + */ +async function compileTemplate(file, templateCtx) { + const contents = await readFile(file); + const scriptTemplate = await hbs.compile(contents); + const name = path.basename(file, '.lua.hbs'); + + return { + name, + lua: scriptTemplate(templateCtx), + }; +} + +/** + * Prepares context for script template + * @param redisOptions + * @returns {{keys: *, keyTemplates: *, fields: *, KEY_SEPARATOR: *, sso: *, throttleActions: *}} + */ +function templateContext(redisOptions) { + const { keyPrefix } = redisOptions; + + return { + KEY_SEPARATOR, + fields, + throttleActions, + templates, + keys: prefixify(keyPrefix, keys), + keyTemplates: prefixify(keyPrefix, keyTemplates), + sso: SSO_PROVIDERS, + }; +} + +/** + * Compiles and registers script + * @param {ioredis} redis + * @param redisConfig + * @returns {Promise} + */ +async function defineCommand(redis, redisConfig) { + const { options, luaScripts } = redisConfig; + + const file = path.join(luaScripts, templateName); + const templateCtx = templateContext(options); + const { lua, name } = await compileTemplate(file, templateCtx); + + if (redis[name] !== null) { + redis.defineCommand(name, { lua }); + } else { + this.log.warn(`script ${name} already defined`); + } +} + +module.exports = defineCommand; diff --git a/src/utils/inactiveUsers/delete.js b/src/utils/inactiveUsers/delete.js new file mode 100644 index 000000000..8a60a0cd4 --- /dev/null +++ b/src/utils/inactiveUsers/delete.js @@ -0,0 +1,17 @@ + +const { + USERS_INACTIVATED, +} = require('../../constants'); + +/** + * Clean all users, who did't pass activation + * see scripts/deleteInactivated.lua + * @param {ioredis} redis + * @param {int} ttl - seconds + */ +async function deleteInactiveUsers(redis, ttl) { + const expire = Date.now() - (ttl * 1000); + return redis.deleteInactivatedUsers(1, USERS_INACTIVATED, expire); +} + +module.exports = deleteInactiveUsers; diff --git a/src/utils/inactiveUsers/index.js b/src/utils/inactiveUsers/index.js new file mode 100644 index 000000000..90fed4db4 --- /dev/null +++ b/src/utils/inactiveUsers/index.js @@ -0,0 +1,44 @@ +const { USERS_INACTIVATED } = require('../../constants'); + +const deleteInactiveUsers = require('./delete'); +const defineCommand = require('./defineCommand'); + +/** + * Add user id to inacive users list + * @param {ioredis} redis + * @param {userId} userId + */ +function addToInactiveUsers(redis, userId) { + const created = Date.now(); + redis.zadd(USERS_INACTIVATED, created, userId); +} + +/** + * Remove user id from inactive users list + * @param {ioredis} redis + * @param {userId} userId + */ +function removeFromInactiveUsers(redis, userId) { + redis.zrem(USERS_INACTIVATED, userId); +} + +async function cleanUsers() { + const { redis } = this; + const { deleteInactiveAccounts } = this.config; + let deletedUsers = 0; + + try { + deletedUsers = await deleteInactiveUsers(redis, deleteInactiveAccounts); + } catch (e) { + this.log.error(e); + } + + return deletedUsers; +} + +module.exports = { + addToInactiveUsers, + removeFromInactiveUsers, + cleanUsers, + defineCommand, +}; diff --git a/src/utils/setOrganizationMetadata.js b/src/utils/setOrganizationMetadata.js index a9f9b47db..6b7e63f2c 100644 --- a/src/utils/setOrganizationMetadata.js +++ b/src/utils/setOrganizationMetadata.js @@ -3,9 +3,18 @@ const Promise = require('bluebird'); const is = require('is'); const { HttpStatusError } = require('common-errors'); const redisKey = require('../utils/key.js'); -const handlePipeline = require('../utils/pipelineError.js'); -const { handleAudience } = require('../utils/updateMetadata.js'); -const { ORGANIZATIONS_METADATA } = require('../constants.js'); +const { prepareOps } = require('./updateMetadata'); +const { ORGANIZATIONS_METADATA, ORGANIZATIONS_AUDIENCE } = require('../constants.js'); + +const JSONStringify = data => JSON.stringify(data); + +function callUpdateMetadataScript(redis, id, ops) { + const audienceKeyTemplate = redisKey('{id}', ORGANIZATIONS_AUDIENCE); + const metaDataTemplate = redisKey('{id}', ORGANIZATIONS_METADATA, '{audience}'); + + return redis + .updateMetadata(2, audienceKeyTemplate, metaDataTemplate, id, JSONStringify(ops)); +} /** * Updates metadata on a organization object @@ -19,20 +28,17 @@ async function setOrganizationMetadata(opts) { } = opts; const audiences = is.array(audience) ? audience : [audience]; - // keys - const keys = audiences.map((aud) => redisKey(organizationId, ORGANIZATIONS_METADATA, aud)); - // if we have meta, then we can if (metadata) { - const pipe = redis.pipeline(); - const metaOps = is.array(metadata) ? metadata : [metadata]; - - if (metaOps.length !== audiences.length) { + const rawMetaOps = is.array(metadata) ? metadata : [metadata]; + if (rawMetaOps.length !== audiences.length) { return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); } - metaOps.forEach((meta, idx) => handleAudience(pipe, keys[idx], meta)); - return pipe.exec().then(handlePipeline); + const metaOps = rawMetaOps.map(opBlock => prepareOps(opBlock)); + + const scriptOpts = { metaOps, audiences }; + return callUpdateMetadataScript(redis, organizationId, scriptOpts); } return true; diff --git a/src/utils/updateMetadata.js b/src/utils/updateMetadata.js index 6fa5f56b8..7c4d791fa 100644 --- a/src/utils/updateMetadata.js +++ b/src/utils/updateMetadata.js @@ -1,98 +1,51 @@ /* eslint-disable no-mixed-operators */ const Promise = require('bluebird'); -const mapValues = require('lodash/mapValues'); const is = require('is'); +const ld = require('lodash'); const { HttpStatusError } = require('common-errors'); +const mapValues = require('lodash/mapValues'); const redisKey = require('../utils/key.js'); -const sha256 = require('./sha256.js'); -const handlePipeline = require('../utils/pipelineError.js'); -const { USERS_METADATA } = require('../constants.js'); +const { USERS_METADATA, USERS_AUDIENCE } = require('../constants.js'); const JSONStringify = (data) => JSON.stringify(data); +const JSONParse = data => JSON.parse(data); +const has = Object.prototype.hasOwnProperty; -/** - * Process metadata update operation for a passed audience - * @param {Object} pipeline - * @param {String} audience - * @param {Object} metadata - */ -function handleAudience(pipeline, key, metadata) { - const { $remove } = metadata; - const $removeOps = $remove && $remove.length || 0; - if ($removeOps > 0) { - pipeline.hdel(key, $remove); - } - - const { $set } = metadata; - const $setKeys = $set && Object.keys($set); - const $setLength = $setKeys && $setKeys.length || 0; - if ($setLength > 0) { - pipeline.hmset(key, mapValues($set, JSONStringify)); - } +function callUpdateMetadataScript(redis, userId, ops) { + const audienceKeyTemplate = redisKey('{id}', USERS_AUDIENCE); + const metaDataTemplate = redisKey('{id}', USERS_METADATA, '{audience}'); - const { $incr } = metadata; - const $incrFields = $incr && Object.keys($incr); - const $incrLength = $incrFields && $incrFields.length || 0; - if ($incrLength > 0) { - $incrFields.forEach((fieldName) => { - pipeline.hincrby(key, fieldName, $incr[fieldName]); - }); - } - - return { - $removeOps, $setLength, $incrLength, $incrFields, - }; + return redis + .updateMetadata(2, audienceKeyTemplate, metaDataTemplate, userId, JSONStringify(ops)); } -/** - * Maps updateMetadata ops - * @param {Array} responses - * @param {Array} operations - * @return {Object|Array} - */ -function mapMetaResponse(operations, responses) { - let cursor = 0; - return Promise - .map(operations, (props) => { - const { - $removeOps, $setLength, $incrLength, $incrFields, - } = props; - const output = {}; - - if ($removeOps > 0) { - output.$remove = responses[cursor]; - cursor += 1; - } - - if ($setLength > 0) { - output.$set = responses[cursor]; - cursor += 1; - } - - if ($incrLength > 0) { - const $incrResponse = output.$incr = {}; - $incrFields.forEach((fieldName) => { - $incrResponse[fieldName] = responses[cursor]; - cursor += 1; - }); +// Stabilizes Lua script response +function mapUpdateResponse(jsonStr) { + const decodedData = JSONParse(jsonStr); + const result = ld.map(decodedData, (audienceProcessResult) => { + return ld.mapValues(audienceProcessResult, (ops) => { + if (ops.length === undefined) { + return ops; } + return ops.length > 1 ? ops : ops[0]; + }); + }); - return output; - }) - .then((ops) => (ops.length > 1 ? ops : ops[0])); + return result.length > 1 ? result : result[0]; } /** - * Handle script, mutually exclusive with metadata - * @param {Array} scriptKeys - * @param {Array} responses + * Encodes operation field values ito json string + * If encoding performed in LUA script using CJSON lib, empty arrays become empty objects. + * This breaks logic + * @param metaOps + * @returns {*} */ -function mapScriptResponse(scriptKeys, responses) { - const output = {}; - scriptKeys.forEach((fieldName, idx) => { - output[fieldName] = responses[idx]; - }); - return output; +function prepareOps(ops) { + if (has.call(ops, '$set')) { + ops.$set = mapValues(ops.$set, JSONStringify); + } + return ops; } /** @@ -107,38 +60,39 @@ function updateMetadata(opts) { } = opts; const audiences = is.array(audience) ? audience : [audience]; - // keys - const keys = audiences.map((aud) => redisKey(userId, USERS_METADATA, aud)); + let scriptOpts = { + audiences, + }; - // if we have meta, then we can if (metadata) { - const pipe = redis.pipeline(); - const metaOps = is.array(metadata) ? metadata : [metadata]; - - if (metaOps.length !== audiences.length) { + const rawMetaOps = is.array(metadata) ? metadata : [metadata]; + if (rawMetaOps.length !== audiences.length) { return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); } - const operations = metaOps.map((meta, idx) => handleAudience(pipe, keys[idx], meta)); - return pipe.exec() - .then(handlePipeline) - .then((res) => mapMetaResponse(operations, res)); + const metaOps = rawMetaOps.map(opBlock => prepareOps(opBlock)); + + scriptOpts = { metaOps, ...scriptOpts }; + return callUpdateMetadataScript(redis, userId, scriptOpts) + .then(mapUpdateResponse); } // dynamic scripts const $scriptKeys = Object.keys(script); const scripts = $scriptKeys.map((scriptName) => { const { lua, argv = [] } = script[scriptName]; - const sha = sha256(lua); - const name = `ms_users_${sha}`; - if (!is.fn(redis[name])) { - redis.defineCommand(name, { lua }); - } - return redis[name](keys.length, keys, argv); + return { + lua, + argv, + name: scriptName, + }; }); - return Promise.all(scripts).then((res) => mapScriptResponse($scriptKeys, res)); + scriptOpts = { scripts, ...scriptOpts }; + return callUpdateMetadataScript(redis, userId, scriptOpts) + .then(result => JSONParse(result)); } -updateMetadata.handleAudience = handleAudience; +updateMetadata.callUpdateMetadataScript = callUpdateMetadataScript; +updateMetadata.prepareOps = prepareOps; module.exports = updateMetadata; From c42e14a27d0a843c058447d130ae2e40698f0913 Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 23 Aug 2019 18:07:17 +0600 Subject: [PATCH 3/9] feat: delete inactivated users * tests --- test/suites/deleteInactivatedUsers.js | 96 +++++++++++++++++++++++++++ test/suites/updateMetadata.js | 32 +++++++++ 2 files changed, 128 insertions(+) create mode 100644 test/suites/deleteInactivatedUsers.js diff --git a/test/suites/deleteInactivatedUsers.js b/test/suites/deleteInactivatedUsers.js new file mode 100644 index 000000000..50ed990f1 --- /dev/null +++ b/test/suites/deleteInactivatedUsers.js @@ -0,0 +1,96 @@ +/* eslint-disable promise/always-return, no-prototype-builtins */ +const { inspectPromise } = require('@makeomatic/deploy'); +const Promise = require('bluebird'); +const { expect } = require('chai'); +const faker = require('faker'); +const ld = require('lodash'); + +const simpleDispatcher = require('./../helpers/simpleDispatcher'); + +const { cleanUsers } = require('../../src/utils/inactiveUsers'); +const { createOrganization } = require('../helpers/organization'); + +const delay = fn => Promise.delay(1000).then(fn); + +describe('#inactive user', function registerSuite() { + beforeEach(global.startService); + afterEach(global.clearRedis); + + const regUser = { + username: 'v@makeomatic.ru', + audience: 'matic.ninja', + alias: 'bondthebest', + activate: false, + metadata: { + service: 'craft', + }, + // eslint-disable-next-line max-len + sso: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJtcy11c2VycyIsInByb2ZpbGUiOnsiZDEiOjEyLCJubSI6InBhaiJ9LCJpbnRlcm5hbHMiOnsidWlkIjoxNTE2MjM5MDIyfSwicHJvdmlkZXIiOiJmYWNlYm9vayIsInVpZCI6MTUxNjIzOTAyMiwidXNlcm5hbWUiOiJmb29AYmFyLmJheiJ9.QXLcP-86A3ly-teJt_C_XQo3hFUVC0pVALb84Eitozo', + }; + + const regUserNoAlias = { + username: 'v2@makeomatic.ru', + audience: 'matic.log', + activate: false, + metadata: { + service: 'log', + }, + }; + + beforeEach(async function pretest() { + this.users.config.deleteInactiveAccounts = 1; + await createOrganization.call(this); + await simpleDispatcher(this.users.router)('users.register', { ...regUser }); + return simpleDispatcher(this.users.router)('users.register', { ...regUserNoAlias }); + }); + + it('deletes inactive user', function test() { + return delay(() => { + return cleanUsers.call(this.users) + .then(async (res) => { + expect(res).to.be.eq(2); + + const { username } = regUser; + const dispatcher = simpleDispatcher(this.users.router); + + await dispatcher('users.getInternalData', { username }) + .reflect() + .then(inspectPromise(false)); + }); + }); + }); + + it('removes org member if user not passed activation', async function test() { + const opts = { + organizationId: this.organization.id, + member: { + email: regUser.username, + firstName: faker.name.firstName(), + lastName: faker.name.lastName(), + }, + }; + + await this.dispatch('users.organization.members.add', opts); + + return delay(() => { + return cleanUsers.call(this.users) + .then(() => { + const dispatcher = simpleDispatcher(this.users.router); + const reqOpts = { + organizationId: this.organization.id, + }; + + return dispatcher('users.organization.members.list', reqOpts) + .reflect() + .then(inspectPromise(true)) + .then(({ data }) => { + const { attributes } = data; + const membersWithUsername = ld.filter(attributes, (record) => { + return record.id === regUser.username; + }); + expect(membersWithUsername.length).to.be.eq(0); + }); + }); + }); + }); +}); diff --git a/test/suites/updateMetadata.js b/test/suites/updateMetadata.js index ef0a67265..485c14263 100644 --- a/test/suites/updateMetadata.js +++ b/test/suites/updateMetadata.js @@ -64,6 +64,7 @@ describe('#updateMetadata', function getMetadataSuite() { $incr: { b: 2, }, + $remove: ['c'], }, { $incr: { @@ -106,4 +107,35 @@ describe('#updateMetadata', function getMetadataSuite() { ]); }); }); + + it('must be able to run dynamic scripts / namespace fully available', function test() { + const dispatch = simpleDispatcher(this.users.router); + const lua = ` + local t = {} + table.insert(t, "foo") + local jsonDec = cjson.decode('{"bar": 1}') + local typeCheck = type(t) + return {jsonDec.bar, redis.call("TIME"), typeCheck, unpack(t)} + `; + + const params = { + username, + audience: [audience], + script: { + check: { + lua, + argv: ['nom-nom'], + }, + }, + }; + + return dispatch('users.updateMetadata', params) + .reflect() + .then(inspectPromise()) + .then(({ check }) => { + const [jsonVal, redisTime] = check; + expect(jsonVal).to.be.eq(1); + expect(redisTime).to.be.an('array'); + }); + }); }); From 77ce48b849190834ed4898b0e2203de43a5a3830 Mon Sep 17 00:00:00 2001 From: pajgo Date: Sat, 24 Aug 2019 21:24:57 +0600 Subject: [PATCH 4/9] feat: delete inactivated users * doc lex and typo fixes --- rfcs/inactive_users_cleanup.md | 67 +++++++++++++++++----------------- rfcs/update_metadata_lua.md | 36 ++++++++++-------- 2 files changed, 53 insertions(+), 50 deletions(-) diff --git a/rfcs/inactive_users_cleanup.md b/rfcs/inactive_users_cleanup.md index 1f0d1e36c..4da8bb315 100644 --- a/rfcs/inactive_users_cleanup.md +++ b/rfcs/inactive_users_cleanup.md @@ -1,64 +1,63 @@ # Inactive user cleanup ## Overview and motivation -Currently `ms-users` service using TTL based cleanup for users who didn't pass the activation process. -In this case, there are lots of data keys staying in the database. That brings additional 'mess' into DB data. -To provide generally better data handling and clean database structure, I introduce some changes in service logic. +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. Bound to `USER_ACTIVATE` constant. + 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 + - `deleteInactivatedUsers` Redis script, handling all cleanup logic. ## Organization Members -The `organization add member` process doesn't have validation whether -the user passed activation. This case allows inviting inactive users into an organization. The script checks whether inactivated user assigned to any organization +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 -On Activation and Registration request event `users:cleanup` is emitted, and executed as a hook. -When Activation action executes, the hook starts before all actions. This strategy saves from inactive users -that hit TTL but tried to pass the activation process. -When Registration action executed, hook executed after `username` exists check. -Handler not breaking general logic and not throwing errors, logs error into a logfile. +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 record added to `inactive-users`. -Record contains `userId` and `currenttimestamp` +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` record deleted from `inactive-users`. +When the user succeeds activation `userId`,the entry deleted from `inactive-users`. ## `users:cleanup` hook -Calls `deleteInactivatedUsers` script with TTL parameter from `service.config.deleteInactiveAccounts` +Calls `deleteInactivatedUsers` script with TTL parameter from `service.config.deleteInactiveAccounts`. ## Redis Delete Inactive User Script -When service connects to Redis server and fires event "plugin:connect:$redisType" `utils/inactive/defineCommand.js` executed. -Function processes `deleteInactivatedUsers.lua.hbs` and loads resulting script into IORedis. -Script depends on lots of constants and key templates, so all these values rendered inside of the template. +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. #### 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 +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` where score < `now() - TTL * 1000` -2. Gets dependent userData such username, alias, and SSO provider information, used in delete procedure. +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 adopted from `actions/removeUsers.js`. -Using provided username, id, alias and SSO providers fields, script checks and deletes dependent data from the database: -* Alias to id binding -* Username to id binding -* All assigned metadata. Key names created from the provided template and `user-audiences`. -* SSO provider to id binding. Script decodes Data stored in JSON string and iterates over assigned `uid's`. -UID's extracted from Internal data by `SSO_PROVIDERS` name as the field name. -* 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 +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 +* Throttle actions (???). **THROTTLE_PREFIX constant doesn't exist in constants.js, so assuming this left for backward compatibility with previous version** diff --git a/rfcs/update_metadata_lua.md b/rfcs/update_metadata_lua.md index a6b2c9c0a..e65780b70 100644 --- a/rfcs/update_metadata_lua.md +++ b/rfcs/update_metadata_lua.md @@ -1,9 +1,11 @@ # User/Organization metadata update rework ## Overview and Motivation -The `ms-users` Script that updates user or organization metadata records using the Redis pipeline javascript code. +When user or organization metadata needs to be updated, the Service uses the Redis pipeline javascript code. +For each assigned meta hash always exists a single `audience`, but there is no list of `audiences` assigned to the user or company. +To achieve easier audience tracking and a combined metadata update, I advise using a Lua based script. ## Audience lists -Audiences stored in sets formed from `USERS_AUDIENCE` or `ORGANISATION_AUDIENCE` constants and `Id` +Audiences stored in sets formed from `USERS_AUDIENCE` or `ORGANISATION_AUDIENCE` constants and `Id` (eg: `{ms-users}10110110111!audiences`). Both keys contain `audience` names that are currently have assigned values. ## utils/updateMetadata.js @@ -17,21 +19,21 @@ After commands execution result returned from the script, decoded from JSON stri Script repeats all logic including custom scripts support. ### Script parameters: -1. KEYS[1] Audiences key template -2. KEYS[2] used as metadata key template, eg: "{ms-users}{id}!metadata!{audience}" -3. ARGV[1] Id - organization or user-id -4. ARGV[2] JSON encoded opts parameter opts.{script, metadata, audiences} +1. KEYS[1] Audiences key template. +2. KEYS[2] used as metadata key template, eg: "{ms-users}{id}!metadata!{audience}". +3. ARGV[1] Id - organization or user-id. +4. ARGV[2] JSON encoded opts parameter opts.{script, metadata, audiences}. ### Depending on metadata or script set: If `opt.metadata` set: * Script starts iterating audiences. - * On each audience, creates metadata key from provided template - * Iterates over provided in `opt.metadata` operations, based on index of `opts.audiences`. + * On each audience, creates metadata key from provided template. + * Iterates operations from `opt.metadata`, based on index of `opts.audiences`. ```javascript const opts = { audiences: ['first', 'second'], metadata: [{ - // first audience commands, + // first audience commands }, { // second audience commands }], @@ -40,10 +42,10 @@ If `opt.metadata` set: Commands execute in order: `audiences[0]` => `metadata[0]`,`audiences[1]` => `metadata[1]`, If `opt.script` set: - * Script starts iterating audiences and creates metadata keys from provided template - * Iterates over `opt.script`: - * EVAL's script from `script.lua` and executes with params generated from: metadata keys(generated in previous step) - and passed `script.argv` +* Script iterates `audiences` and creates metadata keys from provided template + * Iterates `opt.script`: + * EVAL's script from `script.lua` and executes with params generated from: metadata keys(look to the previous step) + and passed `script.argv`. * If script evaluation fails, script returns redis.error witch description. When operations/scripts processed, the script forms JSON object like @@ -67,7 +69,9 @@ const scriptResponse = { ``` ### Audience list update -When all update operations succeeded, script get's current list of user's or organization's audiences from HSET `KEYS[1]`, -unions them with `opts.audiences` and generates full list metadata keys. Iterates over them to check whether some data exists. -If no data exists, the script deletes the corresponding audience from HSET `KEYS[1]` +When all update operations succeeded: +* Script get's current list of user's or organization's audiences from HSET `KEYS[1]`, +unions them with `opts.audiences` and generates full list metadata keys. +* Iterates over them to check whether some data exists. +* If no data exists, the script deletes the corresponding audience from HSET `KEYS[1]`. From 879ebb59c928c96df258210ae44c6b24976eee8b Mon Sep 17 00:00:00 2001 From: pajgo Date: Sat, 24 Aug 2019 22:17:09 +0600 Subject: [PATCH 5/9] feat: delete inactivated users * cluster bug fix --- scripts/deleteInactivatedUsers.lua.hbs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/deleteInactivatedUsers.lua.hbs b/scripts/deleteInactivatedUsers.lua.hbs index ee8768120..951d99ec6 100644 --- a/scripts/deleteInactivatedUsers.lua.hbs +++ b/scripts/deleteInactivatedUsers.lua.hbs @@ -144,7 +144,7 @@ local function deleteUser(userID, userData) local userTokensKey = makeKey(usersTokenKeyTemplate, { id = userID }) redis.call("DEL", userTokensKey) - redis.call("DEL", usersAudienceKey, userID) + redis.call("DEL", usersAudienceKey) -- throttle constants {{#each throttle_actions}} From 9309387b2fa46115dff1d83a33db7a389b2a389e Mon Sep 17 00:00:00 2001 From: pajgo Date: Mon, 26 Aug 2019 21:00:34 +0600 Subject: [PATCH 6/9] feat: delete inactivated users * updateMetadata no lodash * doc addons * cleanUsers supperess error parameter --- rfcs/inactive_users_cleanup.md | 14 ++++++++++++- src/utils/inactiveUsers/index.js | 13 ++++++++++-- src/utils/updateMetadata.js | 18 ++++++++++------- test/suites/deleteInactivatedUsers.js | 29 +++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 10 deletions(-) diff --git a/rfcs/inactive_users_cleanup.md b/rfcs/inactive_users_cleanup.md index 4da8bb315..54413e875 100644 --- a/rfcs/inactive_users_cleanup.md +++ b/rfcs/inactive_users_cleanup.md @@ -31,7 +31,19 @@ Record contains `userId` and `current timestamp`. ## Activation process When the user succeeds activation `userId`,the entry deleted from `inactive-users`. -## `users:cleanup` hook +## `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. + +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`. ## Redis Delete Inactive User Script diff --git a/src/utils/inactiveUsers/index.js b/src/utils/inactiveUsers/index.js index 90fed4db4..5f96e9279 100644 --- a/src/utils/inactiveUsers/index.js +++ b/src/utils/inactiveUsers/index.js @@ -22,7 +22,12 @@ function removeFromInactiveUsers(redis, userId) { redis.zrem(USERS_INACTIVATED, userId); } -async function cleanUsers() { +/** + * Deletes users that didn't pass activation + * @param suppressError boolean throw or suppress error + * @returns {Promise} + */ +async function cleanUsers(suppressError = true) { const { redis } = this; const { deleteInactiveAccounts } = this.config; let deletedUsers = 0; @@ -30,7 +35,11 @@ async function cleanUsers() { try { deletedUsers = await deleteInactiveUsers(redis, deleteInactiveAccounts); } catch (e) { - this.log.error(e); + if (suppressError) { + this.log.error({ error: e }, e.message); + } else { + throw e; + } } return deletedUsers; diff --git a/src/utils/updateMetadata.js b/src/utils/updateMetadata.js index 7c4d791fa..1c6744d89 100644 --- a/src/utils/updateMetadata.js +++ b/src/utils/updateMetadata.js @@ -1,7 +1,6 @@ /* eslint-disable no-mixed-operators */ const Promise = require('bluebird'); const is = require('is'); -const ld = require('lodash'); const { HttpStatusError } = require('common-errors'); const mapValues = require('lodash/mapValues'); const redisKey = require('../utils/key.js'); @@ -22,13 +21,18 @@ function callUpdateMetadataScript(redis, userId, ops) { // Stabilizes Lua script response function mapUpdateResponse(jsonStr) { const decodedData = JSONParse(jsonStr); - const result = ld.map(decodedData, (audienceProcessResult) => { - return ld.mapValues(audienceProcessResult, (ops) => { - if (ops.length === undefined) { - return ops; + const result = []; + + decodedData.forEach((metaResult) => { + const opResult = {}; + for (const [key, ops] of Object.entries(metaResult)) { + if (ops.length !== undefined && ops.length === 1) { + [opResult[key]] = ops; + } else { + opResult[key] = ops; } - return ops.length > 1 ? ops : ops[0]; - }); + } + result.push(opResult); }); return result.length > 1 ? result : result[0]; diff --git a/test/suites/deleteInactivatedUsers.js b/test/suites/deleteInactivatedUsers.js index 50ed990f1..97a29d0dd 100644 --- a/test/suites/deleteInactivatedUsers.js +++ b/test/suites/deleteInactivatedUsers.js @@ -44,6 +44,35 @@ describe('#inactive user', function registerSuite() { return simpleDispatcher(this.users.router)('users.register', { ...regUserNoAlias }); }); + describe('throw suppress error', function test() { + const lua = 'return { foo bar }'; + + beforeEach(function pretest() { + const { redis } = this.users; + redis.defineCommand('deleteInactivatedUsers', { lua }); + }); + + it('throws error', async function subtest() { + let err; + try { + await cleanUsers.call(this.users, false); + } catch (e) { + err = e; + } + expect(err).to.be.an('error'); + }); + + it('suppresses error', async function subtest() { + let err; + try { + await cleanUsers.call(this.users); + } catch (e) { + err = e; + } + expect(err).to.be.an('undefined'); + }); + }); + it('deletes inactive user', function test() { return delay(() => { return cleanUsers.call(this.users) From 0ad48fac4c8d0eb24a82ea7ac1a826e44f664e0a Mon Sep 17 00:00:00 2001 From: pajgo Date: Tue, 27 Aug 2019 23:01:51 +0600 Subject: [PATCH 7/9] feat: delete inactivated users * organization action tokens remove * user action tokens remove --- rfcs/inactive_users_cleanup.md | 8 +-- scripts/deleteInactivatedUsers.lua.hbs | 63 +++++++++++++++--------- src/utils/inactiveUsers/defineCommand.js | 28 ++++------- src/utils/inactiveUsers/index.js | 41 +++++++++++++-- test/suites/deleteInactivatedUsers.js | 8 +-- 5 files changed, 96 insertions(+), 52 deletions(-) diff --git a/rfcs/inactive_users_cleanup.md b/rfcs/inactive_users_cleanup.md index 54413e875..9820014c4 100644 --- a/rfcs/inactive_users_cleanup.md +++ b/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** + diff --git a/scripts/deleteInactivatedUsers.lua.hbs b/scripts/deleteInactivatedUsers.lua.hbs index 951d99ec6..b745ffe88 100644 --- a/scripts/deleteInactivatedUsers.lua.hbs +++ b/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 diff --git a/src/utils/inactiveUsers/defineCommand.js b/src/utils/inactiveUsers/defineCommand.js index 9c503d0ca..62508ed54 100644 --- a/src/utils/inactiveUsers/defineCommand.js +++ b/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,12 +18,6 @@ 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'); @@ -32,16 +25,17 @@ const { 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,21 +46,16 @@ 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); @@ -74,9 +63,11 @@ const readFile = f => Promise.fromCallback((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), diff --git a/src/utils/inactiveUsers/index.js b/src/utils/inactiveUsers/index.js index 5f96e9279..b1088f2c6 100644 --- a/src/utils/inactiveUsers/index.js +++ b/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} + * @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); diff --git a/test/suites/deleteInactivatedUsers.js b/test/suites/deleteInactivatedUsers.js index 97a29d0dd..7fcb3f418 100644 --- a/test/suites/deleteInactivatedUsers.js +++ b/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); From 9557c3247c698a733994964239fe6abc96fc30a2 Mon Sep 17 00:00:00 2001 From: pajgo Date: Wed, 28 Aug 2019 17:39:26 +0600 Subject: [PATCH 8/9] feat: delete inactivated users * fs.promises use * delete token array reassignment fix * doc update --- rfcs/inactive_users_cleanup.md | 3 +++ src/users.js | 5 +++-- src/utils/inactiveUsers/defineCommand.js | 9 ++------- src/utils/inactiveUsers/index.js | 7 +++---- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/rfcs/inactive_users_cleanup.md b/rfcs/inactive_users_cleanup.md index 9820014c4..c6bc9fea0 100644 --- a/rfcs/inactive_users_cleanup.md +++ b/rfcs/inactive_users_cleanup.md @@ -54,6 +54,9 @@ Function rendering `deleteInactivatedUsers.lua.hbs` and evals resulting script i 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. diff --git a/src/users.js b/src/users.js index 8f428d0b4..b5540d536 100644 --- a/src/users.js +++ b/src/users.js @@ -9,6 +9,7 @@ const RedisCluster = require('ioredis').Cluster; const Flakeless = require('ms-flakeless'); const conf = require('./config'); const get = require('./utils/get-value'); +const inactiveUsers = require('./utils/inactiveUsers'); /** * @namespace Users @@ -71,13 +72,13 @@ module.exports = class Users extends Microfleet { }); this.on(`plugin:connect:${this.redisType}`, (redis) => { - const inactiveUsers = require('./utils/inactiveUsers'); fsort.attach(redis, 'fsort'); // init token manager const tokenManagerOpts = { backend: { connection: redis } }; this.tokenManager = new TokenManager(merge({}, config.tokenManager, tokenManagerOpts)); + // load deleteInactivatedUsers script from template and assign listener inactiveUsers.defineCommand(redis, config.redis); this.on('users:cleanup', inactiveUsers.cleanUsers); }); @@ -100,7 +101,7 @@ module.exports = class Users extends Microfleet { this.on(`plugin:close:${this.redisType}`, () => { this.dlock = null; this.tokenManager = null; - this.removeListener('users:cleanup'); + this.removeListener('users:cleanup', inactiveUsers.cleanUsers); }); // add migration connector diff --git a/src/utils/inactiveUsers/defineCommand.js b/src/utils/inactiveUsers/defineCommand.js index 62508ed54..a147fc0a0 100644 --- a/src/utils/inactiveUsers/defineCommand.js +++ b/src/utils/inactiveUsers/defineCommand.js @@ -1,7 +1,6 @@ -const Promise = require('bluebird'); const hbs = require('handlebars'); const path = require('path'); -const fs = require('fs'); +const fs = require('fs').promises; const key = require('../key'); const { @@ -57,10 +56,6 @@ const fields = { USERS_USERNAME_FIELD, }; -const readFile = f => Promise.fromCallback((cb) => { - return fs.readFile(f, 'utf-8', cb); -}); - const prefixify = (prefix, obj) => { if (prefix !== '') { const objEntries = Object.entries(obj); @@ -79,7 +74,7 @@ const prefixify = (prefix, obj) => { * @returns {Promise} */ async function compileTemplate(file, templateCtx) { - const contents = await readFile(file); + const contents = await fs.readFile(file, { encoding: 'utf-8' }); const scriptTemplate = await hbs.compile(contents); const name = path.basename(file, '.lua.hbs'); diff --git a/src/utils/inactiveUsers/index.js b/src/utils/inactiveUsers/index.js index b1088f2c6..e1b73452d 100644 --- a/src/utils/inactiveUsers/index.js +++ b/src/utils/inactiveUsers/index.js @@ -43,14 +43,13 @@ function cleanTokens(userIds) { USERS_ACTION_ORGANIZATION_INVITE, USERS_ACTION_ORGANIZATION_REGISTER, ]; - const work = userIds.reduce((prev, id) => { - const delTokenActions = []; + const work = userIds.reduce((accum, id) => { for (const action of actions) { - delTokenActions.push( + accum.push( this.tokenManager.remove({ id, action }) ); } - return [...prev, ...delTokenActions]; + return accum; }, []); return Promise.all(work); From 9b773a553c6c98895bfcf9ac4549a40041f0bf5e Mon Sep 17 00:00:00 2001 From: pajgo Date: Sun, 15 Sep 2019 20:02:42 +0600 Subject: [PATCH 9/9] feat: delete inactivated users * linter parantheses fix --- src/utils/setOrganizationMetadata.js | 4 ++-- src/utils/updateMetadata.js | 6 +++--- test/suites/deleteInactivatedUsers.js | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/utils/setOrganizationMetadata.js b/src/utils/setOrganizationMetadata.js index 6b7e63f2c..82ef4c7bd 100644 --- a/src/utils/setOrganizationMetadata.js +++ b/src/utils/setOrganizationMetadata.js @@ -6,7 +6,7 @@ const redisKey = require('../utils/key.js'); const { prepareOps } = require('./updateMetadata'); const { ORGANIZATIONS_METADATA, ORGANIZATIONS_AUDIENCE } = require('../constants.js'); -const JSONStringify = data => JSON.stringify(data); +const JSONStringify = (data) => JSON.stringify(data); function callUpdateMetadataScript(redis, id, ops) { const audienceKeyTemplate = redisKey('{id}', ORGANIZATIONS_AUDIENCE); @@ -35,7 +35,7 @@ async function setOrganizationMetadata(opts) { return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); } - const metaOps = rawMetaOps.map(opBlock => prepareOps(opBlock)); + const metaOps = rawMetaOps.map((opBlock) => prepareOps(opBlock)); const scriptOpts = { metaOps, audiences }; return callUpdateMetadataScript(redis, organizationId, scriptOpts); diff --git a/src/utils/updateMetadata.js b/src/utils/updateMetadata.js index 1c6744d89..9ab68f57b 100644 --- a/src/utils/updateMetadata.js +++ b/src/utils/updateMetadata.js @@ -7,7 +7,7 @@ const redisKey = require('../utils/key.js'); const { USERS_METADATA, USERS_AUDIENCE } = require('../constants.js'); const JSONStringify = (data) => JSON.stringify(data); -const JSONParse = data => JSON.parse(data); +const JSONParse = (data) => JSON.parse(data); const has = Object.prototype.hasOwnProperty; function callUpdateMetadataScript(redis, userId, ops) { @@ -74,7 +74,7 @@ function updateMetadata(opts) { return Promise.reject(new HttpStatusError(400, 'audiences must match metadata entries')); } - const metaOps = rawMetaOps.map(opBlock => prepareOps(opBlock)); + const metaOps = rawMetaOps.map((opBlock) => prepareOps(opBlock)); scriptOpts = { metaOps, ...scriptOpts }; return callUpdateMetadataScript(redis, userId, scriptOpts) @@ -94,7 +94,7 @@ function updateMetadata(opts) { scriptOpts = { scripts, ...scriptOpts }; return callUpdateMetadataScript(redis, userId, scriptOpts) - .then(result => JSONParse(result)); + .then((result) => JSONParse(result)); } updateMetadata.callUpdateMetadataScript = callUpdateMetadataScript; diff --git a/test/suites/deleteInactivatedUsers.js b/test/suites/deleteInactivatedUsers.js index 7fcb3f418..5c8e6d5f7 100644 --- a/test/suites/deleteInactivatedUsers.js +++ b/test/suites/deleteInactivatedUsers.js @@ -10,7 +10,7 @@ const simpleDispatcher = require('./../helpers/simpleDispatcher'); const { cleanUsers } = require('../../src/utils/inactiveUsers'); const { createOrganization } = require('../helpers/organization'); -const delay = fn => Promise.delay(1000).then(fn); +const delay = (fn) => Promise.delay(1000).then(fn); describe('#inactive user', function registerSuite() { beforeEach(global.startService);