At present, os.access under Windows simply calls GetFileAttributes to
determine the readonly attribute (ignoring directories). The patch
attached combines this with the use of the AccessCheck API to compare
the user's permissions with those required for the path.

I'm assuming that ATTRIB and CACLS will be available for use in the unit
tests included.

I haven't altered the structure of the posix_access function at all
although I suspect that it could now be simplified now that we're not
supporting Win9x.

A quick look tells me that the patch seems clean. However it involves changes to posixmodule.c and I don't (yet) want to get involved with doing builds, so could someone else please give this a try.

But what are the benefits of this change?

Although I'm the implementer of the patch (the concept
was discussed way back on c.l.py after a naive poster's
original request) I'm probably +0 myself. It's an attempt
to replace os.access' next-to-useless behaviour on Windows
with something which at least uses current security mechanisms
to determine its answer.

However, it's just one function among the many Posixesque
functions which don't quite map to Windows. And one which
you might well be advised to avoid in any case since there's
an race condition inherent in checking for a file's accessibility
and then making use of that fact. Better to try and fall back.

Tim, do you want more time to think about this, could we close as "won't fix" or what?

Do you want to provide an updated patch for 3.4?

Here's an updated patch against trunk with tests & doc changes

... and to answer Amaury's question in msg109871 it creates a reasonable consistency between the results of os.access and the user's actual ability to read / write a file. eg, you might have no permissions whatsoever on the file but as long as it wasn't read-only, os.access would return True for reading, writing and executing.

The solution proposed here could help resolve bpo-22107.

In msg243815 you asked me to look over this patch. I hope this helps.

For GetFileSecurity you need to also request LABEL_SECURITY_INFORMATION. To test this in Vista+, use a file in the root directory of the system drive. This will inherit a high integrity level w/ no write up, which denies write access to a medium integrity (non-elevated) process. If you don't include the label information, the check will erroneously claim a non-elevated token has write access.

You can set a file's integrity level to high using icacls filename /setintegritylevel High. This prevents write up. You can prevent read up and execute up by using the Windows API directly, or with other administration tools such as chml.

As a performance tweak you could get the file security with an initial buffer of 512 bytes. That should be big enough for most file security descriptors. I'd use PyMem_RawRealloc in a do loop (i.e. starting with pSD == NULL and cbSD == 512). It should set PyExc_MemoryError on failure.

Regarding ImpersonateSelf, what if the thread is already impersonating? That's the token the kernel will actually use when checking access. Given that, I'd try OpenThreadToken, and only if it fails call ImpersonateSelf. Then RevertToSelf immediately after the 2nd OpenThreadToken call. Reverting doesn't have to be delayed until after the check. An alternative to ImpersonateSelf is to manually duplicate the process token as an impersonation token (i.e. OpenProcessToken; DuplicateToken). The benefit of this approach is that the duplicated token can be cached in a static variable.

For the access check itself, use the FILE_GENERIC_* / FILE_ALL_ACCESS constants, which are the standard and specific rights for the corresponding GENERIC_* rights. These include the standard READ_CONTROL and SYNCHRONIZE rights that are required for accessing files.

Also, there's no need to call MapGenericMask here, since you know that access_desired doesn't contain any generic rights.

Finally, for backward compatibility this new implementation should default to using only the old file-attribute check. Chaining to the ACL-based check should require a keyword-only argument such as "use_acl".

Thanks for the very thorough review. This isn't going to make it into
3.5, but I'll rework it in the light of your comments and see if people
are happy with it in the optional argument variation.

With increasing use of os.access() in shutil and tempfile, it would be nice to have a real implementation of os.access() for Windows.

Instead of manually evaluating the security of the file/directory, as bpo-2528.2.patch attempts to do, I'd rather just open the file with the desired access (e.g. GENERIC_READ, GENERIC_WRITE, GENERIC_EXECUTE). An open-based check supports checking for sharing violations, filesystem policy (e.g. FILE_READ_ATTRIBUTES granted by the parent directory), non-filesystem devices, and access policy implemented by filter drivers in the device stack.

The code to open the file/directory can be factored out and generalized from the stat() implementation. The common open function can implement the flags AT_SYMLINK_NOFOLLOW and AT_EACCESS (without which it should temporarily revert to the process access token). Also, when a directory is opened with GENERIC_WRITE access, it can re-try the open with FILE_DELETE_CHILD access, which POSIX includes in write access for a directory.

An S_OK flag could also be supported to ignore a sharing violation in Windows. [MS-FSA] section 2.1.5.1.2 (Open of an Existing File) specifies that access sharing is checked after the readonly attribute and file security access check. So if an open fails with a sharing violation, the caller knows that access was otherwise granted.