[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Change freeze request: Fix invalid login pages



When I built the last python-fedora, I built the package in my working tree instead of making a fresh branch. This means the python-fedora package I deployed on Friday has some incompatible changes that weren't meant to go in until the next release. This is leading people to get an internal server error when they try to login with an invalid username/password.

There's two options: spin a new package based off the actual python-fedora-0.2.99.8. The diff for that would look like bzr-0.2.99.8-current.patch.

The alternative is to only fix the problem that we know we're having with BaseClient. The patch for that is quite a bit smaller: it's just a few lines to change exception handling in fas2.py and jsonfasprovider.py to use the new exception hierarchy in BaseClient.

Risk for option #1: we have had the new python-fedora deployed since Friday and this is the only problem reported so far. The patch is bigger than for option #2 and thus there's more room for unexpected problems.

Risk for option #2: We definitely do not want to push this package out to the other servers as it's likely to break error handling in clients because of the new exception hierarchy. Since we're in change freeze we're not likely to do that for a while.

I'm inclined for option #2.  What do others think?

-Toshio
=== modified file 'fedora/accounts/fas2.py'
--- fedora/accounts/fas2.py	2008-04-15 19:06:21 +0000
+++ fedora/accounts/fas2.py	2008-04-12 20:10:55 +0000
@@ -18,9 +18,10 @@
 # Author(s): Ricky Zhou <ricky fedoraproject org>
 #            Toshio Kuratomi <tkuratom redhat com>
 #
-'''
-Provide a client module for talking to the Fedora Account System.
-'''
+import urllib
+import urllib2
+import simplejson
+from urlparse import urljoin
 from fedora.client import BaseClient, AuthError, ServerError
 
 class FASError(Exception):
@@ -183,28 +184,34 @@
         '''Generate a cert for a user'''
         try:
             request = self.send_request('user/gencert', auth=True)
-        except ServerError:
+        except ServerError, e:
             raise
         if not request['cla']:
             raise CLAError
         return "%(cert)s\n%(key)s" % request
 
-    def verify_password(self, username, password):
-        '''Return whether the username and password pair match are valid.
-        
-        Arguments:
-        :username: username to try authenticating
-        :password: password for the user
-
-        Returns: True if the username/password are valid.  False otherwise.
-        '''
-        asUser = BaseClient(self.baseURL, username, password)
-        try:
-            # This will attempt to authenticate to the account system and
-            # raise an AuthError if the password and username don't match. 
-            asUser._authenticate(force=True)
-        except AuthError:
-            return False
-        except:
-            raise
-        return True
+    def authenticate(self, username, password):
+        """TODO"""
+        req = urllib2.Request(urljoin(self.baseURL, 'login?tg_format=json'))
+        req.add_data(urllib.urlencode({
+            'user_name' : username,
+            'password'  : password,
+            'login'     : 'Login'
+            }))
+        try:
+            response = urllib2.urlopen(req)
+        except urllib2.HTTPError, e:
+            if e.msg == 'Forbidden':
+                return False
+        jsonString = response.read()
+        try:
+            data = simplejson.loads(jsonString)
+        except ValueError, e:
+            # The response wasn't JSON data
+            raise ServerError, str(e)
+        try:
+            if data['user']:
+                return data['user']['username'] == username
+        except KeyError:
+            pass
+        return False

=== modified file 'fedora/client/__init__.py'
--- fedora/client/__init__.py	2008-04-15 19:06:21 +0000
+++ fedora/client/__init__.py	2008-04-12 20:48:14 +0000
@@ -35,28 +35,17 @@
 from urlparse import urljoin
 
 import gettext
-translation = gettext.translation('python-fedora', '/usr/share/locale',
-        fallback=True)
-_ = translation.ugettext
+t = gettext.translation('python-fedora', '/usr/share/locale', fallback=True)
+_ = t.ugettext
 
 log = logging.getLogger(__name__)
 
 SESSION_FILE = path.join(path.expanduser('~'), '.fedora_session')
 
-class FedoraServiceError(Exception):
-    '''Base Exception for any problem talking with the Service.'''
-    pass
-
-class ServerError(FedoraServiceError):
-    '''Unable to talk to the server properly.'''
-    pass
-
-class AuthError(FedoraServiceError):
-    '''Error during authentication.  For instance, invalid password.'''
-    pass
-
-class AppError(FedoraServiceError):
-    '''Error condition that the server is passing back to the client.'''
+class ServerError(Exception):
+    pass
+
+class AuthError(ServerError):
     pass
 
 class BaseClient(object):
@@ -259,9 +248,13 @@
         except ValueError, e:
             # The response wasn't JSON data
             raise ServerError, str(e)
-
-        if 'exc' in data:
-            raise AppError(name = data['exc'], message = data['tg_flash'])
+        except Exception, e:
+            regex = re.compile('<span class="fielderror">(.*)</span>')
+            match = regex.search(jsonString)
+            if match and len(match.groups()):
+                return dict(tg_flash=match.groups()[0])
+            else:
+                raise ServerError, e.message
 
         if 'logging_in' in data:
             if (inspect.currentframe().f_back.f_code !=

=== modified file 'fedora/tg/identity/jsonfasprovider.py'
--- fedora/tg/identity/jsonfasprovider.py	2008-04-15 19:06:21 +0000
+++ fedora/tg/identity/jsonfasprovider.py	2008-04-12 20:00:17 +0000
@@ -29,7 +29,7 @@
 from cherrypy import response
 from turbogears import config, identity
 
-from fedora.client import BaseClient, ServerError, AuthError
+from fedora.client import BaseClient, ServerError
 
 import gettext
 translation = gettext.translation('python-fedora', '/usr/share/locale',
@@ -203,7 +203,7 @@
         try:
             user = JsonFasIdentity(visit_key, username = user_name,
                     password = password)
-        except (ServerError, AuthError), e:
+        except ServerError, e:
             log.warning('Error logging in %(user)s: %(error)s' % {
                 'user': user_name, 'error': e})
             return None

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]