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

Users: Log unauthorized requests #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

networkException
Copy link

This patch adds a new configuration option to the web.config.file which
makes it possible to enable logging of unauthorized requests.

If "log_unauthorized" at config file's top level is set to true,
any unauthorized request will have the ip as well as the X-Forwarded-For
header logged. This way, a program that might parse the logs can determent
for itself if the X-Forwarded-For header can be trusted.

web/users.go Outdated Show resolved Hide resolved
@SuperQ
Copy link
Member

SuperQ commented Sep 7, 2021

Seems like a good idea to me.

@roidelapluie
Copy link
Member

Can we log to a file? We might also log successful queries, with usernames?

@networkException
Copy link
Author

I think logging to a file is a bit out of scope of this pr, usually there's always a way to pipe the output somewhere. If people want it I can also implement logging on success

@roidelapluie
Copy link
Member

We should not mix the two streams together (application / access logs). it is separate things.

@roidelapluie
Copy link
Member

prometheus also uses json for logging the queries.

This patch adds new configuration options to the web.config.file which
makes it possible to enable logging of http requests.

The "request_log_config" struct takes two arguments, a "file" which
gets written to on request and "header_for_ip" which optionally
allows a user to override how the ip gets retrieved.

A log entry is formatted as json and currently contains the ip
of the request, if authentication was successful, the url path
and a timestamp

Signed-off-by: networkException <[email protected]>
@networkException
Copy link
Author

I have rewritten the implementation to output to a json file

@SuperQ
Copy link
Member

SuperQ commented Sep 11, 2021

I would prefer to avoid multiple log streams. The fact that Prometheus logs queries to a separate file is an anti-pattern for operations.

We separate logging types in our tools like ELK and Loki.

@networkException
Copy link
Author

For my usecase it doesn't matter, I'd just like to have some way of outputting failed requests. I'm not able to decide what design would be best for the project but I'll happily implement a different one if that gets agreed on.

@SuperQ
Copy link
Member

SuperQ commented Sep 11, 2021

@networkException Yea, let me have a discussion with @roidelapluie about this policy. I really want to avoid a proliferation of log files in the Prometheus ecosystem. It makes dealing with deployments a lot more work.

@roidelapluie
Copy link
Member

You can put /dev/stdout if you don't want a log file

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

Successfully merging this pull request may close these issues.

3 participants