HTTP 401 on failed login

[rems4e]

• Gitea version (or commit ref): 1.1.3
• Git version: 2.11
• Operating system: Debian Stretch
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL) https://try.gitea.io/user/login
  • No
  • Not relevant
• Log gist:

Description

When purposely causing a login error on Gitea, I can see (using web inspector and/or logfiles) that the HTTP return code is 200, i.e. "everything is ok", for the page that presents the error to the user.

It would be great if Gitea would return a 401 ("Unauthorized", see here).

Indeed, I think a 401 code in the webserver logs has great security value, and makes it easy to integrate with solutions such as Fail2Ban and others.

If this change were made, there would be absolutely no impact on the user.

Screenshots

[daviian]

I agree with that.

@lunny Can I work on that as a starter for contribution? ;-)

[lafriks]

@daviian sure, go ahead but this should be tested on all supported browsers if this works as expected and displays correct content not generic browser error page

[sapk]

@daviian This should be easy to start (should be near https://github.com/go-gitea/gitea/blob/master/routers/user/auth.go#L133).
Don't forget to read : CONTRIBUTING.
Feel free to add integration test (this will maybe broke some)

And don't hesitate to ask any question here or on discord

Have fun 😄

[daviian]

After having a look into the code I found out that the HTTP status code is wrong in multiple cases.
The function used for prompting errors in form validation should generally return the 400 Bad Request status code, but instead it returns 200..
But this is a major change because it is used throughout gitea.

I think the most beautiful solution would be to extend this function with a status code parameter, but that would have a huge impact on other parts of the code.
In a first step I could remove the usage of this function for the authentication errors to return a 401 code.

[ethantkoenig]

I'm not sure returning a 401 on failed login is the correct thing to do; see https://stackoverflow.com/questions/8273757/should-a-failed-login-attempt-result-in-a-http-401-response

I've investigated several public-facing sites (Google, Amazon, Instagram, Github, Gitlab), and none of them returned 401 on failed login.

[rems4e]

To my surprise, I've seen that they don't. I guess they don't apply Fail2Ban rules on an Apache reverse proxy's log file like I'm currently doing!

Maybe it was an XY-problem and what we should have is a log file with the IP of the offenders (but this won't solve the issue if the reverse proxy is on another machine).

But I know that Jenkins CI app does return a 401 on failed logins, and beside infringing the RFC quoted on StackOverflow, it's damn pratical :)

[bkcsoft]

Fail2Ban can never be as good at catching intrusion attemts as the application itself can. So most sites just returns 200 because that at least slows the intrusion down (the bot has to parse and check that they got in or not)

[rems4e]

I do not share this point of view entirely. I did not thought about the benefit of a 200 code you stated, though.
But I see that relevant logging information on the offending intrusion attempts will be added to Gitea, so it's OK for me :)

[daviian]

Issue can be closed. Is fixed by #2334