Changed so an admin can't change a user's password

Author: EricKit

README.md

@@ -106,7 +106,16 @@ Users can modify or view their own data. Admins can do anything except refresh a
 
 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.
+The `UsernameEmailAdminGuard` is the same as the `UsernameEmailGuard` except it also allows admins. Admins should not be allowed to change everything. For example, an admin should not be allowed to set another user's password. This would allow the admin to impersonate that user. The `@AdminAllowedArgs` decorator has been added for this reason to this guard. If this decorator is used, only the arguments specified are allowed. Placing the below decorator above the `updateUser` resolver will not allow an admin to specify the `fieldsToUpdate.password` argument.
+
+```Typescript
+@AdminAllowedArgs(
+    'username',
+    'fieldsToUpdate.username',
+    'fieldsToUpdate.email',
+    'fieldsToUpdate.enabled',
+  )
+```
 
 The `AdminGuard` only allows admins.
 

src/auth/guards/username-email-admin.guard.ts

@@ -3,20 +3,40 @@ import { GqlExecutionContext } from '@nestjs/graphql';
 import { User } from '../../graphql.classes';
 import { UsersService } from '../../users/users.service';
 import { AuthenticationError } from 'apollo-server-core';
+import { Reflector } from '@nestjs/core';
 
 // Check if username in field for query matches authenticated user's username
 // or if the user is admin
 @Injectable()
 export class UsernameEmailAdminGuard implements CanActivate {
-  constructor(private usersService: UsersService) {}
+  constructor(
+    private usersService: UsersService,
+    private readonly reflector: Reflector,
+  ) {}
+
+  // Returns an array of all the properties of an object seperated by a .
+  getPropertiesArray(object: any): string[] {
+    let result: string[] = [];
+    Object.entries(object).forEach(([key, value]) => {
+      const field = key;
+      if (typeof value === 'object' && value !== null) {
+        const objectProperties = this.getPropertiesArray(value).map(
+          prop => `${field}.${prop}`,
+        );
+        result = result.concat(objectProperties);
+      } else {
+        result.push(field);
+      }
+    });
+    return result;
+  }
 
   canActivate(context: ExecutionContext): boolean {
     const ctx = GqlExecutionContext.create(context);
     const request = ctx.getContext().req;
     let shouldActivate = false;
     if (request.user) {
       const user = <User> request.user;
-      if (this.usersService.isAdmin(user.permissions)) return true;
       const args = ctx.getArgs();
       if (args.username && typeof args.username === 'string') {
         shouldActivate =
@@ -26,6 +46,29 @@ export class UsernameEmailAdminGuard implements CanActivate {
       } else if (!args.username && !args.email) {
         shouldActivate = true;
       }
+
+      if (
+        shouldActivate === false &&
+        this.usersService.isAdmin(user.permissions)
+      ) {
+        const adminAllowedArgs = this.reflector.get<string[]>(
+          'adminAllowedArgs',
+          context.getHandler(),
+        );
+
+        shouldActivate = true;
+
+        if (adminAllowedArgs) {
+          const argFields = this.getPropertiesArray(args);
+          argFields.forEach(field => {
+            if (!adminAllowedArgs.includes(field)) {
+              throw new AuthenticationError(
+                `Admin is not allowed to modify ${field}`,
+              );
+            }
+          });
+        }
+      }
     }
     if (!shouldActivate) {
       throw new AuthenticationError('Could not authenticate with token');

src/decorators/admin-allowed-args.ts

@@ -0,0 +1,4 @@
+import { SetMetadata } from '@nestjs/common';
+
+export const AdminAllowedArgs = (...adminAllowedArgs: string[]) =>
+  SetMetadata('adminAllowedArgs', adminAllowedArgs);

src/users/users.resolvers.ts

@@ -7,6 +7,7 @@ import { UsernameEmailAdminGuard } from '../auth/guards/username-email-admin.gua
 import { AdminGuard } from '../auth/guards/admin.guard';
 import { UserInputError, ValidationError } from 'apollo-server-core';
 import { UserDocument } from './schemas/user.schema';
+import { AdminAllowedArgs } from '../decorators/admin-allowed-args';
 
 @Resolver('User')
 export class UserResolver {
@@ -74,6 +75,12 @@ export class UserResolver {
   }
 
   @Mutation('updateUser')
+  @AdminAllowedArgs(
+    'username',
+    'fieldsToUpdate.username',
+    'fieldsToUpdate.email',
+    'fieldsToUpdate.enabled',
+  )
   @UseGuards(JwtAuthGuard, UsernameEmailAdminGuard)
   async updateUser(
     @Args('username') username: string,

test/users.e2e-spec.ts

@@ -876,10 +876,6 @@ describe('Users (e2e)', () => {
             fieldsToUpdate: {
             username: "newUsernameByAdmin",
             email: "[email protected]",
-            password: {
-              oldPassword: "password",
-              newPassword: "newPassword",
-            }
           }) {username, email}}`,
       };
 
@@ -894,12 +890,39 @@ describe('Users (e2e)', () => {
             email: '[email protected]',
           });
         });
+    });
 
-      const login = await authService.validateUserByPassword({
-        username: 'newUsernameByAdmin',
-        password: 'newPassword',
+    it('fails with admin changing another user\'s password', async () => {
+      await usersService.create({
+        username: 'userToUpdateByAdmin2',
+        email: '[email protected]',
+        password: 'password',
       });
-      expect(login).toBeTruthy();
+
+      const data = {
+        query: `mutation {
+          updateUser(
+            username: "userToUpdateByAdmin2",
+            fieldsToUpdate: {
+            username: "newUsernameByAdmin2",
+            email: "[email protected]",
+            password: {
+              oldPassword: "password",
+              newPassword: "newPassword",
+            }
+          }) {username, email}}`,
+      };
+
+      await request(app.getHttpServer())
+        .post('/graphql')
+        .set('Authorization', `Bearer ${adminLogin.token!}`)
+        .send(data)
+        .expect(200)
+        .expect(response => {
+          expect(response.body.errors[0].extensions.code).toEqual(
+            'UNAUTHENTICATED',
+          );
+        });
     });
 
     it('updates other fields with changed username already in use', async () => {