User tests: Successful: Unsuccessful:
If $app->login() returns false or throws exception then it should be the same effect, means do not use 303 redirection.
Redirection 303 should be only use on login success.
Backend login fails if:
Or success if:
Currently Joomla uses redirection if login method return false which is incorrect.
After that patch Apache module security can be use to limit POST requests.
Request POST with http code 200 (login fails) or 303 (login success)
Apply patch and
Fixed. Thanks.
Labels |
Added:
?
|
I have tested this item
2) try to login with invalid username/password. Joomla should not use redirection.
Afterwards I see a page like this:
After that patch mod_security can be use to limit POST request to login.
There is no mod_security in Joomla core.
So: ????????????
This PR show different template/view after login fails, probably I have to fix it.
About mod security:
This is apache module which can be configured to block IP access for some time
if attacker send POST requests to index.php?task=login too many times.
Ex:
if request is POST and task=login and returned code is 200 5 times per last hour
then block IP for 15 minutes.
This PR has received new commits.
CC: @bertmert
Incorrect view fixed.
Please try to test once again.
Category | ⇒ | Administration Authentication |
I have tested this item
After patch status 200 when invalid credentials instead of 303.
Title |
|
Title |
|
Sorry, I do not know what & how to test exactly.
Furthermore I do not understand when & why the 303 status code should be displayed.
IMHO all screens (login screen, login fail screen and login successful) all exist at the server and the server correctly returns the http status code for succss: 200.
I have tested this PR with Google Chrome + Webdeveloper addon ( http://chrispederick.com/work/web-developer/chrome/ )
Before the patch:
Login attempt /administrator/ with valid credentials: HTTP status code: 200
Login attempt /administrator/ with invalid credentials, login error screen: HTTP status code: 200
After the patch:
Login attempt /administrator/ with valid credentials: HTTP status code: 200
Login attempt /administrator/ with invalid credentials, login error screen: HTTP status code: 200
I have tested this item
Cannot reproduce this issue.
Before and after the applying the patch, I get code 200. Both for login fail & login success.
For Chromium and Google Chrome:
Opening Chrome DevTools
Open from Browser Menu
You can open Chrome DevTools from Chrome’s menu , click into “More tools” and then click into “Developer Tools.”
Or shortcut Ctrl+Shift+I
Before patch when you use wrong credentials you should get (after POST request):
means 303 and after that 200 for index.php (which is incorrect, after patch will be only 200)
After patch when you use wrong credentials you should get (after POST request):
means only one request to index.php with response status code 200.
For correct credentials there is no changes and should looks like:
I have tested this item
Thank you.
The change is working as expected.
No more redirect status codes when using wrong passwords.
I have tested this item
@brianteeman RTC please
Just wanted to point out that wp gives a 200 and there is a long standing
issue requesting it to be changed to a 303 which would help mod_security
So just because this pr works I'm personally not convinced yet that it is
the correct thing to do and would like to see more input from sysadmins
Then we need to wait on other SysAdmins and DevOps like me. 303 is wrong, 200 is better but 403 is really the correct one.
When you don't have a correct authentication and you don't redirect to a other page/address and therefore don't use any 301,302 or 303. It's a HTTP OK with wrong credentials, because there is nothing wrong with the request.
If you want to communicate it correctly and follow the HTTP status codes you need to use 403: Forbidden
See: https://en.m.wikipedia.org/wiki/List_of_HTTP_status_codes
In simple terms 200 is OK without redirect.
303 is See other (redirect)
403 is forbidden
For reference this was one of the first posts I found on this https://www.longren.io/wordpress-sending-correct-http-status-code-on-login-failure/ and see the links
I thought of it as regular form and validation.
When the form is validated with success then 303,
when the form has invalid field then 200.
He has written the same as my other post.
Use 403 which is the correct status code.
Yes, the other arguments would suggest you need to use 401 but this is the error code for 401 Authentication which send you a 401 Challenge which you need to respond with the challenge. This can be credentials, token codes, 2-Way SSL certificate and many more.
Because you already received this challenge when you opened the login page you want to receive the 403 when using wrong credentials.
I hope some other people can contribute on this, so we can make a decision.
@yireo @pe7er @roland-d please comment on this with your developer and Linux experience.
403 is the correct response. 401 is only used with the WWW-Authenticate challenge-response protocol (eg. "Basic authentication"), which is not what we're using here. See RFC7235.
+1 for 403
updated:
after readding the PR discussion better i think maybe 200 is ok.
This is a submit action that is submited and the page as returned ok even if the credencials are invalid so for me is a 200 Ok
IMHO 403 is more like you don't have access to that page, when you try a direct URL access to a restricted page, which is not the case.
This visual chart of all status codes is useful http://racksburg.com/choosing-an-http-status-code/
Status | New | ⇒ | Pending |
Setting to Pending as we have code.
Status | Pending | ⇒ | Ready to Commit |
Labels |
Added:
?
|
Setting RTC as there is 3 successful tests.
Sorry but I am removing the RTC setting as it is still not agreed that this is the correct thing to do. Setting to needs Review
Status | Ready to Commit | ⇒ | Needs Review |
Labels |
Labels |
Removed:
?
|
Thanks and I apologies if I don't understand and did rtc.
I have tested this item
@csthomas I had a look at this the other day and been thinking what would be the right response. So the 200 response is not correct if you ask me because it doesn't say anything about the authorization attempt. It just tells us the page was loaded, well yeah we know that much. The 403 error @chrisdavenport mentioned is the correct one reading the RFC he mentions as the RFC states:
The 403 (Forbidden) status code indicates that the server understood
the request but refuses to authorize it.
So if you ask me, we should give a 403.
Discussion from wordpress:
https://core.trac.wordpress.org/ticket/25446
Does anyone send a non-200 for a failed web login? I can't say I've ever noticed this in practice.
Nope. Example Twitter, Facebook, GitHub, Google and BitBucket all returns 200. So I'll +1 for staying with 200
My thoughts about www:
If we send POST request (with incorrect credentials) then the request is not forbidden (it is processed) but the result not allow us to go to the next secure page, then we get the login page instead.
if we send POST request (with correct credentials) then the request is not forbidden (it is processed) but after authentication success we are redirected to secure page (but also may go to public page too).
if we send POST/GET request (example delete data/ get access to secure information) but we do not have rights then we should get 403.
BUT login form do not need to redirect to secure page only.
I agree. A 403 is supposed to be when you are trying to access secure info. Here we have invalid credentials but importantly the login form is NOT a restricted page. Therefore the correct response is a 200 even tho the login credentials are incorrect
How about returning a 403 with a Location header pointing to the login form? The browser will follow the redirect and the subsequent GET will return a 200.
@chrisdavenport Can you show us any website (global known) that return 403 code on the login fails?
Stay with current 303 code is the worse solution.
Can we merge that PR and then discuss about replacing 200 to 403 on a new issue?
Laracasts issues a 422 response
KNPUniversity (built on Symfony) issues a 302
drupal.org issues a 200
Laracasts issues a 422 response
I read http://www.restpatterns.org/HTTP_Status_Codes/422_-_Unprocessable_Entity
The 422 (Unprocessable Entity) status code means the server understands the content type of the request entity [...] but was unable to process the contained instructions.
IMHO 422 is incorrect for the login fails.
How about blocked user.
Login process is processed, login and password is OK but user are blocked.
KNPUniversity (built on Symfony) issues a 302
After POST request we should not use 302 redirection.
http://www.restpatterns.org/HTTP_Status_Codes/302_-_Found
mantainers ... why is this not RTC. this as 4 success tests ...
As far as I can see, it's not about tests, but about different opinions on the error code that should be issued.
The errors core is not changes in this pr. Look at code ...
Title |
|
Category | Administration Authentication | ⇒ | Administration com_login Authentication |
Status | Needs Review | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-02-23 01:20:21 |
Closed_By | ⇒ | wilsonge |
Something isnt correct with this PR as you have a changed memcache file