http://bugzilla.moblin.org/show_bug.cgi?id=3604





--- Comment #26 from pohly <[email protected]>  2009-09-15 04:53:18 ---
(In reply to comment #20)
> There's another issue here: properties can be added by backends dynamically.
> Therefore the list of passwords that the UI's savePassword() is called for
> shouldn't be fixed. For askPassword() this is handled by reading all
> properties, which triggers askPassword(). We should do something similar for
> savePassword() and call it "preFlush()", or something like that, because it
> might be used for other checks besides storing passwords in a keyring.

There's still a hard-coded list of passwords inside
EvolutionSyncConfig::preFlush(). Passwords added dynamically by backends
therefore won't be stored in the keyring.

Here's how this can be solved:
* add a virtual checkPassword(ConfigUserInterface &ui,
                               ConfigNode &globalConfigNode,
                               const boost::shared_ptr<FilterConfigNode>
&sourceConfigNode = boost::shared_ptr<FilterConfigNode>())
  to ConfigProperty, with an empty implementation. Similar for savePassword().
* Override these methods in the actual PasswordProperty implementations.
* iterate over *all* sync and source properties and invoke the virtual
  methods

The ordering of description and passwordName seem to be confused somewhere.
When asking me interactively, it prompted me for "proxyPassword" (=
passwordName) instead of using "proxy" (= description).

When I described how to store point 3 in the list (backend password) I
suggested "to err on the side of caution and use server name, source name, and
a
fixed string "Backend" (for object) as GNOME keyring keys. When I say "server
name" and "source name", I mean the names as used in the SyncEvolution config
("scheduleworld" + "addressbook", for example)."

You didn't comment on that and now used "server URL" as part of the "object"
key, leading to "ns5.scheduleworld.com/funambol/ds ical20 backend". The backend
password is not related to the server URL, which might even be empty for other
transports. I still think the server configuration name would be a better
choice; later we should switch to the "source set configuration name".

The --keyring options must be listed and documented in the "--help" output and
README. I can add that. Note to self: one side-effect of "--keyring --configure
--source-property evolutionpassword=foo" is that *all* passwords are moved to
the keyring, not just evolutionpassword. This is okay, but somewhat unexpected
and thus needs to be documented.

GNOME keyring distinguishes between empty and NULL string keys. Using NULL
instead of empty strings looks nice in "seahorse", so let's avoid empty
strings:

diff --git a/src/CmdlineSyncClient.cpp b/src/CmdlineSyncClient.cpp
index 83b85b1..d095659 100644
--- a/src/CmdlineSyncClient.cpp
+++ b/src/CmdlineSyncClient.cpp
@@ -35,8 +35,18 @@ CmdlineSyncClient::CmdlineSyncClient(const string &server,
 {
 }

-string CmdlineSyncClient::askPassword(const string &passwordName, 
-                                      const string &descr, 
+/**
+ * GNOME keyring distinguishes between empty and unset
+ * password keys. This function returns NULL for an
+ * empty std::string.
+ */
+inline const char *passwdStr(const std::string &str)
+{
+    return str.empty() ? NULL : str.c_str();
+}
+
+string CmdlineSyncClient::askPassword(const string &descr,
+                                      const string &passwordName,
                                       const ConfigPasswordKey &key) 
 {
     string password;
@@ -49,12 +59,12 @@ string CmdlineSyncClient::askPassword(const string
&passwordName,
         GnomeKeyringResult result;
         GList* list;

-        result = gnome_keyring_find_network_password_sync(key.user.c_str(),
-                                                          key.domain.c_str(),
-                                                          key.server.c_str(),
-                                                          key.object.c_str(),
-                                                         
key.protocol.c_str(),
-                                                         
key.authtype.c_str(),
+        result = gnome_keyring_find_network_password_sync(passwdStr(key.user),
+                                                         
passwdStr(key.domain),
+                                                         
passwdStr(key.server),
+                                                         
passwdStr(key.object),
+                                                         
passwdStr(key.protocol),
+                                                         
passwdStr(key.authtype),
                                                           key.port,
                                                           &list);

@@ -87,12 +97,12 @@ bool CmdlineSyncClient::savePassword(const string
&passwordName,
         guint32 itemId;
         GnomeKeyringResult result;
         result = gnome_keyring_set_network_password_sync(NULL,
-                                                         key.user.c_str(),
-                                                         key.domain.c_str(),
-                                                         key.server.c_str(),
-                                                         key.object.c_str(),
-                                                         key.protocol.c_str(),
-                                                         key.authtype.c_str(),
+                                                         passwdStr(key.user),
+                                                        
passwdStr(key.domain),
+                                                        
passwdStr(key.server),
+                                                        
passwdStr(key.object),
+                                                        
passwdStr(key.protocol),
+                                                        
passwdStr(key.authtype),
                                                          key.port,
                                                          password.c_str(),

-- 
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are watching someone on the CC list of the bug.
_______________________________________________
Syncevolution-issues mailing list
[email protected]
http://lists.syncevolution.org/listinfo/syncevolution-issues

Reply via email to