Keepassxc-browser: Credential fill button appears at invalid location, never disappears

[oliversalzburg]

Overview

Steps to Reproduce

1. Open a website where login information can be filled by KeePassXC.

Expected Behavior

The "Fill credentials with KeePassXC" button appears in login fields and allows me to fill credentials with KeePassXC.

Actual Behavior

The "Fill credentials with KeePassXC" button appears in random locations and does nothing when clicked.

Context

The button also appears in _valid_ locations, but it also appears in invalid locations in addition to that.

I'm seeing this in the Chrome-based Edge.

KeePassXC - Version 2.6.0
Revision: 0765954

Qt 5.15.0
Debugging mode is disabled.

Operating system: Windows 10 Version 2004
CPU architecture: x86_64
Kernel: winnt 10.0.19041

Enabled extensions:

• Auto-Type
• Browser Integration
• SSH Agent
• KeeShare (signed and unsigned sharing)
• YubiKey

Cryptographic libraries:
libgcrypt 1.8.5

Operating System: Windows

[varjolintu]

This happens with every site? Or just the one with the screenshot?

[oliversalzburg]

@varjolintu I've noticed it on several sites. So far, I've not been able to detect the pattern that causes it. I also can't reliably reproduce it yet, but it keeps happening.

[varjolintu]

What browser are you using? The issue template doesn't reveal that.

[oliversalzburg]

That's the new Chrome-based Edge.

[varjolintu]

That explains some because it's still one version number lower than the latest. Microsoft validation takes days.

[oliversalzburg]

By now I've upgraded and this is still happening on the same page. I haven't noticed it anywhere else though. Might be something weird with pgAdmin.

KeePassXC - Version 2.6.1
Revision: 9a35bba

Qt 5.15.0
Debugging mode is disabled.

Operating system: Windows 10 Version 2004
CPU architecture: x86_64
Kernel: winnt 10.0.19041

Enabled extensions:

• Auto-Type
• Browser Integration
• SSH Agent
• KeeShare (signed and unsigned sharing)
• YubiKey

Cryptographic libraries:
libgcrypt 1.8.5

[varjolintu]

Please test this with https://github.com/keepassxreboot/keepassxc-browser/releases/tag/1.7.0-beta1
Instructions: https://github.com/keepassxreboot/keepassxc-browser/wiki/Loading-the-extension-manually

[oliversalzburg]

Seems to be stuck in a "Key exchange was not successful." state.

[varjolintu]

You'll also have to modify the JSON script to include the temporary extension's ID. Did you do that?

[oliversalzburg]

Yes, I did. I copied the ID prefix from the extensions page with it. That's why it wasn't working.

It's working now, and the behavior regarding this issue is unchanged.

[varjolintu]

Ok. Thank you for testing. I would need to setup an identical page for testing it myself.

[oliversalzburg]

This is pgAadmin and I'm running it through Docker. You may use this docker-compose.yml to start a similar environment.

version: "3.7"
services:
    postgres:
        image: postgis/postgis:10-3.0
        ports:
            - "5432:5432"
        environment:
            POSTGRES_PASSWORD: postgres
            PGDATA: /var/lib/postgresql/data/pgdata
    postgres-gui:
        image: dpage/pgadmin4
        ports:
            - "1234:80"
        depends_on:
            - postgres
        environment:
            PGADMIN_DEFAULT_EMAIL: postgres@localhost
            PGADMIN_DEFAULT_PASSWORD: postgres

The pgAdmin interface will be running at localhost:1234 and use postgres@localhost as login with password postgres. The hostname, username and password of the server will be postgres.

When setting up the integration, password entry will be offered at this point:

After confirming the dialog, that overlay icon will stay in its place:

[oliversalzburg]

Just FYI, the next day, I can't get the browser plugin to work anymore. It's going through these errors:

• Cannot connect to KeePassXC. Check that browser integration is enabled in KeePassXC settings.
• Key exchange was not successful.

Going to revert back to stable for now. It also has the same issues, but usually recovers after a couple of reloads.

[varjolintu]

