Aldaviva / AuthenticatorChooser

🗝️ Background program that skips the phone option and chooses the USB security key in Windows credential prompts.
Apache License 2.0
51 stars 1 forks source link

windowOpened fires too quickly #5

Closed Omzig closed 1 month ago

Omzig commented 2 months ago

Problem

nothing happens when the exe is running

Solution

put a pause in and wait for the window to open before selecting the usb key option

    private void onWindowOpened(object? sender, ShellEventArgs args) {
        //Debug.WriteLine($"Window Open Dectected: {args.shellEvent}");
        if (args.shellEvent == ShellEventArgs.ShellEvent.HSHELL_WINDOWCREATED) {
            Thread.Sleep(1000); // << there might be a better way
            windowOpened?.Invoke(this, new SystemWindow(args.windowHandle));
        }
    }
Aldaviva commented 2 months ago

Hi @Omzig, thank you for alerting me to this issue.

Yeah there is a little bit of delicate timing when interacting with this UI. I already have seen this type of issue when finding the CredentialsList element, which is lazily rendered roughly 20ms after the dialog box appears.

Since this race turns out differently on my computer and yours, can I send you a debug build that logs which step is the problem, and you can tell me which error message you see? Once we know which element isn't being found in time, I can add retry logic to fix it.

Omzig commented 2 months ago

ya, my corporate environment is quite strict, but i can tell you what line gets a null

Extensions.cs

 public static IEnumerable<AutomationElement> children(this AutomationElement parent) {
        if(parent == null)
        {
            return null; //this is where i traced it to, that led me to putting in the delay. Funny thing, when i am in debug, it works because debug slows it down, but then when i run it as normal, it gets kranky.
        }
        return parent.FindAll(TreeScope.Children, Condition.TrueCondition).Cast<AutomationElement>();
    }
Aldaviva commented 2 months ago

Interesting, that is a method that gets retried when it throws an exception (for example if parent is null), so I would expect 2-3 caught and handled exceptions from that line but for it to ultimately succeed. Maybe we're not waiting long enough there (currently only 200 ms), or maybe it's masking the real error later, outside of the children() call.

Can you try changing the maxAttempts: 25 argument to maxAttempts: 125 and see if that fixes it?

waltersenekal commented 1 month ago

Hi, I am getting the same issue, I am also not sure why this happens because it is in a try loop. I can also confirm that if i implement the code (checking for null in the Extensions.cs ) it fixes the issue for me. As the returned null is then caught by the try and on the next try it continues.

Aldaviva commented 1 month ago

Thanks for the additional info. I will try reproducing it on more computers, and also upload a development build that both increases the timeout and prints log messages, which may be useful for spotting the cause. At this point there is still something I am not understanding about the root cause here, because the given workaround doesn't seem like it should change the behavior, so there's more to it than I currently can see.

Aldaviva commented 1 month ago

This issue did not reproduce on two additional computers I tested with.

Here is a development build with a longer timeout for finding CredentialsList and log messages to help diagnose this problem. AuthenticatorChooser.zip

You can try this out and let me know if it fixes the problem, and what the stdout is. Here is what I'm expecting:

Caught NullReferenceException while finding CredentialsList
No CredentialsList found in dialog box yet, retrying
Caught NullReferenceException while finding CredentialsList
No CredentialsList found in dialog box yet, retrying
Caught NullReferenceException while finding CredentialsList
No CredentialsList found in dialog box yet, retrying
authenticatorChoices has 2 items
USB security key selected
Next button pressed
waltersenekal commented 1 month ago

Yes thanks, it is working nicely now

Caught NullReferenceException while finding CredentialsList
No CredentialsList found in dialog box yet, retrying
Caught NullReferenceException while finding CredentialsList
No CredentialsList found in dialog box yet, retrying
Caught NullReferenceException while finding CredentialsList
No CredentialsList found in dialog box yet, retrying
authenticatorChoices has 2 items
USB security key selected
Next button pressed
Aldaviva commented 1 month ago

Thank you very much, adding this fix to the upcoming 0.2.0 release.

Aldaviva commented 1 month ago

Released in 0.2.0.