fix: Prevents failed captchas from causing login lock

packages/auth/src/Bootstrap/Container.ts

@@ -1171,6 +1171,24 @@ export class ContainerConfigLoader {
           container.get<CaptchaServerInterface>(TYPES.Auth_CaptchaServer),
         ),
       )
+    container
+      .bind<ClearLoginAttempts>(TYPES.Auth_ClearLoginAttempts)
+      .toConstantValue(
+        new ClearLoginAttempts(
+          container.get<UserRepositoryInterface>(TYPES.Auth_UserRepository),
+          container.get<LockRepositoryInterface>(TYPES.Auth_LockRepository),
+          container.get<winston.Logger>(TYPES.Auth_Logger),
+        ),
+      )
+    container
+      .bind<IncreaseLoginAttempts>(TYPES.Auth_IncreaseLoginAttempts)
+      .toConstantValue(
+        new IncreaseLoginAttempts(
+          container.get<UserRepositoryInterface>(TYPES.Auth_UserRepository),
+          container.get<LockRepositoryInterface>(TYPES.Auth_LockRepository),
+          container.get<number>(TYPES.Auth_MAX_LOGIN_ATTEMPTS),
+        ),
+      )
     container
       .bind<SignIn>(TYPES.Auth_SignIn)
       .toConstantValue(
@@ -1186,6 +1204,8 @@ export class ContainerConfigLoader {
           container.get<number>(TYPES.Auth_MAX_LOGIN_ATTEMPTS),
           container.get<LockRepositoryInterface>(TYPES.Auth_LockRepository),
           container.get<VerifyHumanInteraction>(TYPES.Auth_VerifyHumanInteraction),
+          container.get<IncreaseLoginAttempts>(TYPES.Auth_IncreaseLoginAttempts),
+          container.get<ClearLoginAttempts>(TYPES.Auth_ClearLoginAttempts),
         ),
       )
     container
@@ -1204,24 +1224,6 @@ export class ContainerConfigLoader {
           container.get<winston.Logger>(TYPES.Auth_Logger),
         ),
       )
