search

keepassxreboot / keepassxc-browser

KeePassXC Browser Extension
GNU General Public License v3.0
1.78k stars 188 forks source link

Fix credential sorting #2262

Closed varjolintu closed 4 months ago

varjolintu commented 4 months ago

Regression from #2119. In some cases (reason unknown?) the credential's group is missing. An empty value should be used instead with some added .? checks.

Hotfix release is needed for this because multiple people are affected.

Fixes #2260.

pushcom commented 4 months ago

Hi! I am using Keepass 2.57 and have in case of "geschaeftskunden.dhl.de" three entries:

KeePassXC-Browser Error KeePassXC-Browser Error2

Two of the entries are in a group (in Keepass) named "DHL Geschäftskundenportal", one isn't i a specified group.

pushcom commented 4 months ago

Hi!

I've tested the fix #2262 (replace keepassxc-browser.js) and it seems to work again.

Thanks a lot for your work!

Regards Pushcom

droidmonkey commented 4 months ago

Are you using keepassnatmessage plugin? That would explain why the data format isn't compliant.

wadabum commented 4 months ago

In some cases (reason unknown?) the credential's group is missing

Keepass 2.57 (64-bit) with NatMsg v2.0.0.17.0 poking at it a bit with inspecting, sortLoginItemBy is what breaks as every single one of my records has "group: undefined" "broke like 10 minutes ago without any changes anywhere"

droidmonkey commented 4 months ago

Well YMMV when not using keepassxc...

pushcom commented 4 months ago

Yes, i'm using the keepassnatmessage plugin.

varjolintu commented 4 months ago

That explains why the group is undefined. With KeePassXC and KeePassXC-Browser that should never happen. Well, these checks are still necessary.

I'll move this to 1.9.2 instead.

wadabum commented 4 months ago

so... were not getting an emergency-fix release anymore because we were truthful in narrowing down the cause to keepassnatmessage? great...

varjolintu commented 4 months ago

so... were not getting an emergency-fix release anymore because we were truthful in narrowing down the cause to keepassnatmessage? great...

Yes. This is eventually a bug in KeePassNatMessage project and should be reported there. They don't pass the group information properly to the extension, so in my opinion this is not a hotfix, especially because it doesn't concern KeePassXC.

chotaire commented 4 months ago

So a hotfix (which already exists) is not released because it's someone elses fault. In the meantime there are plenty of users with major issues and no way to downgrade. May I ask why this isn't released quickly?

varjolintu commented 4 months ago

So a hotfix (which already exists) is not released because it's someone elses fault. In the meantime there are plenty of users with major issues and no way to downgrade. May I ask why this isn't released quickly?

Users having this issue can download the current develop branch and use it instead.

chotaire commented 4 months ago

Users having this issue can download the current develop branch and use it instead.

Using Google Chrome Version 126.0.6478.127 (64-bit Windows 11 Pro [Version 10.0.22631.3810]).

I've tried that (also with KeepassXC-Browser v1.9.0 downloaded from this repo). When trying to connect to my Keepass 2.5.7 database via KeepassNatMsg 2.0.17.0 using the "unpacked extension", nothing happens. A connection dialog never appears. I double-checked I can delete the key and still connect to the database when using the Chrome Webstore version.

Is there something else I need to do to make it work with KeepassXC-Browser other than enabling developer mode in chrome://extensions and installing from an unpacked extension?

varjolintu commented 4 months ago

@chotaire Sure. With Chrome you'll have to use the temporary extension ID in the script files: https://github.com/keepassxreboot/keepassxc-browser/wiki/Loading-the-extension-manually

droidmonkey commented 4 months ago

So a hotfix (which already exists) is not released because it's someone elses fault. In the meantime there are plenty of users with major issues and no way to downgrade. May I ask why this isn't released quickly?

Consider this, we are also addressing several other issues right now and can bundle the fixes. The Firefox extension store hasn't even approved 1.9.1 yet either. So relax please, we will get a patch out very soon.

chotaire commented 4 months ago

Users having this issue can download the current develop branch and use it instead.

Using Google Chrome Version 126.0.6478.127 (64-bit Windows 11 Pro [Version 10.0.22631.3810]).

I've tried that (also with KeepassXC-Browser v1.9.0 downloaded from this repo). When trying to connect to my Keepass 2.5.7 database via KeepassNatMsg 2.0.17.0 using the "unpacked extension", nothing happens. A connection dialog never appears. I double-checked I can delete the key and still connect to the database when using the Chrome Webstore version.

Is there something else I need to do to make it work with KeepassXC-Browser other than enabling developer mode in chrome://extensions and installing from an unpacked extension?

For other Windows users running into this using KeepassNatMsg, that's what I additionally did:

  1. I grabbed the extension ID from the unpacked extension at chrome://extensions
  2. I went to %LocalAppData%\KeePassNatMsg
  3. I added the extension ID, followed by a / to both _kpnmchrome.json and _kpnmchromium.json (don't forget the commas)
  4. Just to be safe I exited Chrome completely (in the traybar, right-click the Chrome icon and exit)

Then I restarted Chrome and I was able to connect to the database. I can confirm everything works again.

varjolintu commented 4 months ago

I think we'll release a hotfix just in case, because it seems other users might be affected too.