diff --git a/rfcs/inactive_users_cleanup.md b/rfcs/inactive_users_cleanup.md new file mode 100644 index 000000000..c6bc9fea0 --- /dev/null +++ b/rfcs/inactive_users_cleanup.md @@ -0,0 +1,80 @@ +# Inactive user cleanup + +## Overview and motivation +For users who didn't pass the activation process, the service using TTL keyspace cleanup and only `Internal Data` deleted. +A lot of linked keys remain in the database, and this leads to keyspace pollution. +For better data handling and clean database structure, I introduce some changes in service logic: + +## General defs + - `inactive-users` + Redis database sorted set key. Assigned to `USER_ACTIVATE` constant. + Record contains `userId` as value and `timestamp` as score. + - `user-audiences` [Described here](update_metadata_lua.md#audience-list-update) + - `deleteInactivatedUsers` Redis script, handling all cleanup logic. + +## Organization Members +The `organization add member` process doesn't have validation whether the user passed activation and allows +inviting inactive users into an organization. The script checks whether inactivated user assigned to any organization +and deletes users from organization members and user->organization bindings. + +## Registration and activation +Every Activation and Registration request event executes `users:cleanup` hook. +The Activation request executes the hook first this strategy saves from inactive +users that hit TTL but tried to pass the activation process. +The Registration request executes the hook after `username` exists check. + +## Registration process +When the user succeeds registration but activation not requested, the new entry added to `inactive-users`. +Record contains `userId` and `current timestamp`. + +## Activation process +When the user succeeds activation `userId`,the entry deleted from `inactive-users`. + +## `users:cleanup` hook `cleanUsers(suppress?)` +`suppress` parameter defines function error behavior. If parameter set, the function throws errors, +otherwise, function calls `log.error` with `error.message` as message. +Default value is `true`. IMHO User shouldn't know about our problems. + +Other option, is to define new config parameter as object and move `config.deleteInactiveAccounts` into it: +```javascript +const conf = { + deleteInactiveUsers: { + ttl: seconds, // replaces deleteInactiveAccounts + suppressErrors: true || false, + }, +} +``` +Calls `deleteInactivatedUsers` script with TTL parameter from `service.config.deleteInactiveAccounts`. +When script finished, calls TokenManager to delete Action tokens(`USER_ACTION_*`, ``). +*NOTE*: Should we update TokenManager to add support of pipeline? + +## Redis Delete Inactive User Script +When the service connects to the Redis server and fires event "plugin:connect:$redisType" `utils/inactive/defineCommand.js` executed. +Function rendering `deleteInactivatedUsers.lua.hbs` and evals resulting script into IORedis. +The Script using a dozen constants, keys, and templates, so all these values rendered inside of the template using template context. +Returns list of deleted users. + +*NOTE*: Using experimental `fs.promises.readFile` API function. On `node` 10.x it's an experimental feature, +on `node` >= 11.x function becomes stable without any changes in API. + +#### deleteInactivatedUsers `USERS_ACTIVATED` `TTL` as seconds +##### Script paramerters: +1. KEYS[1] Sorted Set name containing the list of users that didn't pass activation. +2. ARGS[1] TTL in seconds. + +##### When started: +1. Gets UserId's from ZSET `USERS_ACTIVATED` where score < `now() - TTL * 1000` and iterates over them. +2. Gets dependent userData such as username, alias, and SSO provider information used in delete procedure and calls [Delete process](#delete-process). +3. Deletes processed user ids from `USER_ACTIVATED` key. + +##### Delete process +The main logic is based on `actions/removeUsers.js`. +Using the username, id, alias and SSO providers fields, script checks and removes dependent data from the database: +* Alias to id binding. +* Username to id binding. +* All assigned metadata. Key names rendered from the template and `user-audiences`. +* SSO provider to id binding. Using `SSO_PROVIDERS` items as the field name decodes and extracts UID's from Internal data. +* User tokens. +* Private and public id indexes +* Links and data used in Organization assignment + diff --git a/rfcs/update_metadata_lua.md b/rfcs/update_metadata_lua.md new file mode 100644 index 000000000..e65780b70 --- /dev/null +++ b/rfcs/update_metadata_lua.md @@ -0,0 +1,77 @@ +# User/Organization metadata update rework +## Overview and Motivation +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` +(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 operations from `opt.metadata`, 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 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 +```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]`. + diff --git a/scripts/deleteInactivatedUsers.lua.hbs b/scripts/deleteInactivatedUsers.lua.hbs new file mode 100644 index 000000000..b745ffe88 --- /dev/null +++ b/scripts/deleteInactivatedUsers.lua.hbs @@ -0,0 +1,190 @@ +local usersInactiveKey = KEYS[1] +local exTime = ARGV[1] +--- +-- var defs +--- +local delimiter = '{{ KEY_SEPARATOR }}' +local keyPrefix = '{{ keyPrefix }}' + +local ssoProviders = { + {{#each sso}} + "{{ this }}", + {{/each}} +}; + +-- key templates +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 }}' + +-- 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 }}' + +-- fields +local usersUsernameField = '{{ fields.USERS_USERNAME_FIELD }}' +local usersAliasField = '{{ fields.USERS_ALIAS_FIELD }}' + + +--- +-- 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 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 + str = str:gsub('{'..param..'}', value, 1) + end + 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 to table + for i = 1, #data, 1 do + result[fields[i]] = data[i] + end + return result; + end + + 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) + + 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 + +-- 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 + + 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) + 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 data + 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) + + -- delete user audiences list + redis.call("DEL", usersAudienceKey) + +end + +--- +-- Script Logic +--- + +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 emails of deleted users +-- helps to remove TokenManager tokens +return inactiveUserNames 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..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 @@ -76,6 +77,10 @@ module.exports = class Users extends Microfleet { // 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); }); this.on('plugin:start:http', (server) => { @@ -96,6 +101,7 @@ module.exports = class Users extends Microfleet { this.on(`plugin:close:${this.redisType}`, () => { this.dlock = null; this.tokenManager = null; + this.removeListener('users:cleanup', inactiveUsers.cleanUsers); }); // add migration connector diff --git a/src/utils/inactiveUsers/defineCommand.js b/src/utils/inactiveUsers/defineCommand.js new file mode 100644 index 000000000..a147fc0a0 --- /dev/null +++ b/src/utils/inactiveUsers/defineCommand.js @@ -0,0 +1,125 @@ +const hbs = require('handlebars'); +const path = require('path'); +const fs = require('fs').promises; + +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, + ORGANIZATIONS_MEMBERS, + ORGANIZATIONS_INVITATIONS_INDEX, +} = require('../../constants'); + +const templateName = 'deleteInactivatedUsers.lua.hbs'; +const KEY_SEPARATOR = '!'; + +// keys used in script +const keys = { + USERS_ALIAS_TO_ID, + USERS_SSO_TO_ID, + USERS_USERNAME_TO_ID, + USERS_INDEX, + USERS_PUBLIC_INDEX, + ORGANIZATIONS_INVITATIONS_INDEX, +}; + +// key templates used in script +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}'), +}; + +// 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 prefixify = (prefix, obj) => { + if (prefix !== '') { + const objEntries = Object.entries(obj); + + for (const [prop, value] of objEntries) { + obj[prop] = `${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 fs.readFile(file, { encoding: 'utf-8' }); + 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, + 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..e1b73452d --- /dev/null +++ b/src/utils/inactiveUsers/index.js @@ -0,0 +1,87 @@ +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'); + +/** + * 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); +} + +/** + * 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((accum, id) => { + for (const action of actions) { + accum.push( + this.tokenManager.remove({ id, action }) + ); + } + return accum; + }, []); + + return Promise.all(work); +} + +/** + * 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 = []; + + try { + deletedUsers = await deleteInactiveUsers(redis, deleteInactiveAccounts); + await cleanTokens.call(this, deletedUsers); + } catch (e) { + if (suppressError) { + this.log.error({ error: e }, e.message); + } else { + throw e; + } + } + + return deletedUsers; +} + +module.exports = { + addToInactiveUsers, + removeFromInactiveUsers, + cleanUsers, + defineCommand, +}; diff --git a/src/utils/setOrganizationMetadata.js b/src/utils/setOrganizationMetadata.js index a9f9b47db..82ef4c7bd 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..9ab68f57b 100644 --- a/src/utils/updateMetadata.js +++ b/src/utils/updateMetadata.js @@ -1,98 +1,55 @@ /* eslint-disable no-mixed-operators */ const Promise = require('bluebird'); -const mapValues = require('lodash/mapValues'); const is = require('is'); 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 = []; + + 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; } + } + result.push(opResult); + }); - 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 +64,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; diff --git a/test/suites/deleteInactivatedUsers.js b/test/suites/deleteInactivatedUsers.js new file mode 100644 index 000000000..5c8e6d5f7 --- /dev/null +++ b/test/suites/deleteInactivatedUsers.js @@ -0,0 +1,127 @@ +/* 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; + const dispatcher = simpleDispatcher(this.users.router); + + await createOrganization.call(this); + await dispatcher('users.register', { ...regUser }); + return dispatcher('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) + .then(async (res) => { + expect(res.length).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'); + }); + }); });