Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Higher loglevel for authentication errors #1463

Open
heull001 opened this issue Aug 15, 2023 · 2 comments
Open

Higher loglevel for authentication errors #1463

heull001 opened this issue Aug 15, 2023 · 2 comments

Comments

@heull001
Copy link

Wrong username or password is currently logged on DEBUG-level. I think this information is more important, so should be logged on a higher level. I think NOTICE could be okay for this.

This would allow to use tools like fail2ban without creating gigantic logfiles.

@heull001 heull001 changed the title Lower loglevel for authentication errors Higher loglevel for authentication errors Aug 15, 2023
@jtojnar
Copy link
Member

jtojnar commented Aug 15, 2023

Thanks for reporting, could you describe your use case a bit more?

I would expect that for it to be useful for fail2ban, we would need to log more info (e.g. an IP address), so just raising the level would not help.

Maybe it would be better to match web server logs which should already contain the response code, something like:

127.0.0.1 - - [15/Aug/2023:22:00:30 +0200] "POST /selfoss/login HTTP/1.1" 400 52

Unfortunately, the login endpoint always returns 200 OK status, and we cannot change it without breaking backwards compatibility.

So there is currently no nice way to distinguish failures just from that log. (Technically, you could rely on the fact that responses containing wrong username/password have size 52 bytes, while ones without an error field are 16 bytes long. But there is no guarantee that the response sizes will remain the same.)

Ideally, we would create a new API endpoint for signing in that uses separate response codes but that will require more thought (e.g. which response code to use, should we support HTTP authentication…)

Or you could change the following line to jsonError risking clients potentially crashing when incorrect credentials are entered:

$this->view->jsonSuccess([

@heull001
Copy link
Author

heull001 commented Aug 16, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants