Opened 8 years ago

Closed 4 years ago

#26480 closed Cleanup/optimization (fixed)

Allow sensitive_variables() to preserve the signature of its decorated function

Reported by: tpazderka Owned by: Baptiste Mispelon
Component: Error reporting Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When the method authenticate of a custom AuthenticationBackend is decorated with sensitive_variables, inspect.getcallargs will always match.

Calling the authenticate function will attempt to call this backend with any set of credentials and will raise an uncaught TypeError for an unmatching backend.

Authentication with such decorated backends used to work in version 1.6.

Change History (8)

comment:1 by Tim Graham, 8 years ago

Could you please try bisecting to find the commit where the behavior changed?

comment:2 by tpazderka, 8 years ago

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:3 by Tim Graham, 8 years ago

Thanks! I'm not sure what can be done to fix this. Any ideas?

comment:4 by tpazderka, 8 years ago

Nothing apart from going back to the previous masking of TypeError... I think that these two behaviours go against each other...

comment:5 by Tim Graham, 8 years ago

Component: contrib.authError reporting
Summary: TypeError in authenticate decorated by sensitive_variablesAllow sensitive_variables() to preserve the signature of its decorated function
Triage Stage: UnreviewedAccepted
Type: BugNew feature

It might be possible to allow sensitive_variables to preserve the signature of whatever it decorates. Here's code that works until @sensitive_variables is added:

import inspect
from django.views.decorators.debug import sensitive_variables

class Backend(object):
    @sensitive_variables
    def authenticate(self, username=None, password=None):
        print(username)

inspect.getcallargs(Backend().authenticate, username='foo', password='bar')

comment:6 by Vlastimil Zíma, 8 years ago

What about something like this:

def sensitive_variables(*variables):
    def decorator(func):
        @functools.wraps(func)
        def sensitive_variables_wrapper(*func_args, **func_kwargs):
            ...
        # Keep the original function for inspection in `authenticate`
        sensitive_variables_wrapper.sensitive_variables_func = func
        return sensitive_variables_wrapper
    return decorator

Function authenticate would then check the sensitive_variables_func first.

comment:7 by Baptiste Mispelon, 4 years ago

Has patch: set
Owner: changed from nobody to Baptiste Mispelon
Status: newassigned
Type: New featureCleanup/optimization

I've looked into this and it turns out the issue lies with inspect.getcallargs which doesn't follow the __wrapped__ attribute set by functools.wraps().

Using inspect.signature(...).bind(...) instead of inspect.getcallargs(...) fixes the issue (as an added bonus, getcallargs() has been deprecated since Python 3.5 so removing it is a plus).

PR here

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 3df3c5e6:

Fixed #26480 -- Fixed crash of contrib.auth.authenticate() on decorated authenticate() methods of authentication backends.

The Signature API (PEP 362) has better support for decorated functions
(by default, it follows the wrapped attribute set by
functools.wraps for example).

Note: See TracTickets for help on using tickets.
Back to Top