Skip to content

Commit f3778b6

Browse files
author
Richard Jones
committed
Add security checks and tests for xmlrpc interface.
1 parent 74f909c commit f3778b6

File tree

2 files changed

+67
-41
lines changed

2 files changed

+67
-41
lines changed

roundup/xmlrpc.py

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,15 @@ def __init__(self, tracker, username, password):
4949
self.tracker = tracker
5050
self.db = self.tracker.open('admin')
5151
try:
52-
userid = self.db.user.lookup(username)
52+
self.userid = self.db.user.lookup(username)
5353
except KeyError: # No such user
5454
self.db.close()
55-
raise Unauthorised, 'Invalid user.'
56-
stored = self.db.user.get(userid, 'password')
57-
if stored != password: # Wrong password
55+
raise Unauthorised, 'Invalid user'
56+
stored = self.db.user.get(self.userid, 'password')
57+
if stored != password:
58+
# Wrong password
5859
self.db.close()
59-
raise Unauthorised, 'Invalid user.'
60+
raise Unauthorised, 'Invalid user'
6061
self.db.setCurrentUser(username)
6162

6263
def close(self):
@@ -112,30 +113,41 @@ def __init__(self, tracker, verbose = False):
112113
self.tracker = roundup.instance.open(tracker)
113114
self.verbose = verbose
114115

115-
def list(self, username, password, classname, propname = None):
116-
116+
def list(self, username, password, classname, propname=None):
117117
r = RoundupRequest(self.tracker, username, password)
118118
cl = r.get_class(classname)
119119
if not propname:
120120
propname = cl.labelprop()
121-
result = [cl.get(id, propname) for id in cl.list()]
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)]
122127
r.close()
123128
return result
124129

125130
def display(self, username, password, designator, *properties):
126-
127131
r = RoundupRequest(self.tracker, username, password)
128-
classname, nodeid = hyperdb.splitDesignator(designator)
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+
129138
cl = r.get_class(classname)
130139
props = properties and list(properties) or cl.properties.keys()
131140
props.sort()
132-
result = [(property, cl.get(nodeid, property)) for property in props]
141+
result = [(property, cl.get(itemid, property)) for property in props]
133142
r.close()
134143
return dict(result)
135144

136145
def create(self, username, password, classname, *args):
137-
138146
r = RoundupRequest(self.tracker, username, password)
147+
148+
if not r.db.security.hasPermission('Create', r.userid, classname):
149+
raise Unauthorised('Permission to create %s denied'%classname)
150+
139151
cl = r.get_class(classname)
140152

141153
# convert types
@@ -157,9 +169,13 @@ def create(self, username, password, classname, *args):
157169
return result
158170

159171
def set(self, username, password, designator, *args):
160-
161172
r = RoundupRequest(self.tracker, username, password)
162173
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+
163179
cl = r.get_class(classname)
164180

165181
# convert types

test/test_xmlrpc.py

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,75 +14,85 @@
1414

1515
NEEDS_INSTANCE = 1
1616

17-
class TestCaseBase(unittest.TestCase):
18-
17+
class TestCase(unittest.TestCase):
1918
def setUp(self):
20-
2119
self.dirname = '_test_xmlrpc'
2220
# set up and open a tracker
2321
self.instance = db_test_base.setupTracker(self.dirname)
2422

2523
# open the database
2624
self.db = self.instance.open('admin')
27-
self.db.user.create(username='joe', password=password.Password('random'),
28-
address='[email protected]',
29-
realname='Joe Random', roles='User')
25+
self.joeid = 'user' + self.db.user.create(username='joe',
26+
password=password.Password('random'), address='[email protected]',
27+
realname='Joe Random', roles='User')
3028

3129
self.db.commit()
3230
self.db.close()
33-
34-
self.server = RoundupServer(self.dirname)
3531

32+
self.server = RoundupServer(self.dirname)
3633

3734
def tearDown(self):
38-
3935
try:
4036
shutil.rmtree(self.dirname)
4137
except OSError, error:
4238
if error.errno not in (errno.ENOENT, errno.ESRCH): raise
4339

44-
class AccessTestCase(TestCaseBase):
45-
46-
def test(self):
47-
40+
def testAccess(self):
4841
# Retrieve all three users.
4942
results = self.server.list('joe', 'random', 'user', 'id')
5043
self.assertEqual(len(results), 3)
44+
5145
# Obtain data for 'joe'.
52-
userid = 'user' + results[-1]
53-
results = self.server.display('joe', 'random', userid)
46+
results = self.server.display('joe', 'random', self.joeid)
5447
self.assertEqual(results['username'], 'joe')
5548
self.assertEqual(results['realname'], 'Joe Random')
49+
50+
def testChange(self):
5651
# Reset joe's 'realname'.
57-
results = self.server.set('joe', 'random', userid, 'realname=Joe Doe')
58-
results = self.server.display('joe', 'random', userid, 'realname')
52+
results = self.server.set('joe', 'random', self.joeid,
53+
'realname=Joe Doe')
54+
results = self.server.display('joe', 'random', self.joeid,
55+
'realname')
5956
self.assertEqual(results['realname'], 'Joe Doe')
60-
# Create test
57+
58+
def testCreate(self):
6159
results = self.server.create('joe', 'random', 'issue', 'title=foo')
6260
issueid = 'issue' + results
6361
results = self.server.display('joe', 'random', issueid, 'title')
6462
self.assertEqual(results['title'], 'foo')
6563

66-
class AuthenticationTestCase(TestCaseBase):
67-
68-
def test(self):
69-
64+
def testAuthUnknown(self):
7065
# Unknown user (caught in XMLRPC frontend).
7166
self.assertRaises(Unauthorised, self.server.list,
72-
'nobody', 'nobody', 'user', 'id')
67+
'nobody', 'nobody', 'user', 'id')
68+
69+
def testAuthDeniedEdit(self):
7370
# Wrong permissions (caught by roundup security module).
74-
results = self.server.list('joe', 'random', 'user', 'id')
75-
userid = 'user' + results[0] # admin
7671
self.assertRaises(Unauthorised, self.server.set,
77-
'joe', 'random', userid, 'realname=someone')
72+
'joe', 'random', 'user1', 'realname=someone')
7873

74+
def testAuthDeniedCreate(self):
75+
self.assertRaises(Unauthorised, self.server.create,
76+
'joe', 'random', 'user', {'username': 'blah'})
77+
78+
def testAuthAllowedEdit(self):
79+
try:
80+
self.server.set('admin', 'sekrit', 'user2', 'realname=someone')
81+
except Unauthorised, err:
82+
self.fail('raised %s'%err)
83+
84+
def testAuthAllowedCreate(self):
85+
try:
86+
self.server.create('admin', 'sekrit', 'user', 'username=blah')
87+
except Unauthorised, err:
88+
self.fail('raised %s'%err)
7989

8090
def test_suite():
8191
suite = unittest.TestSuite()
82-
suite.addTest(unittest.makeSuite(AccessTestCase))
83-
suite.addTest(unittest.makeSuite(AuthenticationTestCase))
92+
suite.addTest(unittest.makeSuite(TestCase))
8493
return suite
8594

8695
if __name__ == '__main__':
8796
runner = unittest.TextTestRunner()
8897
unittest.main(testRunner=runner)
98+

0 commit comments

Comments
 (0)