Skip to content

Commit c00b7e5

Browse files
author
Richard Jones
committed
xml-rpc security checks and tests across all backends [SF#1907211]
also add some leap year tests
1 parent 76a6c47 commit c00b7e5

File tree

5 files changed

+82
-58
lines changed

5 files changed

+82
-58
lines changed

‎CHANGES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ are given with the most recent entry first.
44
2008-03-01 1.4.5
55
Fixed:
66
- 'Make a Copy' failed with more than one person in nosy list (sf #1906147)
7+
- xml-rpc security checks and tests across all backends (sf #1907211)
78

89

910
2008-03-01 1.4.4

‎roundup/xmlrpc.py‎

Lines changed: 49 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,10 @@ def __init__(self, tracker, username, password):
6363
def close(self):
6464
"""Close the database, after committing any changes, if needed."""
6565

66-
if getattr(self, 'db'):
67-
try:
68-
if self.db.transactions:
69-
self.db.commit()
70-
finally:
71-
self.db.close()
72-
66+
try:
67+
self.db.commit()
68+
finally:
69+
self.db.close()
7370

7471
def get_class(self, classname):
7572
"""Return the class for the given classname."""
@@ -94,7 +91,7 @@ def props_from_args(self, cl, args):
9491
if value:
9592
try:
9693
props[key] = hyperdb.rawToHyperdb(self.db, cl, None,
97-
key, value)
94+
key, value)
9895
except hyperdb.HyperdbValueError, message:
9996
raise UsageError, message
10097
else:
@@ -115,51 +112,53 @@ def __init__(self, tracker, verbose = False):
115112

116113
def list(self, username, password, classname, propname=None):
117114
r = RoundupRequest(self.tracker, username, password)
118-
cl = r.get_class(classname)
119-
if not propname:
120-
propname = cl.labelprop()
121-
def has_perm(itemid):
122-
return True
123-
r.db.security.hasPermission('View', r.userid, classname,
124-
itemid=itemid, property=propname)
125-
result = [cl.get(id, propname) for id in cl.list()
126-
if has_perm(id)]
127-
r.close()
115+
try:
116+
cl = r.get_class(classname)
117+
if not propname:
118+
propname = cl.labelprop()
119+
result = [cl.get(itemid, propname)
120+
for itemid in cl.list()
121+
if r.db.security.hasPermission('View', r.userid,
122+
classname, propname, itemid)
123+
]
124+
finally:
125+
r.close()
128126
return result
129127

130128
def display(self, username, password, designator, *properties):
131129
r = RoundupRequest(self.tracker, username, password)
132-
classname, itemid = hyperdb.splitDesignator(designator)
133-
134-
if not r.db.security.hasPermission('View', r.userid, classname,
135-
itemid=itemid):
136-
raise Unauthorised('Permission to view %s denied'%designator)
137-
138-
cl = r.get_class(classname)
139-
props = properties and list(properties) or cl.properties.keys()
140-
props.sort()
141-
result = [(property, cl.get(itemid, property)) for property in props]
142-
r.close()
130+
try:
131+
classname, itemid = hyperdb.splitDesignator(designator)
132+
cl = r.get_class(classname)
133+
props = properties and list(properties) or cl.properties.keys()
134+
props.sort()
135+
for p in props:
136+
if not r.db.security.hasPermission('View', r.userid,
137+
classname, p, itemid):
138+
raise Unauthorised('Permission to view %s of %s denied'%
139+
(p, designator))
140+
result = [(prop, cl.get(itemid, prop)) for prop in props]
141+
finally:
142+
r.close()
143143
return dict(result)
144144

145145
def create(self, username, password, classname, *args):
146146
r = RoundupRequest(self.tracker, username, password)
147+
try:
148+
if not r.db.security.hasPermission('Create', r.userid, classname):
149+
raise Unauthorised('Permission to create %s denied'%classname)
147150

148-
if not r.db.security.hasPermission('Create', r.userid, classname):
149-
raise Unauthorised('Permission to create %s denied'%classname)
150-
151-
cl = r.get_class(classname)
151+
cl = r.get_class(classname)
152152

153-
# convert types
154-
props = r.props_from_args(cl, args)
153+
# convert types
154+
props = r.props_from_args(cl, args)
155155

156-
# check for the key property
157-
key = cl.getkey()
158-
if key and not props.has_key(key):
159-
raise UsageError, 'you must provide the "%s" property.'%key
156+
# check for the key property
157+
key = cl.getkey()
158+
if key and not props.has_key(key):
159+
raise UsageError, 'you must provide the "%s" property.'%key
160160

161-
# do the actual create
162-
try:
161+
# do the actual create
163162
try:
164163
result = cl.create(**props)
165164
except (TypeError, IndexError, ValueError), message:
@@ -170,19 +169,17 @@ def create(self, username, password, classname, *args):
170169

171170
def set(self, username, password, designator, *args):
172171
r = RoundupRequest(self.tracker, username, password)
173-
classname, itemid = hyperdb.splitDesignator(designator)
174-
175-
if not r.db.security.hasPermission('Edit', r.userid, classname,
176-
itemid=itemid):
177-
raise Unauthorised('Permission to edit %s denied'%designator)
178-
179-
cl = r.get_class(classname)
180-
181-
# convert types
182-
props = r.props_from_args(cl, args)
183172
try:
173+
classname, itemid = hyperdb.splitDesignator(designator)
174+
cl = r.get_class(classname)
175+
props = r.props_from_args(cl, args) # convert types
176+
for p in props.iterkeys ():
177+
if not r.db.security.hasPermission('Edit', r.userid,
178+
classname, p, itemid):
179+
raise Unauthorised('Permission to edit %s of %s denied'%
180+
(p, designator))
184181
try:
185-
cl.set(itemid, **props)
182+
return cl.set(itemid, **props)
186183
except (TypeError, IndexError, ValueError), message:
187184
raise UsageError, message
188185
finally:

‎test/db_test_base.py‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
1616
# SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
1717
#
18-
# $Id: db_test_base.py,v 1.96 2008-02-07 03:28:34 richard Exp $
18+
# $Id: db_test_base.py,v 1.97 2008-03-07 01:11:55 richard Exp $
1919

2020
import unittest, os, shutil, errno, imp, sys, time, pprint, sets, base64, os.path
2121

@@ -62,6 +62,7 @@ def setupTracker(dirname, backend="anydbm"):
6262
tracker = instance.open(dirname)
6363
if tracker.exists():
6464
tracker.nuke()
65+
init.write_select_db(dirname, backend)
6566
tracker.init(password.Password('sekrit'))
6667
return tracker
6768

@@ -293,7 +294,7 @@ def testMultilinkChangeIterable(self):
293294
l = [u1,u2]; l.sort()
294295
m = self.db.issue.get(nid, "nosy"); m.sort()
295296
self.assertEqual(l, m)
296-
297+
297298

298299
# XXX one day, maybe...
299300
# def testMultilinkOrdering(self):
@@ -329,6 +330,18 @@ def testDateChange(self):
329330
c = self.db.issue.get(nid, "deadline")
330331
self.assertEqual(c, d)
331332

333+
def testDateLeapYear(self):
334+
nid = self.db.issue.create(title='spam', status='1',
335+
deadline=date.Date('2008-02-29'))
336+
self.assertEquals(str(self.db.issue.get(nid, 'deadline')),
337+
'2008-02-29.00:00:00')
338+
self.db.issue.set(nid, deadline=date.Date('2008-02-29'))
339+
self.assertEquals(str(self.db.issue.get(nid, 'deadline')),
340+
'2008-02-29.00:00:00')
341+
self.assertEquals(self.db.issue.filter(None, {'deadline': '2008-02-29'}),
342+
[nid])
343+
344+
332345
def testDateUnset(self):
333346
for commit in (0,1):
334347
nid = self.db.issue.create(title="spam", status='1')

‎test/test_dates.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# BASIS, AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
1616
# SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
1717
#
18-
# $Id: test_dates.py,v 1.44 2007-12-23 00:23:23 richard Exp $
18+
# $Id: test_dates.py,v 1.45 2008-03-07 01:11:55 richard Exp $
1919
from __future__ import nested_scopes
2020

2121
import unittest
@@ -68,6 +68,9 @@ def testDate(self):
6868
ae(str(Date('1900-02-01')), '1900-02-01.00:00:00')
6969
ae(str(Date('1800-07-15')), '1800-07-15.00:00:00')
7070

71+
def testLeapYear(self):
72+
self.assertEquals(str(Date('2008-02-29')), '2008-02-29.00:00:00')
73+
7174
def testDateError(self):
7275
self.assertRaises(ValueError, Date, "12")
7376
# Date cannot handle dates before year 1

‎test/test_xmlrpc.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,20 @@
99
from roundup.cgi.exceptions import *
1010
from roundup import init, instance, password, hyperdb, date
1111
from roundup.xmlrpc import RoundupServer
12+
from roundup.backends import list_backends
1213

1314
import db_test_base
1415

1516
NEEDS_INSTANCE = 1
1617

1718
class TestCase(unittest.TestCase):
19+
20+
backend = None
21+
1822
def setUp(self):
1923
self.dirname = '_test_xmlrpc'
2024
# set up and open a tracker
21-
self.instance = db_test_base.setupTracker(self.dirname)
25+
self.instance = db_test_base.setupTracker(self.dirname, self.backend)
2226

2327
# open the database
2428
self.db = self.instance.open('admin')
@@ -55,6 +59,10 @@ def testChange(self):
5559
'realname')
5660
self.assertEqual(results['realname'], 'Joe Doe')
5761

62+
# check we can't change admin's details
63+
self.assertRaises(Unauthorised, self.server.set, 'joe', 'random',
64+
'user1', 'realname=Joe Doe')
65+
5866
def testCreate(self):
5967
results = self.server.create('joe', 'random', 'issue', 'title=foo')
6068
issueid = 'issue' + results
@@ -89,10 +97,12 @@ def testAuthAllowedCreate(self):
8997

9098
def test_suite():
9199
suite = unittest.TestSuite()
92-
suite.addTest(unittest.makeSuite(TestCase))
100+
for l in list_backends():
101+
dct = dict(backend = l)
102+
subcls = type(TestCase)('TestCase_%s'%l, (TestCase,), dct)
103+
suite.addTest(unittest.makeSuite(subcls))
93104
return suite
94105

95106
if __name__ == '__main__':
96107
runner = unittest.TextTestRunner()
97108
unittest.main(testRunner=runner)
98-

0 commit comments

Comments
 (0)