rest-server icon indicating copy to clipboard operation
rest-server copied to clipboard

Config+Auth: Add flags to log unauthorized requests

Open networkException opened this issue 4 years ago • 4 comments

What is the purpose of this change? What does it change?

Currently it is not possible to integrate rest-server into a service like fail2ban which prevents brute force login attempts.

This pull request adds new command line flags in order to support logging of unauthorized requests to the server. The flag --log-auth-failure enables the logging and uses the remote address of the request as the default for the logged ip. If the server is used behind a reverse proxy for, --header-for-ip can be used to specify a header like "X-Forwarded-For" to be used for logging the ip.

Was the change discussed in an issue or in the forum before?

There was a forum question about fail2ban without an actual solution: https://forum.restic.net/t/rest-server-and-fail2ban/2569

Checklist

  • [x] I have enabled maintainer edits for this PR
  • [ ] I have added tests for all changes in this PR: Unsure where to implement tests
  • [x] I have added documentation for the changes (in the manual): Documentation has been added to the readme
  • [x] There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • [x] I have run gofmt on the code in all commits
  • [x] All commit messages are formatted in the same style as the other commits in the repo: Unsure, commit style inconsistent
  • [x] I'm done, this Pull Request is ready for review

networkException avatar Sep 11 '21 15:09 networkException

Thanks for your contribution. I'm however thinking that instead of having a separate flag to turn this feature on, it's better with sane defaults and if needed a way to specify different verbosity in the regular log. The latter could also be used for other things, and we already use that type of thing in restic (the --verbose option). In other words, instead of adding flags for every thing we want loggable, better to put them in the right verbosity level and use that.

It's unfortunate that we're using -v for --version, it would be better if --version would be its own command instead, like in restic, as opposed to an option. This can be changed though, it's not like a ton of people are depending on the -v option..

I'm also pondering if the --header-for-ip shouldn't be renamed --ip-header or possibly --header-ip.

rawtaz avatar Sep 11 '21 20:09 rawtaz

I'm not familiar enough with the codebase to refactor the whole logging system to respond to verbosity levels I fear.

As for the flag --ip-header sounds alright, I will change that

networkException avatar Sep 11 '21 21:09 networkException