diff --git a/src/actions/activate.js b/src/actions/activate.js index a6cf56de5..7fde57aa4 100644 --- a/src/actions/activate.js +++ b/src/actions/activate.js @@ -5,7 +5,7 @@ const jwt = require('../utils/jwt.js'); const { getInternalData } = require('../utils/userData'); const getMetadata = require('../utils/get-metadata'); const handlePipeline = require('../utils/pipeline-error.js'); -const InactiveUser = require('../utils/inactive-user/inactive-user'); +const InactiveUser = require('../utils/user/inactive-user'); const User = require('../utils/user/user'); const { @@ -206,7 +206,7 @@ async function activateAction({ params }) { }; const inactiveUsers = new InactiveUser(this); - await inactiveUsers.deleteInactive(config.deleteInactiveAccounts); + await inactiveUsers.cleanUsersOnce(config.deleteInactiveAccounts); return Promise .bind(context) diff --git a/src/actions/register.js b/src/actions/register.js index 65edd7cdf..3d62f80e3 100644 --- a/src/actions/register.js +++ b/src/actions/register.js @@ -22,7 +22,7 @@ const checkLimits = require('../utils/check-ip-limits'); const challenge = require('../utils/challenges/challenge'); const handlePipeline = require('../utils/pipeline-error'); const hashPassword = require('../utils/register/password/hash'); -const InactiveUser = require('../utils/inactive-user/inactive-user'); +const InactiveUser = require('../utils/user/inactive-user'); const { USERS_REF, @@ -175,7 +175,7 @@ async function performRegistration({ service, params }) { } const inactiveUsers = new InactiveUser(service); - await inactiveUsers.deleteInactive(config.deleteInactiveAccounts); + await inactiveUsers.cleanUsersOnce(config.deleteInactiveAccounts); const [creatorAudience] = audience; const defaultAudience = last(audience); diff --git a/src/actions/remove.js b/src/actions/remove.js index eaa76c236..49a4bc7b1 100644 --- a/src/actions/remove.js +++ b/src/actions/remove.js @@ -5,7 +5,7 @@ const intersection = require('lodash/intersection'); const { getInternalData } = require('../utils/userData'); const getMetadata = require('../utils/get-metadata'); const User = require('../utils/user/user'); -const InactiveUser = require('../utils/inactive-user/inactive-user'); +const InactiveUser = require('../utils/user/inactive-user'); const { USERS_ID_FIELD, USERS_ADMIN_ROLE, diff --git a/src/utils/inactive-user/inactive-user.js b/src/utils/inactive-user/inactive-user.js deleted file mode 100644 index 801b114c3..000000000 --- a/src/utils/inactive-user/inactive-user.js +++ /dev/null @@ -1,52 +0,0 @@ -const Promise = require('bluebird'); -const RedisInactiveUser = require('./redis/inactive-user'); -const User = require('../user/user'); - -class InactiveUser { - constructor(service) { - this.service = service; - this.backend = new RedisInactiveUser(service.redis); - } - - acquireLock(lockName) { - const { dlock } = this.service; - return dlock - .once(lockName) - .disposer((l) => { - l.release().reflect(); - }); - } - - add(userId, created, pipeline = undefined) { - return this.backend.add(userId, created, pipeline); - } - - delete(userId) { - return this.backend.delete(userId); - } - - deleteInactive(userTTL) { - return Promise - .using(this.acquireLock('delete-inactive-users'), () => this._deleteInactive(userTTL)) - .catch({ name: 'LockAcquisitionError' }, () => {}); - } - - async _deleteInactive(userTTL) { - const user = new User(this.service); - const inactiveUsers = await this.backend.get(userTTL); - const work = []; - - for (const userId of inactiveUsers) { - work.push( - user - .delete(userId) - .then(() => this.backend.delete(userId)) - ); - } - await Promise.all(work); - - return inactiveUsers.length; - } -} - -module.exports = InactiveUser; diff --git a/src/utils/inactive-user/redis/inactive-user.js b/src/utils/inactive-user/redis/inactive-user.js deleted file mode 100644 index 43db96ec8..000000000 --- a/src/utils/inactive-user/redis/inactive-user.js +++ /dev/null @@ -1,22 +0,0 @@ -class InactiveUser { - static REDIS_KEY = 'users-inactivated'; - - constructor(redis) { - this.redis = redis; - } - - get(interval) { - const expire = Date.now() - (interval * 1000); - return this.redis.zrangebyscore(InactiveUser.REDIS_KEY, '-inf', expire); - } - - add(userId, createTime, redis = this.redis) { - return redis.zadd(InactiveUser.REDIS_KEY, createTime, userId); - } - - delete(userId, redis = this.redis) { - return redis.zrem(InactiveUser.REDIS_KEY, userId); - } -} - -module.exports = InactiveUser; diff --git a/src/utils/metadata/redis/metadata.js b/src/utils/metadata/redis/metadata.js index 591bb4b1c..a0ab915dd 100644 --- a/src/utils/metadata/redis/metadata.js +++ b/src/utils/metadata/redis/metadata.js @@ -1,5 +1,6 @@ const Promise = require('bluebird'); const mapValues = require('lodash/mapValues'); +const pick = require('lodash/pick'); const assert = require('assert'); const { HttpStatusError } = require('common-errors'); const handlePipeline = require('../../pipeline-error'); @@ -38,6 +39,16 @@ class Metadata { return `${id}!${this.metadataKeyBase}!${audience}`; } + /** + * Gets metadata + * @param id + * @param audience + * @returns {*} + */ + get(id, audience) { + return this.redis.hgetall(this.getMetadataKey(id, audience)); + } + /** * Updates metadata hash key * @param {String} id @@ -229,6 +240,14 @@ class Metadata { }); return output; } + + static parse(data, fields) { + const parsedData = mapValues(data, JSON.parse); + if (fields) { + return pick(parsedData, fields); + } + return parsedData; + } } module.exports = Metadata; diff --git a/src/utils/metadata/user.js b/src/utils/metadata/user.js index b7da73115..94983c1cc 100644 --- a/src/utils/metadata/user.js +++ b/src/utils/metadata/user.js @@ -53,7 +53,6 @@ class UserMetadata { /** * Deletes key from user metadata object - * @param {String|Number} id * @param {String} hashKey * @param {String} [audience] * @returns {Promise|void} @@ -93,6 +92,21 @@ class UserMetadata { return this.audience.get(this.userId); } + /** + * Gets metadata for assigned audiences + * @param audiences + * @param fields + * @returns {Promise} + */ + async getMetadata(audiences = this.userAudience, fields = {}) { + const _audiences = Array.isArray(audiences) ? audiences : [audiences]; + const data = await Promise.map(_audiences, (a) => this.metadata.get(this.userId, a)); + const output = _audiences.map((audience, idx) => { + return Metadata.parse(data[idx], fields[audience]); + }); + return _audiences.length === 1 ? output[0] : output; + } + async syncAudience() { const metaKeyTemplate = this.metadata.getMetadataKey(this.userId, '{{AUDIENCE}}'); return this.audience.resyncSet(this.userId, metaKeyTemplate); diff --git a/src/utils/organization/member/member.js b/src/utils/organization/member/member.js new file mode 100644 index 000000000..15e1bb522 --- /dev/null +++ b/src/utils/organization/member/member.js @@ -0,0 +1,35 @@ +const RedisOrgMember = require('./redis/member'); + +/** + * Class handing Organization Member data + */ +class OrganizationMember { + /** + * @param {ioredis}redis + * @param {String|Number}memberId + * @param {String|Number}orgId + */ + constructor(redis, memberId, orgId) { + this.backend = new RedisOrgMember(redis); + this.id = memberId; + this.orgId = orgId; + } + + /** + * Deletes Organization Member record + * @returns {*} + */ + delete() { + return this.backend.delete(this.orgId, this.id); + } + + getOrganizationMemberKey(memberId = this.id, orgId = this.orgId) { + return RedisOrgMember.getRedisKey(orgId, memberId); + } + + static using(redis, memberId, orgId) { + return new OrganizationMember(redis, memberId, orgId); + } +} + +module.exports = OrganizationMember; diff --git a/src/utils/organization/member/redis/member.js b/src/utils/organization/member/redis/member.js new file mode 100644 index 000000000..07fca1493 --- /dev/null +++ b/src/utils/organization/member/redis/member.js @@ -0,0 +1,66 @@ +const mapValues = require('lodash/mapValues'); +const assert = require('assert'); +const { isRedis } = require('../../../asserts/redis'); +const isValidId = require('../../../asserts/id'); + +const { + ORGANIZATIONS_MEMBERS, +} = require('../../../../constants'); + +const JSONStringify = (data) => JSON.stringify(data); + +/** + * Class handling Organization member Redis backend + */ +class OrganizationMember { + /** + * @param {ioredis|Pipeline}redis + */ + constructor(redis) { + assert(isRedis(redis), 'must be a valid redis instance'); + this.redis = redis; + } + + /** + * Generates Organization member Redis key + * @param {String|Number}orgId + * @param {String|Number}memberId + * @returns {string} + */ + static getRedisKey(orgId, memberId) { + assert(isValidId(orgId), 'must be valid organization id'); + assert(isValidId(memberId), 'must be valid member id'); + return `${orgId}!${ORGANIZATIONS_MEMBERS}!${memberId}`; + } + + /** + * Deletes Organization Member key + * @param {String|Number}orgId + * @param {String|Number}memberId + * @param {ioredis|Pipeline}[redis] + * @returns {*} + */ + delete(orgId, memberId, redis = this.redis) { + assert(isRedis(redis), 'must be a valid redis instance'); + return redis.del(OrganizationMember.getRedisKey(orgId, memberId)); + } + + /** + * Updates Organization member hash contents + * @param {String|Number}orgId + * @param {String|Number}memberId + * @param {Object}params + * @param redis + * @returns {*} + */ + update(orgId, memberId, params, redis = this.redis) { + assert(isRedis(redis), 'must be a valid redis instance'); + return redis.hmset(OrganizationMember.getRedisKey(orgId, memberId), OrganizationMember.stringify(params)); + } + + static stringify(params) { + return mapValues(params, JSONStringify); + } +} + +module.exports = OrganizationMember; diff --git a/src/utils/organization/organization.js b/src/utils/organization/organization.js new file mode 100644 index 000000000..6150e3ed2 --- /dev/null +++ b/src/utils/organization/organization.js @@ -0,0 +1,41 @@ +const Promise = require('bluebird'); +const RedisOrganization = require('./redis/organization'); +const OrganizationMember = require('./member/member'); + +/** + * Class Handing Organization actions + */ +class Organization { + id = undefined; + + constructor(redis, orgId) { + this.redis = redis; + this.id = orgId; + this.backend = new RedisOrganization(redis); + } + + removeMember(memberId) { + const orgMember = OrganizationMember.using(this.redis, memberId, this.id); + const memberKey = orgMember.getOrganizationMemberKey(); + return Promise.all([ + orgMember.delete(), + this.backend.removeMember(this.id, memberKey, this.redis), + ]); + } + + static filterIds(obj) { + const ids = []; + const re = /^\d+$/; + for (const [key] of Object.entries(obj)) { + if (re.test(key)) ids.push(key); + } + return ids; + } + + static using(orgId, redis) { + const org = new Organization(redis, orgId); + return org; + } +} + +module.exports = Organization; diff --git a/src/utils/organization/redis/organization.js b/src/utils/organization/redis/organization.js new file mode 100644 index 000000000..f31dca62a --- /dev/null +++ b/src/utils/organization/redis/organization.js @@ -0,0 +1,43 @@ +const assert = require('assert'); +const { isRedis } = require('../../asserts/redis'); +const isValidId = require('../../asserts/id'); +const isNotEmptyString = require('../../asserts/string-not-empty'); + +const { ORGANIZATIONS_MEMBERS } = require('../../../constants'); + +/** + * Class handles Organization Data Redis Backend + */ +class Organization { + /** + * @param {ioredis|Pipeline}redis + */ + constructor(redis) { + assert(isRedis(redis), 'must be a valid redis instance'); + this.redis = redis; + } + + /** + * Gets Key with Organization information + * @param {String|Number}orgId + * @returns {string} + */ + static getMembersKey(orgId) { + assert(isValidId(orgId), 'must be valid organization id'); + return `${orgId}!${ORGANIZATIONS_MEMBERS}`; + } + + /** + * Deletes provided member key from Organization Members list + * @param {String|Number}orgId + * @param {String}memberKey + * @param {ioredis|Pipeline}[redis] + * @returns {*} + */ + removeMember(orgId, memberKey, redis = this.redis) { + assert(isNotEmptyString(memberKey), 'must be not epty string'); + return redis.zrem(Organization.getMembersKey(orgId), memberKey); + } +} + +module.exports = Organization; diff --git a/src/utils/user/inactive-user.js b/src/utils/user/inactive-user.js new file mode 100644 index 000000000..8d5042e35 --- /dev/null +++ b/src/utils/user/inactive-user.js @@ -0,0 +1,109 @@ +const Promise = require('bluebird'); +const assert = require('assert'); +const User = require('./user'); +const UserMetadata = require('../metadata/user'); +const Organization = require('../organization/organization'); +const RedisInactiveUser = require('./redis/inactive-user'); +const isNotEmptyString = require('../asserts/string-not-empty'); + +/** + * Class handling Inactive User actions + */ +class InactiveUser { + /** + * @param {Microfleet}service + * @param {ioredis}service.redis + * @param {dlock}service.dlock + * @param {Object}service.config + */ + constructor(service) { + this.service = service; + this.backend = new RedisInactiveUser(this.service.redis); + } + + /** + * Attempts to acquire operation lock using `dlock` + * @param {String}lockName + * @returns {FunctionDisposer} + */ + acquireLock(lockName) { + assert(isNotEmptyString(lockName), 'must be valid lock name'); + const { dlock } = this.service; + return dlock + .once(lockName) + .disposer((l) => { + l.release().reflect(); + }); + } + + /** + * Adds provided user id into inactive index + * @param {String|Number}userId + * @param {Number}created + * @param {ioredis|pipeline}[pipeline] + * @returns {Promise<*>|Pipeline} + */ + add(userId, created, pipeline = undefined) { + return this.backend.add(userId, created, pipeline); + } + + /** + * Deletes provided user id from inactive index + * @param {String|Number}userId + * @returns {Promise<*>|Pipeline} + */ + delete(userId) { + return this.backend.delete(userId); + } + + /** + * Acquires lock and executes `cleanUsers` + * @param {Number}userTTL - interval seconds + * @returns {Promise | * | Promise} + */ + cleanUsersOnce(userTTL) { + return Promise + .using(this.acquireLock('delete-inactive-users'), () => this.cleanUsers(userTTL)) + .catch({ name: 'LockAcquisitionError' }, () => {}); + } + + /** + * @CAUTION Deletes Users whose ids were in index and their score < Now()-interval + * @param {Number}userTTL -interval seconds + * @returns {Promise<*>} + */ + async cleanUsers(userTTL) { + const { config, redis } = this.service; + const { audience } = config.organizations; + const user = new User(this.service); + const work = []; + + const usersToDelete = await this.backend.get(userTTL); + const userOrganizations = {}; + + await Promise.map(usersToDelete, async (id) => { + const metaData = await UserMetadata + .using(id, audience, redis) + .getMetadata(); + userOrganizations[id] = Organization.filterIds(metaData); + }); + + for (const userId of usersToDelete) { + for (const orgId of userOrganizations[userId] || []) { + work.push( + Organization.using(orgId, redis).removeMember(userId) + ); + } + work.push( + user + .delete(userId) + .then(() => this.delete(userId)) + ); + } + await Promise.all(work); + + return usersToDelete.length; + } +} + +module.exports = InactiveUser; diff --git a/src/utils/user/redis/inactive-user.js b/src/utils/user/redis/inactive-user.js new file mode 100644 index 000000000..2b4397d06 --- /dev/null +++ b/src/utils/user/redis/inactive-user.js @@ -0,0 +1,55 @@ +const assert = require('assert'); +const { isRedis } = require('../../asserts/redis'); +const isValidId = require('../../asserts/id'); +const isInteger = require('../../asserts/integer'); + +/** + * Class Handling Inactive Users index using Redis backend + */ +class InactiveUser { + static REDIS_KEY = 'users-inactivated'; + + /** + * @param {ioredis|Pipeline}redis + */ + constructor(redis) { + assert(isRedis(redis), 'must be a valid redis instance'); + this.redis = redis; + } + + /** + * Get list of Ids having score < Now()-interval + * @param {Number}interval - seconds + * @returns {Promise|Pipeline} + */ + get(interval) { + assert(isInteger(interval), 'must be valid interval'); + const expire = Date.now() - (interval * 1000); + return this.redis.zrangebyscore(InactiveUser.REDIS_KEY, '-inf', expire); + } + + /** + * @param {String|Number}userId + * @param {Number}createTime - timestamp + * @param {ioredis|Pipeline}[redis] + * @returns {Promise<*>|Pipeline} + */ + add(userId, createTime, redis = this.redis) { + assert(isValidId(userId), 'must be valid user id'); + assert(isInteger(createTime), 'must be valid timestamp'); + return redis.zadd(InactiveUser.REDIS_KEY, createTime, userId); + } + + /** + * Deletes passed ID from index + * @param {String|Number}userId + * @param {ioredis|Pipeline}[redis] + * @returns {Promise<*>|Pipeline} + */ + delete(userId, redis = this.redis) { + assert(isValidId(userId), 'must be valid user id'); + return redis.zrem(InactiveUser.REDIS_KEY, userId); + } +} + +module.exports = InactiveUser; diff --git a/src/utils/user/redis/user.js b/src/utils/user/redis/user.js index f6b7e6ff0..122399969 100644 --- a/src/utils/user/redis/user.js +++ b/src/utils/user/redis/user.js @@ -1,4 +1,9 @@ const Promise = require('bluebird'); +const assert = require('assert'); +const { isRedis } = require('../../asserts/redis'); +const isValidId = require('../../asserts/id'); +const isNotEmptyString = require('../../asserts/string-not-empty'); + const getInternalData = require('../../userData/get-internal-data'); const { @@ -11,15 +16,32 @@ const { USERS_TOKENS, } = require('../../../constants'); +/** + * Class handles User data operations using Redis backend + */ class User { + /** + * @param {ioredis|Pipeline}redis + */ constructor(redis) { + assert(isRedis(redis), 'must be a valid redis instance'); this.redis = redis; } + /** + * Generates Redis Hash Key storing user data + * @param {String|Number}userId + * @returns {string} + */ static getUserDataKey(userId) { + assert(isValidId(userId), 'must be valid user id'); return `${userId}!${USERS_DATA}`; } + /** + * Flush caches generated by `redis-fsort` + * @returns {Promise<*>} + */ flushCaches() { const now = Date.now(); return Promise.all([ @@ -28,11 +50,30 @@ class User { ]); } + /** + * Returns User data + * @param {String|Number}userId + */ getData(userId) { + assert(isValidId(userId), 'must be valid user id'); return getInternalData.call({ redis: this.redis }, userId); } + /** + * Deletes User and associated data + * @param user + * @param {String|Number}user.id + * @param {String}user.username + * @param {String}user.alias + * @param {String[]}user.ssoIds + * @param {ioredis|Pipeline} [redis] + * @returns {any} + */ delete({ id, username, alias, ssoIds }, redis = this.redis) { + assert(isRedis(redis), 'must be a valid redis instance'); + assert(isValidId(id), 'must be valid user id'); + assert(isNotEmptyString(username), 'must be valid alias'); + const scriptKeys = [ USERS_ALIAS_TO_ID, USERS_USERNAME_TO_ID, USERS_SSO_TO_ID, USERS_PUBLIC_INDEX, USERS_INDEX, USERS_TOKENS, diff --git a/src/utils/user/user.js b/src/utils/user/user.js index 3f2f7a8bc..216e53d81 100644 --- a/src/utils/user/user.js +++ b/src/utils/user/user.js @@ -9,16 +9,29 @@ const UserMetadata = require('../metadata/user'); const { USERS_ACTION_ACTIVATE, USERS_ACTION_REGISTER, USERS_ACTION_PASSWORD, USERS_ACTION_RESET, + USERS_ACTION_ORGANIZATION_INVITE, SSO_PROVIDERS, } = require('../../constants'); - +/** + * Class managing User data and operations + */ class User { + /** + * @param {Microfleet}service + * @param {ioredis}service.redis + * @param {dlock}service.dlock + */ constructor(service) { this.service = service; this.backend = new RedisUser(service.redis); } + /** + * Attempts to acquire operation lock using `dlock` + * @param lockName + * @returns {Promise | *} + */ acquireLock(lockName) { const { dlock } = this.service; return dlock @@ -26,20 +39,39 @@ class User { .catch(LockAcquisitionError, () => null); } + /** + * Checks whether user is being deleted + * @param {String|Number}userId + * @returns {Promise} + */ isUserDeleting(userId) { return this.acquireLock(`delete-user-${userId}`) .catch(LockAcquisitionError, () => true) .return(false); } + /** + * Gets User data + * @param {String|Number}userId + */ getData(userId) { return this.backend.getData(userId); } + /** + * Flushes caches + * @returns {Promise<*>} + */ flushCaches() { return this.backend.flushCaches(); } + /** + * @CAUTION Deletes user + * @param {Object}user - User Data object + * @param {Boolean}[throwError] - Throws Race Condition error if Delete process was already started + * @returns {Promise} + */ async delete(user, throwError = true) { const { id, username, alias, ...restData } = typeof user === 'object' || await this.getData(user); const lock = await this.acquireLock(`delete-user-${id}`); @@ -53,8 +85,8 @@ class User { try { const { redis } = this.service; - const userAudiences = await UserMetadata.using(id, null, redis).getAudience(); const pipeline = redis.pipeline(); + const userAudiences = await UserMetadata.using(id, null, redis).getAudience(); const pipelinedMetadata = UserMetadata.using(id, null, pipeline); const ssoIds = []; @@ -68,7 +100,9 @@ class User { for (const metaAudience of userAudiences) { pipelinedMetadata.deleteMetadata(metaAudience, pipeline); } + this.backend.delete({ id, username, alias, ssoIds }, pipeline); + const [pipelineResult] = await Promise.all([ pipeline.exec(), this.deleteUserTokens(username), @@ -82,10 +116,16 @@ class User { return id; } + /** + * Cleans User Action tokens + * @param userEmail + * @returns {Promise<*>} + */ deleteUserTokens(userEmail) { const actions = [ USERS_ACTION_ACTIVATE, USERS_ACTION_REGISTER, USERS_ACTION_PASSWORD, USERS_ACTION_RESET, + USERS_ACTION_ORGANIZATION_INVITE, ]; const { tokenManager } = this.service; diff --git a/test/suites/delete-inactive-users.js b/test/suites/delete-inactive-users.js index 57c9f6124..ba5e7f69f 100644 --- a/test/suites/delete-inactive-users.js +++ b/test/suites/delete-inactive-users.js @@ -1,8 +1,10 @@ /* eslint-disable promise/always-return, no-prototype-builtins */ const { inspectPromise } = require('@makeomatic/deploy'); const Promise = require('bluebird'); - -const InactiveUsers = require('../../src/utils/inactive-user/inactive-user'); +const faker = require('faker'); +const ld = require('lodash'); +const { expect } = require('chai'); +const InactiveUsers = require('../../src/utils/user/inactive-user'); const { createOrganization } = require('../helpers/organization'); describe('#inactive user', function registerSuite() { @@ -31,8 +33,6 @@ describe('#inactive user', function registerSuite() { }; beforeEach(async () => { - this.users.config.deleteInactiveAccounts = 1; - await createOrganization.call(this); await this.dispatch('users.register', { ...regUser }); await this.dispatch('users.register', { ...regUserNoAlias }); @@ -42,11 +42,39 @@ describe('#inactive user', function registerSuite() { const inactiveUsers = new InactiveUsers(this.users); await Promise.delay(1000); - await inactiveUsers.deleteInactive(1); + await inactiveUsers.cleanUsers(1); const { username } = regUser; await this.dispatch('users.getInternalData', { username }) .reflect() .then(inspectPromise(false)); }); + + it('removes org member if user not passed activation', async () => { + const opts = { + organizationId: this.organization.id, + member: { + email: regUser.username, + firstName: faker.name.firstName(), + lastName: faker.name.lastName(), + }, + }; + + const reqOpts = { + organizationId: this.organization.id, + }; + + await this.dispatch('users.organization.members.add', opts); + const inactiveUsers = new InactiveUsers(this.users); + await Promise.delay(1200); + + await inactiveUsers.cleanUsers(1); + + const members = await this.dispatch('users.organization.members.list', reqOpts); + const { attributes } = members.data; + const membersWithUsername = ld.filter(attributes, (record) => { + return record.id === regUser.username; + }); + expect(membersWithUsername.length).to.be.eq(0); + }); });