Merge lp:~apachelogger/desktopcouch/kwallet-support into lp:desktopcouch

Proposed by Harald Sitter
Status: Rejected
Rejected by: Vincenzo Di Somma
Proposed branch: lp:~apachelogger/desktopcouch/kwallet-support
Merge into: lp:desktopcouch
Diff against target: 109 lines (+60/-4)
1 file modified
desktopcouch/local_files.py (+60/-4)
To merge this branch: bzr merge lp:~apachelogger/desktopcouch/kwallet-support
Reviewer Review Type Date Requested Status
Rodrigo Moya (community) Needs Fixing
Tim Cole (community) Approve
Review via email: mp+31135@code.launchpad.net

Description of the change

Store tokens also if KWallet (if pykde is available). Please note that this will work with either pykde or python-gnome-keyring available or both to allow applications/libraries to read from both KWallet AND gnome-keyring in a mixed environment.

Also: currently the ubuntu packaging depends on gnome-keyring that probably should be changed to recommends on pygnomekeyring | pykde.

To post a comment you must log in.
Revision history for this message
Tim Cole (tcole) wrote :

Looks like a good start. Is this something we can factor out into a library?

review: Approve
Revision history for this message
Harald Sitter (apachelogger) wrote :

IMHO a library to throw content at should be possible and would be a good idea (supposedly it could use the same approach to cover both gnomekeyring and kwallet).

For now however I would just like to get this in as-is to make the way free for ubuntuone-kde stuff.

Revision history for this message
Rodrigo Moya (rodrigo-moya) wrote :

Wouldn't it be easier to use python-keyring? Also, the way the code is written, it will always check for kwallet first and then, whether it found it or not, check again for gnomekeyring:

22 + if ctx.wallet is not None:
...
43 if ctx.keyring is not None:

review: Needs Fixing
Revision history for this message
Harald Sitter (apachelogger) wrote :

IIRC python-keyring has at least on the KWallet side of things a desktop experience flaw because it will trigger authentication (to the wallet) for Python and not the particular app.
Eitherway I wanted it to be as non-intrusive as possible as to prevent regression.

The fact that it does look for kwallet and keyring is intentional since one user can run apps that try to access either backend, since the specification @ fdo does not go into any detail regarding multiple keyrings. In one implementation it is entirely possible that someone writes a KDE library that only accesses kwallet (which is entirely valid considering the spec says that one has to get the token from the keyring, but not any specific one or in any particular order), so if desktopcouch can access kwallet then it _must_ drop its token there (and technically any other keyring system it can find as to not limit client implementations).
If it did not do that then it sort of requires client implementations to search all keyring systems for tokens since desktopcouch might have dropped them in either of them.

Revision history for this message
Vincenzo Di Somma (vds) wrote :

Due to changes this branch doesn't apply anymore.

Unmerged revisions

168. By Harald Sitter

