[misc] move /user/activate into a module (#2962)

* [misc] move /user/activate into a module

Co-Authored-By: Nate Stemen <nate.stemen@overleaf.com>

* [misc] setup copybara for the new user-activate module

* [misc] move the /user/activate route behind a feature flag

...which is by default enabled.

Co-authored-by: Nate Stemen <nate.stemen@overleaf.com>
GitOrigin-RevId: 87fc5ae869a7e282ffdbeea0ff7b7c55b8b9b31b
This commit is contained in:
Jakob Ackermann 2020-07-15 12:46:32 +02:00 committed by Copybot
parent c660497859
commit 53927bca95
9 changed files with 157 additions and 96 deletions

View file

@ -1,6 +1,5 @@
const UserGetter = require('./UserGetter')
const UserSessionsManager = require('./UserSessionsManager')
const ErrorController = require('../Errors/ErrorController')
const logger = require('logger-sharelatex')
const Settings = require('settings-sharelatex')
const AuthenticationController = require('../Authentication/AuthenticationController')
@ -27,44 +26,6 @@ const UserPagesController = {
})
},
activateAccountPage(req, res, next) {
// An 'activation' is actually just a password reset on an account that
// was set with a random password originally.
if (req.query.user_id == null || req.query.token == null) {
return ErrorController.notFound(req, res)
}
if (typeof req.query.user_id !== 'string') {
return ErrorController.forbidden(req, res)
}
UserGetter.getUser(
req.query.user_id,
{ email: 1, loginCount: 1 },
(error, user) => {
if (error != null) {
return next(error)
}
if (!user) {
return ErrorController.notFound(req, res)
}
if (user.loginCount > 0) {
// Already seen this user, so account must be activate
// This lets users keep clicking the 'activate' link in their email
// as a way to log in which, if I know our users, they will.
res.redirect(`/login`)
} else {
req.session.doLoginAfterPasswordReset = true
res.render('user/activate', {
title: 'activate_account',
email: user.email,
token: req.query.token
})
}
}
)
},
loginPage(req, res) {
// if user is being sent to /login with explicit redirect (redir=/foo),
// such as being sent from the editor to /login, then set the redirect explicitly

View file

@ -107,9 +107,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
)
}
webRouter.get('/user/activate', UserPagesController.activateAccountPage)
AuthenticationController.addEndpointToLoginWhitelist('/user/activate')
webRouter.get(
'/system/messages',
AuthenticationController.requireLogin(),

View file

@ -558,6 +558,8 @@ module.exports = settings =
"/templates/index": "/templates/"
reloadModuleViewsOnEachRequest: process.env['NODE_ENV'] == 'development'
disableModule:
'user-activate': process.env['DISABLE_MODULE_USER_ACTIVATE'] == 'true'
domainLicences: [

View file

@ -0,0 +1,43 @@
const Path = require('path')
const UserGetter = require('../../../../app/src/Features/User/UserGetter')
const ErrorController = require('../../../../app/src/Features/Errors/ErrorController')
module.exports = {
activateAccountPage(req, res, next) {
// An 'activation' is actually just a password reset on an account that
// was set with a random password originally.
if (req.query.user_id == null || req.query.token == null) {
return ErrorController.notFound(req, res)
}
if (typeof req.query.user_id !== 'string') {
return ErrorController.forbidden(req, res)
}
UserGetter.getUser(
req.query.user_id,
{ email: 1, loginCount: 1 },
(error, user) => {
if (error != null) {
return next(error)
}
if (!user) {
return ErrorController.notFound(req, res)
}
if (user.loginCount > 0) {
// Already seen this user, so account must be activate
// This lets users keep clicking the 'activate' link in their email
// as a way to log in which, if I know our users, they will.
res.redirect(`/login`)
} else {
req.session.doLoginAfterPasswordReset = true
res.render(Path.resolve(__dirname, '../views/user/activate'), {
title: 'activate_account',
email: user.email,
token: req.query.token
})
}
}
)
}
}

View file

@ -0,0 +1,17 @@
const logger = require('logger-sharelatex')
const Settings = require('settings-sharelatex')
const UserActivateController = require('./UserActivateController')
const AuthenticationController = require('../../../../app/src/Features/Authentication/AuthenticationController')
module.exports = {
apply(webRouter) {
if (Settings.disableModule['user-activate']) {
logger.log({}, 'Skipping Init UserActivate router')
return
}
logger.log({}, 'Init UserActivate router')
webRouter.get('/user/activate', UserActivateController.activateAccountPage)
AuthenticationController.addEndpointToLoginWhitelist('/user/activate')
}
}

View file

@ -1,4 +1,4 @@
extends ../layout
extends ../../../../app/views/layout
block content
.content.content-alt

View file

@ -0,0 +1,2 @@
const UserActivateRouter = require('./app/src/UserActivateRouter')
module.exports = { router: UserActivateRouter }

View file

@ -0,0 +1,92 @@
const Path = require('path')
const SandboxedModule = require('sandboxed-module')
const sinon = require('sinon')
const MODULE_PATH = Path.join(
__dirname,
'../../../app/src/UserActivateController.js'
)
const VIEW_PATH = Path.join(__dirname, '../../../app/views/user/activate')
describe('UserActivateController', function() {
beforeEach(function() {
this.user = {
_id: (this.user_id = 'kwjewkl'),
features: {},
email: 'joe@example.com'
}
this.UserGetter = { getUser: sinon.stub() }
this.ErrorController = { notFound: sinon.stub() }
this.UserActivateController = SandboxedModule.require(MODULE_PATH, {
globals: {
console: console
},
requires: {
'../../../../app/src/Features/User/UserGetter': this.UserGetter,
'../../../../app/src/Features/Errors/ErrorController': this
.ErrorController
}
})
this.req = {
query: {},
session: {
user: this.user
}
}
this.res = {}
})
describe('activateAccountPage', function() {
beforeEach(function() {
this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user)
this.req.query.user_id = this.user_id
this.req.query.token = this.token = 'mock-token-123'
})
it('should 404 without a user_id', function(done) {
delete this.req.query.user_id
this.ErrorController.notFound = () => done()
this.UserActivateController.activateAccountPage(this.req, this.res)
})
it('should 404 without a token', function(done) {
delete this.req.query.token
this.ErrorController.notFound = () => done()
this.UserActivateController.activateAccountPage(this.req, this.res)
})
it('should 404 without a valid user_id', function(done) {
this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, null)
this.ErrorController.notFound = () => done()
this.UserActivateController.activateAccountPage(this.req, this.res)
})
it('should 403 for complex user_id', function(done) {
this.ErrorController.forbidden = () => done()
this.req.query.user_id = { first_name: 'X' }
this.UserActivateController.activateAccountPage(this.req, this.res)
})
it('should redirect activated users to login', function(done) {
this.user.loginCount = 1
this.res.redirect = url => {
this.UserGetter.getUser.calledWith(this.user_id).should.equal(true)
url.should.equal('/login')
return done()
}
this.UserActivateController.activateAccountPage(this.req, this.res)
})
it('render the activation page if the user has not logged in before', function(done) {
this.user.loginCount = 0
this.res.render = (page, opts) => {
page.should.equal(VIEW_PATH)
opts.email.should.equal(this.user.email)
opts.token.should.equal(this.token)
return done()
}
this.UserActivateController.activateAccountPage(this.req, this.res)
})
})
})

