Changing password requires old password

Author: EricKit

README.md

@@ -104,7 +104,7 @@ Admin must be set manually as a string in permissions for the first user (add `a
 
 Users can modify or view their own data. Admins can do anything except refresh another user's token, which would allow the admin to impersonate that user.
 
-The `UsernameEmailGuard` compares the user's email or username with the same field in a query. If any query or mutation in the resolver has `doAnythingWithUser(username: string)` or `doAnythingWithUser(email: string)` and that email / username matches the user which is requesting the action, it will be approved. Username and email are unique, and the user has already been verified via JWT.
+The `UsernameEmailGuard` compares the user's email or username with the same field in a query. If any query or mutation in the resolver has `doAnythingWithUser(username: string)` or `doAnythingWithUser(email: string)` and that email / username matches the user which is requesting the action, it will be approved. Username and email are unique, and the user has already been verified via JWT. **If there is not a username or email in the request, it will pass.** This is because the resolvers will set the action on the user making the request. For example, on updateUser if no username is specified, the modification is on the user making the request.
 
 The `UsernameEmailAdminGuard` is the same as the `UsernameEmailGuard` except it also allows admins.
 
@@ -269,7 +269,6 @@ mutation updateUser($updateUser: UpdateUserInput!) {
 {
   "updateUser": {
     "username": "newUserName",
-    "password": "newPassword",
     "email": "[email protected]",
     "enabled": false
   }

src/auth/auth.module.ts

@@ -1,4 +1,4 @@
-import { Module } from '@nestjs/common';
+import { Module, forwardRef } from '@nestjs/common';
 import { AuthService } from './auth.service';
 import { PassportModule } from '@nestjs/passport';
 import { JwtModule, JwtModuleOptions } from '@nestjs/jwt';
@@ -26,7 +26,7 @@ import { ConfigModule } from '../config/config.module';
       },
       inject: [ConfigService],
     }),
-    UsersModule,
+    forwardRef(() => UsersModule),
     ConfigModule,
   ],
   controllers: [],

src/auth/auth.resolvers.ts

@@ -4,16 +4,11 @@ import { AuthService } from './auth.service';
 import { AuthenticationError } from 'apollo-server-core';
 import { JwtAuthGuard } from './guards/jwt-auth.guard';
 import { UseGuards } from '@nestjs/common';
-import { UsernameEmailGuard } from './guards/username-email.guard';
-import { UsersService } from '../users/users.service';
 import { UserDocument } from '../users/schemas/user.schema';
 
 @Resolver('Auth')
 export class AuthResolver {
-  constructor(
-    private authService: AuthService,
-    private usersService: UsersService,
-  ) {}
+  constructor(private authService: AuthService) {}
 
   @Query('login')
   async login(@Args('user') user: LoginUserInput): Promise<LoginResult> {

src/auth/auth.service.ts

@@ -1,4 +1,4 @@
-import { Injectable } from '@nestjs/common';
+import { Injectable, forwardRef, Inject } from '@nestjs/common';
 import { JwtService } from '@nestjs/jwt';
 import { UsersService } from '../users/users.service';
 import { JwtPayload } from './interfaces/jwt-payload.interface';
@@ -10,6 +10,7 @@ import { resolve } from 'path';
 @Injectable()
 export class AuthService {
   constructor(
+    @Inject(forwardRef(() => UsersService))
     private usersService: UsersService,
     private jwtService: JwtService,
     private configService: ConfigService,

src/graphql.classes.ts

@@ -17,10 +17,15 @@ export class LoginUserInput {
     password: string;
 }
 
+export class UpdatePasswordInput {
+    oldPassword: string;
+    newPassword: string;
+}
+
 export class UpdateUserInput {
     username?: string;
     email?: string;
-    password?: string;
+    password?: UpdatePasswordInput;
     enabled?: boolean;
 }
 
@@ -32,7 +37,7 @@ export class LoginResult {
 export abstract class IMutation {
     abstract createUser(createUserInput?: CreateUserInput): User | Promise<User>;
 
-    abstract updateUser(username: string, fieldsToUpdate: UpdateUserInput): User | Promise<User>;
+    abstract updateUser(fieldsToUpdate: UpdateUserInput, username?: string): User | Promise<User>;
 
     abstract addAdminPermission(username: string): User | Promise<User>;
 

src/users/users.module.ts

@@ -1,16 +1,18 @@
-import { Module } from '@nestjs/common';
+import { Module, forwardRef } from '@nestjs/common';
 import { UsersService } from './users.service';
 import { MongooseModule } from '@nestjs/mongoose';
 import { UserSchema } from './schemas/user.schema';
 import { UserResolver } from './users.resolvers';
 import { DateScalar } from '../scalars/date.scalar';
 import { ConfigModule } from '../config/config.module';
+import { AuthModule } from '../auth/auth.module';
 
 @Module({
   imports: [
     MongooseModule.forFeature([{ name: 'User', schema: UserSchema }]),
     UsersModule,
     ConfigModule,
+    forwardRef(() => AuthModule),
   ],
   exports: [UsersService],
   controllers: [],

src/users/users.resolvers.ts

@@ -78,8 +78,10 @@ export class UserResolver {
   async updateUser(
     @Args('username') username: string,
     @Args('fieldsToUpdate') fieldsToUpdate: UpdateUserInput,
+    @Context('req') request: any,
   ): Promise<User> {
     let user: UserDocument | undefined;
+    if (!username && request.user) username = request.user.username;
     try {
       user = await this.usersService.update(username, fieldsToUpdate);
     } catch (error) {

src/users/users.service.ts

@@ -7,12 +7,14 @@ import { randomBytes } from 'crypto';
 import { createTransport, SendMailOptions } from 'nodemailer';
 import { ConfigService } from '../config/config.service';
 import { MongoError } from 'mongodb';
+import { AuthService } from '../auth/auth.service';
 
 @Injectable()
 export class UsersService {
   constructor(
     @InjectModel('User') private readonly userModel: Model<UserDocument>,
     private configService: ConfigService,
+    private authService: AuthService,
   ) {}
 
   /**
@@ -97,11 +99,22 @@ export class UsersService {
       if (duplicateUser || !emailValid) fieldsToUpdate.email = undefined;
     }
 
-    const fields = {};
+    const fields: any = {};
+
+    if (fieldsToUpdate.password) {
+      if (
+        await this.authService.validateUserByPassword({
+          username,
+          password: fieldsToUpdate.password.oldPassword,
+        })
+      ) {
+        fields.password = fieldsToUpdate.password.newPassword;
+      }
+    }
 
     // Remove undefined keys for update
     for (const key in fieldsToUpdate) {
-      if (typeof fieldsToUpdate[key] !== 'undefined') {
+      if (typeof fieldsToUpdate[key] !== 'undefined' && key !== 'password') {
         fields[key] = fieldsToUpdate[key];
       }
     }

src/users/users.types.graphql

@@ -9,7 +9,7 @@ type Query {
 
 type Mutation {
   createUser(createUserInput: CreateUserInput): User!
-  updateUser(username: String!, fieldsToUpdate: UpdateUserInput!): User!
+  updateUser(fieldsToUpdate: UpdateUserInput!, username: String): User!
   addAdminPermission(username: String!): User!
   removeAdminPermission(username: String!): User!
   resetPassword(username: String!, code: String!, password: String!): User!
@@ -35,6 +35,11 @@ input CreateUserInput {
 input UpdateUserInput {
   username: String
   email: String
-  password: String
+  password: UpdatePasswordInput
   enabled: Boolean
 }
+
+input UpdatePasswordInput {
+  oldPassword: String!
+  newPassword: String!
+}

test/users.e2e-spec.ts

@@ -794,7 +794,10 @@ describe('Users (e2e)', () => {
             fieldsToUpdate: {
             username: "newUsername1",
             email: "[email protected]",
-            password: "newPassword",
+            password: {
+              oldPassword: "password",
+              newPassword: "newPassword",
+            }
             enabled: true
           }) {username, email, enabled}}`,
       };
