Skip to content

Commit 99cfbcc

Browse files
committed
no more sql injection and better tests
1 parent b521c6e commit 99cfbcc

File tree

4 files changed

+57
-11
lines changed

4 files changed

+57
-11
lines changed

Diff for: auth/controllers/group_controller.py

+11-3
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,17 @@ def get_group(db, group_name):
3232
if not rows:
3333
return error(404, {'error': 'Not a valid group'})
3434

35-
userids = ["'%s'" % x['userid'] for x in rows if x['userid']]
36-
users = db.query("SELECT * FROM users WHERE userid IN (%s) " % ','.join(userids))
37-
ret = {group_name: list(users) }
35+
userids = [x['userid'] for x in rows if x['userid']]
36+
if not userids:
37+
return {group_name: []}
38+
39+
params = {}
40+
for i, userid in enumerate(userids,1):
41+
params['userid_' + str(i)] = str(userid)
42+
where_clause = 'userid IN(:' + ",:".join(params.keys()) + ')' # b/c sqlalchemy can't use a list!?
43+
q = "SELECT * FROM users WHERE " + where_clause
44+
users = db.executable.execute(q, params).fetchall()
45+
ret = {group_name: [dict(x.items()) for x in users] }
3846
return ret
3947

4048

Diff for: auth/controllers/user_controller.py

+9-6
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def delete_user(db, userid):
3838

3939

4040
@route('/users/<userid>', method=['POST', 'PUT'])
41-
def create__update_user(db, userid):
41+
def create_update_user(db, userid):
4242
data = request.json
4343
data_keys = data.keys()
4444
required_fields = ['first_name', 'last_name', 'userid', 'groups']
@@ -71,10 +71,13 @@ def create__update_user(db, userid):
7171

7272
if request.method == 'PUT':
7373
#get rid of any old groups for this user
74-
q = '''DELETE FROM groups
75-
WHERE userid='{userid}'
76-
AND name not in ({group_names})
77-
'''.format(userid=userid, group_names=','.join(["'%s'" % x for x in groups]))
78-
db.query(q)
74+
params = {}
75+
for counter, group in enumerate(groups,1):
76+
params["group_name" + str(counter)] = group
77+
counter += 1
78+
where_clause = 'name NOT IN(:' + ",:".join(params.keys()) + ')' # b/c sqlalchemy can't use a list!?
79+
params['userid'] = userid
80+
q = '''DELETE FROM groups WHERE userid=:userid AND ''' + where_clause
81+
db.executable.execute(q, params)
7982

8083
return {'status': 200, 'user': get_user(db, userid)}

Diff for: tests/test_auth.py renamed to tests/test_integration.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
app = TestApp(simpleauth.app)
1010

11-
def test_users():
11+
def test_user_integration():
1212
headers = [('Content-type', 'application/json')]
1313
payload = { "first_name": "Joe",
1414
"last_name": "Smith",
@@ -55,7 +55,7 @@ def test_users():
5555
assert delete_user.status == '200 OK'
5656

5757

58-
def test_groups():
58+
def test_groups_integration():
5959
headers = [('Content-type', 'application/json')]
6060
payload = { "first_name": "Joe",
6161
"last_name": "Smith",

Diff for: tests/test_units.py

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import logging
2+
logging.basicConfig(level=logging.DEBUG)
3+
import json
4+
5+
from webtest import TestApp #Docs: http://webtest.pythonpaste.org/en/latest/
6+
from auth import simpleauth
7+
8+
9+
app = TestApp(simpleauth.app)
10+
11+
def test_sql_injection():
12+
13+
pass
14+
15+
16+
def test_create_user():
17+
pass
18+
19+
def test_update_user():
20+
pass
21+
22+
def test_delete_user():
23+
pass
24+
25+
26+
27+
def test_create_group():
28+
pass
29+
30+
def test_update_group():
31+
pass
32+
33+
def test_delete_group():
34+
pass
35+

0 commit comments

Comments
 (0)