View file

@ -292,57 +292,4 @@ describe('UserPagesController', function() {
})
})
})
describe('activateAccountPage', function() {
beforeEach(function() {
this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user)
this.req.query.user_id = this.user_id
return (this.req.query.token = this.token = 'mock-token-123')
})
it('should 404 without a user_id', function(done) {
delete this.req.query.user_id
this.ErrorController.notFound = () => done()
return this.UserPagesController.activateAccountPage(this.req, this.res)
})
it('should 404 without a token', function(done) {
delete this.req.query.token
this.ErrorController.notFound = () => done()
return this.UserPagesController.activateAccountPage(this.req, this.res)
})
it('should 404 without a valid user_id', function(done) {
this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, null)
this.ErrorController.notFound = () => done()
return this.UserPagesController.activateAccountPage(this.req, this.res)
})
it('should 403 for complex user_id', function(done) {
this.ErrorController.forbidden = () => done()
this.req.query.user_id = { first_name: 'X' }
return this.UserPagesController.activateAccountPage(this.req, this.res)
})
it('should redirect activated users to login', function(done) {
this.user.loginCount = 1
this.res.redirect = url => {
this.UserGetter.getUser.calledWith(this.user_id).should.equal(true)
url.should.equal('/login')
return done()
}
return this.UserPagesController.activateAccountPage(this.req, this.res)
})
it('render the activation page if the user has not logged in before', function(done) {
this.user.loginCount = 0
this.res.render = (page, opts) => {
page.should.equal('user/activate')
opts.email.should.equal(this.user.email)
opts.token.should.equal(this.token)
return done()
}
return this.UserPagesController.activateAccountPage(this.req, this.res)
})
})
})