Skip to content

Commit 0583036

Browse files
bmispelonbmispelon
committed
Used a plain HTML form for DjangoProject login (no more basic auth)
Fixes #95, #61 and #60 Also possibly #100 and #64
1 parent 3647196 commit 0583036

File tree

9 files changed

+240
-110
lines changed

9 files changed

+240
-110
lines changed

‎DjangoPlugin/tracdjangoplugin/djangoauth.py

-101
This file was deleted.

‎DjangoPlugin/tracdjangoplugin/plugins.py

+50-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
1+
from urlparse import urlparse
2+
13
from trac.core import Component, implements
24
from trac.web.chrome import INavigationContributor
3-
from trac.web.api import IRequestFilter, IRequestHandler
5+
from trac.web.api import IRequestFilter, IRequestHandler, RequestDone
6+
from trac.web.auth import LoginModule
47
from trac.wiki.web_ui import WikiModule
58
from trac.util import Markup
69
from trac.util.html import tag
710
from tracext.github import GitHubBrowser
811

12+
from django.conf import settings
13+
from django.contrib.auth.forms import AuthenticationForm
14+
from django.utils.http import is_safe_url
15+
916

1017
class CustomTheme(Component):
1118
implements(IRequestFilter)
@@ -91,3 +98,45 @@ def _format_changeset_link(self, formatter, ns, chgset, label, fullmatch=None):
9198
return super(GitHubBrowserWithSVNChangesets, self)._format_changeset_link(
9299
formatter, ns, chgset, label, fullmatch
93100
)
101+
102+
103+
class PlainLoginComponent(Component):
104+
"""
105+
Enable login through a plain HTML form (no more HTTP basic auth)
106+
"""
107+
108+
implements(IRequestHandler)
109+
110+
def match_request(self, req):
111+
return req.path_info == "/login"
112+
113+
def process_request(self, req):
114+
if req.method == "POST":
115+
return self.do_post(req)
116+
elif req.method == "GET":
117+
return self.do_get(req)
118+
else:
119+
req.send_response(405)
120+
raise RequestDone
121+
122+
def do_get(self, req):
123+
return "plainlogin.html", {
124+
"form": AuthenticationForm(),
125+
"next": req.args.get("next", ""),
126+
}
127+
128+
def do_post(self, req):
129+
form = AuthenticationForm(data=req.args)
130+
if form.is_valid():
131+
req.environ["REMOTE_USER"] = form.get_user().username
132+
LoginModule(self.compmgr)._do_login(req)
133+
req.redirect(self._get_safe_redirect_url(req))
134+
return "plainlogin.html", {"form": form, "next": req.args.get("next", "")}
135+
136+
def _get_safe_redirect_url(self, req):
137+
host = urlparse(req.base_url).hostname
138+
redirect_url = req.args.get("next", "") or settings.LOGIN_REDIRECT_URL
139+
if is_safe_url(redirect_url, allowed_hosts=[host]):
140+
return redirect_url
141+
else:
142+
return settings.LOGIN_REDIRECT_URL

‎DjangoPlugin/tracdjangoplugin/settings.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@
2525

2626
SECRET_KEY = str(SECRETS["secret_key"])
2727

28-
BASIC_AUTH_REALM = "Django's Trac"
28+
LOGIN_REDIRECT_URL = "/"

‎DjangoPlugin/tracdjangoplugin/settings_tests.py

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
DEBUG = False
2+
3+
DATABASES = {
4+
"default": {
5+
"ENGINE": "django.db.backends.sqlite3",
6+
"NAME": "djangoproject",
7+
},
8+
}
9+
10+
INSTALLED_APPS = [
11+
"django.contrib.auth",
12+
"django.contrib.contenttypes",
13+
]
14+
15+
16+
SECRET_KEY = "test"
17+
18+
LOGIN_REDIRECT_URL = "/"

‎DjangoPlugin/tracdjangoplugin/tests.py

