Line 0
Link Here
|
|
|
1 |
Obtained-from: http://hg.moinmo.in/moin/1.9/raw-rev/7b9f39289e16 |
2 |
|
3 |
# HG changeset patch |
4 |
# User Thomas Waldmann <tw AT waldmann-edv DOT de> |
5 |
# Date 1346679035 -7200 |
6 |
# Node ID 7b9f39289e16b37344480025f191d8b64480c834 |
7 |
# Parent 0e58d9bcd3bd8ab3a89506d66bc0c8df85c16d2c |
8 |
security fix: fix virtual group bug in ACL evaluation, add a test for it |
9 |
|
10 |
affected moin releases: all 1.9 releases up to and including 1.9.4 |
11 |
|
12 |
moin releases < 1.9 are NOT affected. |
13 |
|
14 |
You can find out the moin version by looking at SystemInfo page or at the |
15 |
output of <<SystemInfo>> macro. |
16 |
|
17 |
Issue description: |
18 |
|
19 |
We have code that checks whether a group has special members "All" or "Known" |
20 |
or "Trusted", but there was a bug that checked whether these are present in |
21 |
the group NAME (not, as intended, in the group MEMBERS). |
22 |
|
23 |
a) If you have group MEMBERS like "All" or "Known" or "Trusted", they did not |
24 |
work until now, but will start working with this changeset. |
25 |
|
26 |
E.g. SomeGroup: |
27 |
* JoeDoe |
28 |
* Trusted |
29 |
|
30 |
SomeGroup will now (correctly) include JoeDoe and also all trusted users. |
31 |
|
32 |
It (erroneously) contained only "JoeDoe" and "Trusted" (as a username, not |
33 |
as a virtual group) before. |
34 |
|
35 |
b) If you have group NAMES containing "All" or "Known" or "Trusted", they behaved |
36 |
wrong until now (they erroneously included All/Known/Trusted users even if |
37 |
you did not list them as members), but will start working correctly with this |
38 |
changeset. |
39 |
|
40 |
E.g. AllFriendsGroup: |
41 |
* JoeDoe |
42 |
|
43 |
AllFriendsGroup will now (correctly) include only JoeDoe. |
44 |
It (erroneously) contained all users (including JoeDoe) before. |
45 |
|
46 |
E.g. MyTrustedFriendsGroup: |
47 |
* JoeDoe |
48 |
|
49 |
MyTrustedFriendsGroup will now (correctly) include only JoeDoe. |
50 |
It (erroneously) contained all trusted users and JoeDoe before. |
51 |
|
52 |
diff -r 0e58d9bcd3bd -r 7b9f39289e16 MoinMoin/security/__init__.py |
53 |
--- MoinMoin/security/__init__.py Fri Aug 03 17:36:02 2012 +0200 |
54 |
+++ MoinMoin/security/__init__.py Mon Sep 03 15:30:35 2012 +0200 |
55 |
@@ -320,11 +320,12 @@ |
56 |
handler = getattr(self, "_special_"+entry, None) |
57 |
allowed = handler(request, name, dowhat, rightsdict) |
58 |
elif entry in groups: |
59 |
- if name in groups[entry]: |
60 |
+ this_group = groups[entry] |
61 |
+ if name in this_group: |
62 |
allowed = rightsdict.get(dowhat) |
63 |
else: |
64 |
for special in self.special_users: |
65 |
- if special in entry: |
66 |
+ if special in this_group: |
67 |
handler = getattr(self, "_special_" + special, None) |
68 |
allowed = handler(request, name, dowhat, rightsdict) |
69 |
break # order of self.special_users is important |
70 |
diff -r 0e58d9bcd3bd -r 7b9f39289e16 MoinMoin/security/_tests/test_security.py |
71 |
--- MoinMoin/security/_tests/test_security.py Fri Aug 03 17:36:02 2012 +0200 |
72 |
+++ MoinMoin/security/_tests/test_security.py Mon Sep 03 15:30:35 2012 +0200 |
73 |
@@ -16,10 +16,11 @@ |
74 |
acliter = security.ACLStringIterator |
75 |
AccessControlList = security.AccessControlList |
76 |
|
77 |
+from MoinMoin.datastruct import ConfigGroups |
78 |
from MoinMoin.PageEditor import PageEditor |
79 |
from MoinMoin.user import User |
80 |
|
81 |
-from MoinMoin._tests import become_trusted, create_page, nuke_page |
82 |
+from MoinMoin._tests import wikiconfig, become_trusted, create_page, nuke_page |
83 |
|
84 |
class TestACLStringIterator(object): |
85 |
|
86 |
@@ -248,6 +249,50 @@ |
87 |
assert not acl.may(self.request, user, right) |
88 |
|
89 |
|
90 |
+class TestGroupACL(object): |
91 |
+ |
92 |
+ class Config(wikiconfig.Config): |
93 |
+ def groups(self, request): |
94 |
+ groups = { |
95 |
+ u'PGroup': frozenset([u'Antony', u'Beatrice', ]), |
96 |
+ u'AGroup': frozenset([u'All', ]), |
97 |
+ # note: the next line is a INTENDED misnomer, there is "All" in |
98 |
+ # the group NAME, but not in the group members. This makes |
99 |
+ # sure that a bug that erroneously checked "in groupname" (instead |
100 |
+ # of "in groupmembers") does not reappear. |
101 |
+ u'AllGroup': frozenset([]), # note: intended misnomer |
102 |
+ } |
103 |
+ return ConfigGroups(request, groups) |
104 |
+ |
105 |
+ def testApplyACLByGroup(self): |
106 |
+ """ security: applying acl by group name""" |
107 |
+ # This acl string... |
108 |
+ acl_rights = [ |
109 |
+ "PGroup,AllGroup:read,write,admin " |
110 |
+ "AGroup:read " |
111 |
+ ] |
112 |
+ acl = security.AccessControlList(self.request.cfg, acl_rights) |
113 |
+ |
114 |
+ # Should apply these rights: |
115 |
+ users = ( |
116 |
+ # user, rights |
117 |
+ ('Antony', ('read', 'write', 'admin', )), # in PGroup |
118 |
+ ('Beatrice', ('read', 'write', 'admin', )), # in PGroup |
119 |
+ ('Charles', ('read', )), # virtually in AGroup |
120 |
+ ) |
121 |
+ |
122 |
+ # Check rights |
123 |
+ for user, may in users: |
124 |
+ mayNot = [right for right in self.request.cfg.acl_rights_valid |
125 |
+ if right not in may] |
126 |
+ # User should have these rights... |
127 |
+ for right in may: |
128 |
+ assert acl.may(self.request, user, right) |
129 |
+ # But NOT these: |
130 |
+ for right in mayNot: |
131 |
+ assert not acl.may(self.request, user, right) |
132 |
+ |
133 |
+ |
134 |
class TestPageAcls(object): |
135 |
""" security: real-life access control list on pages testing |
136 |
""" |
137 |
|