From b3d55fa65e77dcf849b94cfd2aa56a54885597a2 Mon Sep 17 00:00:00 2001 From: Davinder Singh Date: Thu, 7 Apr 2022 14:41:05 +0100 Subject: [PATCH] Move admin register to user activate module Move admin register to user activate module Co-authored-by: John Lees-Miller & Davinder Singh GitOrigin-RevId: 79428f2932783086435bdad9b1efb5300c467511 --- .../Features/ServerAdmin/AdminController.js | 4 --- .../app/src/Features/User/UserController.js | 20 ----------- services/web/app/src/router.js | 17 +--------- .../app/src/UserActivateController.js | 24 +++++++++++++ .../app/src/UserActivateRouter.js | 18 ++++++++++ .../app/views/user}/register.pug | 2 +- .../unit/src/UserActivateControllerTests.js | 34 ++++++++++++++++++- .../test/unit/src/User/UserControllerTests.js | 28 --------------- 8 files changed, 77 insertions(+), 70 deletions(-) rename services/web/{app/views/admin => modules/user-activate/app/views/user}/register.pug (97%) diff --git a/services/web/app/src/Features/ServerAdmin/AdminController.js b/services/web/app/src/Features/ServerAdmin/AdminController.js index 6f8e8b50bb..11f09b5313 100644 --- a/services/web/app/src/Features/ServerAdmin/AdminController.js +++ b/services/web/app/src/Features/ServerAdmin/AdminController.js @@ -101,10 +101,6 @@ const AdminController = { }) }, - registerNewUser(req, res, next) { - return res.render('admin/register') - }, - disconnectAllUsers: (req, res) => { logger.warn('disconecting everyone') const delay = (req.query && req.query.delay) > 0 ? req.query.delay : 10 diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index e36fae0364..5ed1e022c6 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/services/web/app/src/Features/User/UserController.js @@ -3,7 +3,6 @@ const UserDeleter = require('./UserDeleter') const UserGetter = require('./UserGetter') const { User } = require('../../models/User') const NewsletterManager = require('../Newsletter/NewsletterManager') -const UserRegistrationHandler = require('./UserRegistrationHandler') const logger = require('@overleaf/logger') const metrics = require('@overleaf/metrics') const AuthenticationManager = require('../Authentication/AuthenticationManager') @@ -463,25 +462,6 @@ const UserController = { }) }, - register(req, res, next) { - const { email } = req.body - if (email == null || email === '') { - return res.sendStatus(422) // Unprocessable Entity - } - UserRegistrationHandler.registerNewUserAndSendActivationEmail( - email, - (error, user, setNewPasswordUrl) => { - if (error != null) { - return next(error) - } - res.json({ - email: user.email, - setNewPasswordUrl, - }) - } - ) - }, - changePassword: expressify(changePassword), } diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 72bfb4bf9a..17467d77b0 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -1010,27 +1010,12 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { } }) - // Admin Stuff webRouter.get( '/admin', AuthorizationMiddleware.ensureUserIsSiteAdmin, AdminController.index ) - webRouter.get( - '/admin/user', - AuthorizationMiddleware.ensureUserIsSiteAdmin, - (req, res) => res.redirect('/admin/register') - ) // this gets removed by admin-panel addon - webRouter.get( - '/admin/register', - AuthorizationMiddleware.ensureUserIsSiteAdmin, - AdminController.registerNewUser - ) - webRouter.post( - '/admin/register', - AuthorizationMiddleware.ensureUserIsSiteAdmin, - UserController.register - ) + if (!Features.hasFeature('saas')) { webRouter.post( '/admin/openEditor', diff --git a/services/web/modules/user-activate/app/src/UserActivateController.js b/services/web/modules/user-activate/app/src/UserActivateController.js index 1da5304503..5399a0cf6f 100644 --- a/services/web/modules/user-activate/app/src/UserActivateController.js +++ b/services/web/modules/user-activate/app/src/UserActivateController.js @@ -1,8 +1,32 @@ const Path = require('path') const UserGetter = require('../../../../app/src/Features/User/UserGetter') +const UserRegistrationHandler = require('../../../../app/src/Features/User/UserRegistrationHandler') const ErrorController = require('../../../../app/src/Features/Errors/ErrorController') module.exports = { + registerNewUser(req, res, next) { + res.render(Path.resolve(__dirname, '../views/user/register')) + }, + + register(req, res, next) { + const { email } = req.body + if (email == null || email === '') { + return res.sendStatus(422) // Unprocessable Entity + } + UserRegistrationHandler.registerNewUserAndSendActivationEmail( + email, + (error, user, setNewPasswordUrl) => { + if (error != null) { + return next(error) + } + res.json({ + email: user.email, + setNewPasswordUrl, + }) + } + ) + }, + activateAccountPage(req, res, next) { // An 'activation' is actually just a password reset on an account that // was set with a random password originally. diff --git a/services/web/modules/user-activate/app/src/UserActivateRouter.js b/services/web/modules/user-activate/app/src/UserActivateRouter.js index a63cb9d1ad..c6294ed04c 100644 --- a/services/web/modules/user-activate/app/src/UserActivateRouter.js +++ b/services/web/modules/user-activate/app/src/UserActivateRouter.js @@ -1,12 +1,30 @@ const logger = require('@overleaf/logger') const UserActivateController = require('./UserActivateController') const AuthenticationController = require('../../../../app/src/Features/Authentication/AuthenticationController') +const AuthorizationMiddleware = require('../../../../app/src/Features/Authorization/AuthorizationMiddleware') module.exports = { apply(webRouter) { logger.log({}, 'Init UserActivate router') + webRouter.get( + '/admin/user', + AuthorizationMiddleware.ensureUserIsSiteAdmin, + (req, res) => res.redirect('/admin/register') + ) + webRouter.get('/user/activate', UserActivateController.activateAccountPage) AuthenticationController.addEndpointToLoginWhitelist('/user/activate') + + webRouter.get( + '/admin/register', + AuthorizationMiddleware.ensureUserIsSiteAdmin, + UserActivateController.registerNewUser + ) + webRouter.post( + '/admin/register', + AuthorizationMiddleware.ensureUserIsSiteAdmin, + UserActivateController.register + ) }, } diff --git a/services/web/app/views/admin/register.pug b/services/web/modules/user-activate/app/views/user/register.pug similarity index 97% rename from services/web/app/views/admin/register.pug rename to services/web/modules/user-activate/app/views/user/register.pug index 4078c49484..3dc0921a06 100644 --- a/services/web/app/views/admin/register.pug +++ b/services/web/modules/user-activate/app/views/user/register.pug @@ -1,4 +1,4 @@ -extends ../layout +extends ../../../../../app/views/layout block content .content.content-alt diff --git a/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.js b/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.js index bc68c9276b..9259b48043 100644 --- a/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.js +++ b/services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.js @@ -17,21 +17,27 @@ describe('UserActivateController', function () { } this.UserGetter = { getUser: sinon.stub() } + this.UserRegistrationHandler = {} this.ErrorController = { notFound: sinon.stub() } this.UserActivateController = SandboxedModule.require(MODULE_PATH, { requires: { '../../../../app/src/Features/User/UserGetter': this.UserGetter, + '../../../../app/src/Features/User/UserRegistrationHandler': + this.UserRegistrationHandler, '../../../../app/src/Features/Errors/ErrorController': this.ErrorController, }, }) this.req = { + body: {}, query: {}, session: { user: this.user, }, } - this.res = {} + this.res = { + json: sinon.stub(), + } }) describe('activateAccountPage', function () { @@ -86,4 +92,30 @@ describe('UserActivateController', function () { this.UserActivateController.activateAccountPage(this.req, this.res) }) }) + + describe('register', function () { + beforeEach(function () { + this.UserRegistrationHandler.registerNewUserAndSendActivationEmail = sinon + .stub() + .callsArgWith(1, null, this.user, (this.url = 'mock/url')) + this.req.body.email = this.user.email = this.email = 'email@example.com' + this.UserActivateController.register(this.req, this.res) + }) + + it('should register the user and send them an email', function () { + sinon.assert.calledWith( + this.UserRegistrationHandler.registerNewUserAndSendActivationEmail, + this.email + ) + }) + + it('should return the user and activation url', function () { + this.res.json + .calledWith({ + email: this.email, + setNewPasswordUrl: this.url, + }) + .should.equal(true) + }) + }) }) diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index 8440e6c4e4..aebbd9bd5e 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/services/web/test/unit/src/User/UserControllerTests.js @@ -43,7 +43,6 @@ describe('UserController', function () { } this.User = { findById: sinon.stub().callsArgWith(1, null, this.user) } this.NewsLetterManager = { unsubscribe: sinon.stub().callsArgWith(1) } - this.UserRegistrationHandler = { registerNewUser: sinon.stub() } this.AuthenticationController = { establishUserSession: sinon.stub().callsArg(2), } @@ -104,7 +103,6 @@ describe('UserController', function () { User: this.User, }, '../Newsletter/NewsletterManager': this.NewsLetterManager, - './UserRegistrationHandler': this.UserRegistrationHandler, '../Authentication/AuthenticationController': this.AuthenticationController, '../Authentication/SessionManager': this.SessionManager, @@ -558,32 +556,6 @@ describe('UserController', function () { }) }) - describe('register', function () { - beforeEach(function () { - this.UserRegistrationHandler.registerNewUserAndSendActivationEmail = sinon - .stub() - .callsArgWith(1, null, this.user, (this.url = 'mock/url')) - this.req.body.email = this.user.email = this.email = 'email@example.com' - this.UserController.register(this.req, this.res) - }) - - it('should register the user and send them an email', function () { - sinon.assert.calledWith( - this.UserRegistrationHandler.registerNewUserAndSendActivationEmail, - this.email - ) - }) - - it('should return the user and activation url', function () { - this.res.json - .calledWith({ - email: this.email, - setNewPasswordUrl: this.url, - }) - .should.equal(true) - }) - }) - describe('clearSessions', function () { describe('success', function () { it('should call revokeAllUserSessions', function (done) {