HTTP 401 on failed login #960

New Issue

GiteaMirror · 2025-11-02T03:43:20-06:00

GiteaMirror commented

2025-11-02 03:43:20 -06:00

Originally created by @rems4e on GitHub (Aug 13, 2017).

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

Originally created by @rems4e on GitHub (Aug 13, 2017).  - Gitea version (or commit ref): 1.1.3 - Git version: 2.11 - Operating system: Debian Stretch - Database (use `[x]`): - [ ] PostgreSQL - [x] MySQL - [ ] MSSQL - [ ] SQLite - Can you reproduce the bug at https://try.gitea.io: - [x] 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](https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#4xx_Client_errors)). 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  ![gitea](https://user-images.githubusercontent.com/11176837/29253921-b4cd9216-8089-11e7-8006-634f59fd8cad.png)

GiteaMirror added the type/proposal label 2025-11-02 03:43:20 -06:00

GiteaMirror closed this issue

2025-11-02 03:43:20 -06:00

GiteaMirror commented

2025-11-02 03:43:20 -06:00

@daviian commented on GitHub (Aug 17, 2017):

I agree with that.

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

@daviian commented on GitHub (Aug 17, 2017): I agree with that. @lunny Can I work on that as a starter for contribution? ;-)

GiteaMirror commented

2025-11-02 03:43:21 -06:00

@lafriks commented on GitHub (Aug 17, 2017):

@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

@lafriks commented on GitHub (Aug 17, 2017): @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

GiteaMirror commented

2025-11-02 03:43:21 -06:00

@sapk commented on GitHub (Aug 17, 2017):

@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 😄

@sapk commented on GitHub (Aug 17, 2017): @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](https://github.com/go-gitea/gitea/blob/master/CONTRIBUTING.md). Feel free to add integration test (this will maybe broke some) And don't hesitate to ask any question here or on [discord](https://discord.gg/NsatcWJ) Have fun :smile:

GiteaMirror commented

2025-11-02 03:43:21 -06:00

@daviian commented on GitHub (Aug 18, 2017):

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.

@daviian commented on GitHub (Aug 18, 2017): 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.

GiteaMirror commented

2025-11-02 03:43:21 -06:00

@ethantkoenig commented on GitHub (Aug 20, 2017):

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.

@ethantkoenig commented on GitHub (Aug 20, 2017): 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.

GiteaMirror commented

2025-11-02 03:43:22 -06:00

@rems4e commented on GitHub (Aug 20, 2017):

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 :)

@rems4e commented on GitHub (Aug 20, 2017): 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 :)

GiteaMirror commented

2025-11-02 03:43:22 -06:00

@bkcsoft commented on GitHub (Aug 24, 2017):

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)

@bkcsoft commented on GitHub (Aug 24, 2017): 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)

GiteaMirror commented

2025-11-02 03:43:22 -06:00

@rems4e commented on GitHub (Aug 24, 2017):

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 :)

@rems4e commented on GitHub (Aug 24, 2017): 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 :)

GiteaMirror commented

2025-11-02 03:43:22 -06:00

@daviian commented on GitHub (Aug 24, 2017):

Issue can be closed. Is fixed by https://github.com/go-gitea/gitea/pull/2334

@daviian commented on GitHub (Aug 24, 2017): Issue can be closed. Is fixed by https://github.com/go-gitea/gitea/pull/2334

GiteaMirror referenced this issue

2025-11-02 11:52:12 -06:00

[PR #960] [MERGED] Remove the default console logger when it is not set in the configuration #15681

Sign in to join this conversation.