Switching back and forth with a temporary extension can cause that sometimes. Making reloads doesn't sound normal with the stable version.

[oliversalzburg]

I didn't switch back to the old version though. I had the stable browser extension still disabled, the beta enabled. Couldn't get it to work quickly enough and reverted back to stable.

I will open a new ticket about the reload behavior. So far I haven't been able to fully understand the pattern, but it happens regularly. I wake my machine from sleep, unlock KPXC, open my browser and the connection to the extension isn't there (red X on the icon). I reload it, it fails, I reload it again, connection established. It always goes something like that.

[varjolintu]

Ah. The sleep causes this. For some reason it cannot recover from that situation without a reload.

[oliversalzburg]

Ok, cool. Glad I didn't discover any new issue 😄 Sleep is causing other issues too, I'll manage 😊

[oliversalzburg]

So the textbox for the password input remains in the DOM. It seems like the modal is only shown/hidden through CSS.

Most relevant here appears to be the .ajs-hidden class.

[varjolintu]

@oliversalzburg I know where the bug is but still haven't figured out how to fix it properly. Have you already tried the new 1.7.0 update? It included one more change for CSS style changes.

[oliversalzburg]

Okay, then I won't post more information :)

I had reverted back to the stable release for now. I'd be happy to verify the 1.7 release, but the previews were causing me too much trouble.

[varjolintu]

The 1.7.0 is already officially released. It should update to your browser very soon.

[oliversalzburg]

Excellent. I'll keep watching for it. Edge always takes a while to my understanding :)

[oliversalzburg]

I'm still seeing this behavior in 1.7.0 on Chrome. I'd assume it would be the same on Edge.

[varjolintu]

If you want to debug it quickly, put a breakpoint here before you close the password dialog and see how many mutations there are, and where it goes, if possible:
https://github.com/keepassxreboot/keepassxc-browser/blob/develop/keepassxc-browser/content/keepassxc-browser.js#L1601

[oliversalzburg]

It identifies 8 mutations at this time. Some of which relate to the dialog wrapper that is being hidden. You can see the dialog and the contained password field in this DOM snapshot:

It will enter kpxcObserverHelper.handleObserverAdd() where it will bail out because inputs.length === 0 after kpxcObserverHelper.getInputs(target);.
The input.length === 0 is a result of the if (!ignoreVisibility && !kpxcFields.isVisible(field)) check.

Not sure if that helps.

[varjolintu]

So the line 1621 is where it triggers? This problem might be related to this issue: https://github.com/keepassxreboot/keepassxc-browser/issues/850

And thanks for the help!

[oliversalzburg]

I put the breakpoint at line 1601 per your instructions and followed it to 1621 and further.

The check in kpxcFields.isVisible also seems fine to me. When the dialog is closed, the password form field is passed into it and correctly identified as being invisible (elemStyle.visibility === 'hidden').

I would have to follow an entire _correct_ code path to understand what's wrong, I guess :/

[varjolintu]

Maybe I'll install docker etc. when I have the time and try to test this myself.

[oliversalzburg]

I just realized you can repro this at https://www.pgadmin.org/try/ :)

[florealcab]

It seems appear when the navigation is done with javascript, so there is no full page reload. It's the case on pgadmin, and I have the same problem with another website which change page through JS and without full page reload (it's a private website at work, so I can't share it)

I have the same issue on Firefox 80 on Ubuntu 20.04 OS.

[oliversalzburg]

@florealcab To my understanding, the extension is designed for this scenario, but there are just a lot of different variations to show/hide elements. It requires a very sophisticated approach.

[oliversalzburg]

I looked into this again, and it appears like it just runs into the section commented // There's an issue here. We cannot know for sure if the class attribute if added or removed.. Which seems to pretty clearly identify the issue, as this is invoked when the element is removed.

[varjolintu]

@oliversalzburg That's exactly the reason. The only way it could be identified if the class was added or removed is to check it with getComputedStyle() to get the calculated CSS values. The problem is that it's a very slow function and can possibly affect the page loading times. So far I haven't been able to do that otherwise.