-    container
-      .bind<ClearLoginAttempts>(TYPES.Auth_ClearLoginAttempts)
-      .toConstantValue(
-        new ClearLoginAttempts(
-          container.get<UserRepositoryInterface>(TYPES.Auth_UserRepository),
-          container.get<LockRepositoryInterface>(TYPES.Auth_LockRepository),
-          container.get<winston.Logger>(TYPES.Auth_Logger),
-        ),
-      )
-    container
-      .bind<IncreaseLoginAttempts>(TYPES.Auth_IncreaseLoginAttempts)
-      .toConstantValue(
-        new IncreaseLoginAttempts(
-          container.get<UserRepositoryInterface>(TYPES.Auth_UserRepository),
-          container.get<LockRepositoryInterface>(TYPES.Auth_LockRepository),
-          container.get<number>(TYPES.Auth_MAX_LOGIN_ATTEMPTS),
-        ),
-      )
     container
       .bind<GetUserKeyParamsRecovery>(TYPES.Auth_GetUserKeyParamsRecovery)
       .toConstantValue(
@@ -1871,7 +1873,6 @@ export class ContainerConfigLoader {
           container.get<SignIn>(TYPES.Auth_SignIn),
           container.get<GetUserKeyParams>(TYPES.Auth_GetUserKeyParams),
           container.get<ClearLoginAttempts>(TYPES.Auth_ClearLoginAttempts),
-          container.get<IncreaseLoginAttempts>(TYPES.Auth_IncreaseLoginAttempts),
           container.get<winston.Logger>(TYPES.Auth_Logger),
           container.get<AuthController>(TYPES.Auth_AuthController),
           container.get<Register>(TYPES.Auth_Register),

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

@@ -17,6 +17,8 @@ import { Session } from '../Session/Session'
 import { LockRepositoryInterface } from '../User/LockRepositoryInterface'
 import { VerifyHumanInteraction } from './VerifyHumanInteraction/VerifyHumanInteraction'
 import { Result } from '@standardnotes/domain-core'
+import { IncreaseLoginAttempts } from './IncreaseLoginAttempts'
+import { ClearLoginAttempts } from './ClearLoginAttempts'
 
 describe('SignIn', () => {
   let user: User
@@ -33,7 +35,8 @@ describe('SignIn', () => {
   let maxNonCaptchaAttempts: number
   let lockRepository: LockRepositoryInterface
   let verifyHumanInteractionUseCase: VerifyHumanInteraction
-
+  let increaseLoginAttempts: IncreaseLoginAttempts
+  let clearLoginAttempts: ClearLoginAttempts
   const createUseCase = () =>
     new SignIn(
       userRepository,
@@ -47,6 +50,8 @@ describe('SignIn', () => {
       maxNonCaptchaAttempts,
       lockRepository,
       verifyHumanInteractionUseCase,
+      increaseLoginAttempts,
+      clearLoginAttempts,
     )
 
   beforeEach(() => {
@@ -92,8 +97,22 @@ describe('SignIn', () => {
     lockRepository.getLockCounter = jest.fn().mockReturnValue(0)
 
     maxNonCaptchaAttempts = 6
+
+    increaseLoginAttempts = {} as jest.Mocked<IncreaseLoginAttempts>
+    increaseLoginAttempts.execute = jest.fn().mockReturnValue(Result.ok({ isNonCaptchaLimitReached: false }))
+
+    clearLoginAttempts = {} as jest.Mocked<ClearLoginAttempts>
+    clearLoginAttempts.execute = jest.fn()
+
+    verifyHumanInteractionUseCase = {} as jest.Mocked<VerifyHumanInteraction>
+    verifyHumanInteractionUseCase.execute = jest.fn().mockReturnValue(Result.ok())
   })
 
+  const requireHumanVerification = () => {
+    lockRepository.getLockCounter = jest.fn().mockReturnValueOnce(maxNonCaptchaAttempts).mockReturnValueOnce(0)
+    verifyHumanInteractionUseCase.execute = jest.fn()
+  }
+
   it('should fail sign in a legacy user without code verifier', async () => {
     pkceRepository.removeCodeChallenge = jest.fn().mockReturnValue(false)
 
@@ -113,6 +132,7 @@ describe('SignIn', () => {
       success: false,
       errorCode: 410,
       errorMessage: 'Please update your client application.',
+      isNonCaptchaLimitReached: false,
     })
   })
 
@@ -132,6 +152,7 @@ describe('SignIn', () => {
       success: false,
       errorCode: 410,
       errorMessage: 'Please update your client application.',
+      isNonCaptchaLimitReached: false,
     })
   })
 
@@ -157,6 +178,7 @@ describe('SignIn', () => {
       success: false,
       errorCode: 410,
       errorMessage: 'Please update your client application.',
+      isNonCaptchaLimitReached: false,
     })
   })
 
@@ -173,6 +195,7 @@ describe('SignIn', () => {
     ).toEqual({
       success: false,
       errorMessage: 'Username cannot be empty',
+      isNonCaptchaLimitReached: false,
     })
 
     expect(domainEventFactory.createEmailRequestedEvent).not.toHaveBeenCalled()
@@ -192,6 +215,7 @@ describe('SignIn', () => {
     ).toEqual({
       success: false,
       errorMessage: 'Invalid api version: invalid',
+      isNonCaptchaLimitReached: false,
     })
 
     expect(domainEventFactory.createEmailRequestedEvent).not.toHaveBeenCalled()
@@ -218,6 +242,7 @@ describe('SignIn', () => {
 
     expect(domainEventFactory.createEmailRequestedEvent).toHaveBeenCalled()
     expect(domainEventPublisher.publish).toHaveBeenCalled()
+    expect(clearLoginAttempts.execute).toHaveBeenCalledWith({ email: '[email protected]' })
   })
 
   it('should sign in a user even if publishing a sign in event fails', async () => {
@@ -256,6 +281,7 @@ describe('SignIn', () => {
     ).toEqual({
       success: false,
       errorMessage: 'Invalid email or password',
+      isNonCaptchaLimitReached: false,
     })
   })
 
@@ -274,6 +300,7 @@ describe('SignIn', () => {
     ).toEqual({
       success: false,
       errorMessage: 'Invalid email or password',
+      isNonCaptchaLimitReached: false,
     })
   })
 
@@ -292,6 +319,7 @@ describe('SignIn', () => {
     ).toEqual({
       success: false,
       errorMessage: 'Invalid email or password',
+      isNonCaptchaLimitReached: false,
     })
   })
 
@@ -363,6 +391,131 @@ describe('SignIn', () => {
     ).toEqual({
       success: false,
       errorMessage: 'Human verification step failed.',
+      isNonCaptchaLimitReached: true,
     })
   })
+
+  it('should return isNonCaptchaLimitReached when incrementing login attempts reaches the limit', async () => {
+    increaseLoginAttempts.execute = jest.fn().mockReturnValue(Result.ok({ isNonCaptchaLimitReached: true }))
+
+    expect(
+      await createUseCase().execute({
+        email: '[email protected]',
+        password: 'asdasd123123',
+        userAgent: 'Google Chrome',
+        apiVersion: '20190520',
+        ephemeralSession: false,
+        codeVerifier: 'test',
+      }),
+    ).toEqual({
+      success: false,
+      errorMessage: 'Invalid email or password',
+      isNonCaptchaLimitReached: true,
+    })
+  })
+
+  it('should increment login attempts on invalid password', async () => {
+    await createUseCase().execute({
+      email: '[email protected]',
+      password: 'asdasd123123',
+      userAgent: 'Google Chrome',
+      apiVersion: '20190520',
+      ephemeralSession: false,
+      codeVerifier: 'test',
+    })
+
+    expect(increaseLoginAttempts.execute).toHaveBeenCalledWith({
+      email: '[email protected]',
+      skipUsernameValidation: true,
+    })
+  })
+
+  it('should not increment login attempts when human verification fails', async () => {
+    requireHumanVerification()
+    verifyHumanInteractionUseCase.execute = jest
+      .fn()
+      .mockReturnValueOnce(Result.fail('Human verification step failed.'))
+
+    await createUseCase().execute({
+      email: '[email protected]',
+      password: 'qweqwe123123',
+      userAgent: 'Google Chrome',
+      apiVersion: '20190520',
+      ephemeralSession: false,
+      codeVerifier: 'test',
+      hvmToken: 'bad-token',
+    })
+
+    expect(increaseLoginAttempts.execute).not.toHaveBeenCalled()
+  })
+
+  it('should not increment login attempts when human verification token is missing', async () => {
+    requireHumanVerification()
+    verifyHumanInteractionUseCase.execute = jest.fn().mockReturnValueOnce(Result.fail('No HVM token available.'))
+
+    await createUseCase().execute({
+      email: '[email protected]',
+      password: 'qweqwe123123',
+      userAgent: 'Google Chrome',
+      apiVersion: '20190520',
+      ephemeralSession: false,
+      codeVerifier: 'test',
+    })
+
+    expect(increaseLoginAttempts.execute).not.toHaveBeenCalled()
+  })
+
+  it('should not set isNonCaptchaLimitReached when increasing login attempts fails', async () => {
+    increaseLoginAttempts.execute = jest.fn().mockReturnValue(Result.fail('invalid email'))
+
+    expect(
+      await createUseCase().execute({
+        email: '[email protected]',
+        password: 'asdasd123123',
+        userAgent: 'Google Chrome',
+        apiVersion: '20190520',
+        ephemeralSession: false,
+        codeVerifier: 'test',
+      }),
+    ).toEqual({
+      success: false,
+      errorMessage: 'Invalid email or password',
+    })
+  })
+
+  it('should require human verification in captcha mode and not increment on failure', async () => {
+    lockRepository.getLockCounter = jest.fn().mockReturnValueOnce(0).mockReturnValueOnce(1)
+    verifyHumanInteractionUseCase.execute = jest
+      .fn()
+      .mockReturnValueOnce(Result.fail('Human verification step failed.'))
+
+    await createUseCase().execute({
+      email: '[email protected]',
+      password: 'qweqwe123123',
+      userAgent: 'Google Chrome',
+      apiVersion: '20190520',
+      ephemeralSession: false,
+      codeVerifier: 'test',
+      hvmToken: 'bad-token',
+    })
+
+    expect(increaseLoginAttempts.execute).not.toHaveBeenCalled()
+  })
+
+  it('should not increment login attempts on human verification failure for unknown user', async () => {
+    userRepository.findOneByUsernameOrEmail = jest.fn().mockReturnValue(null)
+    lockRepository.getLockCounter = jest.fn().mockReturnValueOnce(maxNonCaptchaAttempts).mockReturnValueOnce(0)
+    verifyHumanInteractionUseCase.execute = jest.fn().mockReturnValueOnce(Result.fail('No HVM token available.'))
+
+    await createUseCase().execute({
+      email: '[email protected]',
+      password: 'asdasd123123',
+      userAgent: 'Google Chrome',
+      apiVersion: '20190520',
+      ephemeralSession: false,
+      codeVerifier: 'test',
+    })
+
+    expect(increaseLoginAttempts.execute).not.toHaveBeenCalled()
+  })
 })

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

@@ -18,6 +18,8 @@ import { ApiVersion } from '../Api/ApiVersion'
 import { HttpStatusCode } from '@standardnotes/responses'
 import { VerifyHumanInteraction } from './VerifyHumanInteraction/VerifyHumanInteraction'
 import { LockRepositoryInterface } from '../User/LockRepositoryInterface'
+import { IncreaseLoginAttempts } from './IncreaseLoginAttempts'
+import { ClearLoginAttempts } from './ClearLoginAttempts'
 
 export class SignIn implements UseCaseInterface {
   constructor(
@@ -32,43 +34,36 @@ export class SignIn implements UseCaseInterface {
     private maxNonCaptchaAttempts: number,
     private lockRepository: LockRepositoryInterface,
     private verifyHumanInteractionUseCase: VerifyHumanInteraction,
+    private increaseLoginAttempts: IncreaseLoginAttempts,
+    private clearLoginAttempts: ClearLoginAttempts,
   ) {}
 
   async execute(dto: SignInDTO): Promise<SignInResponse> {
     if (!dto.codeVerifier) {
-      return {
-        success: false,
-        errorMessage: 'Please update your client application.',
-        errorCode: HttpStatusCode.Gone,
-      }
+      return this.failAfterIncrementingLoginAttempts(
+        dto.email,
+        'Please update your client application.',
+        HttpStatusCode.Gone,
+      )
     }
 
     const validCodeVerifier = await this.validateCodeVerifier(dto.codeVerifier)
     if (!validCodeVerifier) {
       this.logger.debug('Code verifier does not match')
 
-      return {
-        success: false,
-        errorMessage: 'Invalid email or password',
-      }
+      return this.failAfterIncrementingLoginAttempts(dto.email, 'Invalid email or password')
     }
 
     const apiVersionOrError = ApiVersion.create(dto.apiVersion)
     if (apiVersionOrError.isFailed()) {
-      return {
-        success: false,
-        errorMessage: apiVersionOrError.getError(),
-      }
+      return this.failAfterIncrementingLoginAttempts(dto.email, apiVersionOrError.getError())
     }
     const apiVersion = apiVersionOrError.getValue()
 
     /** Skip validation which was newly added in 2025, to allow existing users to continue to sign in */
     const usernameOrError = Username.create(dto.email, { skipValidation: true })
     if (usernameOrError.isFailed()) {
-      return {
-        success: false,
-        errorMessage: usernameOrError.getError(),
-      }
+      return this.failAfterIncrementingLoginAttempts(dto.email, usernameOrError.getError())
     }
     const username = usernameOrError.getValue()
 
@@ -83,26 +78,21 @@ export class SignIn implements UseCaseInterface {
       return {
         success: false,
         errorMessage: humanVerificationBeforeCheckingUsernameAndPasswordResult.getError(),
+        isNonCaptchaLimitReached: true,
       }
     }
 
     if (!user) {
       this.logger.debug(`User with email ${dto.email} was not found`)
 
-      return {
-        success: false,
-        errorMessage: 'Invalid email or password',
-      }
+      return this.failAfterIncrementingLoginAttempts(dto.email, 'Invalid email or password')
     }
 
     const passwordMatches = await bcrypt.compare(dto.password, user.encryptedPassword)
     if (!passwordMatches) {
       this.logger.debug('Password does not match')
 
-      return {
-        success: false,
-        errorMessage: 'Invalid email or password',
-      }
+      return this.failAfterIncrementingLoginAttempts(dto.email, 'Invalid email or password')
     }
 
     const authResponseFactory = this.authResponseFactoryResolver.resolveAuthResponseFactoryVersion(apiVersion)
@@ -119,12 +109,34 @@ export class SignIn implements UseCaseInterface {
       application: dto.application,
     })
 
+    await this.clearLoginAttempts.execute({ email: dto.email })
+
     return {
       success: true,
       result,
     }
   }
 
+  private async failAfterIncrementingLoginAttempts(
+    email: string,
+    errorMessage: string,
+    errorCode?: HttpStatusCode,
+  ): Promise<SignInResponse> {
+    const increaseResultOrError = await this.increaseLoginAttempts.execute({
+      email,
+      skipUsernameValidation: true,
+    })
+
+    return {
+      success: false,
+      errorMessage,
+      errorCode,
+      isNonCaptchaLimitReached: increaseResultOrError.isFailed()
+        ? undefined
+        : increaseResultOrError.getValue().isNonCaptchaLimitReached,
+    }
+  }
+
   private async validateCodeVerifier(codeVerifier: string): Promise<boolean> {
     const codeChallenge = this.crypter.base64URLEncode(this.crypter.sha256Hash(codeVerifier))
 

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

@@ -7,6 +7,7 @@ export type SignInResponse =
       success: false
       errorMessage: string
       errorCode?: HttpStatusCode
+      isNonCaptchaLimitReached?: boolean
     }
   | {
       success: true

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

@@ -1,6 +1,5 @@
 import { Result } from '@standardnotes/domain-core'
 
-import { AuthResponse20200115 } from '../../Auth/AuthResponse20200115'
 import { AuthResponseFactory20200115 } from '../../Auth/AuthResponseFactory20200115'
 import { AuthenticatorRepositoryInterface } from '../../Authenticator/AuthenticatorRepositoryInterface'
 import { CrypterInterface } from '../../Encryption/CrypterInterface'
@@ -58,7 +57,7 @@ describe('SignInWithRecoveryCodes', () => {
     } as jest.Mocked<User>)
 
     authResponseFactory = {} as jest.Mocked<AuthResponseFactory20200115>
-    authResponseFactory.createResponse = jest.fn().mockReturnValue({} as jest.Mocked<AuthResponse20200115>)
+    authResponseFactory.createResponse = jest.fn().mockReturnValue({ response: { foo: 'bar' }, session: {} })
 
     pkceRepository = {} as jest.Mocked<PKCERepositoryInterface>
     pkceRepository.removeCodeChallenge = jest.fn().mockReturnValue(true)
@@ -76,7 +75,7 @@ describe('SignInWithRecoveryCodes', () => {
     generateRecoveryCodes.execute = jest.fn().mockReturnValue(Result.ok('1234 5678'))
 
     increaseLoginAttempts = {} as jest.Mocked<IncreaseLoginAttempts>
-    increaseLoginAttempts.execute = jest.fn()
+    increaseLoginAttempts.execute = jest.fn().mockReturnValue(Result.ok({ isNonCaptchaLimitReached: false }))
 
     clearLoginAttempts = {} as jest.Mocked<ClearLoginAttempts>
     clearLoginAttempts.execute = jest.fn()
@@ -91,8 +90,16 @@ describe('SignInWithRecoveryCodes', () => {
     lockRepository.getLockCounter = jest.fn().mockReturnValue(0)
 
     maxNonCaptchaAttempts = 6
+
+    verifyHumanInteractionUseCase = {} as jest.Mocked<VerifyHumanInteraction>
+    verifyHumanInteractionUseCase.execute = jest.fn().mockReturnValue(Result.ok())
   })
 
+  const requireHumanVerification = () => {
+    lockRepository.getLockCounter = jest.fn().mockReturnValueOnce(maxNonCaptchaAttempts).mockReturnValueOnce(0)
+    verifyHumanInteractionUseCase.execute = jest.fn()
+  }
+
   it('should return error if password is not provided', async () => {
     const result = await createUseCase().execute({
       apiVersion: ApiVersion.VERSIONS.v20200115,
@@ -103,8 +110,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Empty password')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Empty password',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if username is not provided', async () => {
@@ -117,8 +127,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Could not sign in with recovery codes: Username cannot be empty')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Could not sign in with recovery codes: Username cannot be empty',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if code verifier is not provided', async () => {
@@ -131,8 +144,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Invalid code verifier')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid code verifier',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if recovery codes are not provided', async () => {
@@ -145,8 +161,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Empty recovery codes')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Empty recovery codes',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if code verifier is invalid', async () => {
@@ -161,8 +180,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Invalid code verifier')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid code verifier',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if user is not found', async () => {
@@ -177,8 +199,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Could not find user')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Could not find user',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if recovery codes are invalid', async () => {
@@ -191,8 +216,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Invalid recovery codes')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid recovery codes',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if api version is invalid', async () => {
@@ -205,7 +233,15 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid api version: invalid',
+      isNonCaptchaLimitReached: false,
+    })
+    expect(increaseLoginAttempts.execute).toHaveBeenCalledWith({
+      email: '[email protected]',
+      skipUsernameValidation: true,
+    })
   })
 
   it('should return error if api version does not support recovery sign in', async () => {
@@ -218,7 +254,15 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Unsupported api version',
+      isNonCaptchaLimitReached: false,
+    })
+    expect(increaseLoginAttempts.execute).toHaveBeenCalledWith({
+      email: '[email protected]',
+      skipUsernameValidation: true,
+    })
   })
 
   it('should return error if password does not match', async () => {
@@ -231,8 +275,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Invalid password')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid password',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if recovery codes are not generated for user', async () => {
@@ -247,8 +294,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: '1234 5678',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('User does not have recovery codes generated')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'User does not have recovery codes generated',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if generating new recovery codes fails', async () => {
@@ -263,8 +313,12 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: 'foo',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Could not sign in with recovery codes: Oops')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Could not sign in with recovery codes: Oops',
+      isNonCaptchaLimitReached: false,
+    })
+    expect(authResponseFactory.createResponse).not.toHaveBeenCalled()
   })
 
   it('should return error if user has an invalid uuid', async () => {
@@ -282,8 +336,11 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: 'foo',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Invalid user uuid')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid user uuid',
+      isNonCaptchaLimitReached: false,
+    })
   })
 
   it('should return error if user requires human verification but no hvmtoken provided', async () => {
@@ -302,8 +359,100 @@ describe('SignInWithRecoveryCodes', () => {
       recoveryCodes: 'foo',
     })
 
-    expect(result.isFailed()).toBe(true)
-    expect(result.getError()).toBe('Human verification step failed.')
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Human verification step failed.',
+      isNonCaptchaLimitReached: true,
+    })
+  })
+
+  it('should return isNonCaptchaLimitReached when incrementing login attempts reaches the limit', async () => {
+    increaseLoginAttempts.execute = jest.fn().mockReturnValue(Result.ok({ isNonCaptchaLimitReached: true }))
+
+    const result = await createUseCase().execute({
+      apiVersion: ApiVersion.VERSIONS.v20200115,
+      userAgent: 'user-agent',
+      username: '[email protected]',
+      password: 'asdasd123123',
+      codeVerifier: 'code-verifier',
+      recoveryCodes: '1234 5678',
+    })
+
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid password',
+      isNonCaptchaLimitReached: true,
+    })
+  })
+
+  it('should increment login attempts once on invalid password', async () => {
+    await createUseCase().execute({
+      apiVersion: ApiVersion.VERSIONS.v20200115,
+      userAgent: 'user-agent',
+      username: '[email protected]',
+      password: 'asdasd123123',
+      codeVerifier: 'code-verifier',
+      recoveryCodes: '1234 5678',
+    })
+
+    expect(increaseLoginAttempts.execute).toHaveBeenCalledTimes(1)
+    expect(increaseLoginAttempts.execute).toHaveBeenCalledWith({
+      email: '[email protected]',
+      skipUsernameValidation: true,
+    })
+  })
+
+  it('should not increment login attempts when human verification fails', async () => {
+    requireHumanVerification()
+    verifyHumanInteractionUseCase.execute = jest
+      .fn()
+      .mockReturnValueOnce(Result.fail('Human verification step failed.'))
+
+    await createUseCase().execute({
+      apiVersion: ApiVersion.VERSIONS.v20200115,
+      userAgent: 'user-agent',
+      username: '[email protected]',
+      password: 'qweqwe123123',
+      codeVerifier: 'code-verifier',
+      recoveryCodes: 'foo',
+      hvmToken: 'bad-token',
+    })
+
+    expect(increaseLoginAttempts.execute).not.toHaveBeenCalled()
+  })
+
+  it('should not increment login attempts when human verification token is missing', async () => {
+    requireHumanVerification()
+    verifyHumanInteractionUseCase.execute = jest.fn().mockReturnValueOnce(Result.fail('No HVM token available.'))
+
+    await createUseCase().execute({
+      apiVersion: ApiVersion.VERSIONS.v20200115,
+      userAgent: 'user-agent',
+      username: '[email protected]',
+      password: 'qweqwe123123',
+      codeVerifier: 'code-verifier',
+      recoveryCodes: 'foo',
+    })
+
+    expect(increaseLoginAttempts.execute).not.toHaveBeenCalled()
+  })
+
+  it('should not set isNonCaptchaLimitReached when increasing login attempts fails', async () => {
+    increaseLoginAttempts.execute = jest.fn().mockReturnValue(Result.fail('invalid email'))
+
+    const result = await createUseCase().execute({
+      apiVersion: ApiVersion.VERSIONS.v20200115,
+      userAgent: 'user-agent',
+      username: '[email protected]',
+      password: 'asdasd123123',
+      codeVerifier: 'code-verifier',
+      recoveryCodes: '1234 5678',
+    })
+
+    expect(result).toEqual({
+      success: false,
+      errorMessage: 'Invalid password',
+    })
   })
 
   it('should return auth response with human verification required and passing', async () => {
@@ -326,7 +475,7 @@ describe('SignInWithRecoveryCodes', () => {
       userUuid: '00000000-0000-0000-0000-000000000000',
     })
     expect(authenticatorRepository.removeByUserUuid).toHaveBeenCalled()
-    expect(result.isFailed()).toBe(false)
+    expect(result.success).toBe(true)
   })
 
   it('should return auth response', async () => {
@@ -345,6 +494,6 @@ describe('SignInWithRecoveryCodes', () => {
       userUuid: '00000000-0000-0000-0000-000000000000',
     })
     expect(authenticatorRepository.removeByUserUuid).toHaveBeenCalled()
-    expect(result.isFailed()).toBe(false)
+    expect(result.success).toBe(true)
   })
 })

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

