Used a plain HTML form for DjangoProject login.

Fixes #95, #61 and #60
Also possibly #100 and #64

.github/workflows/tests.yml

@@ -24,3 +24,17 @@ jobs:
  python-version: '3.7'
  - run: pip install "tinycss2>=1.2.0"
  - run: python noshadows.py --tests
+
+ tracdjangoplugin:
+ runs-on: ubuntu-latest
+ container:
+ image: python:2.7.18-buster
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v4
+ - name: Install requirements
+ run: python -m pip install -r requirements.txt
+ - name: Run tests
+ run: python -m django test tracdjangoplugin.tests
+ env:
+ DJANGO_SETTINGS_MODULE: tracdjangoplugin.settings_tests

DjangoPlugin/tracdjangoplugin/plugins.py

@@ -1,11 +1,18 @@
+from urlparse import urlparse
+
 from trac.core import Component, implements
 from trac.web.chrome import INavigationContributor
-from trac.web.api import IRequestFilter, IRequestHandler
+from trac.web.api import IRequestFilter, IRequestHandler, RequestDone
+from trac.web.auth import LoginModule
 from trac.wiki.web_ui import WikiModule
 from trac.util import Markup
 from trac.util.html import tag
 from tracext.github import GitHubBrowser
 
+from django.conf import settings
+from django.contrib.auth.forms import AuthenticationForm
+from django.utils.http import is_safe_url
+
 
 class CustomTheme(Component):
  implements(IRequestFilter)
@@ -91,3 +98,45 @@ def _format_changeset_link(self, formatter, ns, chgset, label, fullmatch=None):
  return super(GitHubBrowserWithSVNChangesets, self)._format_changeset_link(
  formatter, ns, chgset, label, fullmatch
  )
+
+
+class PlainLoginComponent(Component):
+ """
+ Enable login through a plain HTML form (no more HTTP basic auth)
+ """
+
+ implements(IRequestHandler)
+
+ def match_request(self, req):
+ return req.path_info == "/login"
+
+ def process_request(self, req):
+ if req.method == "POST":
+ return self.do_post(req)
+ elif req.method == "GET":
+ return self.do_get(req)
+ else:
+ req.send_response(405)
+ raise RequestDone
+
+ def do_get(self, req):
+ return "plainlogin.html", {
+ "form": AuthenticationForm(),
+ "next": req.args.get("next", ""),
+ }
+
+ def do_post(self, req):
+ form = AuthenticationForm(data=req.args)
+ if form.is_valid():
+ req.environ["REMOTE_USER"] = form.get_user().username
+ LoginModule(self.compmgr)._do_login(req)
+ req.redirect(self._get_safe_redirect_url(req))
+ return "plainlogin.html", {"form": form, "next": req.args.get("next", "")}
+
+ def _get_safe_redirect_url(self, req):
+ host = urlparse(req.base_url).hostname
+ redirect_url = req.args.get("next", "") or settings.LOGIN_REDIRECT_URL
+ if is_safe_url(redirect_url, allowed_hosts=[host]):
+ return redirect_url
+ else:
+ return settings.LOGIN_REDIRECT_URL

DjangoPlugin/tracdjangoplugin/settings.py

@@ -1,8 +1,11 @@
 import json
 import os
 
-with open(os.environ.get("SECRETS_FILE")) as handle:
- SECRETS = json.load(handle)
+if os.environ.get("SECRETS_FILE"):
+ with open(os.environ.get("SECRETS_FILE")) as handle:
+ SECRETS = json.load(handle)
+else:
+ SECRETS = {}
 
 DEBUG = False
 
@@ -23,6 +26,6 @@
 ]
 
 
-SECRET_KEY = str(SECRETS["secret_key"])
+SECRET_KEY = SECRETS.get("secret_key", "")
 
-BASIC_AUTH_REALM = "Django's Trac"
+LOGIN_REDIRECT_URL = "/"

DjangoPlugin/tracdjangoplugin/settings_tests.py

@@ -0,0 +1,13 @@
+from .settings import *
+
+DATABASES = {
+ "default": {
+ "ENGINE": "django.db.backends.sqlite3",
+ },
+}
+
+PASSWORD_HASHERS = [
+ "django.contrib.auth.hashers.MD5PasswordHasher",
+]
+
+SECRET_KEY = "test"

DjangoPlugin/tracdjangoplugin/tests.py

