fix(auth): changing user credentials to work both on http proxy and direct code call

packages/auth/src/Domain/UseCase/ChangeCredentials/ChangeCredentials.spec.ts

@@ -11,6 +11,7 @@ import { User } from '../../User/User'
 import { UserRepositoryInterface } from '../../User/UserRepositoryInterface'
 
 import { ChangeCredentials } from './ChangeCredentials'
+import { Username } from '@standardnotes/domain-core'
 
 describe('ChangeCredentials', () => {
   let userRepository: UserRepositoryInterface
@@ -25,9 +26,6 @@ describe('ChangeCredentials', () => {
     new ChangeCredentials(userRepository, authResponseFactoryResolver, domainEventPublisher, domainEventFactory, timer)
 
   beforeEach(() => {
-    userRepository = {} as jest.Mocked<UserRepositoryInterface>
-    userRepository.save = jest.fn()
-
     authResponseFactory = {} as jest.Mocked<AuthResponseFactoryInterface>
     authResponseFactory.createResponse = jest.fn().mockReturnValue({ foo: 'bar' })
 
@@ -39,6 +37,10 @@ describe('ChangeCredentials', () => {
     user.uuid = '1-2-3'
     user.email = '[email protected]'
 
+    userRepository = {} as jest.Mocked<UserRepositoryInterface>
+    userRepository.save = jest.fn()
+    userRepository.findOneByUsernameOrEmail = jest.fn().mockReturnValue(user)
+
     domainEventPublisher = {} as jest.Mocked<DomainEventPublisherInterface>
     domainEventPublisher.publish = jest.fn()
 
@@ -52,7 +54,7 @@ describe('ChangeCredentials', () => {
   it('should change password', async () => {
     expect(
       await createUseCase().execute({
-        user,
+        username: Username.create('[email protected]').getValue(),
         apiVersion: '20190520',
         currentPassword: 'qweqwe123123',
         newPassword: 'test234',
@@ -82,11 +84,11 @@ describe('ChangeCredentials', () => {
   })
 
   it('should change email', async () => {
-    userRepository.findOneByUsernameOrEmail = jest.fn().mockReturnValue(null)
+    userRepository.findOneByUsernameOrEmail = jest.fn().mockReturnValueOnce(user).mockReturnValueOnce(null)
 
     expect(
       await createUseCase().execute({
-        user,
+        username: Username.create('[email protected]').getValue(),
         apiVersion: '20190520',
         currentPassword: 'qweqwe123123',
         newPassword: 'test234',
@@ -117,11 +119,14 @@ describe('ChangeCredentials', () => {
   })
 
   it('should not change email if already taken', async () => {
-    userRepository.findOneByUsernameOrEmail = jest.fn().mockReturnValue({} as jest.Mocked<User>)
+    userRepository.findOneByUsernameOrEmail = jest
+      .fn()
+      .mockReturnValueOnce(user)
+      .mockReturnValueOnce({} as jest.Mocked<User>)
 
     expect(
       await createUseCase().execute({
-        user,
+        username: Username.create('[email protected]').getValue(),
         apiVersion: '20190520',
         currentPassword: 'qweqwe123123',
         newPassword: 'test234',
@@ -144,7 +149,7 @@ describe('ChangeCredentials', () => {
   it('should not change email if the new email is invalid', async () => {
     expect(
       await createUseCase().execute({
-        user,
+        username: Username.create('[email protected]').getValue(),
         apiVersion: '20190520',
         currentPassword: 'qweqwe123123',
         newPassword: 'test234',
@@ -164,10 +169,35 @@ describe('ChangeCredentials', () => {
     expect(domainEventPublisher.publish).not.toHaveBeenCalled()
   })
 
+  it('should not change email if the user is not found', async () => {
+    userRepository.findOneByUsernameOrEmail = jest.fn().mockReturnValue(null)
+
+    expect(
+      await createUseCase().execute({
+        username: Username.create('[email protected]').getValue(),
+        apiVersion: '20190520',
+        currentPassword: 'qweqwe123123',
+        newPassword: 'test234',
+        newEmail: '',
+        pwNonce: 'asdzxc',
+        updatedWithUserAgent: 'Google Chrome',
+        kpCreated: '123',
+        kpOrigination: 'password-change',
+      }),
+    ).toEqual({
+      success: false,
+      errorMessage: 'User not found.',
+    })
+
+    expect(userRepository.save).not.toHaveBeenCalled()
+    expect(domainEventFactory.createUserEmailChangedEvent).not.toHaveBeenCalled()
+    expect(domainEventPublisher.publish).not.toHaveBeenCalled()
+  })
+
   it('should not change password if current password is incorrect', async () => {
     expect(
       await createUseCase().execute({
-        user,
+        username: Username.create('[email protected]').getValue(),
         apiVersion: '20190520',
         currentPassword: 'test123',
         newPassword: 'test234',
@@ -185,7 +215,7 @@ describe('ChangeCredentials', () => {
   it('should update protocol version while changing password', async () => {
     expect(
       await createUseCase().execute({
-        user,
+        username: Username.create('[email protected]').getValue(),
         apiVersion: '20190520',
         currentPassword: 'qweqwe123123',
         newPassword: 'test234',

packages/auth/src/Domain/UseCase/ChangeCredentials/ChangeCredentials.ts

@@ -25,14 +25,22 @@ export class ChangeCredentials implements UseCaseInterface {
   ) {}
 
   async execute(dto: ChangeCredentialsDTO): Promise<ChangeCredentialsResponse> {
-    if (!(await bcrypt.compare(dto.currentPassword, dto.user.encryptedPassword))) {
+    const user = await this.userRepository.findOneByUsernameOrEmail(dto.username)
+    if (!user) {
+      return {
+        success: false,
+        errorMessage: 'User not found.',
+      }
+    }
+
+    if (!(await bcrypt.compare(dto.currentPassword, user.encryptedPassword))) {
       return {
         success: false,
         errorMessage: 'The current password you entered is incorrect. Please try again.',
       }
     }
 
-    dto.user.encryptedPassword = await bcrypt.hash(dto.newPassword, User.PASSWORD_HASH_COST)
+    user.encryptedPassword = await bcrypt.hash(dto.newPassword, User.PASSWORD_HASH_COST)
 
     let userEmailChangedEvent: UserEmailChangedEvent | undefined = undefined
     if (dto.newEmail !== undefined) {
@@ -54,27 +62,27 @@ export class ChangeCredentials implements UseCaseInterface {
       }
 
       userEmailChangedEvent = this.domainEventFactory.createUserEmailChangedEvent(
-        dto.user.uuid,
-        dto.user.email,
+        user.uuid,
+        user.email,
         newUsername.value,
       )
 
-      dto.user.email = newUsername.value
+      user.email = newUsername.value
     }
 
-    dto.user.pwNonce = dto.pwNonce
+    user.pwNonce = dto.pwNonce
     if (dto.protocolVersion) {
-      dto.user.version = dto.protocolVersion
+      user.version = dto.protocolVersion
     }
     if (dto.kpCreated) {
-      dto.user.kpCreated = dto.kpCreated
+      user.kpCreated = dto.kpCreated
     }
     if (dto.kpOrigination) {
-      dto.user.kpOrigination = dto.kpOrigination
+      user.kpOrigination = dto.kpOrigination
     }
-    dto.user.updatedAt = this.timer.getUTCDate()
+    user.updatedAt = this.timer.getUTCDate()
 
-    const updatedUser = await this.userRepository.save(dto.user)
+    const updatedUser = await this.userRepository.save(user)
 
     if (userEmailChangedEvent !== undefined) {
       await this.domainEventPublisher.publish(userEmailChangedEvent)

packages/auth/src/Domain/UseCase/ChangeCredentials/ChangeCredentialsDTO.ts

@@ -1,7 +1,7 @@
-import { User } from '../../User/User'
+import { Username } from '@standardnotes/domain-core'
 
 export type ChangeCredentialsDTO = {
-  user: User
+  username: Username
   apiVersion: string
   currentPassword: string
   newPassword: string

packages/auth/src/Infra/InversifyExpressUtils/InversifyExpressUsersController.spec.ts

@@ -4,7 +4,7 @@ import * as express from 'express'
 
 import { InversifyExpressUsersController } from './InversifyExpressUsersController'
 import { results } from 'inversify-express-utils'
-import { ControllerContainerInterface } from '@standardnotes/domain-core'
+import { ControllerContainerInterface, Username } from '@standardnotes/domain-core'
 import { DeleteAccount } from '../../Domain/UseCase/DeleteAccount/DeleteAccount'
 import { ChangeCredentials } from '../../Domain/UseCase/ChangeCredentials/ChangeCredentials'
 import { ClearLoginAttempts } from '../../Domain/UseCase/ClearLoginAttempts'
@@ -321,10 +321,7 @@ describe('InversifyExpressUsersController', () => {
       kpOrigination: 'change-password',
       pwNonce: 'asdzxc',
       protocolVersion: '004',
-      user: {
-        uuid: '123',
-        email: '[email protected]',
-      },
+      username: Username.create('[email protected]').getValue(),
     })
 
     expect(clearLoginAttempts.execute).toHaveBeenCalled()

packages/auth/src/Infra/InversifyExpressUtils/InversifyExpressUsersController.ts

@@ -19,7 +19,7 @@ import { GetUserSubscription } from '../../Domain/UseCase/GetUserSubscription/Ge
 import { ClearLoginAttempts } from '../../Domain/UseCase/ClearLoginAttempts'
 import { IncreaseLoginAttempts } from '../../Domain/UseCase/IncreaseLoginAttempts'
 import { ChangeCredentials } from '../../Domain/UseCase/ChangeCredentials/ChangeCredentials'
-import { ControllerContainerInterface } from '@standardnotes/domain-core'
+import { ControllerContainerInterface, Username } from '@standardnotes/domain-core'
 
 @controller('/users')
 export class InversifyExpressUsersController extends BaseHttpController {
@@ -203,9 +203,21 @@ export class InversifyExpressUsersController extends BaseHttpController {
         400,
       )
     }
+    const usernameOrError = Username.create(response.locals.user.email)
+    if (usernameOrError.isFailed()) {
+      return this.json(
+        {
+          error: {
+            message: 'Invalid username.',
+          },
+        },
+        400,
+      )
+    }
+    const username = usernameOrError.getValue()
 
     const changeCredentialsResult = await this.changeCredentialsUseCase.execute({
-      user: response.locals.user,
+      username,
       apiVersion: request.body.api,
       currentPassword: request.body.current_password,
       newPassword: request.body.new_password,