Merge lp:~deejay1/lernid/keyring-integration into lp:lernid

[Łukasz Jernaś]

[Łukasz Jernaś (deejay1)]

Works for me...

[John S. Gruber (jsjgruber)]

I'm sorry it took so long for anyone to get back to you. I recognize that you put significant effort into learning the python gnome keyring interface, and then coding and testing your patch. Michael is no longer maintaining Lernid and the project was without anyone to look after its code for a while. I just started.

I reviewed your code and then tried it. I'm afraid that I have significant reservations about what it is doing (and, I guess, about the validity of the bug for Lernid).

It seems to me that Lernid should, first and foremost, care for students inexperienced with Ubuntu and #ubuntu-classroom. It goes to some pains to negotiate an acceptable nick for them that is like the one they requested. It seems to me that the additional password panel is an enhancement for the experienced user who has figured out how to use other irc clients, and who has negotiated how to set a nickserv password. That sounds like you and me, but not like our inexperienced students.

The bug is about an enhancement to that password enhancement, and it has costs:

1. As I understand it, once a student has entered a password he or she must thereafter enter what should be a high complexity password to unlock the default keyring for Lernid to retrieve the nick password. This *might* happen just once a login session, saving the student who really wants to use his or her registered nick some effort for subsequent launches of Lernid during the same login session. If it is the result of a less experienced student who tried entering a password on a lark, they are stuck entering their gnome password at least once a signon session, and maybe more, for the life of their keyring.

2. It adds a dependency to the package.

3. It adds code and complexity

It simply doesn't seem worth it to me.

The basic problem here is not your code--I think your code is sound. The problem is the nature of the enhancement requested in bug LP: #527370. I think I'll wait a couple of days and then mark the bug "won't fix".

I regret that you have waited over a year for a response, much less for a response like this, and I regret that this is the nature of our first contact. I sincerely I hope that I haven't put you off Lernid with my opinion.

review: Disapprove

Unmerged revisions

183. By Łukasz Jernaś on 2010-02-25

Fix exceptions, remove unnecessary dictionary

182. By Łukasz Jernaś on 2010-02-25

Keep NickServ password in GNOME keyring

=== modified file 'lernid/ConnectDialog.py'
--- lernid/ConnectDialog.py	2010-02-13 15:43:30 +0000
+++ lernid/ConnectDialog.py	2010-02-27 09:35:21 +0000
@@ -99,6 +99,9 @@
     def get_password(self):
         return self.builder.get_object('password').get_text()
 
+    def set_password(self, password):
+        return self.builder.get_object('password').set_text(password)
+
     def is_valid_nick(self, nick_te):
         alpha = 'abcdefghijklmnopqrstuvwxyz'
         alpha = alpha + alpha.upper()
 
=== modified file 'lernid/EventManager.py'
--- lernid/EventManager.py	2010-02-13 15:43:30 +0000
+++ lernid/EventManager.py	2010-02-27 09:35:21 +0000
@@ -23,6 +23,7 @@
 import ConfigParser
 import urllib
 import logging
+import gnomekeyring
 
 from lernid import ConnectDialog
 from lernid.PasswordDialog import PasswordDialog
@@ -59,6 +60,8 @@
         self._passworddialog = None
         self._widgets = {}
         self._read_config()
+        
+        self._keyring = gnomekeyring.get_default_keyring_sync()
 
     @classmethod
     def get_instance(cls):
@@ -96,7 +99,29 @@
         Statusbar.push_message(_('Connecting to event'), duration=10)
         self._isconnected = True
         self.emit('event-connect', self._event)
+    
+    # Checks the GNOME keyring for a password for the specified nick
+    def get_keyring_password(self, nick):
+        try:
+            # Reads only the first entry, because the search shouldn't return more
+            password = gnomekeyring.find_network_password_sync(nick, None, 'irc.freenode.net',
+                                                    'Lernid', 'irc')[0]['password']
+            return password
+        except gnomekeyring.NoMatchError:
+            return ''
+        except gnomekeyring.DeniedError:
+            logging.debug('Failed to read from keyring')
+            return ''
+        return ''
 
+    # Sets or updates keyring password for the specified nick
+    def set_keyring_password(self, nick, password):
+        keyring_data = {'user': nick, 'server': 'irc.freenode.net', 'protocol': 'irc', 'object': 'Lernid'}
+        try:
+            gnomekeyring.item_create_sync(self._keyring, gnomekeyring.ITEM_NETWORK_PASSWORD,'Lernid', keyring_data, password, True)
+        except gnomekeyring.DeniedError:
+            logging.debug('Failed to add keyring item')
+    
     def _identify_nick(self, nick, password):
         server = Server.get_server('irc.freenode.net', nick)
         identifier = server.identify_nick(password)
@@ -108,10 +133,10 @@
                 self._passworddialog = PasswordDialog.new()
             password = self._passworddialog.run()
             if password != None:
+                self.set_keyring_password(nick,password)
                 identifier.retry(password)
         identifier.connect('invalid-password', retry)
-
-
+    
     def connect_dialog(self, widget=None, parent=None):
         """Show the connect dialog and connect to the resources."""
         if not self._connectdialog:
@@ -119,6 +144,7 @@
             self._connectdialog.set_transient_for(parent)
             self._connectdialog.connect('response', self._connect_dialog_reponse)
             self._connectdialog.set_nick(Preferences.get('nick'))
+            self._connectdialog.set_password(self.get_keyring_password(Preferences.get('nick')))
             self._connectdialog.set_transient_for(widget)
         self._connectdialog.show()
 
@@ -130,6 +156,7 @@
             self._connect_event(event, nick)
             password = self._connectdialog.get_password()
             if password:
+                self.set_keyring_password(nick, password)
                 self._identify_nick(nick, password)
 
     def disconnect(self):
@@ -159,3 +186,4 @@
             return self._widgets[name]
         return None
 
+