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
=== modified file 'tests/platform/windows/test_os_helper.py'
--- tests/platform/windows/test_os_helper.py 2011-07-21 13:40:32 +0000
+++ tests/platform/windows/test_os_helper.py 2011-07-26 20:09:31 +0000
@@ -19,7 +19,6 @@
19import os19import os
20import errno20import errno
2121
22from win32api import GetUserName
23from ntsecuritycon import (22from ntsecuritycon import (
24 FILE_ALL_ACCESS,23 FILE_ALL_ACCESS,
25 FILE_GENERIC_READ,24 FILE_GENERIC_READ,
@@ -27,6 +26,10 @@
27)26)
2827
29from ubuntuone.platform.windows.os_helper import (28from ubuntuone.platform.windows.os_helper import (
29 EVERYONE_SID,
30 LONG_PATH_PREFIX,
31 USER_SID,
32 WINDOWS_ILLEGAL_CHARS_MAP,
30 _set_file_attributes,33 _set_file_attributes,
31 access,34 access,
32 can_write,35 can_write,
@@ -41,12 +44,9 @@
41 rename,44 rename,
42 replace_illegal_chars_with_unicode,45 replace_illegal_chars_with_unicode,
43 replace_unicode_with_illegal_chars,46 replace_unicode_with_illegal_chars,
47 set_file_readwrite,
44 set_no_rights,48 set_no_rights,
45 set_file_readwrite,
46 stat_path,49 stat_path,
47 EVERYONE_GROUP,
48 LONG_PATH_PREFIX,
49 WINDOWS_ILLEGAL_CHARS_MAP,
50)50)
51from contrib.testing.testcase import BaseTwistedTestCase51from contrib.testing.testcase import BaseTwistedTestCase
5252
@@ -278,75 +278,73 @@
278 def test_access_read_write_user(self):278 def test_access_read_write_user(self):
279 """Test when the user sid has rw rights."""279 """Test when the user sid has rw rights."""
280 # set the file to be read and write just by the user280 # set the file to be read and write just by the user
281 groups = {}281 groups = [(USER_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
282 groups[GetUserName()] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
283 _set_file_attributes(self.testfile, groups)282 _set_file_attributes(self.testfile, groups)
284 self.assertTrue(access(self.testfile))283 self.assertTrue(access(self.testfile))
285284
286 def test_access_read_write_everyone(self):285 def test_access_read_write_everyone(self):
287 """Test when the everyone sid has rw rights."""286 """Test when the everyone sid has rw rights."""
288 groups = {}287 groups = [(EVERYONE_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
289 groups[EVERYONE_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
290 _set_file_attributes(self.testfile, groups)288 _set_file_attributes(self.testfile, groups)
291 self.assertTrue(access(self.testfile))289 self.assertTrue(access(self.testfile))
292290
293 def test_access_write_user_everyone_read(self):291 def test_access_write_user_everyone_read(self):
294 """Test when the user sid has w rights."""292 """Test when the user sid has w rights."""
295 groups = {}293 groups = [
296 groups[GetUserName()] = FILE_GENERIC_WRITE294 (USER_SID, FILE_GENERIC_WRITE),
297 groups[EVERYONE_GROUP] = FILE_GENERIC_READ295 (EVERYONE_SID, FILE_GENERIC_READ),
296 ]
298 _set_file_attributes(self.testfile, groups)297 _set_file_attributes(self.testfile, groups)
299 self.assertTrue(access(self.testfile))298 self.assertTrue(access(self.testfile))
300299
301 def test_access_write_everyone_user_read(self):300 def test_access_write_everyone_user_read(self):
302 """Test when the everyone sid has w rights"""301 """Test when the everyone sid has w rights"""
303 groups = {}302 groups = [
304 groups[GetUserName()] = FILE_GENERIC_READ303 (USER_SID, FILE_GENERIC_READ),
305 groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE304 (EVERYONE_SID, FILE_GENERIC_WRITE),
305 ]
306 _set_file_attributes(self.testfile, groups)306 _set_file_attributes(self.testfile, groups)
307 self.assertTrue(access(self.testfile))307 self.assertTrue(access(self.testfile))
308308
309 def test_access_write_user_everyone(self):309 def test_access_write_user_everyone(self):
310 """Test when everyone and user have w rights."""310 """Test when everyone and user have w rights."""
311 groups = {}311 groups = [
312 groups[GetUserName()] = FILE_GENERIC_WRITE312 (USER_SID, FILE_GENERIC_WRITE),
313 groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE313 (EVERYONE_SID, FILE_GENERIC_WRITE),
314 ]
314 _set_file_attributes(self.testfile, groups)315 _set_file_attributes(self.testfile, groups)
315 self.assertFalse(access(self.testfile))316 self.assertFalse(access(self.testfile))
316317
317 def test_access_read_user(self):318 def test_access_read_user(self):
318 """Test when the sid has r rights."""319 """Test when the sid has r rights."""
319 groups = {}320 groups = [(USER_SID, FILE_GENERIC_READ)]
320 groups[GetUserName()] = FILE_GENERIC_READ
321 _set_file_attributes(self.testfile, groups)321 _set_file_attributes(self.testfile, groups)
322 self.assertTrue(access(self.testfile))322 self.assertTrue(access(self.testfile))
323323
324 def test_access_read_everyone(self):324 def test_access_read_everyone(self):
325 """Test when everyone has r rights."""325 """Test when everyone has r rights."""
326 groups = {}326 groups = [(EVERYONE_SID, FILE_GENERIC_READ)]
327 groups[EVERYONE_GROUP] = FILE_GENERIC_READ
328 _set_file_attributes(self.testfile, groups)327 _set_file_attributes(self.testfile, groups)
329 self.assertTrue(access(self.testfile))328 self.assertTrue(access(self.testfile))
330329
331 def test_access_read_user_everyone(self):330 def test_access_read_user_everyone(self):
332 """Test when user and everyone have r rights."""331 """Test when user and everyone have r rights."""
333 groups = {}332 groups = [
334 groups[GetUserName()] = FILE_GENERIC_READ333 (USER_SID, FILE_GENERIC_READ),
335 groups[EVERYONE_GROUP] = FILE_GENERIC_READ334 (EVERYONE_SID, FILE_GENERIC_READ),
335 ]
336 _set_file_attributes(self.testfile, groups)336 _set_file_attributes(self.testfile, groups)
337 self.assertTrue(access(self.testfile))337 self.assertTrue(access(self.testfile))
338338
339 def test_access_full_user(self):339 def test_access_full_user(self):
340 """Test when the sid has full control."""340 """Test when the sid has full control."""
341 groups = {}341 groups = [(USER_SID, FILE_ALL_ACCESS)]
342 groups[GetUserName()] = FILE_ALL_ACCESS
343 _set_file_attributes(self.testfile, groups)342 _set_file_attributes(self.testfile, groups)
344 self.assertTrue(access(self.testfile))343 self.assertTrue(access(self.testfile))
345344
346 def test_access_full_everyone(self):345 def test_access_full_everyone(self):
347 """test when everyone has full control."""346 """test when everyone has full control."""
348 groups = {}347 groups = [(EVERYONE_SID, FILE_ALL_ACCESS)]
349 groups[EVERYONE_GROUP] = FILE_ALL_ACCESS
350 _set_file_attributes(self.testfile, groups)348 _set_file_attributes(self.testfile, groups)
351 self.assertTrue(access(self.testfile))349 self.assertTrue(access(self.testfile))
352350
@@ -360,44 +358,42 @@
360 def test_can_write_read_write_user(self):358 def test_can_write_read_write_user(self):
361 """Test when the user sid has rw rights."""359 """Test when the user sid has rw rights."""
362 # set the file to be read and write just by the user360 # set the file to be read and write just by the user
363 groups = {}361 groups = [(USER_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
364 groups[GetUserName()] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
365 _set_file_attributes(self.testfile, groups)362 _set_file_attributes(self.testfile, groups)
366 self.assertTrue(can_write(self.testfile))363 self.assertTrue(can_write(self.testfile))
367364
368 def test_can_write_read_write_everyone(self):365 def test_can_write_read_write_everyone(self):
369 """Test when the everyone sid has rw rights."""366 """Test when the everyone sid has rw rights."""
370 groups = {}367 groups = [(EVERYONE_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE)]
371 groups[EVERYONE_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
372 _set_file_attributes(self.testfile, groups)368 _set_file_attributes(self.testfile, groups)
373 self.assertTrue(can_write(self.testfile))369 self.assertTrue(can_write(self.testfile))
374370
375 def test_can_write_write_user_everyone_read(self):371 def test_can_write_write_user_everyone_read(self):
376 """Test when the user sid has w rights."""372 """Test when the user sid has w rights."""
377 groups = {}373 groups = [
378 groups[GetUserName()] = FILE_GENERIC_WRITE374 (USER_SID, FILE_GENERIC_WRITE),
379 groups[EVERYONE_GROUP] = FILE_GENERIC_READ375 (EVERYONE_SID, FILE_GENERIC_READ),
376 ]
380 _set_file_attributes(self.testfile, groups)377 _set_file_attributes(self.testfile, groups)
381 self.assertTrue(can_write(self.testfile))378 self.assertTrue(can_write(self.testfile))
382379
383 def test_can_write_write_everyone_user_read(self):380 def test_can_write_write_everyone_user_read(self):
384 """Test when the everyone sid has w rights"""381 """Test when the everyone sid has w rights"""
385 groups = {}382 groups = [
386 groups[GetUserName()] = FILE_GENERIC_READ383 (USER_SID, FILE_GENERIC_READ),
387 groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE384 (EVERYONE_SID, FILE_GENERIC_WRITE),
385 ]
388 _set_file_attributes(self.testfile, groups)386 _set_file_attributes(self.testfile, groups)
389 self.assertTrue(can_write(self.testfile))387 self.assertTrue(can_write(self.testfile))
390388
391 def test_can_write_full_user(self):389 def test_can_write_full_user(self):
392 """Test when the sid has full control."""390 """Test when the sid has full control."""
393 groups = {}391 groups = [(USER_SID, FILE_ALL_ACCESS)]
394 groups[GetUserName()] = FILE_ALL_ACCESS
395 _set_file_attributes(self.testfile, groups)392 _set_file_attributes(self.testfile, groups)
396 self.assertTrue(can_write(self.testfile))393 self.assertTrue(can_write(self.testfile))
397394
398 def test_can_write_full_everyone(self):395 def test_can_write_full_everyone(self):
399 """test when everyone has full control."""396 """test when everyone has full control."""
400 groups = {}397 groups = [(EVERYONE_SID, FILE_ALL_ACCESS)]
401 groups[EVERYONE_GROUP] = FILE_ALL_ACCESS
402 _set_file_attributes(self.testfile, groups)398 _set_file_attributes(self.testfile, groups)
403 self.assertTrue(can_write(self.testfile))
404\ No newline at end of file399\ No newline at end of file
400 self.assertTrue(can_write(self.testfile))
405401
=== modified file 'ubuntuone/platform/windows/os_helper.py'
--- ubuntuone/platform/windows/os_helper.py 2011-07-21 13:40:32 +0000
+++ ubuntuone/platform/windows/os_helper.py 2011-07-26 20:09:31 +0000
@@ -37,13 +37,15 @@
37from win32security import (37from win32security import (
38 ACL,38 ACL,
39 ACL_REVISION,39 ACL_REVISION,
40 CONTAINER_INHERIT_ACE,
41 CreateWellKnownSid,
40 DACL_SECURITY_INFORMATION,42 DACL_SECURITY_INFORMATION,
41 CONTAINER_INHERIT_ACE,
42 OBJECT_INHERIT_ACE,
43 GetFileSecurity,43 GetFileSecurity,
44 LookupAccountName,44 LookupAccountName,
45 LookupAccountSid,45 OBJECT_INHERIT_ACE,
46 SetFileSecurity,46 SetFileSecurity,
47 WinBuiltinAdministratorsSid,
48 WinWorldSid,
47)49)
48from ntsecuritycon import (50from ntsecuritycon import (
49 FILE_GENERIC_READ,51 FILE_GENERIC_READ,
@@ -62,8 +64,9 @@
62platform = 'win32'64platform = 'win32'
6365
64LONG_PATH_PREFIX = '\\\\?\\'66LONG_PATH_PREFIX = '\\\\?\\'
65EVERYONE_GROUP = u'Everyone'67USER_SID = LookupAccountName("", GetUserName())[0]
66ADMINISTRATORS_GROUP = u'Administrators'68EVERYONE_SID = CreateWellKnownSid(WinWorldSid)
69ADMINISTRATORS_SID = CreateWellKnownSid(WinBuiltinAdministratorsSid)
67# a map that contains the illegal chars and their 'utf8' representation on70# a map that contains the illegal chars and their 'utf8' representation on
68# windows so that we can show something to the user71# windows so that we can show something to the user
69WINDOWS_ILLEGAL_CHARS_MAP = {72WINDOWS_ILLEGAL_CHARS_MAP = {
@@ -201,12 +204,11 @@
201 raise204 raise
202205
203 dacl = ACL()206 dacl = ACL()
204 for group_name in groups:207 for group_sid, attributes in groups:
205 # set the attributes of the group only if not null208 # set the attributes of the group only if not null
206 if groups[group_name]:209 if attributes:
207 group_sid = _get_group_sid(group_name)
208 dacl.AddAccessAllowedAceEx(ACL_REVISION,210 dacl.AddAccessAllowedAceEx(ACL_REVISION,
209 CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, groups[group_name],211 CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, attributes,
210 group_sid)212 group_sid)
211 # the dacl has all the info of the diff groups passed in the parameters213 # the dacl has all the info of the diff groups passed in the parameters
212 security_descriptor.SetSecurityDescriptorDacl(1, dacl, 0)214 security_descriptor.SetSecurityDescriptorDacl(1, dacl, 0)
@@ -264,7 +266,7 @@
264def _set_no_rights_helper(path):266def _set_no_rights_helper(path):
265 """Set the rights to be none."""267 """Set the rights to be none."""
266 os.chmod(path, 0o000)268 os.chmod(path, 0o000)
267 groups = {}269 groups = []
268 _set_file_attributes(path, groups)270 _set_file_attributes(path, groups)
269271
270272
@@ -284,9 +286,10 @@
284 """Change path permissions to readonly in a file."""286 """Change path permissions to readonly in a file."""
285 # we use the win32 api because chmod just sets the readonly flag and287 # we use the win32 api because chmod just sets the readonly flag and
286 # we want to have more control over the permissions288 # we want to have more control over the permissions
287 groups = {}289 groups = [
288 groups[ADMINISTRATORS_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE290 (ADMINISTRATORS_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE),
289 groups[GetUserName()] = FILE_GENERIC_READ291 (USER_SID, FILE_GENERIC_READ),
292 ]
290 # the above equals more or less to 0444293 # the above equals more or less to 0444
291 _set_file_attributes(path, groups)294 _set_file_attributes(path, groups)
292295
@@ -294,10 +297,11 @@
294@longpath(paths_indexes=[0])297@longpath(paths_indexes=[0])
295def set_file_readwrite(path):298def set_file_readwrite(path):
296 """Change path permissions to readwrite in a file."""299 """Change path permissions to readwrite in a file."""
297 groups = {}300 groups = [
298 groups[EVERYONE_GROUP] = FILE_GENERIC_READ301 (EVERYONE_SID, FILE_GENERIC_READ),
299 groups[ADMINISTRATORS_GROUP] = FILE_ALL_ACCESS302 (ADMINISTRATORS_SID, FILE_ALL_ACCESS),
300 groups[GetUserName()] = FILE_ALL_ACCESS303 (USER_SID, FILE_ALL_ACCESS),
304 ]
301 # the above equals more or less to 0774305 # the above equals more or less to 0774
302 _set_file_attributes(path, groups)306 _set_file_attributes(path, groups)
303 try:307 try:
@@ -311,9 +315,10 @@
311@longpath(paths_indexes=[0])315@longpath(paths_indexes=[0])
312def set_dir_readonly(path):316def set_dir_readonly(path):
313 """Change path permissions to readonly in a dir."""317 """Change path permissions to readonly in a dir."""
314 groups = {}318 groups = [
315 groups[ADMINISTRATORS_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE319 (ADMINISTRATORS_SID, FILE_GENERIC_READ | FILE_GENERIC_WRITE),
316 groups[GetUserName()] = FILE_GENERIC_READ320 (USER_SID, FILE_GENERIC_READ),
321 ]
317 # the above equals more or less to 0444322 # the above equals more or less to 0444
318 _set_file_attributes(path, groups)323 _set_file_attributes(path, groups)
319324
@@ -321,10 +326,11 @@
321@longpath(paths_indexes=[0])326@longpath(paths_indexes=[0])
322def set_dir_readwrite(path):327def set_dir_readwrite(path):
323 """Change path permissions to readwrite in a dir."""328 """Change path permissions to readwrite in a dir."""
324 groups = {}329 groups = [
325 groups[EVERYONE_GROUP] = FILE_GENERIC_READ330 (EVERYONE_SID, FILE_GENERIC_READ),
326 groups[ADMINISTRATORS_GROUP] = FILE_ALL_ACCESS331 (ADMINISTRATORS_SID, FILE_ALL_ACCESS),
327 groups[GetUserName()] = FILE_ALL_ACCESS332 (USER_SID, FILE_ALL_ACCESS),
333 ]
328 # the above equals more or less to 0774334 # the above equals more or less to 0774
329 _set_file_attributes(path, groups)335 _set_file_attributes(path, groups)
330 # remove the read only flag336 # remove the read only flag
@@ -485,7 +491,7 @@
485 path += '.lnk'491 path += '.lnk'
486 if os.path.exists(path):492 if os.path.exists(path):
487 os.unlink(path)493 os.unlink(path)
488 494
489495
490@longpath(paths_indexes=[0])496@longpath(paths_indexes=[0])
491def listdir(directory):497def listdir(directory):
@@ -517,16 +523,7 @@
517 ace = dacl.GetAce(index)523 ace = dacl.GetAce(index)
518 if _has_read_mask(ace[1]):524 if _has_read_mask(ace[1]):
519 sids.append(ace[2])525 sids.append(ace[2])
520 accounts = []526 return (USER_SID in sids or EVERYONE_SID in sids) and\
521 for sid in sids:
522 try:
523 accounts.append(LookupAccountSid('', sid)[0])
524 except PyWinError:
525 # means that the sid is not linked with a 'visible' account
526 # in our case we can ignore this since we are looking for visible
527 # users
528 continue
529 return (GetUserName() in accounts or EVERYONE_GROUP in accounts) and\
530 os.access(path, os.R_OK)527 os.access(path, os.R_OK)
531528
532529
@@ -551,16 +548,7 @@
551 ace = dacl.GetAce(index)548 ace = dacl.GetAce(index)
552 if _has_read_mask(ace[1]):549 if _has_read_mask(ace[1]):
553 sids.append(ace[2])550 sids.append(ace[2])
554 accounts = []551 return (USER_SID in sids or EVERYONE_SID in sids) and\
555 for sid in sids:
556 try:
557 accounts.append(LookupAccountSid('', sid)[0])
558 except PyWinError:
559 # means that the sid is not linked with a 'visible' account
560 # in our case we can ignore this since we are looking for visible
561 # users
562 continue
563 return (GetUserName() in accounts or EVERYONE_GROUP in accounts) and\
564 os.access(path, os.R_OK)552 os.access(path, os.R_OK)
565553
566@longpath(paths_indexes=[0])554@longpath(paths_indexes=[0])
@@ -622,4 +610,4 @@
622 # as a trick for long paths, lets check it is is there610 # as a trick for long paths, lets check it is is there
623 if path.startswith(LONG_PATH_PREFIX):611 if path.startswith(LONG_PATH_PREFIX):
624 path = path.replace(LONG_PATH_PREFIX, '')612 path = path.replace(LONG_PATH_PREFIX, '')
625 return LONG_PATH_PREFIX + os.path.normpath(path)
626\ No newline at end of file613\ No newline at end of file
614 return LONG_PATH_PREFIX + os.path.normpath(path)

Subscribers

People subscribed via source and target branches