@@ -1,13 +1,13 @@
 import * as bcrypt from 'bcryptjs'
-import { Result, SettingName, UseCaseInterface, Username, Uuid, Validator } from '@standardnotes/domain-core'
+import { Result, SettingName, Username, Uuid, Validator } from '@standardnotes/domain-core'
 
-import { AuthResponse20200115 } from '../../Auth/AuthResponse20200115'
 import { CrypterInterface } from '../../Encryption/CrypterInterface'
 import { PKCERepositoryInterface } from '../../User/PKCERepositoryInterface'
 import { UserRepositoryInterface } from '../../User/UserRepositoryInterface'
 import { GenerateRecoveryCodes } from '../GenerateRecoveryCodes/GenerateRecoveryCodes'
 
 import { SignInWithRecoveryCodesDTO } from './SignInWithRecoveryCodesDTO'
+import { SignInWithRecoveryCodesResponse } from './SignInWithRecoveryCodesResponse'
 import { AuthResponseFactory20200115 } from '../../Auth/AuthResponseFactory20200115'
 import { IncreaseLoginAttempts } from '../IncreaseLoginAttempts'
 import { ClearLoginAttempts } from '../ClearLoginAttempts'
@@ -17,8 +17,9 @@ import { ApiVersion } from '../../Api/ApiVersion'
 import { GetSetting } from '../GetSetting/GetSetting'
 import { LockRepositoryInterface } from '../../User/LockRepositoryInterface'
 import { VerifyHumanInteraction } from '../VerifyHumanInteraction/VerifyHumanInteraction'
