[gen] Security: added missing checks at the code level, ensuring that a user can create instances of a given class (root classes, or instances created via an initiator field); bugfixes in the test system, which works again (was broken after deplonization); [shared] XmlUnmarshaller can now be ran in 'non utf-8' mode: if enabled, any marshalled string will no be Python unicode, but simple str.

gen/__init__.py

@@ -1990,9 +1990,8 @@ class Ref(Type):
         res.select = None # Not callable from tool.
         return res
 
-    def mayAdd(self, obj, folder):
+    def mayAdd(self, obj):
-        '''May the user create a new referred object to p_obj via this Ref,
+        '''May the user create a new referred object from p_obj via this Ref?'''
-           in p_folder?'''
         # We can't (yet) do that on back references.
         if self.isBack: return
         # Check if this Ref is addable
@@ -2007,13 +2006,21 @@ class Ref(Type):
             if refCount >= self.multiplicity[1]: return
         # May the user edit this Ref field?
         if not obj.allows(self.writePermission): return
-        # Have the user the correct add permission on p_folder?
+        # Have the user the correct add permission?
         tool = obj.getTool()
         addPermission = '%s: Add %s' % (tool.getAppName(),
                                         tool.getPortalType(self.klass))
+        folder = obj.getCreateFolder()
         if not obj.getUser().has_permission(addPermission, folder): return
         return True
 
