pykong / YubiGuard

Python script to prevent accidental triggering of YubiKeys on Linux.
GNU General Public License v3.0
24 stars 8 forks source link

Add button to toggle (unlock) yubikey to indicator applet #6

Closed mozfreddyb closed 7 years ago

mozfreddyb commented 7 years ago

@bfelder What do you think? One could rename "Toggle" to "Unlock" of course.

(This patch /might/ not cleanly apply, if you take my other pull request)

mozfreddyb commented 7 years ago

I'm only unlocking, similar to the key combo. It's even doing the exact same thing. Calling the file with the -t param.

Should I rename things to "unlock"?

mozfreddyb commented 7 years ago

Should I rename things to "unlock"? I just did that.

pykong commented 7 years ago

The toggle should be 'unclickable' during unlock and become 'clickable' after lock again. (See the logic which changes the icon to know how to do it.)

It is inelegant calling the script again via subprocess. There should be a method call here.

mozfreddyb commented 7 years ago

The toggle should be 'unclickable' during unlock and become 'clickable' after lock again. (See the logic which changes the icon to know how to do it.)

Good point. I will do that when I have an impression on how to change the state without calling the script again (see below)

It is inelegant calling the script again via subprocess. There should be a method call here.

True, but calling change_state on the YubiGuard class somehow did not work. I assumed it was a problem with things happening in different threads and I couldn't figure that out. Any pointers?

pykong commented 7 years ago

Unlock will now be triggered via Zmq directly. Actually it would have been most elegant to use the inbuilt queue for that purpose. I will check whether I can do that.

I have also marked the respective lines that should hold activate/deactivate commands for the button you added.

pykong commented 7 years ago

I have now wired the button to respective queue. It should now also activate/deactivate when unlocked/locked. I have not tested this commit yet.

You may do that. Give me a feedback so I can merge into the main branch.

mozfreddyb commented 7 years ago

Wow, I was expecting just a few pointers, but your commits helped me understand how things work. The Unlock item is now correctly disabled, except when there's no YubiKey found. I don't know why update_icon isn't called in that case. Will possibly take a look later

pykong commented 7 years ago

Have you fixed the issue of not-updating logo? I like to merge into the major branch.

And do not forget to STAR YubiGuard! I would appreciate it! THX!

mozfreddyb commented 7 years ago

What's still left is that the "Unlock" menu item does not automatically lock if you remove the Yubikey. I couldn't figure out why the https://github.com/bfelder/YubiGuard/pull/6/files#diff-0da1838ca3c4eb687e8a6d41b6b87856R162 branch is never called.

pykong commented 7 years ago

OK. fixed that. Two little lines only. It works perfectly now. Tell me your opinion. If you find everything to be OK, I am going to merge into the major brand.

mozfreddyb commented 7 years ago

looks good, thanks 👍

pykong commented 7 years ago

Merged. Was a pleasure working with you. Just get in touch in case you need anything in the future...

mozfreddyb commented 7 years ago

Thank you. Belated happy holidays 🎆 🎉