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

Unable to use no_logging on admin login view #123

Open
wesleyjellis opened this issue Feb 22, 2021 · 4 comments
Open

Unable to use no_logging on admin login view #123

wesleyjellis opened this issue Feb 22, 2021 · 4 comments

Comments

@wesleyjellis
Copy link

I'm trying to avoid logging the login request for the default django admin site, since it contains a password in the request body. Is there a way to apply the no_logging decorator to an arbitrary view?

I tried manually setting the NO_LOGGING_ATTR on django.contrib.admin.sites.login (source) which is passed as the view/func to _should_log_route like so:

from django.contrib.admin.sites import site

from request_logging.middleware import NO_LOGGING_ATTR, NO_LOGGING_MSG_ATTR

def no_log_admin_login():
    setattr(site.login, NO_LOGGING_ATTR, True)
    setattr(site.login, NO_LOGGING_MSG_ATTR, "Admin login view contains password")

but this fails with AttributeError: 'method' object has no attribute 'no_logging'

Given how invasive this kind of patch is in general, I'm not sure I'd be happy even if I could get it to work...

So, I was wondering if you would be open to a new setting like REQUEST_LOGGING_SKIP_ROUTES which would contain a list of routes like ['/admin/login/', '/api/sensitive/'], which could be checked in _should_log_route in addition to the NO_LOGGING_ATTR. I feel this would be useful in general for controlling how logging is done for third party views/apps

@wesleyjellis wesleyjellis changed the title Unable no_logging on admin login view Unable to use no_logging on admin login view Feb 22, 2021
@mindflayer
Copy link

Good point, I was wondering how to manage this scenario. Another idea would be to filter out not only some headers but specific fields from the body.

@wesleyjellis
Copy link
Author

body level filtering would be amazing as well, although I don't know how easy that is to implement when you have only the raw request body

@Wissperwind
Copy link

Wissperwind commented Dec 14, 2021

Hi,
I have a similar problem: I have a custom login view and in case of a failed login the passwort is logged. But @no_logging() does not have an effect:

@no_logging()
def login(request):

It still loggs: b'{"username":"user","password":"123"}'

@jakubkrysl
Copy link

Hi,

We've encountered logging credentials in plaintext in case POST request failed on /login/. The way we handle it for now is using custom filter that masks the password. This is than applied through logging configuration.

def mask_password(record):
    record.msg = re.sub(r"password=(.*?)($|&|\n)", r"password=*****\g<2>", record.msg)
    return True

This way the actual password is replaced with *****. Though this has a downside that if the filter fails for any reason, it does not get applied and password will get logged in plaintext.

I like the skipping routes option as this would allow any admin to skip sensitive routes completely instead of relying on the devs of the app to apply patch on those routes in code. They might be very reluctant to do that since it would introduce this middleware as new dependency. This is our case - this middleware is deployed through config on our side, not by the devs of the app.

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

4 participants