Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tb78 #33

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Tb78 #33

wants to merge 26 commits into from

Conversation

antixn
Copy link

@antixn antixn commented Apr 11, 2021

I'm a kee user and i was quite frustrated recently : during a few months i had thunderbird 68 and kee working again... and then TB78 came and broke stuff again...

@dlech I've seen your comment on the issue 30 and i've decided to give a hand. I've choosen to implement the upgrade to TB78 the quick way with the WindowListener API

My first target was to be able to debug it, it's not proper debug but the new target in the makefile launches the node app web-ext with the local plugin files and ease tries and tests.

The only functionnal element i was able to see working is the overload of the login popup providing the text "You are logged out of your database (it may be locked)". It guided me through the TB68, TB78 and helped me being sure that i wasn't breaking anything in the process.
After the upgrade, as a sort of bonus i've also been able to see that the options page was also displaying "correctly"

I'm pretty sure a lot of string resources and management code could be removed (TB40) but i wanted to avoid making big changes as the objective here was mainly to allow kee to work with thunderbird 78. I would be interested to invest more time for the mail extension real upgrade but this PR seems quite enough for a first contact :)

@wohauser
Copy link

Tried to install on TB 78, but it won't install --> "incompatible to TB78"
How to solve that ?

@antixn
Copy link
Author

antixn commented Jul 24, 2021

Tried to install on TB 78, but it won't install --> "incompatible to TB78"
How to solve that ?

it should, the manifest states "strict_min_version": "78.1.0"
make sure you're on the right branch : TB78
git checkout TB78
I've re-checked locally to build the xpi and it install without issue

@kevinkk525
Copy link

kevinkk525 commented Jul 28, 2021

I just tried this and compiling and installing works just fine, connecting to the DB too.
However, after that it seems to just sit idle and doesn't fill any password requests. Can't even get it to logging something to the error console even when I set it to debug. Any idea as to why? (I have been successfully using the old version with old Thunderbird but slowly trying to migrate to TB78)

@wohauser
Copy link

OK, successfully installed this PR, as I see all what I need is working well.

@wohauser
Copy link

wohauser commented Jul 30, 2021

Further tests: The smtp passwords for outgoing messages are not filled.
For pop/imap incomming it works.
But there is no debug log output in log file.

@antixn
Copy link
Author

antixn commented Aug 1, 2021

I just tried this and compiling and installing works just fine, connecting to the DB too.
However, after that it seems to just sit idle and doesn't fill any password requests. Can't even get it to logging something to the error console even when I set it to debug. Any idea as to why? (I have been successfully using the old version with old Thunderbird but slowly trying to migrate to TB78)

From what i saw during debugs, queries to Keepass DB could be very restrictive to find out the good entry.
Maybe you could see what's been injected into KeepPassRPC by adding extra consol.warn() in xul-ext/modules/jsonrpcClient.js in the function findLogins() that could help you find out which value does not match.

To enable builtin logging, go to about:config of thunderbird and then filter by keepass, set logLevel to 4 (debug), you should see keefox debug messages in console output

@antixn
Copy link
Author

antixn commented Aug 1, 2021

Further tests: The smtp passwords for outgoing messages are not filled.
For pop/imap incomming it works.
But there is no debug log output in log file.

I observed the same for outgoing smtp, no detection. It must be related with the checks, there's an iteration of string resources in xul-ext/chrome/content/KFcommonDialog.js, the function LoadDialogData is used to gather all string resources and help identify if a popup is related to a password query. The resources keys for smtp must be obsolete.

Builtin logs does work but you have to set these config settings via about:config

@antixn
Copy link
Author

antixn commented Aug 7, 2021

I just tried this and compiling and installing works just fine, connecting to the DB too.
However, after that it seems to just sit idle and doesn't fill any password requests. Can't even get it to logging something to the error console even when I set it to debug. Any idea as to why? (I have been successfully using the old version with old Thunderbird but slowly trying to migrate to TB78)

@kevinkk525

I've experienced issues with kee recently and my changes in settings had an impact on keebird. No matching entries were found in thunderbird password request popup and it seemed broken. The fix came from
https://forum.kee.pm/t/keepassrpc-home-group/2362
By setting the kee home group in KeePass, the entries where identified and suggested properly in Kee. It's the starting point of the search area in KeePass db. Hope it could help.

@antixn
Copy link
Author

antixn commented Aug 7, 2021

hello @wohauser, i've found a text resource to match the check on outgoing smtp prompt request.
Please check on the last commit, it should work for you also.
If not, please note the exact texts of the popup : title and request and i'll look for other resource keys for this popup.

@kevinkk525
Copy link