+    def checkAdd(self, obj):
+        '''Compute m_mayAdd above, and raise an Unauthorized exception if
+           m_mayAdd returns False.'''
+        if not self.mayAdd(obj):
+            from AccessControl import Unauthorized
+            raise Unauthorized("User can't write Ref field '%s'." % self.name)
+
 class Computed(Type):
     def __init__(self, validator=None, multiplicity=(0,1), index=None,
                  default=None, optional=False, editDefault=False, show='view',

gen/installer.py

@@ -214,9 +214,15 @@ class ZopeInstaller:
         zopeContent = self.app.objectIds()
         from OFS.Folder import manage_addFolder
 
+        if 'config' not in zopeContent:
+            toolName = '%sTool' % self.productName
+            createObject(self.app, 'config', toolName, self.productName,
+                         wf=False, noSecurity=True)
+
         if 'data' not in zopeContent:
             manage_addFolder(self.app, 'data')
             data = self.app.data
+            tool = self.app.config
             # Manager has been granted Add permissions for all root classes.
             # This may not be desired, so remove this.
             for className in self.config.rootClasses:
@@ -230,15 +236,12 @@ class ZopeInstaller:
                 if not klass.__dict__.has_key('root') or \
                    not klass.__dict__['root']:
                     continue # It is not a root class
-                creators = getattr(klass, 'creators', None)
-                if not creators: creators = self.config.defaultAddRoles
                 className = self.config.appClassNames[i]
+                wrapperClass = tool.getAppyClass(className, wrapper=True)
+                creators = wrapperClass.getCreators(self.config)
                 permission = self.getAddPermission(className)
                 updateRolesForPermission(permission, tuple(creators), data)
 
-        if 'config' not in zopeContent:
-            toolName = '%sTool' % self.productName
-            createObject(self.app, 'config', toolName,self.productName,wf=False)
         # Remove some default objects created by Zope but not useful to Appy
         for name in ('standard_html_footer', 'standard_html_header',\
                      'standard_template.pt'):
@@ -261,15 +264,15 @@ class ZopeInstaller:
             # may still be in the way for migration purposes.
             users = ('admin',) # We suppose there is at least a user.
         if not users:
-            appyTool.create('users', login='admin', password1='admin',
+            appyTool.create('users', noSecurity=True, login='admin',
-                            password2='admin',
+                            password1='admin', password2='admin',
                             email='admin@appyframework.org', roles=['Manager'])
             appyTool.log('Admin user "admin" created.')
 
         # Create group "admins" if it does not exist
         if not appyTool.count('Group', noSecurity=True, login='admins'):
-            appyTool.create('groups', login='admins', title='Administrators',
+            appyTool.create('groups', noSecurity=True, login='admins',
-                            roles=['Manager'])
+                            title='Administrators', roles=['Manager'])
             appyTool.log('Group "admins" created.')
 
         # Create a group for every global role defined in the application
@@ -277,8 +280,8 @@ class ZopeInstaller:
             relatedGroup = '%s_group' % role
             if appyTool.count('Group', noSecurity=True, login=relatedGroup):
                 continue
-            appyTool.create('groups', login=relatedGroup, title=relatedGroup,
+            appyTool.create('groups', noSecurity=True, login=relatedGroup,
-                            roles=[role])
+                            title=relatedGroup, roles=[role])
             appyTool.log('Group "%s", related to global role "%s", was ' \
                          'created.' % (relatedGroup, role))
 
@@ -320,7 +323,8 @@ class ZopeInstaller:
                 title = '%s (%s)' % (langEn, langNat)
             else:
                 title = langEn
-            appyTool.create('translations', id=language, title=title)
+            appyTool.create('translations', noSecurity=True,
+                            id=language, title=title)
             appyTool.log('Translation object created for "%s".' % language)
         # Now, we synchronise every Translation object with the corresponding
         # "po" file on disk.

gen/mixins/TestMixin.py

@@ -1,14 +1,14 @@
 # ------------------------------------------------------------------------------
 import os, os.path, sys
+try:
+    from AccessControl.SecurityManagement import \
+         newSecurityManager, noSecurityManager
+except ImportError:
+    pass
 
 # ------------------------------------------------------------------------------
 class TestMixin:
     '''This class is mixed in with any ZopeTestCase.'''
-    def createUser(self, userId, roles):
-        '''Creates a user with id p_userId with some p_roles.'''
-        self.acl_users.addMember(userId, 'password', [], [])
-        self.setRoles(roles, name=userId)
-
     def changeUser(self, userId):
         '''Logs out currently logged user and logs in p_loginName.'''
         self.logout()
@@ -55,6 +55,16 @@ class TestMixin:
                 return arg[10:].strip(']')
         return None
 
+    def login(self, name='admin'):
+        user = self.app.acl_users.getUserById(name)
+        newSecurityManager(None, user)
+
+    def logout(self):
+        '''Logs out.'''
+        noSecurityManager()
+
+    def _setup(self): pass
+
 # Functions executed before and after every test -------------------------------
 def beforeTest(test):
     '''Is executed before every test.'''
@@ -64,7 +74,6 @@ def beforeTest(test):
     g['appFolder'] = cfg.diskFolder
     moduleOrClassName = g['test'].name # Not used yet.
     # Initialize the test
-    test.createUser('admin', ('Member','Manager'))
     test.login('admin')
     g['t'] = g['test']
 

gen/mixins/ToolMixin.py

@@ -898,11 +898,16 @@ class ToolMixin(BaseMixin):
         userId = self.getUser().getId()
         # Perform the logout in acl_users
         rq.RESPONSE.expireCookie('__ac', path='/')
-        # Invalidate existing sessions.
+        # Invalidate session.
-        sdm = self.session_data_manager
+        try:
-        session = sdm.getSessionData(create=0)
+            sdm = self.session_data_manager
-        if session is not None:
+        except AttributeError, ae:
-            session.invalidate()
+            # When ran in test mode, session_data_manager is not there.
+            sdm = None
+        if sdm:
+            session = sdm.getSessionData(create=0)
+            if session is not None:
+                session.invalidate()
         self.log('User "%s" has been logged out.' % userId)
         # Remove user from variable "loggedUsers"
         from appy.gen.installer import loggedUsers

gen/mixins/__init__.py

@@ -25,6 +25,20 @@ class BaseMixin:
         return self
     o = property(get_o)
 
+    def getInitiatorInfo(self):
+        '''Gets information about a potential initiator object from the request.
+           Returns a 3-tuple (initiator, pageName, field):
+           * initiator is the initiator (Zope) object;
+           * pageName is the page on the initiator where the origin of the Ref
+             field lies;
+           * field is the Ref instance.
+        '''
+        if not rq.get('nav', '').startswith('ref.'): return (None,)*4
+        splitted = rq['nav'].split('.')
+        initiator = self.tool.getObject(splitted[1])
+        fieldName, page = splitted[2].split(':')
+        return (initiator, page, self.getAppyType(fieldName))
+
     def createOrUpdate(self, created, values,
                        initiator=None, initiatorField=None):
         '''This method creates (if p_created is True) or updates an object.
@@ -43,10 +57,9 @@ class BaseMixin:
             if not initiator:
                 folder = tool.getPath('/data')
             else:
-                if initiator.isPrincipiaFolderish:
+                folder = initiator.getCreateFolder()
-                    folder = initiator
+                # Check that the user can add objects through this Ref.
-                else:
+                initiatorField.checkAdd(initiator)
-                    folder = initiator.getParentNode()
             obj = createObject(folder, id, obj.portal_type, tool.getAppName())
         previousData = None
         if not created: previousData = obj.rememberPreviousData()
@@ -61,7 +74,7 @@ class BaseMixin:
             obj.historizeData(previousData)
 
         # Manage potential link with an initiator object
-        if created and initiator: initiator.appy().link(initiatorField, obj)
+        if created and initiator: initiator.appy().link(initiatorField.name,obj)
 
         # Manage "add" permissions and reindex the object
         obj._appy_managePermissions()
@@ -112,13 +125,16 @@ class BaseMixin:
         # Create the params to add to the URL we will redirect the user to
         # create the object.
         urlParams = {'mode':'edit', 'page':'main', 'nav':''}
-        if rq.get('nav', None):
+        initiator, initiatorPage, initiatorField = self.getInitiatorInfo()
+        if initiator:
             # The object to create will be linked to an initiator object through
-            # a ref field. We create here a new navigation string with one more
+            # a Ref field. We create here a new navigation string with one more
             # item, that will be the currently created item.
             splitted = rq.get('nav').split('.')
             splitted[-1] = splitted[-2] = str(int(splitted[-1])+1)
             urlParams['nav'] = '.'.join(splitted)
+            # Check that the user can add objects through this Ref field
+            initiatiorField.checkAdd(initiator)
         # Create a temp object in /temp_folder
         tool = self.getTool()
         id = tool.generateUid(className)
@@ -188,13 +204,7 @@ class BaseMixin:
         errorMessage = 'Please correct the indicated errors.' # XXX Translate
         isNew = rq.get('is_new') == 'True'
         # If this object is created from an initiator, get info about him.
-        initiator = None
+        initiator, initiatorPage, initiatorField = self.getInitiatorInfo()
-        initiatorPage = None
-        initiatorField = None
-        if rq.get('nav', '').startswith('ref.'):
-            splitted = rq['nav'].split('.')
-            initiator = tool.getObject(splitted[1])
-            initiatorField, initiatorPage = splitted[2].split(':')
         # If the user clicked on 'Cancel', go back to the previous page.
         if rq.get('buttonCancel.x', None):
             if initiator:
@@ -434,6 +444,14 @@ class BaseMixin:
         # broken on returned object.
         return getattr(self, methodName, None)
 
+    def getCreateFolder(self):
+        '''When an object must be created from this one through a Ref field, we
+           must know where to put the newly create object: within this one if it
+           is folderish, besides this one in its parent else.
+        '''
+        if self.isPrincipiaFolderish: return self
+        return self.getParentNode()
+
     def getFieldValue(self, name, onlyIfSync=False, layoutType=None,
                       outerValue=None):
         '''Returns the database value of field named p_name for p_self.
@@ -534,10 +552,10 @@ class BaseMixin:
         if not refs: raise IndexError()
         return refs.index(obj.UID())
 
-    def mayAddReference(self, name, folder):
+    def mayAddReference(self, name):
         '''May the user add references via Ref field named p_name in
            p_folder?'''
-        return self.getAppyType(name).mayAdd(self, folder)
+        return self.getAppyType(name).mayAdd(self)
 
     def isDebug(self):
         '''Are we in debug mode ?'''
@@ -1227,9 +1245,7 @@ class BaseMixin:
            Ref fields; if it is not a folder, we must update permissions on its
            parent folder instead.'''
         # Determine on which folder we need to set "add" permissions
-        folder = self
+        folder = self.getCreateFolder()
-        if not self.isPrincipiaFolderish:
-            folder = self.getParentNode()
         # On this folder, set "add" permissions for every content type that will
         # be created through reference fields
         allCreators = {} # One key for every add permission
@@ -1238,12 +1254,12 @@ class BaseMixin:
             if appyType.type != 'Ref': continue
             if appyType.isBack or appyType.link: continue
             # Indeed, no possibility to create objects with such Refs
-            refType = self.getTool().getPortalType(appyType.klass)
+            tool = self.getTool()
+            refType = tool.getPortalType(appyType.klass)
             if refType not in addPermissions: continue
             # Get roles that may add this content type
-            creators = getattr(appyType.klass, 'creators', None)
+            appyWrapper = tool.getAppyClass(refType, wrapper=True)
-            if not creators:
+            creators = appyWrapper.getCreators(self.getProductConfig())
-                creators = self.getProductConfig().defaultAddRoles
             # Add those creators to the list of creators for this meta_type
             addPermission = addPermissions[refType]
             if addPermission in allCreators:

gen/templates/testAll.pyt

@@ -9,7 +9,7 @@ from appy.gen.mixins.TestMixin import TestMixin, beforeTest, afterTest
 # Initialize the Zope test system ----------------------------------------------
 ZopeTestCase.installProduct('<!applicationName!>')
 
-class Test(ZopeTestCase.ZopeTestCase, TestMixin):
+class Test(TestMixin, ZopeTestCase.ZopeTestCase):
     '''Base test class for <!applicationName!> test cases.'''
 
 # Data needed for defining the tests -------------------------------------------

gen/ui/appy.js

@@ -294,7 +294,8 @@ function initSlaves() {
   while (i >= 0) {
     masterName = getSlaveInfo(slaves[i], 'masterName');
     master = document.getElementById(masterName);
-    updateSlaves(master, slaves[i]);
+    // If master is not here, we can't hide its slaves when appropriate.
+    if (master) updateSlaves(master, slaves[i]);
     i -= 1;
   }
 }

gen/ui/widgets/ref.pt

@@ -124,10 +124,10 @@
                  objs refObjects/objects;
                  totalNumber refObjects/totalNumber;
                  batchSize refObjects/batchSize;
-                 folder python: contextObj.isPrincipiaFolderish and contextObj or contextObj.getParentNode();
+                 folder contextObj/getCreateFolder;
                  linkedPortalType python: tool.getPortalType(appyType['klass']);
                  canWrite python: not appyType['isBack'] and contextObj.allows(appyType['writePermission']);
-                 showPlusIcon python: contextObj.mayAddReference(fieldName, folder);
+                 showPlusIcon python: contextObj.mayAddReference(fieldName);
                  atMostOneRef python: (appyType['multiplicity'][1] == 1) and (len(objs)<=1);
                  addConfirmMsg python: appyType['addConfirm'] and _('%s_addConfirm' % appyType['labelId']) or '';
                  navBaseCall python: 'askRefField(\'%s\',\'%s\',\'%s\',\'%s\',**v**)' % (ajaxHookId, contextObj.absolute_url(), fieldName, innerRef)">

gen/utils.py

@@ -2,7 +2,7 @@
 import re, os, os.path
 
 # Function for creating a Zope object ------------------------------------------
-def createObject(folder, id, className, appName, wf=True):
+def createObject(folder, id, className, appName, wf=True, noSecurity=False):
     '''Creates, in p_folder, object with some p_id. Object will be an instance
        of p_className from application p_appName. In a very special case (the
        creation of the config object), computing workflow-related info is not
@@ -10,6 +10,24 @@ def createObject(folder, id, className, appName, wf=True):
        p_wf=False.'''
     exec 'from Products.%s.%s import %s as ZopeClass' % (appName, className,
                                                          className)
+    if not noSecurity:
+        # Check that the user can create objects of className
+        if folder.meta_type.endswith('Folder'): # Folder or temp folder.
+            tool = folder.config
+        else:
+            tool = folder.getTool()
+        user = tool.getUser()
+        userRoles = user.getRoles()
+        allowedRoles=ZopeClass.wrapperClass.getCreators(tool.getProductConfig())
+        allowed = False
+        for role in userRoles:
+            if role in allowedRoles:
+                allowed = True
+                break
+        if not allowed:
+            from AccessControl import Unauthorized
+            raise Unauthorized("User can't create instances of %s" % \
+                               ZopeClass.__name__)
     obj = ZopeClass(id)
     folder._objects = folder._objects + \
                       ({'id':id, 'meta_type':className},)

gen/wrappers/__init__.py

@@ -22,8 +22,46 @@ FREEZE_FATAL_ERROR = 'A server error occurred. Please contact the system ' \
 
 # ------------------------------------------------------------------------------
 class AbstractWrapper(object):
-    '''Any real Zope object has a companion object that is an instance of this
+    '''Any real Appy-managed Zope object has a companion object that is an
-       class.'''
+       instance of this class.'''
+
+    # --------------------------------------------------------------------------
+    # Class methods
+    # --------------------------------------------------------------------------
+    @classmethod
+    def _getParentAttr(klass, attr):
+        '''Gets value of p_attr on p_klass base classes (if this attr exists).
+           Scan base classes in the reverse order as Python does. Used by
+           classmethods m_getWorkflow and m_getCreators below. Scanning base
+           classes in reverse order allows user-defined elements to override
+           default Appy elements.'''
+        i = len(klass.__bases__) - 1
+        res = None
+        while i >= 0:
+            res = getattr(klass.__bases__[i], attr, None)
+            if res: return res
+            i -= 1
+
+    @classmethod
+    def getWorkflow(klass):
+        '''Returns the workflow tied to p_klass.'''
+        res = klass._getParentAttr('workflow')
+        # Return a default workflow if no workflow was found.
+        if not res: res = WorkflowAnonymous
+        return res
+
+    @classmethod
+    def getCreators(klass, cfg):
+        '''Returns the roles that are allowed to create instances of p_klass.
+           p_cfg is the product config that holds the default value.'''
+        res = klass._getParentAttr('creators')
+        # Return default creators if no creators was found.
+        if not res: res = cfg.defaultAddRoles
+        return res
+
+    # --------------------------------------------------------------------------
+    # Instance methods
+    # --------------------------------------------------------------------------
     def __init__(self, o): self.__dict__['o'] = o
     def appy(self): return self
 
@@ -87,21 +125,6 @@ class AbstractWrapper(object):
                 return customUser.__dict__[methodName](self, *args, **kwargs)
 
     def getField(self, name): return self.o.getAppyType(name)
-    @classmethod
-    def getWorkflow(klass):
-        '''Returns the workflow tied to p_klass.'''
-        # Browse parent classes of p_klass in reverse order. This way, a
-        # user-defined workflow will override a Appy default workflow.
-        i = len(klass.__bases__)-1
-        res = None
-        while i >= 0:
-            res = getattr(klass.__bases__[i], 'workflow', None)
-            if res: break
-            i -= 1
-        # Return a default workflow if no workflow was found.
-        if not res:
-            res = WorkflowAnonymous
-        return res
 
     def link(self, fieldName, obj):
         '''This method links p_obj (which can be a list of objects) to this one
@@ -124,7 +147,7 @@ class AbstractWrapper(object):
                                   getattr(tool.getObject(y), sortKey)))
         if reverse: refs.reverse()
 
