Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions rfcs/inactive_users/user_index.md
Original file line number Diff line number Diff line change
@@ -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.


11 changes: 10 additions & 1 deletion src/actions/activate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions src/actions/register.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/actions/remove.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand Down
4 changes: 4 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
23 changes: 23 additions & 0 deletions src/utils/inactiveUsers/index.js
Original file line number Diff line number Diff line change
@@ -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<void>}
*/
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,
};