+import { UseCaseInterface } from '../UseCaseInterface'
 
-export class SignInWithRecoveryCodes implements UseCaseInterface<AuthResponse20200115> {
+export class SignInWithRecoveryCodes implements UseCaseInterface {
   constructor(
     private userRepository: UserRepositoryInterface,
     private authResponseFactory: AuthResponseFactory20200115,
@@ -35,20 +36,23 @@ export class SignInWithRecoveryCodes implements UseCaseInterface<AuthResponse202
     private verifyHumanInteractionUseCase: VerifyHumanInteraction,
   ) {}
 
-  async execute(dto: SignInWithRecoveryCodesDTO): Promise<Result<AuthResponse20200115>> {
+  async execute(dto: SignInWithRecoveryCodesDTO): Promise<SignInWithRecoveryCodesResponse> {
     const apiVersionOrError = ApiVersion.create(dto.apiVersion)
     if (apiVersionOrError.isFailed()) {
-      return Result.fail(apiVersionOrError.getError())
+      return this.failAfterIncrementingLoginAttempts(dto.username, apiVersionOrError.getError())
     }
     const apiVersion = apiVersionOrError.getValue()
 
     if (!apiVersion.isSupportedForRecoverySignIn()) {
-      return Result.fail('Unsupported api version')
+      return this.failAfterIncrementingLoginAttempts(dto.username, 'Unsupported api version')
     }
 
     const usernameOrError = Username.create(dto.username)
     if (usernameOrError.isFailed()) {
-      return Result.fail(`Could not sign in with recovery codes: ${usernameOrError.getError()}`)
+      return this.failAfterIncrementingLoginAttempts(
+        dto.username,
+        `Could not sign in with recovery codes: ${usernameOrError.getError()}`,
+      )
     }
     const username = usernameOrError.getValue()
 
@@ -60,49 +64,41 @@ export class SignInWithRecoveryCodes implements UseCaseInterface<AuthResponse202
       dto.hvmToken,
     )
     if (humanVerificationBeforeCheckingUsernameAndPasswordResult.isFailed()) {
-      return Result.fail(humanVerificationBeforeCheckingUsernameAndPasswordResult.getError())
+      return {
+        success: false,
+        errorMessage: humanVerificationBeforeCheckingUsernameAndPasswordResult.getError(),
+        isNonCaptchaLimitReached: true,
+      }
     }
 
     const validCodeVerifier = await this.validateCodeVerifier(dto.codeVerifier)
     if (!validCodeVerifier) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('Invalid code verifier')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'Invalid code verifier')
     }
 
     const passwordValidationResult = Validator.isNotEmpty(dto.password)
     if (passwordValidationResult.isFailed()) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('Empty password')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'Empty password')
     }
 
     const recoveryCodesValidationResult = Validator.isNotEmpty(dto.recoveryCodes)
     if (recoveryCodesValidationResult.isFailed()) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('Empty recovery codes')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'Empty recovery codes')
     }
 
     if (!user) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('Could not find user')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'Could not find user')
     }
 
     const userUuidOrError = Uuid.create(user.uuid)
     if (userUuidOrError.isFailed()) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('Invalid user uuid')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'Invalid user uuid')
     }
     const userUuid = userUuidOrError.getValue()
 
     const passwordMatches = await bcrypt.compare(dto.password, user.encryptedPassword)
     if (!passwordMatches) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('Invalid password')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'Invalid password')
     }
 
     const recoveryCodesSettingOrError = await this.getSetting.execute({
@@ -112,19 +108,25 @@ export class SignInWithRecoveryCodes implements UseCaseInterface<AuthResponse202
       allowSensitiveRetrieval: true,
     })
     if (recoveryCodesSettingOrError.isFailed()) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('User does not have recovery codes generated')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'User does not have recovery codes generated')
     }
     const recoveryCodesSetting = recoveryCodesSettingOrError.getValue()
 
     if (recoveryCodesSetting.decryptedValue !== dto.recoveryCodes) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail('Invalid recovery codes')