kevinkk525 commented Aug 7, 2021

I've experienced issues with kee recently and my changes in settings had an impact on keebird. No matching entries were found in thunderbird password request popup and it seemed broken. The fix came from
https://forum.kee.pm/t/keepassrpc-home-group/2362

Thanks a lot. I also found out that my way of having urls doesn't work out. I had multiple urls before separated by "," which works just fine with KeeFox and the old KeeBird.
In the meantime I switched to using the KeePassNatMSG plugin in KeePass and the KeePassXC-mail plugin in Thunderbird when I realized my URLs were wrong. That was easier for me than trying to debug what went wrong before..
Don't want to redirect people from your work with this PR but installing those plugins was extremely easy, so if anyone has trouble getting this plugin to work, that might be an option. (Those 2 plugins were mentioned before in an issue of keebird somewhere but sounded complicated to install, which it isn't. Just have to install both plugins and it'll work right away without further configuration).

@wohauser
Copy link

wohauser commented Aug 9, 2021

Installed the latest commit, and for now all password requests are working for me.
THX for your work.

@kevinkk525
Copy link

I tried this again on my ubuntu installation as KeePassXC just isn't working as well but same problem, even with the Home Group set to the root of my DB it just doesn't find any passwords. Or it doens't look, i don't know because it doesn't log anything and the password prompts don't show any options or if a password was even requested. All I have is the successful connection attempt between the plugin and keepass.

I already changed my urls for keepassxc to like gmx.net to match for imap and smtp but I also tried imap.gmx.net and imap://imap.gmx.net and it just doesn't work.

If nobody has a clue, I might have to try more in depth debugging or live with a rather disappointing keepassxc implementation on firefox.

@snodo
Copy link

snodo commented Aug 24, 2021

Had the same issue that no matching entries were found and made a tiny change which fixed it for me - maybe it will help you too:
https://github.com/snodo/keebird/tree/TB78

@kevinkk525
Copy link

