Potential bugfix for AttributeError: 'PasswordResetFromKeyView' object has no attribute 'redirect_field_name' #178
Conversation
Codecov Report
@@ Coverage Diff @@
## main #178 +/- ##
==========================================
+ Coverage 92.95% 93.06% +0.10%
==========================================
Files 15 15
Lines 568 577 +9
==========================================
+ Hits 528 537 +9
Misses 40 40
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Hi @pbadeer! Thank you! Can you maybe write a test case which can prove that this fixes your bug? |
|
I can certainly try! I'm not well versed on pytest and I've never used Hatch so I'm struggling a bit, but I'll throw some time at it. |
|
BTW I got 34 test failures (18 passes) with a new test environment due to: When I changed the requirements-dev.txt line: One of the fuller stack traces, they're all the same error: |
|
@valberg Test case added. Let me know if I screwed anything up, apologies in advance! |
|
pre-commit.ci autofix |
1 similar comment
|
pre-commit.ci autofix |
allauth_2fa/adapter.py
Outdated
| # Add "next" parameter to the URL if possible. | ||
| # If the view function smells like a class-based view, we can interrogate it. | ||
| if getattr(request.resolver_match.func, "view_class", None): | ||
| if getattr(request, "resolver_match", None) and getattr( | ||
| request.resolver_match.func, | ||
| "view_class", | ||
| None, | ||
| ): | ||
| view = request.resolver_match.func.view_class() | ||
| view.request = request | ||
| success_url = view.get_success_url() | ||
| query_params = request.GET.copy() | ||
| if success_url: | ||
| if success_url and hasattr(view, "redirect_field_name"): | ||
| query_params[view.redirect_field_name] = success_url | ||
| if query_params: | ||
| redirect_url += f"?{urlencode(query_params)}" |
There was a problem hiding this comment.
I think this is getting complex enough to turn it into a separate free utility function, and in that case we can just use try: except:s, something like
def get_next_query_string(request: HttpRequest) -> str | None:
"""
Get the query string (including the prefix `?`) to redirect to after a successful POST.
If a query string can't be determined, returns None.
"""
# If the view function smells like a class-based view, we can interrogate it.
try:
view = request.resolver_match.func.view_class()
redirect_field_name = view.redirect_field_name
except AttributeError:
# Interrogation failed :(
return None
view.request = request
query_params = request.GET.copy()
success_url = view.get_success_url()
if success_url:
query_params[redirect_field_name] = success_url
if query_params:
return f"?{urlencode(query_params)}"
return NoneWhat do you think?
There was a problem hiding this comment.
Sounds fine by me :) Then we can test that directly.
|
This PR would be great. I have the same problem and am waiting for a fix 😅. |
Same here, we are waiting for this to push our code to production. |
|
Ditto the above, I can confirm this fix has worked for us, so a merge and release would be great 👍 Thank you |
|
pre-commit.ci autofix |
Getting this error when submitting the change password form after opening a key-based password reset link. AttributeError: 'PasswordResetFromKeyView' object has no attribute 'redirect_field_name'
…ck so that we can use FixtureRequests on OTPAdapter without it crashing.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Add get_next_query_string util from @akx
Use new get_next_query_string util
Fixes for ruff finds: allauth_2fa/utils.py:40:36: F821 Undefined name `HttpRequest` allauth_2fa/utils.py:42:89: E501 Line too long (91 > 88 characters)
for more information, see https://pre-commit.ci
| if getattr(request.resolver_match.func, "view_class", None): | ||
| if get_next_query_string(request): | ||
| view = request.resolver_match.func.view_class() | ||
| view.request = request | ||
| success_url = view.get_success_url() | ||
| query_params = request.GET.copy() | ||
| if success_url: | ||
| if success_url and hasattr(view, "redirect_field_name"): | ||
| query_params[view.redirect_field_name] = success_url | ||
| if query_params: | ||
| redirect_url += f"?{urlencode(query_params)}" |
There was a problem hiding this comment.
Erm, shouldn't this just be
| if getattr(request.resolver_match.func, "view_class", None): | |
| if get_next_query_string(request): | |
| view = request.resolver_match.func.view_class() | |
| view.request = request | |
| success_url = view.get_success_url() | |
| query_params = request.GET.copy() | |
| if success_url: | |
| if success_url and hasattr(view, "redirect_field_name"): | |
| query_params[view.redirect_field_name] = success_url | |
| if query_params: | |
| redirect_url += f"?{urlencode(query_params)}" | |
| query_string = get_next_query_string(request) | |
| if query_string: | |
| redirect_url += query_string |
now that the logic is refactored..?
There was a problem hiding this comment.
Feel free to take this over! I don't use this library anymore and was just trying to pass this change forwards since it seems many of your users need it.
Getting this error when submitting the change password form after opening a key-based password reset link.
AttributeError: 'PasswordResetFromKeyView' object has no attribute 'redirect_field_name'I'm using an Adapter that combines the OTPAdapter from this library and InvitationsAdapter from django-invitations, so this issue may or may not be present for others, but the change adds a simple safety check.
Full stack trace: