Fix a bug in iterating over the SIDs by adding a feature

[GinoMan]

I'm submitting a ...

• bug report
• feature request
• support request

Do you want to request a feature or report a bug?

It's hard to say. It's a bug in the sense that presumably it would happen everytime someone does it on Windows, but it's kinda a feature request since in theory it's up to the client to catch unhandled exceptions and this would obviate the "bug". It's probably more helpful to explain below:

What is the current behavior?

Currently, if you get a list of sid's from the registry, and then iterate over that list getting the username from each entry, an exception will be raised which is quite obtuse: FileNotFound. But what file? What does accessing the registry have to do with files? Is it actually exceptional that this happens?

It took some debugging, but I discovered that a typical listing of the SIDs includes ".DEFAULT". This same key is not represented in the other location from where usernames are derived. So upon the very first entry, the key is not found. Since the key is not found, a "FileNotFound" exception is raised. But every system has this ".DEFAULT" key.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem

In order to observe this behavior, one need only run the following code:

import os
from lib_registry import *

username = os.getenv('username')

for sid in get_ls_user_sids():
    if get_username_from_sid(sid) == username:
        print(sid)

What is the expected behavior?

So what's the solution? There are three options I can think of:

1: Catch the FileNotFoundError and throw a more appropriate exception, then add a note in documentation comments that this key exists and should be "continued" if encountered. While this is a solution, and at least the issue would waste less developer time since they would understand immediately what's going on, I have some better solutions.

2: Add a flag to the get_ls_user_sids function to ignore the .DEFAULT SID. This would be a good feature especially if the flag had a default "True" (ignore_default=True). Of course however this may break existing code depending on the current behavior of getting all the ids.

3: Check if the key exists in the get_username_from_sid(sid) function. The function would then return "", "Default", or None if the key doesn't exist. The user would then have to determine if the key was None or empty or only contained "Default". As I understand it, that's not very pythonic, but I have another solution that I think would fill all the requirements:

4: (Recommended) Add a function: get_sid_from_username(username). The whole reason I ran into this problem, was that I was trying to determine the SID of the logged in user. I figured it would be easy to just loop through the list of SIDs and call get_username_from_sid(sid) on each one until I found one that matched the username of the logged in user. By adding this function, there's less need to loop through the list. If this option were pursued, I would still recommend option 1 for the existing get_username_from_sid(sid) function.
To increase performance, when either function is called, the library could map the entire thing using a dictionary in a separate thread, then if the dictionary is populated, use the dictionary to lookup the result rather than the registry. If the dictionary is not populated or is out of date, say because a user has been added (one would only need to enumerate the SIDs again to check) the slower registry query would be made while the internal dictionary is repopulated.

What is the motivation / use case for changing the behavior?

The most obvious use case is trying to determine the SID of the currently running user. Mapping the SID to a username is possible, but not the reverse. Also something so easily implementable with the current set of methods still fails unless the not-really-exceptional case of .DEFAULT is accounted for. Since every system contains the .DEFAULT - or at least every windows system, I have not tested this with wine - key, then it would make sense for the failure to obtain the username for that key to either default to "Default", or a blank field, or for a more comprehensible exception to be thrown. Also it would make sense to have a reverse mapping function so that the user doesn't even run into this bug in the first place since they're not trying to roll their own.

Please tell us about your environment:

• Release Number of the Repository used : Whichever release is downloaded from pip install as of April 8th 2020
• Version : 1.0.2
• Python Version : 3.8.1 CPython
• OS, OS Version : Windows 10 Pro 64-bit Build 18363

Other information ## (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, gitter, etc)

Below is an example of the stack trace of the error when this happens:

Traceback (most recent call last):
  File "C:\Users\username\Projects\ProjectName\.venv\lib\site-packages\lib_registry\lib_registry.py", line 100, in _get_username_from_sid_windows
    key = OpenKey(reg, path)
FileNotFoundError: [WinError 2] The system cannot find the file specified

First Draft of Suggested Solution

# Line 66
def get_sid_from_username(username):
    # type: (str) -> str
    """
    >>> user = os.getenv('username')
    Username
    >>> print(f"{get_sid_from_username(user)}")
    1-2-3-45-1234567890-1234567890-12345678-1001
    """
    for sid in get_ls_user_sids():
        if sid == get_username_from_sid(username):
            return sid
        

# Line 78
def get_username_from_sid(sid):
    # type: (str) -> str
    """
    >>> ls_user_sids = get_ls_user_sids()
    >>> assert len(ls_user_sids) > 1
    >>> username = get_username_from_sid(ls_user_sids[1])
    >>> assert len(username) > 1        # 'systemprofile' on windows, '<username>' on wine
    >>> print(f"{get_username_from_sid('.DEFAULT')}")
    Default
    >>> get_username_from_sid('EPIC FAIL')
    Traceback (most recent call last):
    ...
    ValueError: The SID is not valid on this machine
    """
    
    if sid.upper() == r'.DEFAULT':
        return 'Default'
    
    if get_is_platform_windows_wine():
        username = _get_username_from_sid_wine(sid)
    else:
        username = _get_username_from_sid_windows(sid)
    return username

# Line 95
def _get_username_from_sid_windows(sid):
    reg = get_registry_connection('HKEY_LOCAL_MACHINE')
    path = r'SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\ProfileList\\{}'.format(sid)
    try:
    	key = OpenKey(reg, path)
    except:
        raise ValueError("The SID is not valid on this machine")
    val, value_type = QueryValueEx(key, 'ProfileImagePath')
    username = val.rsplit('\\', 1)[1]
    return username

[bitranox]

Gino,

what a perfect error report , You truly deserve 3 Michelin Stars !

It is a FileNotFoundError, because the hive is a file - it comes from winreg, not my idea ;-)

I will look into it - its a relatively old helper library I wrote, but everything sounds very reasonable.
It will take some time since I have lots of other work in this crisis atm.

yours sincerely

Robert
Vienna

[bitranox]

Dear Gino,
lib_registry will be overhauled completely to offer additional pathlib-like interface -
so the interface will change over the next weeks.

I just released an intermediate, working release, but the API will change again a bit - but it is usable.

If You do more windows registry stuff, You might like fake-winreg, check it out !

https://github.com/bitranox/fake_winreg

It offers You fully type annotated interface for winreg, and a "fake registry" for tests -

I would ask You if You can have a look at it.

yours sincerely

Robert