@@ -0,0 +1,129 @@
+from functools import partial
+
+from django.contrib.auth.forms import AuthenticationForm
+from django.contrib.auth.models import User
+from django.test import TestCase
+
+from trac.test import EnvironmentStub, MockRequest
+from trac.web.api import RequestDone
+from trac.web.main import RequestDispatcher
+
+from tracdjangoplugin.plugins import PlainLoginComponent
+
+
+class PlainLoginComponentTestCase(TestCase):
+ def setUp(self):
+ self.env = EnvironmentStub()
+ self.component = PlainLoginComponent(self.env)
+ self.request_factory = partial(MockRequest, self.env)
+
+ def test_component_matches_correct_url(self):
+ request = self.request_factory(path_info="/login")
+ self.assertTrue(self.component.match_request(request))
+
+ def test_component_doesnt_match_another_url(self):
+ request = self.request_factory(path_info="/github/login")
+ self.assertFalse(self.component.match_request(request))
+
+ def test_component(self):
+ request = self.request_factory(path_info="/login")
+ template, context = self.component.process_request(request)
+ self.assertEqual(template, "plainlogin.html")
+ self.assertFalse(context["form"].is_bound)
+
+ def assertLoginSucceeds(
+ self, username, password, check_redirect=None, extra_data=None
+ ):
+ data = {"username": username, "password": password}
+ if extra_data is not None:
+ data.update(extra_data)
+ request = self.request_factory(method="POST", path_info="/login", args=data)
+ with self.assertRaises(RequestDone):
+ self.component.process_request(request)
+
+ self.assertEqual(request.authname, "test")
+ self.assertEqual(request.status_sent, ["303 See Other"])
+ if check_redirect is not None:
+ redirect_url = request.headers_sent["Location"]
+ self.assertEqual(redirect_url, check_redirect)
+
+ def test_login_valid_user(self):
+ User.objects.create_user(username="test", password="test")
+ self.assertLoginSucceeds(username="test", password="test")
+
+ def test_login_valid_default_redirect(self):
+ self.env.config.set("trac", "base_url", "")
+ User.objects.create_user(username="test", password="test")
+ with self.settings(LOGIN_REDIRECT_URL="/test"):
+ self.assertLoginSucceeds(
+ username="test", password="test", check_redirect="/test"
+ )
+
+ def test_login_valid_with_custom_redirection(self):
+ self.env.config.set("trac", "base_url", "")
+ User.objects.create_user(username="test", password="test")
+ self.assertLoginSucceeds(
+ username="test",
+ password="test",
+ check_redirect="/test",
+ extra_data={"next": "/test"},
+ )
+
+ def test_login_valid_with_custom_redirection_with_hostname(self):
+ self.env.config.set("trac", "base_url", "http://localhost")
+ User.objects.create_user(username="test", password="test")
+ self.assertLoginSucceeds(
+ username="test",
+ password="test",
+ check_redirect="http://localhost/test",
+ extra_data={"next": "http://localhost/test"},
+ )
+
+ def test_login_valid_with_malicious_redirection(self):
+ self.env.config.set("trac", "base_url", "http://localhost")
+ User.objects.create_user(username="test", password="test")
+ with self.settings(LOGIN_REDIRECT_URL="/test"):
+ self.assertLoginSucceeds(
+ username="test",
+ password="test",
+ check_redirect="http://localhost/test",
+ extra_data={"next": "http://example.com/evil"},
+ )
+
+ def assertLoginFails(self, username, password, error_message=None):
+ if error_message is None:
+ error_message = AuthenticationForm.error_messages["invalid_login"] % {
+ "username": "username"
+ }
+
+ request = self.request_factory(
+ method="POST",
+ path_info="/login",
+ args={"username": username, "password": password},
+ )
+ template, context = self.component.process_request(request)
+ self.assertEqual(template, "plainlogin.html")
+ self.assertEqual(context["form"].errors, {"__all__": [error_message]})
+
+ def test_login_invalid_no_users(self):
+ self.assertLoginFails(username="test", password="test")
+
+ def test_login_invalid_incorrect_username(self):
+ User.objects.create_user(username="test", password="test")
+ self.assertLoginFails(username="test123", password="test")
+
+ def test_login_invalid_incorrect_password(self):
+ User.objects.create_user(username="test", password="test")
+ self.assertLoginFails(username="test", password="test123")
+
+ def test_login_invalid_incorrect_username_and_password(self):
+ User.objects.create_user(username="test", password="test")
+ self.assertLoginFails(username="test123", password="test123")
+
+ def test_login_invalid_username_uppercased(self):
+ User.objects.create_user(username="test", password="test")
+ self.assertLoginFails(username="TEST", password="test")
+
+ def test_login_invalid_inactive_user(self):
+ User.objects.create_user(username="test", password="test", is_active=False)
+ self.assertLoginFails(username="test", password="test")

DjangoPlugin/tracdjangoplugin/wsgi.py

@@ -4,18 +4,17 @@
 
 application = trac.web.main.dispatch_request
 
+import django
+
+django.setup()
+
 # Massive hack to make Trac fast, otherwise every git call tries to close ulimit -n (1e6) fds
 # Python 3 would perform better here, but we are still on 2.7 for Trac, so leak fds for now.
 from tracopt.versioncontrol.git import PyGIT
 
 PyGIT.close_fds = False
 
-from .djangoauth import DjangoAuth
-
-application = DjangoAuth(application)
-
 trac_dsn = os.getenv("SENTRY_DSN")
-
 if trac_dsn:
  import sentry_sdk
  from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware

trac-env/conf/trac.ini

@@ -22,6 +22,7 @@ trac.ticket.roadmap.roadmapmodule = disabled
 trac.versioncontrol.web_ui.browser.browsermodule = disabled
 trac.versioncontrol.web_ui.changeset.changesetmodule = disabled
 trac.versioncontrol.web_ui.log.logmodule = disabled
+trac.web.auth.loginmodule = disabled; replaced by djangoplugin.PlainLoginComponent
 trac.wiki.web_ui.wikimodule = disabled
 tracdjangoplugin.* = enabled
 tracext.github.githubloginmodule = enabled

trac-env/templates/django_theme.html

@@ -28,8 +28,7 @@
  </form>
  </li>
  # else
- <li><a href="/github/login">GitHub Login</a></li>
- <li><a href="/login">DjangoProject Login</a></li>
+ <li><a href="/login?next=${req.path_info|urlencode()}">Login</a></li>
  # endif
  <li><a href="${req.href.prefs()}">Preferences</a></li>
  # if req.perm.has_permission('XML_RPC'):