+      return this.failAfterIncrementingLoginAttempts(username.value, 'Invalid recovery codes')
     }
 
-    const authResponse = await this.authResponseFactory.createResponse({
+    const generateNewRecoveryCodesResult = await this.generateRecoveryCodes.execute({
+      userUuid: user.uuid,
+    })
+    if (generateNewRecoveryCodesResult.isFailed()) {
+      return this.failAfterIncrementingLoginAttempts(
+        username.value,
+        `Could not sign in with recovery codes: ${generateNewRecoveryCodesResult.getError()}`,
+      )
+    }
+
+    const authResponseCreationResult = await this.authResponseFactory.createResponse({
       user,
       apiVersion,
       userAgent: dto.userAgent,
@@ -134,15 +136,6 @@ export class SignInWithRecoveryCodes implements UseCaseInterface<AuthResponse202
       application: dto.application,
     })
 
-    const generateNewRecoveryCodesResult = await this.generateRecoveryCodes.execute({
-      userUuid: user.uuid,
-    })
-    if (generateNewRecoveryCodesResult.isFailed()) {
-      await this.increaseLoginAttempts.execute({ email: username.value })
-
-      return Result.fail(`Could not sign in with recovery codes: ${generateNewRecoveryCodesResult.getError()}`)
-    }
-
     await this.deleteSetting.execute({
       settingName: SettingName.NAMES.MfaSecret,
       userUuid: user.uuid,
@@ -152,7 +145,28 @@ export class SignInWithRecoveryCodes implements UseCaseInterface<AuthResponse202
 
     await this.clearLoginAttempts.execute({ email: username.value })
 
-    return Result.ok(authResponse.response as AuthResponse20200115)
+    return {
+      success: true,
+      result: authResponseCreationResult,
+    }
+  }
+
+  private async failAfterIncrementingLoginAttempts(
+    email: string,
+    errorMessage: string,
+  ): Promise<SignInWithRecoveryCodesResponse> {
+    const increaseResultOrError = await this.increaseLoginAttempts.execute({
+      email,
+      skipUsernameValidation: true,
+    })
+
+    return {
+      success: false,
+      errorMessage,
+      isNonCaptchaLimitReached: increaseResultOrError.isFailed()
+        ? undefined
+        : increaseResultOrError.getValue().isNonCaptchaLimitReached,
+    }
   }
 
   private async validateCodeVerifier(codeVerifier: string): Promise<boolean> {

packages/auth/src/Domain/UseCase/SignInWithRecoveryCodes/SignInWithRecoveryCodesResponse.ts

@@ -0,0 +1,12 @@
+import { AuthResponseCreationResult } from '../../Auth/AuthResponseCreationResult'
+
+export type SignInWithRecoveryCodesResponse =
+  | {
+      success: false
+      errorMessage: string
+      isNonCaptchaLimitReached?: boolean
+    }
+  | {
+      success: true
+      result: AuthResponseCreationResult
+    }

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

@@ -10,7 +10,6 @@ import TYPES from '../../Bootstrap/Types'
 import { SignIn } from '../../Domain/UseCase/SignIn'
 import { ClearLoginAttempts } from '../../Domain/UseCase/ClearLoginAttempts'
 import { VerifyMFA } from '../../Domain/UseCase/VerifyMFA'
-import { IncreaseLoginAttempts } from '../../Domain/UseCase/IncreaseLoginAttempts'
 import { Logger } from 'winston'
 import { GetUserKeyParams } from '../../Domain/UseCase/GetUserKeyParams/GetUserKeyParams'
 import { AuthController } from '../../Controller/AuthController'
@@ -32,7 +31,6 @@ export class AnnotatedAuthController extends BaseAuthController {
     @inject(TYPES.Auth_SignIn) override signInUseCase: SignIn,
     @inject(TYPES.Auth_GetUserKeyParams) override getUserKeyParams: GetUserKeyParams,
     @inject(TYPES.Auth_ClearLoginAttempts) override clearLoginAttempts: ClearLoginAttempts,
-    @inject(TYPES.Auth_IncreaseLoginAttempts) override increaseLoginAttempts: IncreaseLoginAttempts,
     @inject(TYPES.Auth_Logger) override logger: Logger,
     @inject(TYPES.Auth_AuthController) override authController: AuthController,
     @inject(TYPES.Auth_Register) override registerUser: Register,
@@ -50,7 +48,6 @@ export class AnnotatedAuthController extends BaseAuthController {
       signInUseCase,
       getUserKeyParams,
       clearLoginAttempts,
-      increaseLoginAttempts,
       logger,
       authController,
       registerUser,

packages/auth/src/Infra/InversifyExpressUtils/Base/BaseAuthController.ts

@@ -4,7 +4,6 @@ import { Logger } from 'winston'
 
 import { ClearLoginAttempts } from '../../../Domain/UseCase/ClearLoginAttempts'
 import { GetUserKeyParams } from '../../../Domain/UseCase/GetUserKeyParams/GetUserKeyParams'
-import { IncreaseLoginAttempts } from '../../../Domain/UseCase/IncreaseLoginAttempts'
 import { SignIn } from '../../../Domain/UseCase/SignIn'
 import { VerifyMFA } from '../../../Domain/UseCase/VerifyMFA'
 import { AuthController } from '../../../Controller/AuthController'
@@ -29,7 +28,6 @@ export class BaseAuthController extends BaseHttpController {
     protected signInUseCase: SignIn,
     protected getUserKeyParams: GetUserKeyParams,
     protected clearLoginAttempts: ClearLoginAttempts,
-    protected increaseLoginAttempts: IncreaseLoginAttempts,
     protected logger: Logger,
     protected authController: AuthController,
     protected registerUser: Register,
@@ -147,16 +145,8 @@ export class BaseAuthController extends BaseHttpController {
     })
 
     if (!signInResult.success) {
-      const resultOrError = await this.increaseLoginAttempts.execute({ email: request.body.email, skipUsernameValidation: true })
-      if (resultOrError.isFailed()) {
-        this.logger.error(`Failed to increase login attempts: ${resultOrError.getError()}`, {
-          application: request.headers['x-application-version'] as string,
-        })
-      } else {
-        const result = resultOrError.getValue()
-        if (result.isNonCaptchaLimitReached) {
-          response.setHeader('x-captcha-required', this.captchaUIUrl)
-        }
+      if (signInResult.isNonCaptchaLimitReached) {
+        response.setHeader('x-captcha-required', this.captchaUIUrl)
       }
 
       return this.json(
@@ -169,8 +159,6 @@ export class BaseAuthController extends BaseHttpController {
       )
     }
 
-    await this.clearLoginAttempts.execute({ email: request.body.email })
-
     if (signInResult.result.response !== undefined) {
       const session = signInResult.result.session as Session
       const user = signInResult.result.response.user
@@ -208,7 +196,7 @@ export class BaseAuthController extends BaseHttpController {
   }
 
   async recoveryLogin(request: Request, response: Response): Promise<results.JsonResult> {
-    const result = await this.signInWithRecoveryCodes.execute({
+    const signInResponse = await this.signInWithRecoveryCodes.execute({
       apiVersion: request.body.api_version,
       userAgent: <string>request.headers['user-agent'],
       codeVerifier: request.body.code_verifier,
@@ -220,24 +208,11 @@ export class BaseAuthController extends BaseHttpController {
       application: request.headers['x-application-version'] as string,
     })
 
-    if (result.isFailed()) {
-      this.logger.debug(`Failed to sign in with recovery codes: ${result.getError()}`)
+    if (!signInResponse.success) {
+      this.logger.debug(`Failed to sign in with recovery codes: ${signInResponse.errorMessage}`)
 
-      const increasLoginAttemtpsResultOrError = await this.increaseLoginAttempts.execute({
-        email: request.body.username,
-      })
-      if (increasLoginAttemtpsResultOrError.isFailed()) {
-        this.logger.error(
-          `Failed to increase login attempts on recovery login: ${increasLoginAttemtpsResultOrError.getError()}`,
-          {
-            application: request.headers['x-application-version'] as string,
-          },
-        )
-      } else {
-        const increasLoginAttemtpsResult = increasLoginAttemtpsResultOrError.getValue()
-        if (increasLoginAttemtpsResult.isNonCaptchaLimitReached) {
-          response.setHeader('x-captcha-required', this.captchaUIUrl)
-        }
+      if (signInResponse.isNonCaptchaLimitReached) {
+        response.setHeader('x-captcha-required', this.captchaUIUrl)
       }
 
       return this.json(
@@ -250,14 +225,23 @@ export class BaseAuthController extends BaseHttpController {
       )
     }
 
-    await this.clearLoginAttempts.execute({ email: request.body.username })
+    const authResponse = signInResponse.result.response
 
-    const signInWithRecoveryCodesResult = result.getValue()
+    if (authResponse === undefined) {
+      return this.json(
+        {
+          error: {
+            message: 'Invalid login credentials.',
+          },
+        },
+        HttpStatusCode.Unauthorized,
+      )
+    }
 
     return this.json({
-      session: signInWithRecoveryCodesResult.sessionBody,
-      key_params: signInWithRecoveryCodesResult.keyParams,
-      user: signInWithRecoveryCodesResult.user,
+      session: authResponse.sessionBody,
+      key_params: authResponse.keyParams,
+      user: authResponse.user,
     })
   }