From 65d9186e0bcbc3a83636a86c19195e0ffefeb801 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Wed, 31 Mar 2021 11:28:19 +0200 Subject: [PATCH] Merge pull request #3727 from overleaf/ab-ta-unique-referal-id Generate User referal_id using longer and more complex token to avoid duplicates GitOrigin-RevId: 302515b0250fec875dcb7b3a505c1c7be4189e2b --- .../Features/Project/ProjectDetailsHandler.js | 6 +-- .../TokenGenerator.js} | 49 +++++++++---------- services/web/app/src/models/User.js | 4 +- .../src/Project/ProjectDetailsHandlerTests.js | 15 +++--- 4 files changed, 35 insertions(+), 39 deletions(-) rename services/web/app/src/Features/{Project/ProjectTokenGenerator.js => TokenGenerator/TokenGenerator.js} (68%) diff --git a/services/web/app/src/Features/Project/ProjectDetailsHandler.js b/services/web/app/src/Features/Project/ProjectDetailsHandler.js index 09d8f39e52..783a817b94 100644 --- a/services/web/app/src/Features/Project/ProjectDetailsHandler.js +++ b/services/web/app/src/Features/Project/ProjectDetailsHandler.js @@ -6,7 +6,7 @@ const logger = require('logger-sharelatex') const TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender') const PublicAccessLevels = require('../Authorization/PublicAccessLevels') const Errors = require('../Errors/Errors') -const ProjectTokenGenerator = require('./ProjectTokenGenerator') +const TokenGenerator = require('../TokenGenerator/TokenGenerator') const ProjectHelper = require('./ProjectHelper') const settings = require('settings-sharelatex') const { callbackify } = require('util') @@ -236,11 +236,11 @@ async function _generateTokens(project, callback) { } const { tokens } = project if (tokens.readAndWrite == null) { - const { token, numericPrefix } = ProjectTokenGenerator.readAndWriteToken() + const { token, numericPrefix } = TokenGenerator.readAndWriteToken() tokens.readAndWrite = token tokens.readAndWritePrefix = numericPrefix } if (tokens.readOnly == null) { - tokens.readOnly = await ProjectTokenGenerator.promises.generateUniqueReadOnlyToken() + tokens.readOnly = await TokenGenerator.promises.generateUniqueReadOnlyToken() } } diff --git a/services/web/app/src/Features/Project/ProjectTokenGenerator.js b/services/web/app/src/Features/TokenGenerator/TokenGenerator.js similarity index 68% rename from services/web/app/src/Features/Project/ProjectTokenGenerator.js rename to services/web/app/src/Features/TokenGenerator/TokenGenerator.js index 60fdd01cb4..d2ae7ef390 100644 --- a/services/web/app/src/Features/Project/ProjectTokenGenerator.js +++ b/services/web/app/src/Features/TokenGenerator/TokenGenerator.js @@ -16,19 +16,21 @@ const Features = require('../../infrastructure/Features') const Async = require('async') const { promisify } = require('util') +// (From Overleaf `random_token.rb`) +// Letters (not numbers! see generate_token) used in tokens. They're all +// consonants, to avoid embarassing words (I can't think of any that use only +// a y), and lower case "l" is omitted, because in many fonts it is +// indistinguishable from an upper case "I" (and sometimes even the number 1). +const TOKEN_LOWERCASE_ALPHA = 'bcdfghjkmnpqrstvwxyz' +const TOKEN_NUMERICS = '123456789' +const TOKEN_ALPHANUMERICS = + TOKEN_LOWERCASE_ALPHA + TOKEN_LOWERCASE_ALPHA.toUpperCase() + TOKEN_NUMERICS + // This module mirrors the token generation in Overleaf (`random_token.rb`), // for the purposes of implementing token-based project access, like the // 'unlisted-projects' feature in Overleaf -const ProjectTokenGenerator = { - // (From Overleaf `random_token.rb`) - // Letters (not numbers! see generate_token) used in tokens. They're all - // consonants, to avoid embarassing words (I can't think of any that use only - // a y), and lower case "l" is omitted, because in many fonts it is - // indistinguishable from an upper case "I" (and sometimes even the number 1). - TOKEN_ALPHA: 'bcdfghjkmnpqrstvwxyz', - TOKEN_NUMERICS: '123456789', - +const TokenGenerator = { _randomString(length, alphabet) { const result = crypto .randomBytes(length) @@ -38,30 +40,25 @@ const ProjectTokenGenerator = { return result }, - // Generate a 12-char token with only characters from TOKEN_ALPHA, + // Generate a 12-char token with only characters from TOKEN_LOWERCASE_ALPHA, // suitable for use as a read-only token for a project readOnlyToken() { - return ProjectTokenGenerator._randomString( - 12, - ProjectTokenGenerator.TOKEN_ALPHA - ) + return TokenGenerator._randomString(12, TOKEN_LOWERCASE_ALPHA) }, // Generate a longer token, with a numeric prefix, // suitable for use as a read-and-write token for a project readAndWriteToken() { - const numerics = ProjectTokenGenerator._randomString( - 10, - ProjectTokenGenerator.TOKEN_NUMERICS - ) - const token = ProjectTokenGenerator._randomString( - 12, - ProjectTokenGenerator.TOKEN_ALPHA - ) + const numerics = TokenGenerator._randomString(10, TOKEN_NUMERICS) + const token = TokenGenerator._randomString(12, TOKEN_LOWERCASE_ALPHA) const fullToken = `${numerics}${token}` return { token: fullToken, numericPrefix: numerics } }, + generateReferralId() { + return TokenGenerator._randomString(16, TOKEN_ALPHANUMERICS) + }, + generateUniqueReadOnlyToken(callback) { if (callback == null) { callback = function(err, token) {} @@ -69,7 +66,7 @@ const ProjectTokenGenerator = { return Async.retry( 10, function(cb) { - const token = ProjectTokenGenerator.readOnlyToken() + const token = TokenGenerator.readOnlyToken() if (!Features.hasFeature('overleaf-integration')) { return cb(null, token) @@ -104,9 +101,9 @@ const ProjectTokenGenerator = { } } -ProjectTokenGenerator.promises = { +TokenGenerator.promises = { generateUniqueReadOnlyToken: promisify( - ProjectTokenGenerator.generateUniqueReadOnlyToken + TokenGenerator.generateUniqueReadOnlyToken ) } -module.exports = ProjectTokenGenerator +module.exports = TokenGenerator diff --git a/services/web/app/src/models/User.js b/services/web/app/src/models/User.js index 38773fc4a0..1a9e82bbbb 100644 --- a/services/web/app/src/models/User.js +++ b/services/web/app/src/models/User.js @@ -1,6 +1,6 @@ const Settings = require('settings-sharelatex') const mongoose = require('../infrastructure/Mongoose') -const uuid = require('uuid') +const TokenGenerator = require('../Features/TokenGenerator/TokenGenerator') const { Schema } = mongoose const { ObjectId } = Schema @@ -136,7 +136,7 @@ const UserSchema = new Schema({ referal_id: { type: String, default() { - return uuid.v4().split('-')[0] + return TokenGenerator.generateReferralId() } }, refered_users: [{ type: ObjectId, ref: 'User' }], diff --git a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js index f2357a8bd5..47e01dce57 100644 --- a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js @@ -56,7 +56,7 @@ describe('ProjectDetailsHandler', function() { moveEntity: sinon.stub().resolves() } } - this.ProjectTokenGenerator = { + this.TokenGenerator = { readAndWriteToken: sinon.stub(), promises: { generateUniqueReadOnlyToken: sinon.stub() @@ -82,7 +82,7 @@ describe('ProjectDetailsHandler', function() { warn() {}, err() {} }, - './ProjectTokenGenerator': this.ProjectTokenGenerator, + '../TokenGenerator/TokenGenerator': this.TokenGenerator, 'settings-sharelatex': this.settings } }) @@ -484,10 +484,10 @@ describe('ProjectDetailsHandler', function() { this.readOnlyToken = 'abc' this.readAndWriteToken = '42def' this.readAndWriteTokenPrefix = '42' - this.ProjectTokenGenerator.promises.generateUniqueReadOnlyToken.resolves( + this.TokenGenerator.promises.generateUniqueReadOnlyToken.resolves( this.readOnlyToken ) - this.ProjectTokenGenerator.readAndWriteToken.returns({ + this.TokenGenerator.readAndWriteToken.returns({ token: this.readAndWriteToken, numericPrefix: this.readAndWriteTokenPrefix }) @@ -506,10 +506,9 @@ describe('ProjectDetailsHandler', function() { it('should update the project with new tokens', async function() { await this.handler.promises.ensureTokensArePresent(this.project._id) - expect(this.ProjectTokenGenerator.promises.generateUniqueReadOnlyToken) - .to.have.been.calledOnce - expect(this.ProjectTokenGenerator.readAndWriteToken).to.have.been - .calledOnce + expect(this.TokenGenerator.promises.generateUniqueReadOnlyToken).to.have + .been.calledOnce + expect(this.TokenGenerator.readAndWriteToken).to.have.been.calledOnce expect(this.ProjectModel.updateOne).to.have.been.calledOnce expect(this.ProjectModel.updateOne).to.have.been.calledWith( { _id: this.project._id },