Awesome, that actually worked! It now finds my entries (weird that it didn't before since they were quite accurate, or so I thought..).
Now the only problem I have is that I manually have to confirm the password prompt. I already tried changing that behaviour in the keebird config to fill the password and submit the prompt but that config options always revert to an empty selection.. Any fix for that?

fix regression created in a84bfa1
@antixn
Copy link
Author

antixn commented Aug 29, 2021

Awesome, that actually worked! It now finds my entries (weird that it didn't before since they were quite accurate, or so I thought..).
Now the only problem I have is that I manually have to confirm the password prompt. I already tried changing that behaviour in the keebird config to fill the password and submit the prompt but that config options always revert to an empty selection.. Any fix for that?

Do you mean you have multiple entries in the list for a given prompt ? And that none of them is preselected ?

@kevinkk525
Copy link

No they are preselected, just not automatically submitted in the dialogue.

@antixn
Copy link
Author

antixn commented Aug 29, 2021

i see the involved piece of code, i've never used this autoSubmit feature.
It's computed from
var autoSubmit = keefox_org._keeFoxExtension.prefs.getValue("autoSubmitDialogs",false);
And then it depends on the properties of the entries returned by KeePassRPC,

        if (matchedLogins[bestMatch].alwaysAutoSubmit)
            autoSubmit = true;
        if (matchedLogins[bestMatch].neverAutoSubmit)
            autoSubmit = false;
            
        if (autoFill)
        {
            // fill in the best matching login
            dialogFindLoginStorage.document.getElementById("loginTextbox").value = matchedLogins[bestMatch].username;
            dialogFindLoginStorage.document.getElementById("password1Textbox").value = matchedLogins[bestMatch].password;
        }
        if (autoSubmit || dialogFindLoginStorage.mustAutoSubmit)
        {            
            Dialog.onButton0();            
            close();
        }

Maybe the Dialog.onButton0(); instruction is silently failing...
I've just tested it locally by changing the AutoFill/AutoSubmit for an entry in my keepass db and i've seen it work.
As your password is filled, it should be a setting issue...

@Janpopan
Copy link

Janpopan commented Oct 2, 2021

Is there an pre or beta release for this fork?
What is todo to merge this and release an new version?

@BSG2000
Copy link

BSG2000 commented Nov 3, 2021

I would agree to the question of @Janpopan and hope. that there will be a release in the near future.

@ikenichiro
Copy link

@Janpopan @BSG2000
If you want an official release from this repo, we need @dlech to (review and) merge this branch, make an official release (or pre/beta release).
There is nothing we can do until dlech checks and approves this merge (or tells us what is the problem that this can not be merged), but until then, you can use @antixn merge request codes by the following procedure.
However, please be noted that since it is not an officially merged code, you need to trust antixn's branch AT YOUR OWN RISK. Since you can check what kind of implementation has been done, and the conversations that has been done here, it is up to you to either wait for dlech or try it.
I can not assure everything since I'm not a decent programmer, but I personally am currently using the method below, and have no issues using it on TB 78.14.0 (64bit).

  1. Download the code from TB78 branch from antixn https://github.com/antixn/keebird/tree/TB78
  2. Unzip the file, then create a zip file containing what is inside the directory "xul-ext", with the file "manifest.json" and the others at the root hierarchy of the zip. Name the zip file to something like "keebird-TB78-test.zip" for clarity.
  3. Open Thunderbird, go to addon page, click the setting button and select something like "install add-on from file"ish (I haven't checked the string in English so it's just something like that).
  4. Select the zip file you made (e.g. keebird-TB-test.zip) and install the file
  5. You should have the add-on installed/updated.

FYI: Although I am unsuccessful making it work on TB 91.x :(

@dlech
Copy link
Collaborator

dlech commented Nov 5, 2021

Sorry I never got around to merging this. As you have seen, it is a loosing battle since it breaks in every Thunderbird release.

@snodo
Copy link

snodo commented Nov 5, 2021

FYI: Although I am unsuccessful making it work on TB 91.x :(

My Thunderbird updated to 91 too breaking the addon yet again. Damn. As @dlech said: It's a loosing battle.

@ikenichiro
Copy link

While there are challenges to keep up with Thunderbird updates, I think having an official TB 78 version of this addon would still mean something. At the least, we could merge this and close #30 , release an update for those who are still on TB 78 ;)

@antixn
Copy link
Author

antixn commented Nov 11, 2021

While there are challenges to keep up with Thunderbird updates, I think having an official TB 78 version of this addon would still mean something. At the least, we could merge this and close #30 , release an update for those who are still on TB 78 ;)

Hello @dlech, i also feel like it's a loosing battle but i find this addon usefull and i was happy to play with its source code.
I think i could manage some time to look at the next migration steps in the upcoming months.
Even if it seems useless to work on this topic on the long term, i am stubborn enough to try to keep my environment up with thunderbird, keebird and keepass. It would be great to be able to share it with the help of official releases.

@antixn
Copy link
Author

antixn commented Sep 1, 2022

Ok so... it's been a long time.
I selfishly made the required changes for my own usage but i failed at making it public. Sorry for that guys.
you can find the needed changes for thunderbird 91 on this branch

https://github.com/antixn/keebird/tree/TB91

I was a bit confused with the idea of all this stuff being a loosing battle. Even if the code have aged a lot and would require more than the little adjustments i made, Keebird is still a useful extension i like to use everyday. It's just too bad if i cannot share it for those interested enough to build a release for themselves. I hope you'll appreciate it despite the delays.

@wohauser
Copy link

wohauser commented Sep 6, 2022

Thanks for the work and the publishing.
I installed https://github.com/antixn/keebird/tree/TB91 and it works for me.

Why you do not make a pull request for that branch ? It may be a good anchor for further discussions.

@ikenichiro
Copy link

Confirmed https://github.com/antixn/keebird/tree/TB91 KeeBird working on 102.2.2 (the latest stable TB).
Thank you so much @antixn !!

I'm not sure if @dlech is still interested, but having discussions on track for other people would be also great.
I'm sure many KeePass + Thunderbird user would be excited to know for an working addon.

@antixn
Copy link
Author

antixn commented Sep 11, 2022

Your enthusiasm is heartwarming :)
I have created another PR to merge changes directly in master

@Janpopan
Copy link

Janpopan commented Sep 18, 2022

Confirmed https://github.com/antixn/keebird/tree/TB91 KeeBird working on 102.2.2 (the latest stable TB). Thank you so much @antixn !!

I'm not sure if @dlech is still interested, but having discussions on track for other people would be also great. I'm sure many KeePass + Thunderbird user would be excited to know for an working addon.

It is not work for me TB 102.2.2 (German). Win10 latest Patches
Install works!
Initial Connect to Keepass 2.52 works!
Configuration - partially?
no Password is used from keepass
Hints?
Thanks
Jan

@Janpopan
Copy link

XML-Verarbeitungsfehler: Präfix nicht an einen Namespace gebunden
Adresse: chrome://keefox/content/siteOptions.xhtml
Zeile Nr. 71, Spalte 15:
<html:input id="gb-form-name-wl" maxlength="50000" flex="2" />
--------------^

@ikenichiro ikenichiro mentioned this pull request Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants