Ubuntu One Client

Merge lp:~facundo/ubuntuone-client/win-user2sid into lp:ubuntuone-client

  1. win-user2sid
  2. Merge into trunk
Proposed by Facundo Batista
Status: Merged
Approved by: Natalia Bidart
Approved revision: 1068
Merged at revision: 1066
Proposed branch: lp:~facundo/ubuntuone-client/win-user2sid
Merge into: lp:ubuntuone-client
Diff against target: 365 lines (+74/-90)
2 files modified
tests/platform/windows/test_os_helper.py (+40/-44)
ubuntuone/platform/windows/os_helper.py (+34/-46)
To merge this branch: bzr merge lp:~facundo/ubuntuone-client/win-user2sid
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Manuel de la Peña (community) Approve
Review via email: mp+69342@code.launchpad.net

Commit message

Use SIDs instead user account names.

Description of the change

Use SIDs instead user account names.

This change not only affects to the current user account, but also the Administrators and Everyone ones.

Tests changed too.

To post a comment you must log in.
Revision history for this message
Manuel de la Peña (mandel) :
review: Approve
lp:~facundo/ubuntuone-client/win-user2sid updated
1067. By Facundo Batista

Make lint happy

1068. By Facundo Batista

PySID is not hashable: use a list

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/platform/windows/test_os_helper.py'
2--- tests/platform/windows/test_os_helper.py 2011-07-21 13:40:32 +0000
3+++ tests/platform/windows/test_os_helper.py 2011-07-26 20:09:31 +0000
4@@ -19,7 +19,6 @@
5 import os
6 import errno
7
8-from win32api import GetUserName
9 from ntsecuritycon import (
10 FILE_ALL_ACCESS,
11 FILE_GENERIC_READ,
12@@ -27,6 +26,10 @@
13 )
14
15 from ubuntuone.platform.windows.os_helper import (
16+ EVERYONE_SID,
17+ LONG_PATH_PREFIX,
18+ USER_SID,
19+ WINDOWS_ILLEGAL_CHARS_MAP,
20 _set_file_attributes,
21 access,
22 can_write,
23@@ -41,12 +44,9 @@
24 rename,
25 replace_illegal_chars_with_unicode,
26 replace_unicode_with_illegal_chars,
27+ set_file_readwrite,
28 set_no_rights,
29- set_file_readwrite,
30 stat_path,
31- EVERYONE_GROUP,
32- LONG_PATH_PREFIX,
33- WINDOWS_ILLEGAL_CHARS_MAP,
34 )
35 from contrib.testing.testcase import BaseTwistedTestCase
36
37@@ -278,75 +278,73 @@
38 def test_access_read_write_user(self):
39 """Test when the user sid has rw rights."""
40 # set the file to be read and write just by the user
41- groups = {}
42- groups[GetUserName()] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
43+ groups = [(USER_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
44 _set_file_attributes(self.testfile, groups)
45 self.assertTrue(access(self.testfile))
46
47 def test_access_read_write_everyone(self):
48 """Test when the everyone sid has rw rights."""
49- groups = {}
50- groups[EVERYONE_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
51+ groups = [(EVERYONE_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
52 _set_file_attributes(self.testfile, groups)
53 self.assertTrue(access(self.testfile))
54
55 def test_access_write_user_everyone_read(self):
56 """Test when the user sid has w rights."""
57- groups = {}
58- groups[GetUserName()] = FILE_GENERIC_WRITE
59- groups[EVERYONE_GROUP] = FILE_GENERIC_READ
60+ groups = [
61+ (USER_SID, FILE_GENERIC_WRITE),
62+ (EVERYONE_SID, FILE_GENERIC_READ),
63+ ]
64 _set_file_attributes(self.testfile, groups)
65 self.assertTrue(access(self.testfile))
66
67 def test_access_write_everyone_user_read(self):
68 """Test when the everyone sid has w rights"""
69- groups = {}
70- groups[GetUserName()] = FILE_GENERIC_READ
71- groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE
72+ groups = [
73+ (USER_SID, FILE_GENERIC_READ),
74+ (EVERYONE_SID, FILE_GENERIC_WRITE),
75+ ]
76 _set_file_attributes(self.testfile, groups)
77 self.assertTrue(access(self.testfile))
78
79 def test_access_write_user_everyone(self):
80 """Test when everyone and user have w rights."""
81- groups = {}
82- groups[GetUserName()] = FILE_GENERIC_WRITE
83- groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE
84+ groups = [
85+ (USER_SID, FILE_GENERIC_WRITE),
86+ (EVERYONE_SID, FILE_GENERIC_WRITE),
87+ ]
88 _set_file_attributes(self.testfile, groups)
89 self.assertFalse(access(self.testfile))
90
91 def test_access_read_user(self):
92 """Test when the sid has r rights."""
93- groups = {}
94- groups[GetUserName()] = FILE_GENERIC_READ
95+ groups = [(USER_SID, FILE_GENERIC_READ)]
96 _set_file_attributes(self.testfile, groups)
97 self.assertTrue(access(self.testfile))
98
99 def test_access_read_everyone(self):
100 """Test when everyone has r rights."""
101- groups = {}
102- groups[EVERYONE_GROUP] = FILE_GENERIC_READ
103+ groups = [(EVERYONE_SID, FILE_GENERIC_READ)]
104 _set_file_attributes(self.testfile, groups)
105 self.assertTrue(access(self.testfile))
106
107 def test_access_read_user_everyone(self):
108 """Test when user and everyone have r rights."""
109- groups = {}
110- groups[GetUserName()] = FILE_GENERIC_READ
111- groups[EVERYONE_GROUP] = FILE_GENERIC_READ
112+ groups = [
113+ (USER_SID, FILE_GENERIC_READ),
114+ (EVERYONE_SID, FILE_GENERIC_READ),
115+ ]
116 _set_file_attributes(self.testfile, groups)
117 self.assertTrue(access(self.testfile))
118
119 def test_access_full_user(self):
120 """Test when the sid has full control."""
121- groups = {}
122- groups[GetUserName()] = FILE_ALL_ACCESS
123+ groups = [(USER_SID, FILE_ALL_ACCESS)]
124 _set_file_attributes(self.testfile, groups)
125 self.assertTrue(access(self.testfile))
126
127 def test_access_full_everyone(self):
128 """test when everyone has full control."""
129- groups = {}
130- groups[EVERYONE_GROUP] = FILE_ALL_ACCESS
131+ groups = [(EVERYONE_SID, FILE_ALL_ACCESS)]
132 _set_file_attributes(self.testfile, groups)
133 self.assertTrue(access(self.testfile))
134
135@@ -360,44 +358,42 @@
136 def test_can_write_read_write_user(self):
137 """Test when the user sid has rw rights."""
138 # set the file to be read and write just by the user
139- groups = {}
140- groups[GetUserName()] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
141+ groups = [(USER_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
142 _set_file_attributes(self.testfile, groups)
143 self.assertTrue(can_write(self.testfile))
144
145 def test_can_write_read_write_everyone(self):
146 """Test when the everyone sid has rw rights."""
147- groups = {}
148- groups[EVERYONE_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
149+ groups = [(EVERYONE_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
150 _set_file_attributes(self.testfile, groups)
151 self.assertTrue(can_write(self.testfile))
152
153 def test_can_write_write_user_everyone_read(self):
154 """Test when the user sid has w rights."""
155- groups = {}
156- groups[GetUserName()] = FILE_GENERIC_WRITE
157- groups[EVERYONE_GROUP] = FILE_GENERIC_READ
158+ groups = [
159+ (USER_SID, FILE_GENERIC_WRITE),
160+ (EVERYONE_SID, FILE_GENERIC_READ),
161+ ]
162 _set_file_attributes(self.testfile, groups)
163 self.assertTrue(can_write(self.testfile))
164
165 def test_can_write_write_everyone_user_read(self):
166 """Test when the everyone sid has w rights"""
167- groups = {}
168- groups[GetUserName()] = FILE_GENERIC_READ
169- groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE
170+ groups = [
171+ (USER_SID, FILE_GENERIC_READ),
172+ (EVERYONE_SID, FILE_GENERIC_WRITE),
173+ ]
174 _set_file_attributes(self.testfile, groups)
175 self.assertTrue(can_write(self.testfile))
176
177 def test_can_write_full_user(self):
178 """Test when the sid has full control."""
179- groups = {}
180- groups[GetUserName()] = FILE_ALL_ACCESS
181+ groups = [(USER_SID, FILE_ALL_ACCESS)]
182 _set_file_attributes(self.testfile, groups)
183 self.assertTrue(can_write(self.testfile))
184
185 def test_can_write_full_everyone(self):
186 """test when everyone has full control."""
187- groups = {}
188- groups[EVERYONE_GROUP] = FILE_ALL_ACCESS
189+ groups = [(EVERYONE_SID, FILE_ALL_ACCESS)]
190 _set_file_attributes(self.testfile, groups)
191- self.assertTrue(can_write(self.testfile))
192\ No newline at end of file
193+ self.assertTrue(can_write(self.testfile))
194
195=== modified file 'ubuntuone/platform/windows/os_helper.py'
196--- ubuntuone/platform/windows/os_helper.py 2011-07-21 13:40:32 +0000
197+++ ubuntuone/platform/windows/os_helper.py 2011-07-26 20:09:31 +0000
198@@ -37,13 +37,15 @@
199 from win32security import (
200 ACL,
201 ACL_REVISION,
202+ CONTAINER_INHERIT_ACE,
203+ CreateWellKnownSid,
204 DACL_SECURITY_INFORMATION,
205- CONTAINER_INHERIT_ACE,
206- OBJECT_INHERIT_ACE,
207 GetFileSecurity,
208 LookupAccountName,
209- LookupAccountSid,
210+ OBJECT_INHERIT_ACE,
211 SetFileSecurity,
212+ WinBuiltinAdministratorsSid,
213+ WinWorldSid,
214 )
215 from ntsecuritycon import (
216 FILE_GENERIC_READ,
217@@ -62,8 +64,9 @@
218 platform = 'win32'
219
220 LONG_PATH_PREFIX = '\\\\?\\'
221-EVERYONE_GROUP = u'Everyone'
222-ADMINISTRATORS_GROUP = u'Administrators'
223+USER_SID = LookupAccountName("", GetUserName())[0]
224+EVERYONE_SID = CreateWellKnownSid(WinWorldSid)
225+ADMINISTRATORS_SID = CreateWellKnownSid(WinBuiltinAdministratorsSid)
226 # a map that contains the illegal chars and their 'utf8' representation on
227 # windows so that we can show something to the user
228 WINDOWS_ILLEGAL_CHARS_MAP = {
229@@ -201,12 +204,11 @@
230 raise
231
232 dacl = ACL()
233- for group_name in groups:
234+ for group_sid, attributes in groups:
235 # set the attributes of the group only if not null
236- if groups[group_name]:
237- group_sid = _get_group_sid(group_name)
238+ if attributes:
239 dacl.AddAccessAllowedAceEx(ACL_REVISION,
240- CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, groups[group_name],
241+ CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, attributes,
242 group_sid)
243 # the dacl has all the info of the diff groups passed in the parameters
244 security_descriptor.SetSecurityDescriptorDacl(1, dacl, 0)
245@@ -264,7 +266,7 @@
246 def _set_no_rights_helper(path):
247 """Set the rights to be none."""
248 os.chmod(path, 0o000)
249- groups = {}
250+ groups = []
251 _set_file_attributes(path, groups)
252
253
254@@ -284,9 +286,10 @@
255 """Change path permissions to readonly in a file."""
256 # we use the win32 api because chmod just sets the readonly flag and
257 # we want to have more control over the permissions
258- groups = {}
259- groups[ADMINISTRATORS_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
260- groups[GetUserName()] = FILE_GENERIC_READ
261+ groups = [
262+ (ADMINISTRATORS_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE),
263+ (USER_SID, FILE_GENERIC_READ),
264+ ]
265 # the above equals more or less to 0444
266 _set_file_attributes(path, groups)
267
268@@ -294,10 +297,11 @@
269 @longpath(paths_indexes=[0])
270 def set_file_readwrite(path):
271 """Change path permissions to readwrite in a file."""
272- groups = {}
273- groups[EVERYONE_GROUP] = FILE_GENERIC_READ
274- groups[ADMINISTRATORS_GROUP] = FILE_ALL_ACCESS
275- groups[GetUserName()] = FILE_ALL_ACCESS
276+ groups = [
277+ (EVERYONE_SID, FILE_GENERIC_READ),
278+ (ADMINISTRATORS_SID, FILE_ALL_ACCESS),
279+ (USER_SID, FILE_ALL_ACCESS),
280+ ]
281 # the above equals more or less to 0774
282 _set_file_attributes(path, groups)
283 try:
284@@ -311,9 +315,10 @@
285 @longpath(paths_indexes=[0])
286 def set_dir_readonly(path):
287 """Change path permissions to readonly in a dir."""
288- groups = {}
289- groups[ADMINISTRATORS_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
290- groups[GetUserName()] = FILE_GENERIC_READ
291+ groups = [
292+ (ADMINISTRATORS_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE),
293+ (USER_SID, FILE_GENERIC_READ),
294+ ]
295 # the above equals more or less to 0444
296 _set_file_attributes(path, groups)
297
298@@ -321,10 +326,11 @@
299 @longpath(paths_indexes=[0])
300 def set_dir_readwrite(path):
301 """Change path permissions to readwrite in a dir."""
302- groups = {}
303- groups[EVERYONE_GROUP] = FILE_GENERIC_READ
304- groups[ADMINISTRATORS_GROUP] = FILE_ALL_ACCESS
305- groups[GetUserName()] = FILE_ALL_ACCESS
306+ groups = [
307+ (EVERYONE_SID, FILE_GENERIC_READ),
308+ (ADMINISTRATORS_SID, FILE_ALL_ACCESS),
309+ (USER_SID, FILE_ALL_ACCESS),
310+ ]
311 # the above equals more or less to 0774
312 _set_file_attributes(path, groups)
313 # remove the read only flag
314@@ -485,7 +491,7 @@
315 path += '.lnk'
316 if os.path.exists(path):
317 os.unlink(path)
318-
319+
320
321 @longpath(paths_indexes=[0])
322 def listdir(directory):
323@@ -517,16 +523,7 @@
324 ace = dacl.GetAce(index)
325 if _has_read_mask(ace[1]):
326 sids.append(ace[2])
327- accounts = []
328- for sid in sids:
329- try:
330- accounts.append(LookupAccountSid('', sid)[0])
331- except PyWinError:
332- # means that the sid is not linked with a 'visible' account
333- # in our case we can ignore this since we are looking for visible
334- # users
335- continue
336- return (GetUserName() in accounts or EVERYONE_GROUP in accounts) and\
337+ return (USER_SID in sids or EVERYONE_SID in sids) and\
338 os.access(path, os.R_OK)
339
340
341@@ -551,16 +548,7 @@
342 ace = dacl.GetAce(index)
343 if _has_read_mask(ace[1]):
344 sids.append(ace[2])
345- accounts = []
346- for sid in sids:
347- try:
348- accounts.append(LookupAccountSid('', sid)[0])
349- except PyWinError:
350- # means that the sid is not linked with a 'visible' account
351- # in our case we can ignore this since we are looking for visible
352- # users
353- continue
354- return (GetUserName() in accounts or EVERYONE_GROUP in accounts) and\
355+ return (USER_SID in sids or EVERYONE_SID in sids) and\
356 os.access(path, os.R_OK)
357
358 @longpath(paths_indexes=[0])
359@@ -622,4 +610,4 @@
360 # as a trick for long paths, lets check it is is there
361 if path.startswith(LONG_PATH_PREFIX):
362 path = path.replace(LONG_PATH_PREFIX, '')
363- return LONG_PATH_PREFIX + os.path.normpath(path)
364\ No newline at end of file
365+ return LONG_PATH_PREFIX + os.path.normpath(path)

Subscribers

People subscribed via source and target branches