Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Error Message on Login #900

Open
lautarodragan opened this issue Apr 7, 2019 · 0 comments
Open

Improve Error Message on Login #900

lautarodragan opened this issue Apr 7, 2019 · 0 comments
Labels

Comments

@lautarodragan
Copy link
Member

lautarodragan commented Apr 7, 2019

Login currently returns Resource Not Found for anything that isn't a success case, for example:

  • If the account doesn't exist
  • If the account exists but the password doesn't match
  • If any error at all is thrown

export const Login = (verifiedAccount: boolean, pwnedCheckerRoot: string) => async (ctx: any, next: any) => {
const logger = ctx.logger(__dirname)
const { ResourceNotFound } = errors
try {
const user = ctx.request.body
const usersController = new AccountsController(ctx.logger, verifiedAccount, pwnedCheckerRoot)
const response = await usersController.get(user.email)
if (isNil(response)) {
ctx.status = ResourceNotFound.code
ctx.body = ResourceNotFound.message
} else {
await verify(user.password, response.password)
const token = await getToken(user.email, Token.Login)
ctx.body = { token, issuer: response.issuer }
}
} catch (exception) {
logger.error({ exception }, 'api.Login')
ctx.throw(ResourceNotFound.code, ResourceNotFound.message)
}
}

For the first and second case, return a different, more explicit message in: No account matching email and password found.. The same message must be returned in both cases to mitigate brute force attacks that try and guess users' emails.

In the case an error is thrown, omit the try/catch block here, let the error middleware take care of it.

Should become something like the following code.

  const user = ctx.request.body
  const usersController = new AccountsController(ctx.logger, verifiedAccount, pwnedCheckerRoot)
  const response = await usersController.get(user.email)

  if (!response)
    throw new AccountNotFound()
    
  const passwordsMatch = await verify(user.password, response.password) // if we refactor verify

  if (!passwordsMatch)
    throw new AccountNotFound()

  const token = await getToken(user.email, Token.Login)
  ctx.body = { token, issuer: response.issuer }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant