diff --git a/rfcs/inactive_users/user_index.md b/rfcs/inactive_users/user_index.md new file mode 100644 index 000000000..ea594ea69 --- /dev/null +++ b/rfcs/inactive_users/user_index.md @@ -0,0 +1,28 @@ +# Inactive users index + +## Overview and motivation +As other task dependency and to provide additional internal statistics the Service needs additional index +to track users that didn't pass the activation. + +## `inactive-users` index +It's an Additional Redis Sorted Set which contains the list of IDs of the `inactive` users. +Each item score is equal to the `timestamp` set on user creation. + +To avoid hardcoded SET name new `USERS_INACTIVATED` constant introduced. + +## Registration process +When the user succeeds registration but activation not requested, the new entry added to `inactive-users`. + +**NOTE:** Old Redis `expire` setting methods left in their place, to save old service behavior. + +## Activation process +When the user succeeds activation the entry deleted from `inactive-users`. + +## Index cleanup +Temporarily add `inactive-users` index cleanup. The functionality will move into one LUA script +with `delete inactive users` logic. This will save us from `dlock` based implementations and all operations will execute in `one-shot` avoiding race conditions. +On `registration` cleanup method executes before user creation. + +On `activation` cleanup method executes before any checks performed by `activation` action. + + diff --git a/src/actions/activate.js b/src/actions/activate.js index 783f13029..460eeb778 100644 --- a/src/actions/activate.js +++ b/src/actions/activate.js @@ -5,8 +5,11 @@ const jwt = require('../utils/jwt.js'); const { getInternalData } = require('../utils/userData'); const getMetadata = require('../utils/getMetadata'); const handlePipeline = require('../utils/pipelineError.js'); +const { cleanInactiveUsersIndex } = require('../utils/inactiveUsers'); + const { USERS_INDEX, + USERS_INACTIVATED, USERS_DATA, USERS_REFERRAL_INDEX, USERS_PUBLIC_INDEX, @@ -126,6 +129,9 @@ function activateAccount(data, metadata) { .persist(userKey) .sadd(USERS_INDEX, userId); + /* delete user id from the inactive users index */ + pipeline.zrem(USERS_INACTIVATED, userId); + if (alias) { pipeline.sadd(USERS_PUBLIC_INDEX, userId); } @@ -173,7 +179,7 @@ function hook(userId) { * @apiParam (Payload) {String} [audience] - additional metadata will be pushed there from custom hooks * */ -function activateAction({ params }) { +async function activateAction({ params }) { // TODO: add security logs // var remoteip = request.params.remoteip; const { token, username } = params; @@ -191,6 +197,9 @@ function activateAction({ params }) { erase: config.token.erase, }; + // TODO: REMOVEME: Execute `DeleteInactiveUsers` LUA script + await cleanInactiveUsersIndex.call(this); + return Promise .bind(context) .then(verifyRequest) diff --git a/src/actions/register.js b/src/actions/register.js index ba0413630..06cbc9383 100644 --- a/src/actions/register.js +++ b/src/actions/register.js @@ -22,10 +22,13 @@ const checkLimits = require('../utils/checkIpLimits'); const challenge = require('../utils/challenges/challenge'); const handlePipeline = require('../utils/pipelineError'); const hashPassword = require('../utils/register/password/hash'); +const { cleanInactiveUsersIndex } = require('../utils/inactiveUsers'); + const { USERS_REF, USERS_INDEX, USERS_SSO_TO_ID, + USERS_INACTIVATED, USERS_DATA, USERS_USERNAME_TO_ID, USERS_ACTIVE_FLAG, @@ -172,6 +175,9 @@ async function performRegistration({ service, params }) { await verifySSO(service, params); } + // TODO: REMOVEME: Execute `DeleteInactiveUsers` LUA script + await cleanInactiveUsersIndex.call(service); + const [creatorAudience] = audience; const defaultAudience = last(audience); const userId = service.flake.next(); @@ -208,7 +214,11 @@ async function performRegistration({ service, params }) { pipeline.hset(USERS_USERNAME_TO_ID, username, userId); if (activate === false && config.deleteInactiveAccounts >= 0) { + /* TODO Remove this line when last task in group merged */ pipeline.expire(userDataKey, config.deleteInactiveAccounts); + + /* Add user id to the inactive users index */ + redis.zadd(USERS_INACTIVATED, created, userId); } await pipeline.exec().then(handlePipeline); diff --git a/src/actions/remove.js b/src/actions/remove.js index cdad191e2..b13b2625a 100644 --- a/src/actions/remove.js +++ b/src/actions/remove.js @@ -9,6 +9,7 @@ const getMetadata = require('../utils/getMetadata'); const handlePipeline = require('../utils/pipelineError'); const { USERS_INDEX, + USERS_INACTIVATED, USERS_PUBLIC_INDEX, USERS_ALIAS_TO_ID, USERS_SSO_TO_ID, @@ -92,6 +93,9 @@ async function removeUser({ params }) { transaction.srem(USERS_PUBLIC_INDEX, userId); transaction.srem(USERS_INDEX, userId); + /* Delete user from the inactive users index, if user not activated */ + transaction.zrem(USERS_INACTIVATED, 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..0c5867e70 100644 --- a/src/constants.js +++ b/src/constants.js @@ -6,6 +6,10 @@ module.exports = exports = { USERS_PUBLIC_INDEX: 'users-public', USERS_REFERRAL_INDEX: 'users-referral', ORGANIZATIONS_INDEX: 'organization-iterator-set', + + /* inactive user ids set */ + USERS_INACTIVATED: 'users-inactivated', + // id mapping USERS_ALIAS_TO_ID: 'users-alias', USERS_SSO_TO_ID: 'users-sso-hash', diff --git a/src/utils/inactiveUsers/index.js b/src/utils/inactiveUsers/index.js new file mode 100644 index 000000000..6c4bb9e80 --- /dev/null +++ b/src/utils/inactiveUsers/index.js @@ -0,0 +1,23 @@ +const { + USERS_INACTIVATED, +} = require('../../constants'); + +/** + * NOTE: Contents of this file will be removed when `DeleteInactiveUsers` feature merged. + * To avoid `dlock` based locks, index cleanup and inactive user remove process will be merged into one LUA script. + */ + +/** + * Cleans Inactive user index from User IDs that not activated in some period + * @returns {Promise} + */ +async function cleanInactiveUsersIndex() { + const { redis } = this; + const { deleteInactiveAccounts } = this.config; + const expire = Date.now() - (deleteInactiveAccounts * 1000); + await redis.zremrangebyscore(USERS_INACTIVATED, '-inf', expire); +} + +module.exports = { + cleanInactiveUsersIndex, +};