Skip to content

Conversation

@TudorGR
Copy link
Contributor

@TudorGR TudorGR commented Nov 17, 2025

Replace custom middleware with Django's LoginRequiredMiddleware

Description

Replace Weblate's custom RequireLoginMiddleware with Django 5.1's built-in LoginRequiredMiddleware to simplify authentication handling and reduce maintenance burden.

Changes

  • Remove weblate.accounts.middleware.RequireLoginMiddleware class (~75 lines)
  • Use django.contrib.auth.middleware.LoginRequiredMiddleware when REQUIRE_LOGIN=True
  • Add @login_not_required decorator to git_export view for HTTP Basic Auth compatibility
  • Remove obsolete LOGIN_REQUIRED_URLS and LOGIN_REQUIRED_URLS_EXCEPTIONS settings
  • Update tests in weblate.accounts.tests.test_middleware to verify Django middleware behavior
  • Deprecate Docker environment variables: WEBLATE_LOGIN_REQUIRED_URLS_EXCEPTIONS, WEBLATE_ADD_LOGIN_REQUIRED_URLS_EXCEPTIONS, and WEBLATE_REMOVE_LOGIN_REQUIRED_URLS_EXCEPTIONS
  • Update documentation in docs/changes.rst with compatibility notes

Implementation Details

All public views throughout the codebase already use the @login_not_required decorator (introduced in Django 5.1), making this migration straightforward. The middleware is conditionally added only when REQUIRE_LOGIN=True, maintaining backward compatibility.

The Git exporter view required special attention as it implements HTTP Basic Authentication independently. It now uses @login_not_required to bypass the login requirement while maintaining its own authentication logic.

Testing

  • Updated test suite to verify Django's LoginRequiredMiddleware behavior
  • Tests confirm public endpoints remain accessible (/healthz/, /api/, /widgets/, etc.)
  • Tests verify protected views redirect anonymous users to login when REQUIRE_LOGIN=True
  • Tests verify authenticated users can access all views
  • No syntax errors in modified files

Documentation

  • Added comprehensive compatibility notes to changelog
  • Documented deprecation of URL-based exception configuration
  • Added explanatory comments in settings files

Closes #16934

Replace Weblate's custom RequireLoginMiddleware with Django 5.1's
built-in LoginRequiredMiddleware to simplify authentication handling.

- Remove weblate.accounts.middleware.RequireLoginMiddleware class
- Use django.contrib.auth.middleware.LoginRequiredMiddleware when REQUIRE_LOGIN=True
- Add @login_not_required decorator to git_export view for HTTP Basic Auth
- Remove obsolete LOGIN_REQUIRED_URLS and LOGIN_REQUIRED_URLS_EXCEPTIONS settings
- Update tests to verify Django middleware behavior
- Deprecate Docker environment variables for URL exceptions

All public views already use @login_not_required decorator as of Django 5.1.

Closes WeblateOrg#16934
@codecov
Copy link

codecov bot commented Nov 17, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
5163 1 5162 663
View the top 1 failed test(s) by shortest run time
::weblate.accounts.tests.test_middleware
Stack Traces | 0s run time
.../accounts/tests/test_middleware.py:20: in <module>
    class MiddlewareTest(TestCase):
.../accounts/tests/test_middleware.py:29: in MiddlewareTest
    @method_decorator(login_not_required)
     ^^^^^^^^^^^^^^^^
E   NameError: name 'method_decorator' is not defined

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Copy link
Member

@nijel nijel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once the ruff and mypy errors are fixed.

Comment on lines 23 to 24
def public_view(self, request) -> str:
"""A public view marked with @login_not_required decorator."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def public_view(self, request) -> str:
"""A public view marked with @login_not_required decorator."""
def public_view(self, request: HttpRequest) -> str:
"""View not requiring login."""

Comment on lines 27 to 28
def protected_view(self, request) -> str:
"""A protected view without the @login_not_required decorator."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def protected_view(self, request) -> str:
"""A protected view without the @login_not_required decorator."""
def protected_view(self, request: HttpRequest) -> str:
"""View requiring login."""

@nijel nijel added this to the 5.15 milestone Nov 18, 2025
@nijel nijel self-assigned this Nov 18, 2025
@nijel
Copy link
Member

nijel commented Nov 18, 2025

Did you actually test running with the middleware?

  • It fails with the system check auth.E013, probably should be ignored (we're using custom auth middleware).
  • Login ends up in a redirect loop.
  • Admin login is not working either.

TudorGR and others added 5 commits November 19, 2025 10:19
- Fix docstrings to use imperative mood (D401)
- Add HttpRequest type hints to test methods
- Add auth.E013 to SILENCED_SYSTEM_CHECKS for custom auth middleware
- Add @login_not_required decorator to WeblateLoginView and SecondFactorLoginView to prevent redirect loops

These changes fix the ruff linting errors and resolve the login redirect loop issue identified during testing.
@nijel
Copy link
Member

nijel commented Nov 21, 2025

The remaining bit is /admin/login/ which should also work without requiring login (it always allows local login even if it is disabled for user authentication).

Add @login_not_required decorator to WeblateAdminSite.login method
to ensure /admin/login/ is accessible even when REQUIRE_LOGIN=True.
This allows local admin login to work as intended, supporting the
Django admin's authentication flow.
@nijel nijel merged commit faeeea3 into WeblateOrg:main Nov 25, 2025
48 of 50 checks passed
@nijel
Copy link
Member

nijel commented Nov 25, 2025

Merged, thanks for your contribution! I had to do some documentation adjustments before.

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.

Replace custom middleware with LoginRequiredMiddleware

2 participants