Skip to content

Commit f8ec208

Browse files
committed
Security fix default user permissions
Default user permissions should not include all user attributes. We now limit this to the username, realname and some further attributes depending on the schema. Note that we no longer include the email addresses, depending on your installation you may want to further restrict this or add some attributes like ``address`` and ``alternate_addresses``.
1 parent 2427078 commit f8ec208

File tree

8 files changed

+62
-15
lines changed

8 files changed

+62
-15
lines changed

CHANGES.txt

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ and template changes not listed here.
66
Each entry has the developer who committed the change in brackets.
77
Entries without name were done by Richard Jones.
88

9-
**IMPORTANT** The v1.5.x releases of Roundup will be the last to support Python
10-
v2.5. Support for Python v2.5 will be dropped with the v1.6 release of Roundup,
11-
at which point users will need to run Roundup using either Python v2.6 or v2.7.
9+
**IMPORTANT** The v1.5.x releases of Roundup will be the last to support
10+
Python v2.5. Support for Python v2.5 will be dropped with the v1.6
11+
release of Roundup, at which point users will need to run Roundup using
12+
either Python v2.6 or v2.7.
1213

1314

1415
2014-??-??: 1.5.1
@@ -21,6 +22,8 @@ Pay attention:
2122
If you're upgrading from a previous roundup release version
2223
you should look into ``doc/upgrading.txt``. (Ralf Schlatterbeck)
2324

25+
Also note the default user permissions, see ``doc/upgrading.txt``.
26+
2427
Features:
2528

2629
- The example local_replace.py has been updated to show how to link to
@@ -35,7 +38,7 @@ Features:
3538
class can be numeric -- in that case roundup will try to parse the
3639
value as an ID when evaluating form values -- not as a key. Specifying
3740
try_id_parsing='no' for these Link/Multilink will skip the ID step,
38-
default is 'yes'. (Ralf Schlatterbeck)
41+
default is 'yes'. (Ralf Schlatterbeck)
3942
- New configuration option 'isolation_level' in rdbms section. Currently
4043
supported for Postgres and mysql, sets the transaction isolation level.
4144
Wrong history entries for concurrent database updates observed in
@@ -105,6 +108,12 @@ Fixed:
105108
(Thomas Arendsen Hein)
106109
- Fix issue2550841 roundup-demo templates not found in virtualenv (John
107110
Kristensen)
111+
- Security: Default user permissions should not include all user
112+
attributes. We now limit this to the username, realname and some
113+
further attributes depending on the schema. Note that we no longer
114+
include the email addresses, depending on your installation you may
115+
want to further restrict this or add some attributes like ``address``
116+
and ``alternate_addresses``. (Ralf Schlatterbeck)
108117

109118
Minor:
110119
- demo.py usage message improved: explains "nuke" now. (Bernhard Reiter)

doc/upgrading.txt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,23 @@ Contents:
2323
Migrating from 1.5.0 to 1.5.1
2424
=============================
2525

26+
For security reasons you should change the permissions on the user
27+
class. We previously shipped a configuration that allowed users to see
28+
too many of other users details, including hashed passwords under
29+
certain circumstances. In schema.py in your tracker, replace the line::
30+
31+
db.security.addPermissionToRole('User', 'View', 'user')
32+
33+
with::
34+
35+
p = db.security.addPermission(name='View', klass='user',
36+
properties=('id', 'organisation', 'phone', 'realname',
37+
'timezone', 'username'))
38+
db.security.addPermissionToRole('User', p)
39+
40+
Note that this removes visibility of user emails, if you want emails to
41+
be visible you can add 'address' and 'alternate_addresses' to the list
42+
above.
2643
If you have defined your own cgi actions in your tracker instance
2744
(e.g. in a custom ``extensions/spambayes.py`` file) you need to modify
2845
all cases where client.error_message or client.ok_message are modified

share/roundup/templates/classic/schema.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,10 @@
101101

