Merge lp:~nataliabidart/ubuntuone-client/add-tests-access-can-write into lp:ubuntuone-client

[Natalia Bidart]

- Added test case for access and can_write. All credits to Manuel de la Peña
  (LP: #812980).

- Added test case for access and can_write. All credits to Manuel de la Peña
  (LP: #812980).

[Manuel de la Peña (mandel)]

review: Approve

[Alejandro J. Cura (alecu)]

The branch looks good.

review: Approve

=== modified file 'tests/platform/windows/test_os_helper.py'
--- tests/platform/windows/test_os_helper.py	2011-06-29 16:41:34 +0000
+++ tests/platform/windows/test_os_helper.py	2011-07-19 16:59:03 +0000
@@ -19,9 +19,17 @@
 import os
 import errno
 
+from win32api import GetUserName
+from ntsecuritycon import (
+    FILE_ALL_ACCESS,
+    FILE_GENERIC_READ,
+    FILE_GENERIC_WRITE,
+)
+
 from ubuntuone.platform.windows.os_helper import (
-    remove_windows_illegal_chars,
+    _set_file_attributes,
     access,
+    can_write,
     is_illegal_path,
     listdir,
     make_dir,
@@ -32,18 +40,20 @@
     remove_file,
     rename,
     remove_utf8_chars,
+    remove_windows_illegal_chars,
     set_no_rights,
     set_file_readwrite,
     stat_path,
+    EVERYONE_GROUP,
     LONG_PATH_PREFIX,
-    WINDOWS_ILLEGAL_CHARS_MAP
+    WINDOWS_ILLEGAL_CHARS_MAP,
 )
 from contrib.testing.testcase import BaseTwistedTestCase
 
 
 class TestIllegalPaths(BaseTwistedTestCase):
     """Test all the operations using illegal paths."""
-    
+
     def setUp(self):
         """Setup for the tests."""
         super(TestIllegalPaths, self).setUp()
@@ -241,4 +251,153 @@
         for current_path in paths:
             long_path = LONG_PATH_PREFIX + current_path
             self.assertEqual(LONG_PATH_PREFIX + os.path.normpath(current_path),
-                             normpath(long_path))
\ No newline at end of file
+                             normpath(long_path))
+
+
+class TestAccess(BaseTwistedTestCase):
+    """Test specific windows implementation access details."""
+
+    def setUp(self):
+        """Setup for the tests."""
+        super(TestAccess, self).setUp()
+
+        self.basedir = self.mktemp('test_root')
+        self.addCleanup(lambda: self.rmtree(self.basedir))
+
+        self.testfile = os.path.join(self.basedir, "test_file")
+        self.fd = open(self.testfile, 'w')
+        self.addCleanup(self.fd.close)
+
+    def test_access_no_rights(self):
+        """Test when the sid is not present."""
+        # remove all the rights from the test file so that
+        # we cannot read or write
+        set_no_rights(self.testfile)
+        self.assertFalse(access(self.testfile))
+
+    def test_access_read_write_user(self):
+        """Test when the user sid has rw rights."""
+        # set the file to be read and write just by the user
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_read_write_everyone(self):
+        """Test when the everyone sid has rw rights."""
+        groups = {}
+        groups[EVERYONE_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_write_user_everyone_read(self):
+        """Test when the user sid has w rights."""
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_WRITE
+        groups[EVERYONE_GROUP] = FILE_GENERIC_READ
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_write_everyone_user_read(self):
+        """Test when the everyone sid has w rights"""
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_READ
+        groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_write_user_everyone(self):
+        """Test when everyone and user have w rights."""
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_WRITE
+        groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE
+        _set_file_attributes(self.testfile, groups)
+        self.assertFalse(access(self.testfile))
+
+    def test_access_read_user(self):
+        """Test when the sid has r rights."""
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_READ
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_read_everyone(self):
+        """Test when everyone has r rights."""
+        groups = {}
+        groups[EVERYONE_GROUP] = FILE_GENERIC_READ
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_read_user_everyone(self):
+        """Test when user and everyone have r rights."""
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_READ
+        groups[EVERYONE_GROUP] = FILE_GENERIC_READ
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_full_user(self):
+        """Test when the sid has full control."""
+        groups = {}
+        groups[GetUserName()] = FILE_ALL_ACCESS
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_access_full_everyone(self):
+        """test when everyone has full control."""
+        groups = {}
+        groups[EVERYONE_GROUP] = FILE_ALL_ACCESS
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(access(self.testfile))
+
+    def test_canwrite_no_rights(self):
+        """Test when the sid is not present."""
+        # remove all the rights from the test file so that
+        # we cannot read or write
+        set_no_rights(self.testfile)
+        self.assertFalse(can_write(self.testfile))
+
+    def test_can_write_read_write_user(self):
+        """Test when the user sid has rw rights."""
+        # set the file to be read and write just by the user
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(can_write(self.testfile))
+
+    def test_can_write_read_write_everyone(self):
+        """Test when the everyone sid has rw rights."""
+        groups = {}
+        groups[EVERYONE_GROUP] = FILE_GENERIC_READ | FILE_GENERIC_WRITE
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(can_write(self.testfile))
+
+    def test_can_write_write_user_everyone_read(self):
+        """Test when the user sid has w rights."""
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_WRITE
+        groups[EVERYONE_GROUP] = FILE_GENERIC_READ
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(can_write(self.testfile))
+
+    def test_can_write_write_everyone_user_read(self):
+        """Test when the everyone sid has w rights"""
+        groups = {}
+        groups[GetUserName()] = FILE_GENERIC_READ
+        groups[EVERYONE_GROUP] = FILE_GENERIC_WRITE
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(can_write(self.testfile))
+
+    def test_can_write_full_user(self):
+        """Test when the sid has full control."""
+        groups = {}
+        groups[GetUserName()] = FILE_ALL_ACCESS
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(can_write(self.testfile))
+
+    def test_can_write_full_everyone(self):
+        """test when everyone has full control."""
+        groups = {}
+        groups[EVERYONE_GROUP] = FILE_ALL_ACCESS
+        _set_file_attributes(self.testfile, groups)
+        self.assertTrue(can_write(self.testfile))
\ No newline at end of file

=== modified file 'ubuntuone/platform/windows/os_helper.py'
--- ubuntuone/platform/windows/os_helper.py	2011-07-19 16:04:51 +0000
+++ ubuntuone/platform/windows/os_helper.py	2011-07-19 16:59:03 +0000
@@ -26,11 +26,9 @@
 from functools import wraps
 from contextlib import contextmanager
 from pywintypes import error as PyWinError
-from win32api import GetUserName, GetFileAttributes
+from win32api import GetUserName
 from win32file import MoveFileExW
 from win32com.client import Dispatch
-from pywintypes import error
-from win32con import FILE_ATTRIBUTE_READONLY
 from win32file import (
     MOVEFILE_COPY_ALLOWED,
     MOVEFILE_REPLACE_EXISTING,
@@ -50,9 +48,10 @@
 from ntsecuritycon import (
     FILE_GENERIC_READ,
     FILE_GENERIC_WRITE,
-    FILE_ALL_ACCESS
+    FILE_ALL_ACCESS,
 )
 
+
 # ugly trick to stop pylint for complaining about
 # WindowsError on Linux
 if sys.platform != 'win32':
@@ -63,8 +62,8 @@
 platform = 'win32'
 
 LONG_PATH_PREFIX = '\\\\?\\'
-EVERYONE_GROUP = 'Everyone'
-ADMINISTRATORS_GROUP = 'Administrators'
+EVERYONE_GROUP = u'Everyone'
+ADMINISTRATORS_GROUP = u'Administrators'
 # a map that contains the illegal chars and their 'utf8' representation on
 # windows so that we can show something to the user
 WINDOWS_ILLEGAL_CHARS_MAP = {
@@ -522,7 +521,7 @@
     for sid in sids:
         try:
             accounts.append(LookupAccountSid('', sid)[0])
-        except error:
+        except PyWinError:
             # means that the sid is not linked with a 'visible' account
             # in our case we can ignore this since we are looking for visible
             # users
@@ -533,11 +532,36 @@
 
 @longpath(paths_indexes=[0])
 def can_write(path):
-    """Return if the path can be written to."""
-    file_att = GetFileAttributes(path)
-    if file_att and FILE_ATTRIBUTE_READONLY:
+    """Return if the path is at least readable."""
+    # lets consider the access on an illegal path to be a special case
+    # since that will only occur in the case where the user created the path
+    # for a file to be readable it has to be readable either by the user or
+    # by the everyone group
+    if is_illegal_path(path):
+        path = remove_windows_illegal_chars(path)
+    if not os.path.exists(path):
         return False
-    return True
+    security_descriptor = GetFileSecurity(path, DACL_SECURITY_INFORMATION)
+    dacl = security_descriptor.GetSecurityDescriptorDacl()
+    sids = []
+    for index in range(0, dacl.GetAceCount()):
+        # add the sid of the ace if it can read to test that we remove
+        # the r bitmask and test if the bitmask is the same, if not, it means
+        # we could read and removed it.
+        ace = dacl.GetAce(index)
+        if _has_read_mask(ace[1]):
+            sids.append(ace[2])
+    accounts = []
+    for sid in sids:
+        try:
+            accounts.append(LookupAccountSid('', sid)[0])
+        except PyWinError:
+            # means that the sid is not linked with a 'visible' account
+            # in our case we can ignore this since we are looking for visible
+            # users
+            continue
+    return (GetUserName() in accounts or EVERYONE_GROUP in accounts) and\
+            os.access(path, os.R_OK)
 
 @longpath(paths_indexes=[0])
 def stat_path(path, look_for_link=True):