From d1ab89deb79f70c439b58750d044d9cadf1194e5 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 11 Jun 2019 11:54:33 +0200 Subject: [PATCH] Handle email update on server --- .github/FUNDING.yml | 1 + server/controllers/api/users/index.ts | 18 +++--- server/controllers/api/users/me.ts | 16 ++++- server/initializers/constants.ts | 2 +- .../migrations/0390-user-pending-email.ts | 25 ++++++++ server/lib/user.ts | 18 +++++- server/middlewares/validators/users.ts | 39 ++++++++---- server/models/account/user.ts | 6 ++ server/tests/api/server/email.ts | 2 +- server/tests/api/users/users-verification.ts | 59 ++++++++++++++++++- shared/extra-utils/users/users.ts | 7 ++- shared/models/users/user.model.ts | 1 + 12 files changed, 164 insertions(+), 30 deletions(-) create mode 100644 .github/FUNDING.yml create mode 100644 server/initializers/migrations/0390-user-pending-email.ts diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 000000000..cece65761 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1 @@ +custom: https://framasoft.org/en/#soutenir diff --git a/server/controllers/api/users/index.ts b/server/controllers/api/users/index.ts index 99f51a648..c1d72087c 100644 --- a/server/controllers/api/users/index.ts +++ b/server/controllers/api/users/index.ts @@ -6,7 +6,7 @@ import { getFormattedObjects } from '../../../helpers/utils' import { RATES_LIMIT, WEBSERVER } from '../../../initializers/constants' import { Emailer } from '../../../lib/emailer' import { Redis } from '../../../lib/redis' -import { createUserAccountAndChannelAndPlaylist } from '../../../lib/user' +import { createUserAccountAndChannelAndPlaylist, sendVerifyUserEmail } from '../../../lib/user' import { asyncMiddleware, asyncRetryTransactionMiddleware, @@ -147,7 +147,7 @@ usersRouter.post('/:id/reset-password', usersRouter.post('/ask-send-verify-email', askSendEmailLimiter, asyncMiddleware(usersAskSendVerifyEmailValidator), - asyncMiddleware(askSendVerifyUserEmail) + asyncMiddleware(reSendVerifyUserEmail) ) usersRouter.post('/:id/verify-email', @@ -320,14 +320,7 @@ async function resetUserPassword (req: express.Request, res: express.Response) { return res.status(204).end() } -async function sendVerifyUserEmail (user: UserModel) { - const verificationString = await Redis.Instance.setVerifyEmailVerificationString(user.id) - const url = WEBSERVER.URL + '/verify-account/email?userId=' + user.id + '&verificationString=' + verificationString - await Emailer.Instance.addVerifyEmailJob(user.email, url) - return -} - -async function askSendVerifyUserEmail (req: express.Request, res: express.Response) { +async function reSendVerifyUserEmail (req: express.Request, res: express.Response) { const user = res.locals.user await sendVerifyUserEmail(user) @@ -339,6 +332,11 @@ async function verifyUserEmail (req: express.Request, res: express.Response) { const user = res.locals.user user.emailVerified = true + if (req.body.isPendingEmail === true) { + user.email = user.pendingEmail + user.pendingEmail = null + } + await user.save() return res.status(204).end() diff --git a/server/controllers/api/users/me.ts b/server/controllers/api/users/me.ts index ddb239e7b..1750a02e9 100644 --- a/server/controllers/api/users/me.ts +++ b/server/controllers/api/users/me.ts @@ -28,6 +28,7 @@ import { VideoImportModel } from '../../../models/video/video-import' import { AccountModel } from '../../../models/account/account' import { CONFIG } from '../../../initializers/config' import { sequelizeTypescript } from '../../../initializers/database' +import { sendVerifyUserEmail } from '../../../lib/user' const auditLogger = auditLoggerFactory('users-me') @@ -171,17 +172,26 @@ async function deleteMe (req: express.Request, res: express.Response) { async function updateMe (req: express.Request, res: express.Response) { const body: UserUpdateMe = req.body + let sendVerificationEmail = false const user = res.locals.oauth.token.user const oldUserAuditView = new UserAuditView(user.toFormattedJSON({})) if (body.password !== undefined) user.password = body.password - if (body.email !== undefined) user.email = body.email if (body.nsfwPolicy !== undefined) user.nsfwPolicy = body.nsfwPolicy if (body.webTorrentEnabled !== undefined) user.webTorrentEnabled = body.webTorrentEnabled if (body.autoPlayVideo !== undefined) user.autoPlayVideo = body.autoPlayVideo if (body.videosHistoryEnabled !== undefined) user.videosHistoryEnabled = body.videosHistoryEnabled + if (body.email !== undefined) { + if (CONFIG.SIGNUP.REQUIRES_EMAIL_VERIFICATION) { + user.pendingEmail = body.email + sendVerificationEmail = true + } else { + user.email = body.email + } + } + await sequelizeTypescript.transaction(async t => { const userAccount = await AccountModel.load(user.Account.id) @@ -196,6 +206,10 @@ async function updateMe (req: express.Request, res: express.Response) { auditLogger.update(getAuditIdFromRes(res), new UserAuditView(user.toFormattedJSON({})), oldUserAuditView) }) + if (sendVerificationEmail === true) { + await sendVerifyUserEmail(user, true) + } + return res.sendStatus(204) } diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index be30be463..c2b8eff95 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -14,7 +14,7 @@ import { CONFIG, registerConfigChangedHandler } from './config' // --------------------------------------------------------------------------- -const LAST_MIGRATION_VERSION = 385 +const LAST_MIGRATION_VERSION = 390 // --------------------------------------------------------------------------- diff --git a/server/initializers/migrations/0390-user-pending-email.ts b/server/initializers/migrations/0390-user-pending-email.ts new file mode 100644 index 000000000..5ca871746 --- /dev/null +++ b/server/initializers/migrations/0390-user-pending-email.ts @@ -0,0 +1,25 @@ +import * as Sequelize from 'sequelize' + +async function up (utils: { + transaction: Sequelize.Transaction, + queryInterface: Sequelize.QueryInterface, + sequelize: Sequelize.Sequelize, + db: any +}): Promise { + const data = { + type: Sequelize.STRING(400), + allowNull: true, + defaultValue: null + } + + await utils.queryInterface.addColumn('user', 'pendingEmail', data) +} + +function down (options) { + throw new Error('Not implemented.') +} + +export { + up, + down +} diff --git a/server/lib/user.ts b/server/lib/user.ts index b50b09d72..0e4007770 100644 --- a/server/lib/user.ts +++ b/server/lib/user.ts @@ -1,6 +1,6 @@ import * as uuidv4 from 'uuid/v4' import { ActivityPubActorType } from '../../shared/models/activitypub' -import { SERVER_ACTOR_NAME } from '../initializers/constants' +import { SERVER_ACTOR_NAME, WEBSERVER } from '../initializers/constants' import { AccountModel } from '../models/account/account' import { UserModel } from '../models/account/user' import { buildActorInstance, getAccountActivityPubUrl, setAsyncActorKeys } from './activitypub' @@ -12,6 +12,8 @@ import { UserNotificationSetting, UserNotificationSettingValue } from '../../sha import { createWatchLaterPlaylist } from './video-playlist' import { sequelizeTypescript } from '../initializers/database' import { Transaction } from 'sequelize/types' +import { Redis } from './redis' +import { Emailer } from './emailer' type ChannelNames = { name: string, displayName: string } async function createUserAccountAndChannelAndPlaylist (parameters: { @@ -100,12 +102,24 @@ async function createApplicationActor (applicationId: number) { return accountCreated } +async function sendVerifyUserEmail (user: UserModel, isPendingEmail = false) { + const verificationString = await Redis.Instance.setVerifyEmailVerificationString(user.id) + let url = WEBSERVER.URL + '/verify-account/email?userId=' + user.id + '&verificationString=' + verificationString + + if (isPendingEmail) url += '&isPendingEmail=true' + + const email = isPendingEmail ? user.pendingEmail : user.email + + await Emailer.Instance.addVerifyEmailJob(email, url) +} + // --------------------------------------------------------------------------- export { createApplicationActor, createUserAccountAndChannelAndPlaylist, - createLocalAccountWithoutKeys + createLocalAccountWithoutKeys, + sendVerifyUserEmail } // --------------------------------------------------------------------------- diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index b4e09c9b7..a4d4ae46d 100644 --- a/server/middlewares/validators/users.ts +++ b/server/middlewares/validators/users.ts @@ -27,7 +27,6 @@ import { areValidationErrors } from './utils' import { ActorModel } from '../../models/activitypub/actor' import { isActorPreferredUsernameValid } from '../../helpers/custom-validators/activitypub/actor' import { isVideoChannelNameValid } from '../../helpers/custom-validators/video-channels' -import { UserCreate } from '../../../shared/models/users' import { UserRegister } from '../../../shared/models/users/user-register.model' const usersAddValidator = [ @@ -178,13 +177,27 @@ const usersUpdateValidator = [ ] const usersUpdateMeValidator = [ - body('displayName').optional().custom(isUserDisplayNameValid).withMessage('Should have a valid display name'), - body('description').optional().custom(isUserDescriptionValid).withMessage('Should have a valid description'), - body('currentPassword').optional().custom(isUserPasswordValid).withMessage('Should have a valid current password'), - body('password').optional().custom(isUserPasswordValid).withMessage('Should have a valid password'), - body('email').optional().isEmail().withMessage('Should have a valid email attribute'), - body('nsfwPolicy').optional().custom(isUserNSFWPolicyValid).withMessage('Should have a valid display Not Safe For Work policy'), - body('autoPlayVideo').optional().custom(isUserAutoPlayVideoValid).withMessage('Should have a valid automatically plays video attribute'), + body('displayName') + .optional() + .custom(isUserDisplayNameValid).withMessage('Should have a valid display name'), + body('description') + .optional() + .custom(isUserDescriptionValid).withMessage('Should have a valid description'), + body('currentPassword') + .optional() + .custom(isUserPasswordValid).withMessage('Should have a valid current password'), + body('password') + .optional() + .custom(isUserPasswordValid).withMessage('Should have a valid password'), + body('email') + .optional() + .isEmail().withMessage('Should have a valid email attribute'), + body('nsfwPolicy') + .optional() + .custom(isUserNSFWPolicyValid).withMessage('Should have a valid display Not Safe For Work policy'), + body('autoPlayVideo') + .optional() + .custom(isUserAutoPlayVideoValid).withMessage('Should have a valid automatically plays video attribute'), body('videosHistoryEnabled') .optional() .custom(isUserVideosHistoryEnabledValid).withMessage('Should have a valid videos history enabled attribute'), @@ -329,8 +342,14 @@ const usersAskSendVerifyEmailValidator = [ ] const usersVerifyEmailValidator = [ - param('id').isInt().not().isEmpty().withMessage('Should have a valid id'), - body('verificationString').not().isEmpty().withMessage('Should have a valid verification string'), + param('id') + .isInt().not().isEmpty().withMessage('Should have a valid id'), + + body('verificationString') + .not().isEmpty().withMessage('Should have a valid verification string'), + body('isPendingEmail') + .optional() + .toBoolean(), async (req: express.Request, res: express.Response, next: express.NextFunction) => { logger.debug('Checking usersVerifyEmail parameters', { parameters: req.params }) diff --git a/server/models/account/user.ts b/server/models/account/user.ts index 4a9acd703..e75039521 100644 --- a/server/models/account/user.ts +++ b/server/models/account/user.ts @@ -113,6 +113,11 @@ export class UserModel extends Model { @Column(DataType.STRING(400)) email: string + @AllowNull(true) + @IsEmail + @Column(DataType.STRING(400)) + pendingEmail: string + @AllowNull(true) @Default(null) @Is('UserEmailVerified', value => throwIfNotValid(value, isUserEmailVerifiedValid, 'email verified boolean', true)) @@ -540,6 +545,7 @@ export class UserModel extends Model { id: this.id, username: this.username, email: this.email, + pendingEmail: this.pendingEmail, emailVerified: this.emailVerified, nsfwPolicy: this.nsfwPolicy, webTorrentEnabled: this.webTorrentEnabled, diff --git a/server/tests/api/server/email.ts b/server/tests/api/server/email.ts index 5929a3adb..7b7acfd12 100644 --- a/server/tests/api/server/email.ts +++ b/server/tests/api/server/email.ts @@ -250,7 +250,7 @@ describe('Test emails', function () { }) it('Should not verify the email with an invalid verification string', async function () { - await verifyEmail(server.url, userId, verificationString + 'b', 403) + await verifyEmail(server.url, userId, verificationString + 'b', false, 403) }) it('Should verify the email', async function () { diff --git a/server/tests/api/users/users-verification.ts b/server/tests/api/users/users-verification.ts index 3b37a26cf..b8fa1430b 100644 --- a/server/tests/api/users/users-verification.ts +++ b/server/tests/api/users/users-verification.ts @@ -3,18 +3,29 @@ import * as chai from 'chai' import 'mocha' import { - registerUser, flushTests, getUserInformation, getMyUserInformation, killallServers, - userLogin, login, flushAndRunServer, ServerInfo, verifyEmail, updateCustomSubConfig, wait, cleanupTests + cleanupTests, + flushAndRunServer, + getMyUserInformation, + getUserInformation, + login, + registerUser, + ServerInfo, + updateCustomSubConfig, + updateMyUser, + userLogin, + verifyEmail } from '../../../../shared/extra-utils' import { setAccessTokensToServers } from '../../../../shared/extra-utils/users/login' import { MockSmtpServer } from '../../../../shared/extra-utils/miscs/email' import { waitJobs } from '../../../../shared/extra-utils/server/jobs' +import { User } from '../../../../shared/models/users' const expect = chai.expect describe('Test users account verification', function () { let server: ServerInfo let userId: number + let userAccessToken: string let verificationString: string let expectedEmailsLength = 0 const user1 = { @@ -83,11 +94,53 @@ describe('Test users account verification', function () { it('Should verify the user via email and allow login', async function () { await verifyEmail(server.url, userId, verificationString) - await login(server.url, server.client, user1) + + const res = await login(server.url, server.client, user1) + userAccessToken = res.body.access_token + const resUserVerified = await getUserInformation(server.url, server.accessToken, userId) expect(resUserVerified.body.emailVerified).to.be.true }) + it('Should be able to change the user email', async function () { + let updateVerificationString: string + + { + await updateMyUser({ + url: server.url, + accessToken: userAccessToken, + email: 'updated@example.com' + }) + + await waitJobs(server) + expectedEmailsLength++ + expect(emails).to.have.lengthOf(expectedEmailsLength) + + const email = emails[expectedEmailsLength - 1] + + const verificationStringMatches = /verificationString=([a-z0-9]+)/.exec(email['text']) + updateVerificationString = verificationStringMatches[1] + } + + { + const res = await getMyUserInformation(server.url, userAccessToken) + const me: User = res.body + + expect(me.email).to.equal('user_1@example.com') + expect(me.pendingEmail).to.equal('updated@example.com') + } + + { + await verifyEmail(server.url, userId, updateVerificationString, true) + + const res = await getMyUserInformation(server.url, userAccessToken) + const me: User = res.body + + expect(me.email).to.equal('updated@example.com') + expect(me.pendingEmail).to.be.null + } + }) + it('Should register user not requiring email verification if setting not enabled', async function () { this.timeout(5000) await updateCustomSubConfig(server.url, server.accessToken, { diff --git a/shared/extra-utils/users/users.ts b/shared/extra-utils/users/users.ts index c09211b71..1c39881d6 100644 --- a/shared/extra-utils/users/users.ts +++ b/shared/extra-utils/users/users.ts @@ -323,13 +323,16 @@ function askSendVerifyEmail (url: string, email: string) { }) } -function verifyEmail (url: string, userId: number, verificationString: string, statusCodeExpected = 204) { +function verifyEmail (url: string, userId: number, verificationString: string, isPendingEmail = false, statusCodeExpected = 204) { const path = '/api/v1/users/' + userId + '/verify-email' return makePostBodyRequest({ url, path, - fields: { verificationString }, + fields: { + verificationString, + isPendingEmail + }, statusCodeExpected }) } diff --git a/shared/models/users/user.model.ts b/shared/models/users/user.model.ts index 2f6a3c719..b5823b47a 100644 --- a/shared/models/users/user.model.ts +++ b/shared/models/users/user.model.ts @@ -9,6 +9,7 @@ export interface User { id: number username: string email: string + pendingEmail: string | null emailVerified: boolean nsfwPolicy: NSFWPolicyType