Bee-Lab / BeelabUserBundle

:busts_in_silhouette: Simple user management for Symfony.
17 stars 5 forks source link

Login errors management #7

Closed vrobic closed 7 years ago

vrobic commented 7 years ago

Hi,

I would suggest an improvement of login errors management. These errors are caught in: AuthController.php.

This if condition is too restrictive:

if ($error instanceof \Exception && !$error instanceof BadCredentialsException) {
    $this->get('logger')->log('error', $error->getMessage());
    $error = ['message' => 'Unexpected error.'];
}

So, if the error is an Exception and not a BadCredentialsException, then it's silenced as an 'Unexpected error.'. It makes the DisabledException and others appear like an 'Unexpected error.'. 👎

Is there any good reason for doing that? I would simply remove the condition over BadCredentialsException so that it looks like this :

if ($error instanceof \Exception) {
    $this->get('logger')->log('error', $error->getMessage()); // should we really log this?
    $error = ['message' => $error->getMessage()];
}

I'd be glad to propose a PR for this!

Vincent

garak commented 7 years ago

We can't expose any Exception, since it could lead to disclosure potentially confidential information. Anyway, you're right that BadCredentialsException, I'll extend it to use AuthenticationException instead.