getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

XHR: 400 Bad Request error on log-in attempt with invalid credentials #2856

Closed GiantCrocodile closed 3 years ago

GiantCrocodile commented 4 years ago

Describe the bug
You get two errors during a log-in attempt with invalid credentials in the browser console.

  1. a XHR 400 Bad Request request
  2. a code 400 error because the password doesn't match:
code: 400
details: Array []
exception: "Kirby\\Exception\\InvalidArgumentException"
file: "/kirby/kirby/src/Cms/User.php"
key: "error.user.password.notSame"
line: 926
message: "Die Passwörter stimmen nicht überein"
route: "auth/login"
status: "error"

The second error is fine but I think the first one shouldn't happen or has a wrong error code.

To Reproduce
Steps to reproduce the behavior:

  1. Go to your panel to log in
  2. Type in an existing email address
  3. Type in a wrong password
  4. Confirm the dialog
  5. Check your browser console

Expected behavior
Just one proper error message.

Kirby Version
3.4.3

Desktop (please complete the following information):
Win 10, latest Firefox, WAMP setup

Additional context

distantnative commented 3 years ago

The second error is fine but I think the first one shouldn't happen

Why not? Isn't the POST request you sent (with the invalid login details) a request that fails?

GiantCrocodile commented 3 years ago

According to my understanding the HTTP code 400 is used for invalid requests from clients where the request itself is malformed, e. g. missing parameters or invalid parameter values which are never valid for the given request/API. In this case a invalid password is still a valid request regarding its syntax and context. I think there is no other place where you get a HTTP error and a Kirby 3 error at the same time? If the intention was to signalize an invalid logon attempt, then a 403 or 401 might fit better, see https://stackoverflow.com/a/6937030 for further details.

distantnative commented 3 years ago

@bastianallgeier @lukasbestle I think 401 would be the fitting one then for this case. Agreed?

lukasbestle commented 3 years ago

I agree!

bastianallgeier commented 3 years ago