add as non-intrusive kwallet support as possible

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'desktopcouch/local_files.py'
--- desktopcouch/local_files.py 2010-07-14 21:02:43 +0000
+++ desktopcouch/local_files.py 2010-07-28 09:38:43 +0000
@@ -1,4 +1,5 @@
1# Copyright 2009 Canonical Ltd.1# Copyright 2009 Canonical Ltd.
2# Copyright 2010 Harald Sitter <apachelogger@ubuntu.com>
2#3#
3# This file is part of desktopcouch.4# This file is part of desktopcouch.
4#5#
@@ -31,7 +32,6 @@
31import random32import random
32import string33import string
33import re34import re
34import gnomekeyring
35try:35try:
36 import ConfigParser as configparser36 import ConfigParser as configparser
37except ImportError:37except ImportError:
@@ -86,7 +86,29 @@
86 if ctx.with_auth:86 if ctx.with_auth:
87 admin_username = self._make_random_string(10)87 admin_username = self._make_random_string(10)
88 admin_password = self._make_random_string(10)88 admin_password = self._make_random_string(10)
89 if ctx.wallet is not None:
90 if ctx.wallet.isOpen():
91 write_basic = False
92 if ctx.wallet.setFolder("desktopcouch"):
93 fail, value = ctx.wallet.readMap("basic")
94 if not fail and len(value) == 2:
95 admin_username = basic["admin_username"]
96 admin_password = basic["admin_password"]
97 else:
98 write_basic = True
99 else:
100 ctx.wallet.createFolder("desktopcouch")
101 ctx.wallet.setFolder("desktopcouch")
102 write_basic = True
103 if write_basic:
104 value = dict()
105 value["admin_username"] = admin_username
106 value["admin_password"] = admin_password
107 ctx.wallet.writeMap("basic", value)
108 else:
109 logging.warn("There is no wallet to store our admin credentials.")
89 if ctx.keyring is not None:110 if ctx.keyring is not None:
111 import gnomekeyring # (Re)import for errors.
90 try:112 try:
91 data = ctx.keyring.find_items_sync(gnomekeyring.ITEM_GENERIC_SECRET,113 data = ctx.keyring.find_items_sync(gnomekeyring.ITEM_GENERIC_SECRET,
92 {'desktopcouch': 'basic'})114 {'desktopcouch': 'basic'})
@@ -110,9 +132,22 @@
110 token = self._make_random_string(10)132 token = self._make_random_string(10)
111 token_secret = self._make_random_string(10)133 token_secret = self._make_random_string(10)
112134
135 # Save the new OAuth creds so that 3rd-party apps can authenticate by
136 # accessing the keyring first. This is one-way. We don't read from keyring.
137 if ctx.wallet is not None:
138 if ctx.wallet.isOpen():
139 if str(ctx.wallet.currentFolder()) != "desktopcouch":
140 ctx.wallet.setFolder("desktopcouch")
141 value = dict()
142 value["consumer_key"] = consumer_key
143 value["consumer_secret"] = consumer_secret
144 value["token"] = token
145 value["token_secret"] = token_secret
146 ctx.wallet.writeMap("oauth", value)
147 else:
148 logging.warn("There is no wallet to store our oauth credentials.")
113 if ctx.keyring is not None:149 if ctx.keyring is not None:
114 # Save the new OAuth creds so that 3rd-party apps can authenticate by150 import gnomekeyring # (Re)import for errors.
115 # accessing the keyring first. This is one-way. We don't read from keyring.
116 try:151 try:
117 ctx.keyring.item_create_sync(None,152 ctx.keyring.item_create_sync(None,
118 gnomekeyring.ITEM_GENERIC_SECRET,153 gnomekeyring.ITEM_GENERIC_SECRET,
@@ -207,7 +242,27 @@
207 depend on environment variables."""242 depend on environment variables."""
208243
209 def __init__(self, run_dir, db_dir, config_dir, with_auth=True,244 def __init__(self, run_dir, db_dir, config_dir, with_auth=True,
210 keyring=gnomekeyring): # (cache, data, config)245 wallet=None, keyring=None): # (cache, data, config)
246
247 try:
248 from PyQt4.QtCore import QCoreApplication
249 from PyKDE4.kdeui import KWallet
250
251 # KWallet uses a QEventLoop. This only works if a QApp is present.
252 # We only can have one instance - ever!
253 if QCoreApplication.instance() == None:
254 a = QCoreApplication([""])
255 a.setApplicationName("Desktop Couch")
256
257 wallet = KWallet.Wallet.openWallet(KWallet.Wallet.LocalWallet(), 0, KWallet.Wallet.Synchronous)
258 except ImportError:
259 wallet = None # just to be save
260
261 try:
262 import gnomekeyring
263 keyring = gnomekeyring
264 except ImportError:
265 keyring = None # just to be save
211266
212 self.couchdb_log_level = 'notice'267 self.couchdb_log_level = 'notice'
213268
@@ -221,6 +276,7 @@
221 self.config_dir = os.path.realpath(config_dir)276 self.config_dir = os.path.realpath(config_dir)
222 self.db_dir = os.path.realpath(db_dir)277 self.db_dir = os.path.realpath(db_dir)
223 self.with_auth = with_auth278 self.with_auth = with_auth
279 self.wallet = wallet
224 self.keyring = keyring280 self.keyring = keyring
225281
226 self.file_ini = os.path.join(config_dir, "desktop-couchdb.ini")282 self.file_ini = os.path.join(config_dir, "desktop-couchdb.ini")

Subscribers

People subscribed via source and target branches