Cleanup reset user password by admin

And add some tests

18 files changed, 97 insertions, 85 deletions

diff --git a/client/package.json b/client/package.json
index 9e5e87d4a..3eea661f1 100644
--- a/client/package.json
+++ b/client/package.json
@@ -164,7 +164,6 @@
     "webpack-cli": "^3.0.8",
     "webtorrent": "https://github.com/webtorrent/webtorrent#e9b209c7970816fc29e0cc871157a4918d66001d",
     "whatwg-fetch": "^3.0.0",
-    "zone.js": "~0.8.5",
-    "generate-password-browser": "^1.0.2"
+    "zone.js": "~0.8.5"
   }
 }
diff --git a/client/src/app/+admin/users/user-edit/user-edit.component.html b/client/src/app/+admin/users/user-edit/user-edit.component.html
index 3ce246771..c6566da24 100644
--- a/client/src/app/+admin/users/user-edit/user-edit.component.html
+++ b/client/src/app/+admin/users/user-edit/user-edit.component.html
@@ -82,12 +82,16 @@
   <input type="submit" value="{{ getFormButtonTitle() }}" [disabled]="!form.valid">
 </form>
 
-<div *ngIf="!isCreation()">
+<div *ngIf="!isCreation()" class="danger-zone">
   <div class="account-title" i18n>Danger Zone</div>
 
-  <p i18n>Send a link to reset the password by mail to the user.</p>
-  <button style="margin-top:0;" (click)="resetPassword()" i18n>Ask for new password</button>
+  <div class="form-group reset-password-email">
+    <label i18n>Send a link to reset the password by email to the user</label>
+    <button (click)="resetPassword()" i18n>Ask for new password</button>
+  </div>
 
-  <p class="mt-4" i18n>Manually set the user password</p>
-  <my-user-password userId="userId"></my-user-password>
-</div>
\ No newline at end of file
+  <div class="form-group">
+    <label i18n>Manually set the user password</label>
+    <my-user-password [userId]="userId"></my-user-password>
+  </div>
+</div>
diff --git a/client/src/app/+admin/users/user-edit/user-edit.component.scss b/client/src/app/+admin/users/user-edit/user-edit.component.scss
index 2b4aae83c..c1cc4ca45 100644
--- a/client/src/app/+admin/users/user-edit/user-edit.component.scss
+++ b/client/src/app/+admin/users/user-edit/user-edit.component.scss
@@ -32,3 +32,16 @@ input[type=submit], button {
   margin-top: 55px;
   margin-bottom: 30px;
 }
