[Django] #26480: TypeError in authenticate decorated by sensitive_variables
Django
------------------------------+--------------------
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26480>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Could you please try
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/#bisecting-a-regression bisecting] to find the commit where the
behavior changed?
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:1>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tpazderka):
It is a commit `b89c2a5d9eb70ca36629ef657c98e3371e9a5c4f`.
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:2>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Thanks! I'm not sure what can be done to fix this. Any ideas?
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:3>
Django
Reporter: tpazderka | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tpazderka):
Nothing apart from going back to the previous masking of TypeError... I
think that these two behaviours go against each other...
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:4>
Django
function
---------------------------------+------------------------------------
Reporter: tpazderka | Owner: nobody
Type: New feature | Status: new
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Bug => New feature
* component: contrib.auth => Error reporting
* stage: Unreviewed => Accepted
Comment:
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:
{{{ #!python
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')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:5>
Django
function
---------------------------------+------------------------------------
Type: New feature | Status: new
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vzima):
What about something like this:
{{{
#!python
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:6>
Django
function
Reporter: tpazderka | Owner: Baptiste
Type: | Mispelon
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Baptiste Mispelon):
* owner: nobody => Baptiste Mispelon
* status: new => assigned
* has_patch: 0 => 1
* type: New feature => Cleanup/optimization
Comment:
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).
[https://github.com/django/django/pull/12198 PR here]
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:7>
Django
function
-------------------------------------+-------------------------------------
Reporter: tpazderka | Owner: Baptiste
Type: | Mispelon
Component: Error reporting | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3df3c5e67070949887e08282a332cc34c1f05184" 3df3c5e6]:
{{{
#!CommitTicketReference repository=""
revision="3df3c5e67070949887e08282a332cc34c1f05184"
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).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26480#comment:8>