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

Add KWallet support #540

Merged
merged 36 commits into from
Dec 3, 2020
Merged

Add KWallet support #540

merged 36 commits into from
Dec 3, 2020

Conversation

samuel-w
Copy link
Contributor

@samuel-w samuel-w commented Jul 2, 2020

In the future, we should check if the wallet closes in between detecting the current keyring and setting the password.

Also, is there an alternate way to do keyring detection other than a mountain of try excepts? The code is starting to get hard to read.

Fixes #236

@samuel-w
Copy link
Contributor Author

samuel-w commented Jul 2, 2020

Wait, QtDBus exists. Have to change code

@samuel-w samuel-w closed this Jul 2, 2020
@samuel-w
Copy link
Contributor Author

samuel-w commented Jul 2, 2020

Attempting to port to QtDBus, unsucessful.
Python crashes with "No such method 'open' in interface 'org.kde.KWallet' at object path '/modules/kwalletd5' (signature 'sis')"
Even when I convert the wId to qlonglong, it still gets converted back to an int.
However, the existing dbus code still works.
https://gist.github.com/samuel-w/005453d30250265e2c75c167d9b545b3

@samuel-w samuel-w reopened this Jul 2, 2020
@Hofer-Julian
Copy link
Collaborator

Too bad...
But thanks for trying :)

@samuel-w
Copy link
Contributor Author

samuel-w commented Jul 2, 2020

Tested and it works, woohoo. Had to use command line to get the handle.

@samuel-w samuel-w marked this pull request as ready for review July 2, 2020 22:43
samuel-w added 4 commits July 2, 2020 22:11
Old commit messages:
- Add KWallet 4 and 5 support
- Spelling, lint, move comments, remove print
- Use PyQt5,QtDBus over dbus, untested
- Lint
- Use list instead of many args
- Open wallet only on password request
- Remove noqa
- Check if KWallet is enabled
@samuel-w samuel-w closed this Aug 2, 2020
@samuel-w samuel-w deleted the KWallet branch August 2, 2020 04:03
@m3nu
Copy link
Contributor

m3nu commented Aug 2, 2020

Dont want to add this any more? I’m a bit behind merging PRs due to many other things going on, but plan to catch up over Aug.

@rejinka
Copy link

rejinka commented Aug 20, 2020

I really would like to get this feature

@samuel-w samuel-w restored the KWallet branch August 28, 2020 21:57
@samuel-w
Copy link
Contributor Author

Initially closed this because I thought no one would use it, as more code = more maintenance. Guess I was wrong.

@samuel-w samuel-w reopened this Aug 28, 2020
m3nu
m3nu previously approved these changes Aug 30, 2020
@samuel-w
Copy link
Contributor Author

Wait a bit, I may have forgot to catch an exception

@samuel-w samuel-w changed the title Add KWallet 4 and 5 support Add KWallet support Sep 28, 2020
KDE 4 is 6 years out of support
More people use secrestservice password maangerse than KWallet
@samuel-w
Copy link
Contributor Author

What's the status of this?

@codecov-io
Copy link

codecov-io commented Nov 30, 2020

Codecov Report

Merging #540 (c16bb4d) into master (529b1da) will decrease coverage by 0.14%.
The diff coverage is 65.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
- Coverage   74.60%   74.45%   -0.15%     
==========================================
  Files          52       53       +1     
  Lines        3379     3418      +39     
==========================================
+ Hits         2521     2545      +24     
- Misses        858      873      +15     
Impacted Files Coverage Δ
src/vorta/keyring/kwallet.py 57.14% <57.14%> (ø)
src/vorta/keyring/abc.py 92.59% <100.00%> (+1.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 529b1da...c16bb4d. Read the comment docs.

@samuel-w samuel-w requested a review from m3nu November 30, 2020 07:01
@m3nu m3nu merged commit 5c1306e into borgbase:master Dec 3, 2020
@samuel-w samuel-w deleted the KWallet branch December 3, 2020 08:34
@rejinka
Copy link

rejinka commented Dec 3, 2020

Thank you very much @samuel-w and @m3nu!

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.

Support KDE KWallet
5 participants