-    def create(self, fieldNameOrClass, **kwargs):
+    def create(self, fieldNameOrClass, noSecurity=False, **kwargs):
         '''If p_fieldNameOrClass is the name of a field, this method allows to
            create an object and link it to the current one (self) through
            reference field named p_fieldName.
@@ -156,12 +179,13 @@ class AbstractWrapper(object):
         if not isField:
             folder = tool.getPath('/data')
         else:
-            if hasattr(self, 'folder') and self.folder:
+            folder = self.o.getCreateFolder()
-                folder = self.o
+            if not noSecurity:
-            else:
+                # Check that the user can edit this field.
-                folder = self.o.getParentNode()
+                appyType.checkAdd(self.o)
         # Create the object
-        zopeObj = createObject(folder, objId,portalType, tool.getAppName())
+        zopeObj = createObject(folder, objId, portalType, tool.getAppName(),
+                               noSecurity=noSecurity)
         appyObj = zopeObj.appy()
         # Set object attributes
         for attrName, attrValue in kwargs.iteritems():

shared/xml_parser.py

@@ -216,7 +216,8 @@ class XmlUnmarshaller(XmlParser):
        If "object" is specified, it means that the tag contains sub-tags, each
        one corresponding to the value of an attribute for this object.
        if "tuple" is specified, it will be converted to a list.'''
-    def __init__(self, classes={}, tagTypes={}, conversionFunctions={}):
+    def __init__(self, classes={}, tagTypes={}, conversionFunctions={},
+                 utf8=True):
         XmlParser.__init__(self)
         # self.classes below is a dict whose keys are tag names and values are
         # Python classes. During the unmarshalling process, when an object is
@@ -253,6 +254,7 @@ class XmlUnmarshaller(XmlParser):
         # for example convert strings that have specific values (in this case,
         # knowing that the value is a 'string' is not sufficient).        
         self.conversionFunctions = conversionFunctions
+        self.utf8 = utf8
 
     def convertAttrs(self, attrs):
         '''Converts XML attrs to a dict.'''
@@ -360,6 +362,8 @@ class XmlUnmarshaller(XmlParser):
                 setattr(currentContainer, name, attrValue)
 
     def characters(self, content):
+        if not self.utf8:
+            content = content.encode('utf-8')
         e = XmlParser.characters(self, content)
         if e.currentBasicType:
             e.currentContent += content