@@ -873,7 +876,10 @@ describe('Users (e2e)', () => {
             fieldsToUpdate: {
             username: "newUsernameByAdmin",
             email: "[email protected]",
-            password: "newPassword"
+            password: {
+              oldPassword: "password",
+              newPassword: "newPassword",
+            }
           }) {username, email}}`,
       };
 
@@ -915,7 +921,10 @@ describe('Users (e2e)', () => {
             fieldsToUpdate: {
             username: "user1",
             email:"[email protected]",
-            password:"newPassword"
+            password: {
+              oldPassword: "password",
+              newPassword: "newPassword",
+            }
           }) {username, email}}`,
       };
 
@@ -958,7 +967,10 @@ describe('Users (e2e)', () => {
             fieldsToUpdate: {
             username: "newUsername3",
             email:"[email protected]",
-            password:"newPassword"
+            password: {
+              oldPassword: "password",
+              newPassword: "newPassword",
+            }
           }) {username, email}}`,
       };
 
@@ -1001,7 +1013,10 @@ describe('Users (e2e)', () => {
             fieldsToUpdate: {
             username: "newUsername5",
             email:"invalidEmail",
-            password:"newPassword"
+            password: {
+              oldPassword: "password",
+              newPassword: "newPassword",
+            }
           }) {username, email}}`,
       };
 
@@ -1053,6 +1068,60 @@ describe('Users (e2e)', () => {
       ).toBeTruthy();
     });
 
+    it(`update other fields with bad old password - also verifies updates requesting user's info
+    with no username set`, async () => {
+      await usersService.create({
+        username: 'userToUpdate55',
+        email: '[email protected]',
+        password: 'password',
+      });
+
+      const result = await authService.validateUserByPassword({
+        username: 'userToUpdate55',
+        password: 'password',
+      });
+      const token = result!.token;
+
+      const data = {
+        query: `mutation {
+          updateUser(
+            fieldsToUpdate: {
+            username: "newUsername55",
+            email: "[email protected]",
+            password: {
+              oldPassword: "notthepassword",
+              newPassword: "newPassword",
+            }
+            enabled: true
+          }) {username, email, enabled}}`,
+      };
+
+      await request(app.getHttpServer())
+        .post('/graphql')
+        .set('Authorization', `Bearer ${token!}`)
+        .send(data)
+        .expect(200)
+        .expect(response => {
+          expect(response.body.data.updateUser).toMatchObject({
+            username: 'newUsername55',
+            email: '[email protected]',
+            enabled: true,
+          });
+        });
+
+      let login = await authService.validateUserByPassword({
+        username: 'newUsername55',
+        password: 'newPassword',
+      });
+      expect(login).toBeFalsy();
+
+      login = await authService.validateUserByPassword({
+        username: 'newUsername55',
+        password: 'password',
+      });
+      expect(login).toBeTruthy();
+    });
+
     it('fails to update with wrong username', async () => {
       const data = {
         query: `mutation {