Second patch from issue2550688 -- with some changes:

- password.py now has a second class JournalPassword used for journal
  storage. We have some backends that directly store serialized python
  objects. Also when reading from the journal some backends expected the
  string read to be usable as a parameter to a Password constructor.
  This now calls a JournalPassword constructor in all these cases.
  The new JournalPassword just keeps the scheme and has an empty
  password.
- some factoring, move redundant implementation of "history" from
  rdbms_common and back_anydbm to hyperdb.

git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4592 57a73879-2fb5-44c3-a270-3262357dd7e2

diff --git a/CHANGES.txt b/CHANGES.txt
index 7cc26ae7abe357735a1cfe06f44f3ceca86e2e1f..8e167541bd84a7a71e30ed1f3e1967fb710927f2 100644 (file)
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -76,8 +76,9 @@ Fixed:
   (Ralf Schlatterbeck)
 - Fixed bug in mailgw refactoring, patch issue2550697 (thanks Hubert
   Touvet)
-- Fix first part of Password handling security issue2550688 (thanks
-  Joseph Myers for reporting and Eli Collins for fixing)
+- Fix Password handling security issue2550688 (thanks Joseph Myers for
+  reporting and Eli Collins for fixing) -- this fixes all observations
+  by Joseph Myers except for auto-migration of existing passwords.
 
 2010-10-08 1.4.16 (r4541)
 

diff --git a/roundup/backends/back_anydbm.py b/roundup/backends/back_anydbm.py
index f8ba81399c6a232a9c9642f22260facdff304d5c..34ddf936e095cc729349158f331bd103dfadd7b6 100644 (file)
--- a/roundup/backends/back_anydbm.py
+++ b/roundup/backends/back_anydbm.py
@@ -1298,6 +1298,8 @@ class Class(hyperdb.Class):
                     raise TypeError('new property "%s" not a '
                         'Password'%propname)
                 propvalues[propname] = value
+                journalvalues[propname] = \
+                    current and password.JournalPassword(current)
 
             elif value is not None and isinstance(prop, hyperdb.Date):
                 if not isinstance(value, date.Date):
@@ -1423,23 +1425,6 @@ class Class(hyperdb.Class):
             raise hyperdb.DatabaseError(_('Database open read-only'))
         self.db.destroynode(self.classname, nodeid)
 
-    def history(self, nodeid):
-        """Retrieve the journal of edits on a particular node.
-
-        'nodeid' must be the id of an existing node of this class or an
-        IndexError is raised.
-
-        The returned list contains tuples of the form
-
-            (nodeid, date, tag, action, params)
-
-        'date' is a Timestamp object specifying the time of the change and
-        'tag' is the journaltag specified when the database was opened.
-        """
-        if not self.do_journal:
-            raise ValueError('Journalling is disabled for this class')
-        return self.db.getjournal(self.classname, nodeid)
-
     # Locating nodes:
     def hasnode(self, nodeid):
         """Determine if the given nodeid actually exists

diff --git a/roundup/backends/rdbms_common.py b/roundup/backends/rdbms_common.py
index 09425b2ac35ce557ac24c11d5f93829cb8fbd827..5798c8d646e1ab223a16db221e06f52cf47a05e2 100644 (file)
--- a/roundup/backends/rdbms_common.py
+++ b/roundup/backends/rdbms_common.py
@@ -1297,7 +1297,7 @@ class Database(FileStorage, hyperdb.Database, roundupdb.Database):
                         continue
                     cvt = self.to_hyperdb_value(property.__class__)
                     if isinstance(property, Password):
-                        params[param] = cvt(value)
+                        params[param] = password.JournalPassword(value)
                     elif isinstance(property, Date):
                         params[param] = cvt(value)
                     elif isinstance(property, Interval):
@@ -1864,6 +1864,8 @@ class Class(hyperdb.Class):
                 if not isinstance(value, password.Password):
                     raise TypeError('new property "%s" not a Password'%propname)
                 propvalues[propname] = value
+                journalvalues[propname] = \
+                    current and password.JournalPassword(current)
 
             elif value is not None and isinstance(prop, Date):
                 if not isinstance(value, date.Date):
@@ -1995,23 +1997,6 @@ class Class(hyperdb.Class):
             raise DatabaseError(_('Database open read-only'))
         self.db.destroynode(self.classname, nodeid)
 
-    def history(self, nodeid):
-        """Retrieve the journal of edits on a particular node.
-
-        'nodeid' must be the id of an existing node of this class or an
-        IndexError is raised.
-
-        The returned list contains tuples of the form
-
-            (nodeid, date, tag, action, params)
-
-        'date' is a Timestamp object specifying the time of the change and
-        'tag' is the journaltag specified when the database was opened.
-        """
-        if not self.do_journal:
-            raise ValueError('Journalling is disabled for this class')
-        return self.db.getjournal(self.classname, nodeid)
-
     # Locating nodes:
     def hasnode(self, nodeid):
         """Determine if the given nodeid actually exists

diff --git a/roundup/cgi/templating.py b/roundup/cgi/templating.py
index 11e1449470ff25dd1742ebf786ed3d0117e6362a..04cfd76ab6cb3689d59d7ebc108c8d0904f1c3d9 100644 (file)
--- a/roundup/cgi/templating.py
+++ b/roundup/cgi/templating.py
@@ -1108,6 +1108,13 @@ class _HTMLItem(HTMLInputMixin, HTMLPermissions):
                             cell[-1] += ' -> %s'%current[k]
                             current[k] = val
 
