ISD Sentry Server

Merge lp:~ricardokirkner/isd-sentry/require-team-decorator into lp:isd-sentry

  1. require-team-decorator
  2. Merge into trunk
Proposed by Ricardo Kirkner
Status: Merged
Approved by: Natalia Bidart
Approved revision: 21
Merged at revision: 13
Proposed branch: lp:~ricardokirkner/isd-sentry/require-team-decorator
Merge into: lp:isd-sentry
Diff against target: 202 lines (+119/-7)
8 files modified
django_project/config/main.cfg (+1/-0)
django_project/urls.py (+12/-4)
fabtasks/database.py (+1/-1)
fabtasks/development.py (+1/-1)
isd_sentry/decorators.py (+29/-0)
isd_sentry/schema.py (+2/-0)
isd_sentry/tests.py (+69/-0)
requirements.txt (+4/-1)
To merge this branch: bzr merge lp:~ricardokirkner/isd-sentry/require-team-decorator
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Tom Haddon Approve
Review via email: mp+124757@code.launchpad.net

Commit message

added decorator to restrict login to members of certain teams only

Description of the change

Added new decorator and monkeypatched sentry's login_required decorator so that all views requiring login also require users to be members of certain teams

To post a comment you must log in.
lp:~ricardokirkner/isd-sentry/require-team-decorator updated
16. By Ricardo Kirkner

make sure to restrict which version of celery we can use

17. By Ricardo Kirkner

fixed incorrect decorator

18. By Ricardo Kirkner

changed flow following review

Revision history for this message
Tom Haddon (mthaddon) wrote :

Looks good. I'm approving in terms of assuming it does what it's designed to do - this isn't a code review as such, so you may need to have someone else do that if it's not testable.

review: Approve
lp:~ricardokirkner/isd-sentry/require-team-decorator updated
19. By Ricardo Kirkner

make isd_sentry a real django app

20. By Ricardo Kirkner

added tests

21. By Ricardo Kirkner