+
+.danger-zone {
+  .reset-password-email {
+    margin-bottom: 30px;
+    padding-bottom: 30px;
+    border-bottom: 1px solid rgba(0, 0, 0, 0.1);
+
+    button {
+      display: block;
+      margin-top: 0;
+    }
+  }
+}
diff --git a/client/src/app/+admin/users/user-edit/user-edit.ts b/client/src/app/+admin/users/user-edit/user-edit.ts
index 021b1feb4..649b35b0c 100644
--- a/client/src/app/+admin/users/user-edit/user-edit.ts
+++ b/client/src/app/+admin/users/user-edit/user-edit.ts
@@ -8,6 +8,7 @@ export abstract class UserEdit extends FormReactive {
   videoQuotaDailyOptions: { value: string, label: string }[] = []
   roles = Object.keys(USER_ROLE_LABELS).map(key => ({ value: key.toString(), label: USER_ROLE_LABELS[key] }))
   username: string
+  userId: number
 
   protected abstract serverService: ServerService
   protected abstract configService: ConfigService
@@ -37,6 +38,10 @@ export abstract class UserEdit extends FormReactive {
     return multiplier * parseInt(this.form.value['videoQuota'], 10)
   }
 
+  resetPassword () {
+    return
+  }
+
   protected buildQuotaOptions () {
     // These are used by a HTML select, so convert key into strings
     this.videoQuotaOptions = this.configService
diff --git a/client/src/app/+admin/users/user-edit/user-password.component.html b/client/src/app/+admin/users/user-edit/user-password.component.html
index 822e4688e..a1e1f6216 100644
--- a/client/src/app/+admin/users/user-edit/user-password.component.html
+++ b/client/src/app/+admin/users/user-edit/user-password.component.html
@@ -1,19 +1,15 @@
 <form role="form" (ngSubmit)="formValidated()" [formGroup]="form">
   <div class="form-group">
 
-    <div class="input-group mb-3">
-      <div class="input-group-prepend">
-        <div class="input-group-text">
-          <input type="checkbox" aria-label="Show password" (change)="togglePasswordVisibility()">
-        </div>
-      </div>
-      <input id="passwordField" #passwordField
-        [attr.type]="showPassword ? 'text' : 'password'" id="password"
+    <div class="input-group">
+      <input id="password" [attr.type]="showPassword ? 'text' : 'password'"
         formControlName="password" [ngClass]="{ 'input-error': formErrors['password'] }"
       >
       <div class="input-group-append">
-        <button class="btn btn-sm btn-outline-secondary" (click)="generatePassword()  "
-                type="button">Generate</button>
+        <button class="btn btn-sm btn-outline-secondary" (click)="togglePasswordVisibility()" type="button">
+          <ng-container *ngIf="!showPassword" i18n>Show</ng-container>
+          <ng-container *ngIf="!!showPassword" i18n>Hide</ng-container>
+        </button>
       </div>
     </div>
     <div *ngIf="formErrors.password" class="form-error">
diff --git a/client/src/app/+admin/users/user-edit/user-password.component.scss b/client/src/app/+admin/users/user-edit/user-password.component.scss
index 9185e787c..217d585af 100644
--- a/client/src/app/+admin/users/user-edit/user-password.component.scss
+++ b/client/src/app/+admin/users/user-edit/user-password.component.scss
@@ -3,6 +3,7 @@
 
 input:not([type=submit]):not([type=checkbox]) {
   @include peertube-input-text(340px);
+
   display: block;
   border-top-right-radius: 0;
   border-bottom-right-radius: 0;
diff --git a/client/src/app/+admin/users/user-edit/user-password.component.ts b/client/src/app/+admin/users/user-edit/user-password.component.ts
index 30cd21ccd..5b3040440 100644
--- a/client/src/app/+admin/users/user-edit/user-password.component.ts
+++ b/client/src/app/+admin/users/user-edit/user-password.component.ts
@@ -1,14 +1,11 @@
-import { Component, OnDestroy, OnInit, Input } from '@angular/core'
+import { Component, Input, OnInit } from '@angular/core'
 import { ActivatedRoute, Router } from '@angular/router'
-import * as generator from 'generate-password-browser'
-import { NotificationsService } from 'angular2-notifications'
 import { UserService } from '@app/shared/users/user.service'
-import { ServerService } from '../../../core'
+import { Notifier } from '../../../core'
 import { User, UserUpdate } from '../../../../../../shared'
 import { I18n } from '@ngx-translate/i18n-polyfill'
 import { FormValidatorService } from '@app/shared/forms/form-validators/form-validator.service'
 import { UserValidatorsService } from '@app/shared/forms/form-validators/user-validators.service'
-import { ConfigService } from '@app/+admin/config/shared/config.service'
 import { FormReactive } from '../../../shared'
 
 @Component({
@@ -16,7 +13,7 @@ import { FormReactive } from '../../../shared'
   templateUrl: './user-password.component.html',
   styleUrls: [ './user-password.component.scss' ]
 })
-export class UserPasswordComponent extends FormReactive implements OnInit, OnDestroy {
+export class UserPasswordComponent extends FormReactive implements OnInit {
   error: string
   username: string
   showPassword = false
@@ -25,12 +22,10 @@ export class UserPasswordComponent extends FormReactive implements OnInit, OnDes
 
   constructor (
     protected formValidatorService: FormValidatorService,
-    protected serverService: ServerService,
-    protected configService: ConfigService,
     private userValidatorsService: UserValidatorsService,
     private route: ActivatedRoute,
     private router: Router,
-    private notificationsService: NotificationsService,
+    private notifier: Notifier,
     private userService: UserService,
     private i18n: I18n
   ) {
@@ -43,10 +38,6 @@ export class UserPasswordComponent extends FormReactive implements OnInit, OnDes
     })
   }
 
-  ngOnDestroy () {
-    //
-  }
-
   formValidated () {
     this.error = undefined
 
@@ -54,8 +45,7 @@ export class UserPasswordComponent extends FormReactive implements OnInit, OnDes
 
     this.userService.updateUser(this.userId, userUpdate).subscribe(
       () => {
-        this.notificationsService.success(
-          this.i18n('Success'),
+        this.notifier.success(
           this.i18n('Password changed for user {{username}}.', { username: this.username })
         )
       },
@@ -64,16 +54,6 @@ export class UserPasswordComponent extends FormReactive implements OnInit, OnDes
     )
   }
 
-  generatePassword () {
-    this.form.patchValue({
-      password: generator.generate({
-        length: 16,
-        excludeSimilarCharacters: true,
-        strict: true
-      })
-    })
-  }
-
   togglePasswordVisibility () {
     this.showPassword = !this.showPassword
   }
@@ -81,9 +61,4 @@ export class UserPasswordComponent extends FormReactive implements OnInit, OnDes
   getFormButtonTitle () {
     return this.i18n('Update user password')
   }
-
-  private onUserFetched (userJson: User) {
-    this.userId = userJson.id
-    this.username = userJson.username
-  }
 }
diff --git a/client/src/app/+admin/users/user-edit/user-update.component.ts b/client/src/app/+admin/users/user-edit/user-update.component.ts
index 4e4002a73..94ef87b08 100644
--- a/client/src/app/+admin/users/user-edit/user-update.component.ts
+++ b/client/src/app/+admin/users/user-edit/user-update.component.ts
@@ -1,4 +1,4 @@
-import { Component, OnDestroy, OnInit, Input } from '@angular/core'
+import { Component, OnDestroy, OnInit } from '@angular/core'
 import { ActivatedRoute, Router } from '@angular/router'
 import { Subscription } from 'rxjs'
 import { Notifier } from '@app/core'
@@ -93,8 +93,7 @@ export class UserUpdateComponent extends UserEdit implements OnInit, OnDestroy {
   resetPassword () {
     this.userService.askResetPassword(this.userEmail).subscribe(
       () => {
-        this.notificationsService.success(
-          this.i18n('Success'),
+        this.notifier.success(
           this.i18n('An email asking for password reset has been sent to {{username}}.', { username: this.username })
         )
       },
diff --git a/client/src/app/shared/users/user.service.ts b/client/src/app/shared/users/user.service.ts
index d0abc7def..cc5c051f1 100644
--- a/client/src/app/shared/users/user.service.ts
+++ b/client/src/app/shared/users/user.service.ts
@@ -103,11 +103,6 @@ export class UserService {
                )
   }
 
-  resetUserPassword (userId: number) {
-    return this.authHttp.post(UserService.BASE_USERS_URL + userId + '/reset-password', {})
-               .pipe(catchError(err => this.restExtractor.handleError(err)))
-  }
-
   verifyEmail (userId: number, verificationString: string) {
     const url = `${UserService.BASE_USERS_URL}/${userId}/verify-email`
     const body = {
diff --git a/server/controllers/api/users/index.ts b/server/controllers/api/users/index.ts
index beac6d8b1..e3533a7f6 100644
--- a/server/controllers/api/users/index.ts
+++ b/server/controllers/api/users/index.ts
@@ -3,7 +3,6 @@ import * as RateLimit from 'express-rate-limit'
 import { UserCreate, UserRight, UserRole, UserUpdate } from '../../../../shared'
 import { logger } from '../../../helpers/logger'
 import { getFormattedObjects } from '../../../helpers/utils'
-import { pseudoRandomBytesPromise } from '../../../helpers/core-utils'
 import { CONFIG, RATES_LIMIT, sequelizeTypescript } from '../../../initializers'
 import { Emailer } from '../../../lib/emailer'
 import { Redis } from '../../../lib/redis'
@@ -230,7 +229,7 @@ async function unblockUser (req: express.Request, res: express.Response, next: e
   return res.status(204).end()
 }
 
-async function blockUser (req: express.Request, res: express.Response, next: express.NextFunction) {
+async function blockUser (req: express.Request, res: express.Response) {
   const user: UserModel = res.locals.user
   const reason = req.body.reason
 
@@ -239,23 +238,23 @@ async function blockUser (req: express.Request, res: express.Response, next: exp
   return res.status(204).end()
 }
 
-function getUser (req: express.Request, res: express.Response, next: express.NextFunction) {
+function getUser (req: express.Request, res: express.Response) {
   return res.json((res.locals.user as UserModel).toFormattedJSON())
 }
 
-async function autocompleteUsers (req: express.Request, res: express.Response, next: express.NextFunction) {
+async function autocompleteUsers (req: express.Request, res: express.Response) {
   const resultList = await UserModel.autoComplete(req.query.search as string)
 
   return res.json(resultList)
 }
 
-async function listUsers (req: express.Request, res: express.Response, next: express.NextFunction) {
+async function listUsers (req: express.Request, res: express.Response) {
   const resultList = await UserModel.listForApi(req.query.start, req.query.count, req.query.sort, req.query.search)
 
   return res.json(getFormattedObjects(resultList.data, resultList.total))
 }
 
-async function removeUser (req: express.Request, res: express.Response, next: express.NextFunction) {
+async function removeUser (req: express.Request, res: express.Response) {
   const user: UserModel = res.locals.user
 
   await user.destroy()
@@ -265,12 +264,13 @@ async function removeUser (req: express.Request, res: express.Response, next: ex
   return res.sendStatus(204)
 }
 
-async function updateUser (req: express.Request, res: express.Response, next: express.NextFunction) {
+async function updateUser (req: express.Request, res: express.Response) {
   const body: UserUpdate = req.body
   const userToUpdate = res.locals.user as UserModel
   const oldUserAuditView = new UserAuditView(userToUpdate.toFormattedJSON())
   const roleChanged = body.role !== undefined && body.role !== userToUpdate.role
 
+  if (body.password !== undefined) userToUpdate.password = body.password
   if (body.email !== undefined) userToUpdate.email = body.email
   if (body.emailVerified !== undefined) userToUpdate.emailVerified = body.emailVerified
   if (body.videoQuota !== undefined) userToUpdate.videoQuota = body.videoQuota
@@ -280,11 +280,11 @@ async function updateUser (req: express.Request, res: express.Response, next: ex
   const user = await userToUpdate.save()
 
   // Destroy user token to refresh rights
-  if (roleChanged) await deleteUserToken(userToUpdate.id)
+  if (roleChanged || body.password !== undefined) await deleteUserToken(userToUpdate.id)
 
   auditLogger.update(getAuditIdFromRes(res), new UserAuditView(user.toFormattedJSON()), oldUserAuditView)
 
-  // Don't need to send this update to followers, these attributes are not propagated
+  // Don't need to send this update to followers, these attributes are not federated
 
   return res.sendStatus(204)
 }
@@ -294,7 +294,7 @@ async function askResetUserPassword (req: express.Request, res: express.Response
 
   const verificationString = await Redis.Instance.setResetPasswordVerificationString(user.id)
   const url = CONFIG.WEBSERVER.URL + '/reset-password?userId=' + user.id + '&verificationString=' + verificationString
-  await Emailer.Instance.addForgetPasswordEmailJob(user.email, url)
+  await Emailer.Instance.addPasswordResetEmailJob(user.email, url)
 
   return res.status(204).end()
 }
diff --git a/server/controllers/api/users/me.ts b/server/controllers/api/users/me.ts
index 94a2b8732..d5e154869 100644
--- a/server/controllers/api/users/me.ts
+++ b/server/controllers/api/users/me.ts
@@ -167,7 +167,7 @@ async function deleteMe (req: express.Request, res: express.Response) {
   return res.sendStatus(204)
 }
 
-async function updateMe (req: express.Request, res: express.Response, next: express.NextFunction) {
+async function updateMe (req: express.Request, res: express.Response) {
   const body: UserUpdateMe = req.body
 
   const user: UserModel = res.locals.oauth.token.user
diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts
index 98f8f8694..e5c4c4e63 100644
--- a/server/initializers/constants.ts
+++ b/server/initializers/constants.ts
@@ -711,6 +711,8 @@ if (isTestInstance() === true) {
   CACHE.VIDEO_CAPTIONS.MAX_AGE = 3000
   MEMOIZE_TTL.OVERVIEWS_SAMPLE = 1
   ROUTE_CACHE_LIFETIME.OVERVIEWS.VIDEOS = '0ms'
+
+  RATES_LIMIT.LOGIN.MAX = 20
 }
 
 updateWebserverUrls()
diff --git a/server/lib/emailer.ts b/server/lib/emailer.ts
index 7681164b3..672414cc0 100644
--- a/server/lib/emailer.ts
+++ b/server/lib/emailer.ts
@@ -101,22 +101,6 @@ class Emailer {
     return JobQueue.Instance.createJob({ type: 'email', payload: emailPayload })
   }
 
-  addForceResetPasswordEmailJob (to: string, resetPasswordUrl: string) {
-    const text = `Hi dear user,\n\n` +
-      `Your password has been reset on ${CONFIG.WEBSERVER.HOST}! ` +
-      `Please follow this link to reset it: ${resetPasswordUrl}\n\n` +
-      `Cheers,\n` +
-      `PeerTube.`
-
-    const emailPayload: EmailPayload = {
-      to: [ to ],
-      subject: 'Reset of your PeerTube password',
-      text
-    }
-
-    return JobQueue.Instance.createJob({ type: 'email', payload: emailPayload })
-  }
-
   addNewFollowNotification (to: string[], actorFollow: ActorFollowModel, followType: 'account' | 'channel') {
     const followerName = actorFollow.ActorFollower.Account.getDisplayName()
     const followingName = (actorFollow.ActorFollowing.VideoChannel || actorFollow.ActorFollowing.Account).getDisplayName()
@@ -312,9 +296,9 @@ class Emailer {
     return JobQueue.Instance.createJob({ type: 'email', payload: emailPayload })
   }
 
-  addForgetPasswordEmailJob (to: string, resetPasswordUrl: string) {
+  addPasswordResetEmailJob (to: string, resetPasswordUrl: string) {
     const text = `Hi dear user,\n\n` +
-      `It seems you forgot your password on ${CONFIG.WEBSERVER.HOST}! ` +
+      `A reset password procedure for your account ${to} has been requested on ${CONFIG.WEBSERVER.HOST} ` +
       `Please follow this link to reset it: ${resetPasswordUrl}\n\n` +
       `If you are not the person who initiated this request, please ignore this email.\n\n` +
       `Cheers,\n` +
diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts
index 1bb0bfb1b..a52e3060a 100644
--- a/server/middlewares/validators/users.ts
+++ b/server/middlewares/validators/users.ts
@@ -113,6 +113,7 @@ const deleteMeValidator = [
 
 const usersUpdateValidator = [
   param('id').isInt().not().isEmpty().withMessage('Should have a valid id'),
+  body('password').optional().custom(isUserPasswordValid).withMessage('Should have a valid password'),
   body('email').optional().isEmail().withMessage('Should have a valid email attribute'),
   body('emailVerified').optional().isBoolean().withMessage('Should have a valid email verified attribute'),
   body('videoQuota').optional().custom(isUserVideoQuotaValid).withMessage('Should have a valid user quota'),
@@ -233,6 +234,7 @@ const usersAskResetPasswordValidator = [
     logger.debug('Checking usersAskResetPassword parameters', { parameters: req.body })
 
     if (areValidationErrors(req, res)) return
+
     const exists = await checkUserEmailExist(req.body.email, res, false)
     if (!exists) {
       logger.debug('User with email %s does not exist (asking reset password).', req.body.email)
diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts
index a3e8e2e9c..13be8b460 100644
--- a/server/tests/api/check-params/users.ts
+++ b/server/tests/api/check-params/users.ts
@@ -464,6 +464,24 @@ describe('Test users API validators', function () {
       await makePutBodyRequest({ url: server.url, path: path + userId, token: server.accessToken, fields })
     })
 
+    it('Should fail with a too small password', async function () {
+      const fields = {
+        currentPassword: 'my super password',
+        password: 'bla'
+      }
+
+      await makePutBodyRequest({ url: server.url, path: path + userId, token: server.accessToken, fields })
+    })
+
+    it('Should fail with a too long password', async function () {
+      const fields = {
+        currentPassword: 'my super password',
+        password: 'super'.repeat(61)
+      }
+
+      await makePutBodyRequest({ url: server.url, path: path + userId, token: server.accessToken, fields })
+    })
+
     it('Should fail with an non authenticated user', async function () {
       const fields = {
         videoQuota: 42
diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts
index ad98ab1c7..c4465d541 100644
--- a/server/tests/api/users/users.ts
+++ b/server/tests/api/users/users.ts
@@ -501,6 +501,22 @@ describe('Test users', function () {
     accessTokenUser = await userLogin(server, user)
   })
 
+  it('Should be able to update another user password', async function () {
+    await updateUser({
+      url: server.url,
+      userId,
+      accessToken,
+      password: 'password updated'
+    })
+
+    await getMyUserVideoQuotaUsed(server.url, accessTokenUser, 401)
+
+    await userLogin(server, user, 400)
+
+    user.password = 'password updated'
+    accessTokenUser = await userLogin(server, user)
+  })
+
   it('Should be able to list video blacklist by a moderator', async function () {
     await getBlacklistedVideosList(server.url, accessTokenUser)
   })
diff --git a/shared/models/users/user-update.model.ts b/shared/models/users/user-update.model.ts
index abde51321..cd215bab3 100644
--- a/shared/models/users/user-update.model.ts
+++ b/shared/models/users/user-update.model.ts
@@ -1,6 +1,7 @@
 import { UserRole } from './user-role'
 
 export interface UserUpdate {
+  password?: string
   email?: string
   emailVerified?: boolean
   videoQuota?: number
diff --git a/shared/utils/users/users.ts b/shared/utils/users/users.ts
index 61a7e3757..7191b263e 100644
--- a/shared/utils/users/users.ts
+++ b/shared/utils/users/users.ts
@@ -213,11 +213,13 @@ function updateUser (options: {
   emailVerified?: boolean,
   videoQuota?: number,
   videoQuotaDaily?: number,
+  password?: string,
   role?: UserRole
 }) {
   const path = '/api/v1/users/' + options.userId
 
   const toSend = {}
+  if (options.password !== undefined && options.password !== null) toSend['password'] = options.password
   if (options.email !== undefined && options.email !== null) toSend['email'] = options.email
   if (options.emailVerified !== undefined && options.emailVerified !== null) toSend['emailVerified'] = options.emailVerified
   if (options.videoQuota !== undefined && options.videoQuota !== null) toSend['videoQuota'] = options.videoQuota