102102
# May users view other user information? Comment these lines out
103103
# if you don't want them to
104-
db.security.addPermissionToRole('User', 'View', 'user')
104+
p = db.security.addPermission(name='View', klass='user',
105+
properties=('id', 'organisation', 'phone', 'realname', 'timezone',
106+
'username'))
107+
db.security.addPermissionToRole('User', p)
105108

106109
# Users should be able to edit their own details -- this permission is
107110
# limited to only the situation where the Viewed or Edited item is their own.

share/roundup/templates/devel/schema.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,13 @@ def may_edit_file(db, userid, itemid):
292292

293293
# May users view other user information? Comment these lines out
294294
# if you don't want them to
295-
db.security.addPermissionToRole('User', 'View', 'user')
296-
db.security.addPermissionToRole('Developer', 'View', 'user')
295+
p = db.security.addPermission(name='View', klass='user',
296+
properties=('id', 'organisation', 'phone', 'realname', 'timezone',
297+
'vcs_name', 'username'))
298+
db.security.addPermissionToRole('User', p)
299+
db.security.addPermissionToRole('Developer', p)
300+
301+
# Coordinator may also edit users, so they may see everything:
297302
db.security.addPermissionToRole('Coordinator', 'View', 'user')
298303

299304
# Allow Coordinator to edit any user, including their roles.

share/roundup/templates/jinja2/schema.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,10 @@
101101

102102
# May users view other user information? Comment these lines out
103103
# if you don't want them to
104-
db.security.addPermissionToRole('User', 'View', 'user')
104+
p = db.security.addPermission(name='View', klass='user',
105+
properties=('id', 'organisation', 'phone', 'realname', 'timezone',
106+
'username'))
107+
db.security.addPermissionToRole('User', p)
105108

106109
# Users should be able to edit their own details -- this permission is
107110
# limited to only the situation where the Viewed or Edited item is their own.

share/roundup/templates/minimal/schema.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232

3333
# May users view other user information?
3434
# Comment these lines out if you don't want them to
35-
db.security.addPermissionToRole('User', 'View', 'user')
35+
p = db.security.addPermission(name='View', klass='user',
36+
properties=('id', 'username'))
37+
db.security.addPermissionToRole('User', p)
3638

3739
# Users should be able to edit their own details -- this permission is
3840
# limited to only the situation where the Viewed or Edited item is their own.

share/roundup/templates/responsive/schema.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,13 @@ def may_edit_file(db, userid, itemid):
292292

293293
# May users view other user information? Comment these lines out
294294
# if you don't want them to
295-
db.security.addPermissionToRole('User', 'View', 'user')
296-
db.security.addPermissionToRole('Developer', 'View', 'user')
295+
p = db.security.addPermission(name='View', klass='user',
296+
properties=('id', 'organisation', 'phone', 'realname', 'timezone',
297+
'username', 'vcs_name'))
298+
db.security.addPermissionToRole('User', p)
299+
db.security.addPermissionToRole('Developer', p)
300+
301+
# Coordinator may also edit users, so they may see everything:
297302
db.security.addPermissionToRole('Coordinator', 'View', 'user')
298303

299304
# Allow Coordinator to edit any user, including their roles.

website/issues/schema.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -259,10 +259,13 @@ def may_edit_file(db, userid, itemid):
259259

260260
db.security.addPermissionToRole('Coordinator', 'SB: May Classify')
261261

262-
# May users view other user information? Comment these lines out
263-
# if you don't want them to
264-
db.security.addPermissionToRole('User', 'View', 'user')
265-
db.security.addPermissionToRole('Developer', 'View', 'user')
262+
# Allow Users and Developers to view most user properties.
263+
p = db.security.addPermission(name='View', klass='user',
264+
properties=('id', 'username', 'address', 'realname', 'phone',
265+
'organisation', 'alternate_addresses', 'timezone'))
266+
db.security.addPermissionToRole('User', p)
267+
db.security.addPermissionToRole('Developer', p)
268+
# Coordinator may view all user properties.
266269
db.security.addPermissionToRole('Coordinator', 'View', 'user')
267270

268271
# Allow Coordinator to edit any user, including their roles.

0 commit comments

Comments
 (0)