osmank3 / otp-keys

Show and copy otp keys on Gnome Shell panel
GNU General Public License v3.0
9 stars 5 forks source link

Fixed keyring unlocking. #11

Closed dkosmari closed 10 months ago

dkosmari commented 10 months ago

The previous method for unlocking the keyring wasn't working on my systems, so I changed it to use service.unlock_sync().

osmank3 commented 10 months ago

Thanks for your contribution. I could not face with your problem in any of my test systems. I tried your code, it is working extensions settings page but not working in extension's main ui. So I did not merge. If you share your systems specs I will test and develop this extension.

dkosmari commented 10 months ago

I cannot reproduce your test case. Here are the cases I tested, with GNOME 44 on Xorg:

Test 1:

Test 2:

Test 3:

For tests 1 and 2:

My PR fixes this case. At no point it fails to update the menu or the preferences window, after the "Unlock keyring" option is used. It either detects the keyring is correctly unlocked, or the "Unlock keyring" does the expected thing, and the menus/preferences UI gets updated correctly.

For test 3:

Extension fails to start with the error "defaultCol is null". There's no default keyring, because there's no login password. Creating a default keyring, the error changes to:

JS ERROR: Extension otp-keys@osmank3.net: TypeError: otp is null
                                                   OtpMenuItem@/home/nologinpass-keyringpass/.local/share/gnome-shell/extensions/otp-keys@osmank3.net/extension.js:53:13
                                                   _fillList/<@/home/nologinpass-keyringpass/.local/share/gnome-shell/extensions/otp-keys@osmank3.net/extension.js:161:28
                                                   _fillList@/home/nologinpass-keyringpass/.local/share/gnome-shell/extensions/otp-keys@osmank3.net/extension.js:160:27
                                                   _init@/home/nologinpass-keyringpass/.local/share/gnome-shell/extensions/otp-keys@osmank3.net/extension.js:119:14
                                                   ButtonBox@resource:///org/gnome/shell/ui/panelMenu.js:11:1
                                                   PanelMenuButton@resource:///org/gnome/shell/ui/panelMenu.js:97:4
                                                   Indicator@/home/nologinpass-keyringpass/.local/share/gnome-shell/extensions/otp-keys@osmank3.net/extension.js:101:1
                                                   enable@/home/nologinpass-keyringpass/.local/share/gnome-shell/extensions/otp-keys@osmank3.net/extension.js:231:27
                                                   _callExtensionEnable@resource:///org/gnome/shell/ui/extensionSystem.js:196:38
                                                   loadExtension@resource:///org/gnome/shell/ui/extensionSystem.js:398:32
                                                   async*_onInstallButtonPressed@resource:///org/gnome/shell/ui/extensionDownloader.js:294:35
                                                   Async*addButton/<@resource:///org/gnome/shell/ui/dialog.js:130:41

This is clearly a bug elsewhere in the code, and my PR changes nothing.

osmank3 commented 10 months ago

OK, I will test your Test 1-2 cases with Gnome version 44.

About Test 3 case you can set any keyring to default keyring with seahorse or you can wait for a feature which will be added for choosing default or selected keyring for saving otp keys in a later release.

osmank3 commented 10 months ago

OK, I was able to reproduce the error.

However there is a slight issue! There is no phrase to say for the passwordless login part, but the reason for keeping the OTP code on the keyring comes from the necessity of encrypting and hiding the code. When you leave the keyring without a password, anyone accessing your system can easily take over your OTP code. This situation goes against the purpose of using OTP. Therefore, I will not make any changes for Test 1 and 2 cases.

As mentioned in the previous message, for the Test 3 case, I am considering adding an option to utilize a default or desired collection on the keyring through the extension's settings in the future.

dkosmari commented 10 months ago

There's literally an API to unlock the keyring, yet you're trying to rationalize your clown code by saying I'm not allowed to keep the keyring without a password. I'll just maintain my fork then, my OTP codes will never be safe with clown code.

osmank3 commented 10 months ago

You are free to develop the project in your own fork; open-source license is there for that. Best regards...