+129
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
from functools import partial
2+
3+
from django.contrib.auth.forms import AuthenticationForm
4+
from django.contrib.auth.models import User
5+
from django.test import TestCase
6+
7+
from trac.test import EnvironmentStub, MockRequest
8+
from trac.web.api import RequestDone
9+
from trac.web.main import RequestDispatcher
10+
11+
from tracdjangoplugin import PlainLoginComponent
12+
13+
14+
class PlainLoginComponentTestCase(TestCase):
15+
def setUp(self):
16+
self.env = EnvironmentStub()
17+
self.component = PlainLoginComponent(self.env)
18+
self.request_factory = partial(MockRequest, self.env)
19+
20+
def test_component_matches_correct_url(self):
21+
request = self.request_factory(path_info="/login")
22+
self.assertTrue(self.component.match_request(request))
23+
24+
def test_component_doesnt_match_another_url(self):
25+
request = self.request_factory(path_info="/github/login")
26+
self.assertFalse(self.component.match_request(request))
27+
28+
def test_component(self):
29+
request = self.request_factory(path_info="/login")
30+
template, context = self.component.process_request(request)
31+
self.assertEqual(template, "plainlogin.html")
32+
self.assertFalse(context["form"].is_bound)
33+
34+
def assertLoginSucceeds(
35+
self, username, password, check_redirect=None, extra_data=None
36+
):
37+
data = {"username": username, "password": password}
38+
if extra_data is not None:
39+
data.update(extra_data)
40+
request = self.request_factory(method="POST", path_info="/login", args=data)
41+
with self.assertRaises(RequestDone):
42+
self.component.process_request(request)
43+
44+
self.assertEqual(request.authname, "test")
45+
self.assertEqual(request.status_sent, ["303 See Other"])
46+
if check_redirect is not None:
47+
redirect_url = request.headers_sent["Location"]
48+
self.assertEqual(redirect_url, check_redirect)
49+
50+
def test_login_valid_user(self):
51+
User.objects.create_user(username="test", password="test")
52+
self.assertLoginSucceeds(username="test", password="test")
53+
54+
def test_login_valid_default_redirect(self):
55+
self.env.config.set("trac", "base_url", "")
56+
User.objects.create_user(username="test", password="test")
57+
with self.settings(LOGIN_REDIRECT_URL="/test"):
58+
self.assertLoginSucceeds(
59+
username="test", password="test", check_redirect="/test"
60+
)
61+
62+
def test_login_valid_with_custom_redirection(self):
63+
self.env.config.set("trac", "base_url", "")
64+
User.objects.create_user(username="test", password="test")
65+
self.assertLoginSucceeds(
66+
username="test",
67+
password="test",
68+
check_redirect="/test",
69+
extra_data={"next": "/test"},
70+
)
71+
72+
def test_login_valid_with_custom_redirection_with_hostname(self):
73+
self.env.config.set("trac", "base_url", "http://localhost")
74+
User.objects.create_user(username="test", password="test")
75+
self.assertLoginSucceeds(
76+
username="test",
77+
password="test",
78+
check_redirect="http://localhost/test",
79+
extra_data={"next": "http://localhost/test"},
80+
)
81+
82+
def test_login_valid_with_malicious_redirection(self):
83+
self.env.config.set("trac", "base_url", "http://localhost")
84+
User.objects.create_user(username="test", password="test")
85+
with self.settings(LOGIN_REDIRECT_URL="/test"):
86+
self.assertLoginSucceeds(
87+
username="test",
88+
password="test",
89+
check_redirect="http://localhost/test",
90+
extra_data={"next": "http://example.com/evil"},
91+
)
92+
93+
def assertLoginFails(self, username, password, error_message=None):
94+
if error_message is None:
95+
error_message = AuthenticationForm.error_messages["invalid_login"] % {
96+
"username": "username"
97+
}
98+
99+
request = self.request_factory(
100+
method="POST",
101+
path_info="/login",
102+
args={"username": username, "password": password},
103+
)
104+
template, context = self.component.process_request(request)
105+
self.assertEqual(template, "plainlogin.html")
106+
self.assertEqual(context["form"].errors, {"__all__": [error_message]})
107+
108+
def test_login_invalid_no_users(self):
109+
self.assertLoginFails(username="test", password="test")
110+
111+
def test_login_invalid_incorrect_username(self):
112+
User.objects.create_user(username="test", password="test")
113+
self.assertLoginFails(username="test123", password="test")
114+
115+
def test_login_invalid_incorrect_password(self):
116+
User.objects.create_user(username="test", password="test")
117+
self.assertLoginFails(username="test", password="test123")
118+
119+
def test_login_invalid_incorrect_username_and_password(self):
120+
User.objects.create_user(username="test", password="test")
121+
self.assertLoginFails(username="test123", password="test123")
122+
123+
def test_login_invalid_username_uppercased(self):
124+
User.objects.create_user(username="test", password="test")
125+
self.assertLoginFails(username="TEST", password="test")
126+
127+
def test_login_invalid_inactive_user(self):
128+
User.objects.create_user(username="test", password="test", is_active=False)
129+
self.assertLoginFails(username="test", password="test")

‎DjangoPlugin/tracdjangoplugin/wsgi.py

+4-5
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,17 @@
44

55
application = trac.web.main.dispatch_request
66

7+
import django
8+
9+
django.setup()
10+
711
# Massive hack to make Trac fast, otherwise every git call tries to close ulimit -n (1e6) fds
812
# Python 3 would perform better here, but we are still on 2.7 for Trac, so leak fds for now.
913
from tracopt.versioncontrol.git import PyGIT
1014

1115
PyGIT.close_fds = False
1216

13-
from .djangoauth import DjangoAuth
14-
15-
application = DjangoAuth(application)
16-
1717
trac_dsn = os.getenv("SENTRY_DSN")
18-
1918
if trac_dsn:
2019
import sentry_sdk
2120
from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware

‎trac-env/conf/trac.ini

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ trac.ticket.roadmap.roadmapmodule = disabled
2222
trac.versioncontrol.web_ui.browser.browsermodule = disabled
2323
trac.versioncontrol.web_ui.changeset.changesetmodule = disabled
2424
trac.versioncontrol.web_ui.log.logmodule = disabled
25+
trac.web.auth.loginmodule = disabled; replaced by djangoplugin.PlainLoginComponent
2526
trac.wiki.web_ui.wikimodule = disabled
2627
tracdjangoplugin.* = enabled
2728
tracext.github.githubloginmodule = enabled

‎trac-env/templates/django_theme.html

+1-2
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@
2828
</form>
2929
</li>
3030
# else
31-
<li><a href="/github/login">GitHub Login</a></li>
32-
<li><a href="/login">DjangoProject Login</a></li>
31+
<li><a href="/login?next=${req.path_info|urlencode()}">Login</a></li>
3332
# endif
3433
<li><a href="${req.href.prefs()}">Preferences</a></li>
3534
# if req.perm.has_permission('XML_RPC'):

0 commit comments

Comments
 (0)