make sure to require django < 1.4

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good! Also tested IRL and works as expected.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'django_project/config/main.cfg'
2--- django_project/config/main.cfg 2012-02-29 15:48:48 +0000
3+++ django_project/config/main.cfg 2012-09-18 14:10:23 +0000
4@@ -28,6 +28,7 @@
5 south
6 django_openid_auth
7 django_configglue
8+ isd_sentry
9 template_dirs =
10 /usr/share/pyshared/sentry/templates
11 %(basedir)s/isd_sentry/templates
12
13=== modified file 'django_project/urls.py'
14--- django_project/urls.py 2012-02-27 15:17:45 +0000
15+++ django_project/urls.py 2012-09-18 14:10:23 +0000
16@@ -1,9 +1,17 @@
17 from django.contrib.auth.views import logout
18-from sentry.conf.urls import *
19-
20-
21-urlpatterns += patterns('',
22+from django.conf.urls.defaults import include, patterns
23+
24+
25+urlpatterns = patterns('',
26 # OpenID views
27 (r'^openid/', include('django_openid_auth.urls')),
28 (r'^logout/$', logout, {'next_page': '/'}),
29 )
30+
31+# monkey patch sentry so login requires team membership
32+import sentry.web.decorators
33+from isd_sentry.decorators import requires_team
34+sentry.web.decorators.login_required = requires_team
35+
36+from sentry.conf.urls import urlpatterns as sentry_patterns
37+urlpatterns += sentry_patterns
38
39=== modified file 'fabtasks/database.py'
40--- fabtasks/database.py 2012-02-27 15:18:10 +0000
41+++ fabtasks/database.py 2012-09-18 14:10:23 +0000
42@@ -84,7 +84,7 @@
43
44 pg_env = []
45 env.postgres = {
46- 'BIN': '/usr/lib/postgresql/8.4/bin',
47+ 'BIN': '/usr/lib/postgresql/9.1/bin',
48 'DATABASE': env.database['NAME'],
49 }
50
51
52=== modified file 'fabtasks/development.py'
53--- fabtasks/development.py 2012-02-27 13:29:34 +0000
54+++ fabtasks/development.py 2012-09-18 14:10:23 +0000
55@@ -5,7 +5,7 @@
56 from .environment import bootstrap, virtualenv_local
57
58
59-def test(coverage=False, extra=''):
60+def test(coverage=False, extra='isd_sentry'):
61 """Run unit tests."""
62 cmd = ['python django_project/manage.py test --noinput', extra]
63 virtualenv_local(' '.join(cmd), capture=False)
64
65=== added file 'isd_sentry/decorators.py'
66--- isd_sentry/decorators.py 1970-01-01 00:00:00 +0000
67+++ isd_sentry/decorators.py 2012-09-18 14:10:23 +0000
68@@ -0,0 +1,29 @@
69+from functools import wraps
70+
71+from django.conf import settings
72+from django.http import HttpResponseBadRequest
73+from django.template.loader import render_to_string
74+
75+from sentry.web.decorators import login_required
76+
77+
78+def requires_team(func):
79+ @wraps(func)
80+ @login_required
81+ def wrapped(request, *args, **kwargs):
82+ allowed = True
83+ restricted_teams = getattr(
84+ settings, 'OPENID_RESTRICT_ACCESS_TO_TEAMS', [])
85+ user_has_openid = (
86+ getattr(request.user, 'useropenid_set') is not None and
87+ request.user.useropenid_set.count())
88+
89+ if restricted_teams and user_has_openid:
90+ teams = request.user.groups.filter(name__in=restricted_teams)
91+ allowed = teams.count()
92+
93+ if not allowed:
94+ return HttpResponseBadRequest(render_to_string(
95+ 'sentry/missing_permissions.html'))
96+ return func(request, *args, **kwargs)
97+ return wrapped
98
99=== added file 'isd_sentry/models.py'
100=== modified file 'isd_sentry/schema.py'
101--- isd_sentry/schema.py 2012-04-13 20:21:05 +0000
102+++ isd_sentry/schema.py 2012-09-18 14:10:23 +0000
103@@ -29,6 +29,7 @@
104 sentry_udp_host = StringOption()
105 sentry_udp_port = IntOption()
106 sentry_url_prefix = StringOption()
107+ sentry_sample_data = BoolOption(default=True)
108
109 class openid(Section):
110 openid_create_users = BoolOption()
111@@ -38,3 +39,4 @@
112 openid_sreg_extra_fields = ListOption()
113 openid_launchpad_teams_mapping = DictOption()
114 openid_launchpad_staff_teams = ListOption()
115+ openid_restrict_access_to_teams = ListOption()
116
117=== added file 'isd_sentry/tests.py'
118--- isd_sentry/tests.py 1970-01-01 00:00:00 +0000
119+++ isd_sentry/tests.py 2012-09-18 14:10:23 +0000
120@@ -0,0 +1,69 @@
121+from django.test import TestCase
122+from django.contrib.auth.models import User, Group
123+from django.conf import settings
124+from django_openid_auth.models import UserOpenID
125+
126+from mock import patch
127+
128+
129+class RequiresTeamTestCase(TestCase):
130+ def setUp(self):
131+ self.user = User.objects.create_user('testuser', '', 'test')
132+ # add user to the right team
133+ group = Group.objects.create(name='test-team')
134+ self.user.groups.add(group)
135+ # fake user logged in via openid
136+ UserOpenID.objects.create(user=self.user)
137+
138+ def test_deny_access_to_non_members(self):
139+ with patch.object(settings, 'OPENID_RESTRICT_ACCESS_TO_TEAMS',
140+ ['other-team']):
141+ self.client.login(username='testuser', password='test')
142+ response = self.client.get('/1/')
143+ self.assertEqual(response.status_code, 400)
144+ self.assertTemplateUsed(response, 'sentry/missing_permissions.html')
145+
146+ def test_allow_access_to_members(self):
147+ with patch.object(settings, 'OPENID_RESTRICT_ACCESS_TO_TEAMS',
148+ ['test-team']):
149+ self.client.login(username='testuser', password='test')
150+ response = self.client.get('/1/')
151+ self.assertEqual(response.status_code, 200)
152+
153+ def test_dont_restrict_teams(self):
154+ with patch.object(settings, 'OPENID_RESTRICT_ACCESS_TO_TEAMS', []):
155+ self.client.login(username='testuser', password='test')
156+ response = self.client.get('/1/')
157+ self.assertEqual(response.status_code, 200)
158+
159+ def test_dont_restrict_standard_users_if_no_teams(self):
160+ # remove openid link with user
161+ UserOpenID.objects.all().delete()
162+
163+ with patch.object(settings, 'OPENID_RESTRICT_ACCESS_TO_TEAMS', []):
164+ self.client.login(username='testuser', password='test')
165+ response = self.client.get('/1/')
166+ self.assertEqual(response.status_code, 200)
167+
168+ def test_dont_restrict_admin_access_to_staff(self):
169+ staff = User.objects.create_user('staff', '', 'staff')
170+ staff.is_staff = True
171+ staff.save()
172+
173+ with patch.object(settings, 'OPENID_RESTRICT_ACCESS_TO_TEAMS',
174+ ['test-team']):
175+ self.client.login(username='staff', password='staff')
176+ response = self.client.get('/admin/')
177+ self.assertEqual(response.status_code, 200)
178+
179+ def test_dont_restrict_admin_access_to_superuser(self):
180+ superuser = User.objects.create_user('superuser', '', 'superuser')
181+ superuser.is_superuser = True
182+ superuser.save()
183+
184+ with patch.object(settings, 'OPENID_RESTRICT_ACCESS_TO_TEAMS',
185+ ['test-team']):
186+ self.client.login(username='superuser', password='superuser')
187+ response = self.client.get('/admin/')
188+ self.assertEqual(response.status_code, 200)
189+
190
191=== modified file 'requirements.txt'
192--- requirements.txt 2012-02-27 13:29:59 +0000
193+++ requirements.txt 2012-09-18 14:10:23 +0000
194@@ -1,4 +1,7 @@
195-sentry
196+celery<3
197+django<1.4
198 django_openid_auth
199+mock
200 psycopg2
201 python-openid
202+sentry

Subscribers

People subscribed via source and target branches