Skip to content

[fix] Exempt REST API requests from password expiration middleware #511#538

Open
CodingWithSaksham wants to merge 1 commit into
openwisp:masterfrom
CodingWithSaksham:issues/511-fix-password-reset-affecting-api
Open

[fix] Exempt REST API requests from password expiration middleware #511#538
CodingWithSaksham wants to merge 1 commit into
openwisp:masterfrom
CodingWithSaksham:issues/511-fix-password-reset-affecting-api

Conversation

@CodingWithSaksham

Copy link
Copy Markdown

Password expiration was redirecting authenticatated API requests to an HTML password-change page instead of returning a proper API response, breaking API clients. The middleware now detects DRF API views via the resolver's view class (works regardless of where a host project mounts the API) and skips the redirect for them.

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #511

Description of Changes

  • PasswordExpirationMiddleware now checks request.resolver_match.func.cls against APIView (prefix-independent, works regardless of where a host project mounts the API) and skips the redirect for API requests.
  • Test added: test_api_request_not_redirected expired-password user hits a token-authenticated API endpoint, asserts 200 not 302.

…enwisp#511

Password expiration was redirecting authenticatated API requests to an HTML password-change page instead of returning a proper API response, breaking API clients. The middleware now detects DRF API views via the resolver's view class (works regardless of where a host project mounts the API) and skips the redirect for them.
Closes openwisp#511
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 04f9282c-6961-4d87-b918-3e1a31b5486b

📥 Commits

Reviewing files that changed from the base of the PR and between f7bc7b1 and 0346409.

📒 Files selected for processing (2)
  • openwisp_users/middleware.py
  • openwisp_users/tests/test_middlewares.py
📜 Recent review details
⏰ Context from checks skipped due to timeout. (9)
  • GitHub Check: Python==3.11 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=4.2.0
  • GitHub Check: Python==3.13 | django~=5.1.0
  • GitHub Check: Python==3.10 | django~=5.1.0
  • GitHub Check: Python==3.11 | django~=5.1.0
  • GitHub Check: Python==3.11 | django~=5.0.0
  • GitHub Check: Python==3.10 | django~=4.2.0
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.12 | django~=5.2.0
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (Custom checks)

For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework (django.utils.translation.gettext, gettext_lazy, or ugettext aliases)

**/*.py: Mark user-facing strings for translation with Django i18n helpers in Django code
Place imports at the top of the file; only defer imports when necessary (e.g., Django model imports inside functions or methods where the app registry is not yet ready)
Avoid unnecessary blank lines inside function and method bodies
Write comments and docstrings only when they explain why code is shaped a certain way; place comments before the relevant code block instead of scattering them inside it

Files:

  • openwisp_users/middleware.py
  • openwisp_users/tests/test_middlewares.py
🔇 Additional comments (3)
openwisp_users/middleware.py (2)

4-6: LGTM!

Also applies to: 29-40, 42-42


41-41: 🩺 Stability & Availability

Guard issubclass() against non-class cls attributes. url_match.func.cls is assumed to be a class here; if a resolved callable exposes a different value, this raises TypeError before the expired-password redirect path runs.

openwisp_users/tests/test_middlewares.py (1)

9-17: LGTM!

Also applies to: 54-71


📝 Walkthrough

Walkthrough

The PasswordExpirationMiddleware was changed to use request.resolver_match instead of calling resolve(request.path), with a None guard for cases where no resolver match exists. The middleware now detects DRF APIView-based requests via the cls attribute set by APIView.as_view() and skips redirects for such API requests, while retaining the existing exempted_url_names check for non-API requests. A corresponding test was added to verify authenticated API requests with expired passwords return 200 rather than being redirected.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant PasswordExpirationMiddleware
  participant DRFAPIView

  Client->>PasswordExpirationMiddleware: Request with expired password
  PasswordExpirationMiddleware->>PasswordExpirationMiddleware: Check request.resolver_match
  PasswordExpirationMiddleware->>DRFAPIView: Check if view is APIView subclass
  DRFAPIView-->>PasswordExpirationMiddleware: Is API request
  PasswordExpirationMiddleware->>Client: Pass through response (no redirect)
Loading

Related issues: #511 – Password expiration affects REST API

Suggested labels: bug, backend

Suggested reviewers: none identified

Poem:
A rabbit hopped through login gates,
Found APIs stuck behind expired fates.
"No redirect for you, dear token bearer!"
It fixed the flow, made access fairer.
resolver_match now leads the way,
Hooray for tests that guard the day! 🐰

🚥 Pre-merge checks | ✅ 6 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Features ⚠️ Warning Docs do not mention the new API-exemption behavior, and there’s no evidence the linked issue was validated/accepted. Add docs/changelog for the API redirect exemption and provide evidence the linked issue was validated/accepted by an org member.
Changes ⚠️ Warning Tests were added, but docs for PasswordExpirationMiddleware still say it always restricts users to the password-change view; the new API exemption is undocumented. Update the middleware docs/changelog to note that DRF APIView requests bypass password-expiration redirects; no screenshots are needed since there’s no UI change.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title is descriptive, matches the bug fix, and references the linked issue with the required [fix] prefix.
Description check ✅ Passed The description covers the checklist, linked issue, and change summary; only the screenshot/documentation parts are incomplete.
Linked Issues check ✅ Passed The middleware now exempts authenticated DRF API requests from password-expiration redirects and adds a test, matching #511.
Out of Scope Changes check ✅ Passed The changes are limited to the middleware fix and its corresponding test, with no unrelated additions.
Bug Fixes ✅ Passed Middleware now skips redirects for DRF APIView requests; the new regression test hits an expired-password Bearer-auth API endpoint and would 302 without the patch.
General Rules ✅ Passed #511 is validated (bug label, both OpenWISP projects); the middleware change and regression test align with the API-redirect fix.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 98.085% (-0.04%) from 98.128% — CodingWithSaksham:issues/511-fix-password-reset-affecting-api into openwisp:master

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.

[bug] Password expiration affects REST API

2 participants