+                    elif isinstance(prop, hyperdb.Password) and args[k] is not None:
+                        val = args[k].dummystr()
+                        cell.append('%s: %s'%(self._(k), val))
+                        if current.has_key(k):
+                            cell[-1] += ' -> %s'%current[k]
+                            current[k] = val
+
                     elif not args[k]:
                         if current.has_key(k):
                             cell.append('%s: %s'%(self._(k), current[k]))
@@ -1561,7 +1568,10 @@ class PasswordHTMLProperty(HTMLProperty):
 
         if self._value is None:
             return ''
-        return self._('*encrypted*')
+        value = self._value.dummystr()
+        if escape:
+            value = cgi.escape(value)
+        return value
 
     def field(self, size=30, **kwargs):
         """ Render a form edit field for the property.

diff --git a/roundup/hyperdb.py b/roundup/hyperdb.py
index a9bb9e1550c5d35a3556280089158309aec371db..0ed036060d468d1a4fbd1336fb6db4c942cae431 100644 (file)
--- a/roundup/hyperdb.py
+++ b/roundup/hyperdb.py
@@ -951,7 +951,9 @@ class Class:
         'date' is a Timestamp object specifying the time of the change and
         'tag' is the journaltag specified when the database was opened.
         """
-        raise NotImplementedError
+        if not self.do_journal:
+            raise ValueError('Journalling is disabled for this class')
+        return self.db.getjournal(self.classname, nodeid)
 
     # Locating nodes:
     def hasnode(self, nodeid):
@@ -1309,7 +1311,7 @@ class Class:
                     elif isinstance(prop, Interval):
                         value = date.Interval(value)
                     elif isinstance(prop, Password):
-                        value = password.Password(encrypted=value)
+                        value = password.JournalPassword(encrypted=value)
                     params[propname] = value
             elif action == 'create' and params:
                 # old tracker with data stored in the create!

diff --git a/roundup/password.py b/roundup/password.py
index fcf8386bdc131afab8d096bd3d5fe6aff5d583a6..545baeb66690603808961c02c6617cc8ea84e03b 100644 (file)
--- a/roundup/password.py
+++ b/roundup/password.py
@@ -168,7 +168,49 @@ def generatePassword(length=8):
     chars = string.letters+string.digits
     return ''.join([random.choice(chars) for x in range(length)])
 
-class Password:
+class JournalPassword:
+    """ Password dummy instance intended for journal operation.
+        We do not store passwords in the journal any longer.  The dummy
+        version only reads the encryption scheme from the given
+        encrypted password.
+    """
+    default_scheme = 'PBKDF2'        # new encryptions use this scheme
+    pwre = re.compile(r'{(\w+)}(.+)')
+
+    def __init__ (self, encrypted=''):
+        if isinstance(encrypted, self.__class__):
+            self.scheme = encrypted.scheme or self.default_scheme
+        else:
+            m = self.pwre.match(encrypted)
+            if m:
+                self.scheme = m.group(1)
+            else:
+                self.scheme = self.default_scheme
+        self.password = ''
+
+    def dummystr(self):
+        """ return dummy string to store in journal
+            - reports scheme, but nothing else
+        """
+        return "{%s}*encrypted*" % (self.scheme,)
+
+    __str__ = dummystr
+
+    def __cmp__(self, other):
+        """Compare this password against another password."""
+        # check to see if we're comparing instances
+        if isinstance(other, self.__class__):
+            if self.scheme != other.scheme:
+                return cmp(self.scheme, other.scheme)
+            return cmp(self.password, other.password)
+
+        # assume password is plaintext
+        if self.password is None:
+            raise ValueError, 'Password not set'
+        return cmp(self.password, encodePassword(other, self.scheme,
+            self.password or None))
+
+class Password(JournalPassword):
     """The class encapsulates a Password property type value in the database.
 
     The encoding of the password is one if None, 'SHA', 'MD5' or 'plaintext'.
@@ -192,9 +234,7 @@ class Password:
     """
     #TODO: code to migrate from old password schemes.
 
-    default_scheme = 'PBKDF2'        # new encryptions use this scheme
     known_schemes = [ "PBKDF2", "SHA", "MD5", "crypt", "plaintext" ]
-    pwre = re.compile(r'{(\w+)}(.+)')
 
     def __init__(self, plaintext=None, scheme=None, encrypted=None, strict=False):
         """Call setPassword if plaintext is not None."""
@@ -232,20 +272,6 @@ class Password:
         self.password = encodePassword(plaintext, scheme)
         self.plaintext = plaintext
 
-    def __cmp__(self, other):
-        """Compare this password against another password."""
-        # check to see if we're comparing instances
-        if isinstance(other, Password):
-            if self.scheme != other.scheme:
-                return cmp(self.scheme, other.scheme)
-            return cmp(self.password, other.password)
-
-        # assume password is plaintext
-        if self.password is None:
-            raise ValueError, 'Password not set'
-        return cmp(self.password, encodePassword(other, self.scheme,
-            self.password))
-
     def __str__(self):
         """Stringify the encrypted password for database storage."""
         if self.password is None: