From b4e63334728e4e33fed9291248167fe1d681bde8 Mon Sep 17 00:00:00 2001 From: Gaetan Delannay Date: Wed, 14 May 2014 15:10:41 +0200 Subject: [PATCH] [gen] Bugfix: slave groups; bugfix: security check for pod fields; security bugfix for pod fields: write access to the field is required for performing any freeze-related action; migration to Appy 0.9.0: dump frozen pod fields on disk; repaired test system for Appy 0.9.0; more explicit error message when using, on some field, a specific write or read permission that is not used in the workflow. --- fields/file.py | 1 + fields/group.py | 6 ++-- fields/pod.py | 10 ++++--- gen/migrator.py | 56 ++++++++++++++++++++++++++++--------- gen/mixins/TestMixin.py | 17 +---------- gen/mixins/__init__.py | 5 ++++ gen/ui/appy.css | 4 +-- gen/wrappers/ToolWrapper.py | 4 +++ gen/wrappers/__init__.py | 2 +- 9 files changed, 66 insertions(+), 39 deletions(-) diff --git a/fields/file.py b/fields/file.py index 263058f..657b90e 100644 --- a/fields/file.py +++ b/fields/file.py @@ -393,6 +393,7 @@ class File(Field): cfg = zobj.getProductConfig() if isinstance(value, cfg.FileUpload) or isinstance(value, cfg.File): # Cases a, b + value.filename = value.filename.replace('/', '-') info.writeFile(self.name, value, dbFolder) elif isinstance(value, UnmarshalledFile): # Case c diff --git a/fields/group.py b/fields/group.py index 828a5be..2e205ad 100644 --- a/fields/group.py +++ b/fields/group.py @@ -231,12 +231,12 @@ class UiGroup: # PX that renders a group of fields (the group is refered as var "field"). pxView = Px(''' - + tagId='%s_%s' % (zobj.id, field.name)">
diff --git a/fields/pod.py b/fields/pod.py index 6e5e8e7..8438003 100644 --- a/fields/pod.py +++ b/fields/pod.py @@ -278,9 +278,9 @@ class Pod(Field): template = template or self.template[0] format = format or 'odt' # Security check. - if not noSecurity and not queryData and \ - not self.showTemplate(obj, template): - raise Exception(self.UNAUTHORIZED) + if not noSecurity and not queryData: + if self.showTemplate and not self.showTemplate(obj, template): + raise Exception(self.UNAUTHORIZED) # Return the possibly frozen document (not applicable for query-related # pods). if not queryData: @@ -445,7 +445,9 @@ class Pod(Field): def getFreezeFormats(self, obj, template=None): '''What are the formats into which the current user may freeze p_template?''' - # Manager can always perform freeze actions. + # One may have the right to edit the field to freeze anything in it. + if not obj.o.mayEdit(self.writePermission): return () + # Manager can perform all freeze actions. template = template or self.template[0] isManager = obj.user.has_role('Manager') if isManager: return self.getAllFormats(template) diff --git a/gen/migrator.py b/gen/migrator.py index 3f69a64..e4954c5 100644 --- a/gen/migrator.py +++ b/gen/migrator.py @@ -1,6 +1,7 @@ # ------------------------------------------------------------------------------ -import time +import os.path, time from appy.fields.file import FileInfo +from appy.shared import utils as sutils # ------------------------------------------------------------------------------ class Migrator: @@ -13,29 +14,58 @@ class Migrator: self.tool = self.app.config.appy() @staticmethod - def migrateFileFields(obj): - '''Ensures all file fields on p_obj are FileInfo instances.''' + def migrateBinaryFields(obj): + '''Ensures all file and frozen pod fields on p_obj are FileInfo + instances.''' migrated = 0 # Count the number of migrated fields for field in obj.fields: - if field.type != 'File': continue - oldValue = getattr(obj, field.name) - if oldValue and not isinstance(oldValue, FileInfo): - # A legacy File object. Convert it to a FileInfo instance and - # extract the binary to the filesystem. - setattr(obj, field.name, oldValue) - migrated += 1 + if field.type == 'File': + oldValue = getattr(obj, field.name) + if oldValue and not isinstance(oldValue, FileInfo): + # A legacy File object. Convert it to a FileInfo instance + # and extract the binary to the filesystem. + setattr(obj, field.name, oldValue) + migrated += 1 + elif field.type == 'Pod': + frozen = getattr(obj.o, field.name, None) + if frozen: + # Dump this file on disk. + tempFolder = sutils.getOsTempFolder() + fmt = os.path.splitext(frozen.filename)[1][1:] + fileName = os.path.join(tempFolder, + '%f.%s' % (time.time(), fmt)) + f = file(fileName, 'wb') + if frozen.data.__class__.__name__ == 'Pdata': + # The file content is splitted in several chunks. + f.write(frozen.data.data) + nextPart = frozen.data.next + while nextPart: + f.write(nextPart.data) + nextPart = nextPart.next + else: + # Only one chunk + f.write(frozen.data) + f.close() + f = file(fileName) + field.freeze(obj, template=field.template[0], format=fmt, + noSecurity=True, upload=f, + freezeOdtOnError=False) + f.close() + # Remove the legacy in-zodb file object + setattr(obj.o, field.name, None) + migrated += 1 return migrated def migrateTo_0_9_0(self): '''Migrates this DB to Appy 0.9.x.''' # Put all binaries to the filesystem tool = self.tool - tool.log('Migrating file fields...') - context = {'migrate': self.migrateFileFields, 'nb': 0} + tool.log('Migrating binary fields...') + context = {'migrate': self.migrateBinaryFields, 'nb': 0} for className in tool.o.getAllClassNames(): tool.compute(className, context=context, noSecurity=True, expression="ctx['nb'] += ctx['migrate'](obj)") - tool.log('Migrated %d File field(s).' % context['nb']) + tool.log('Migrated %d binary field(s).' % context['nb']) def run(self, force=False): '''Executes a migration when relevant, or do it for sure if p_force is diff --git a/gen/mixins/TestMixin.py b/gen/mixins/TestMixin.py index 431936c..aefe1f2 100644 --- a/gen/mixins/TestMixin.py +++ b/gen/mixins/TestMixin.py @@ -9,11 +9,6 @@ except ImportError: # ------------------------------------------------------------------------------ class TestMixin: '''This class is mixed in with any ZopeTestCase.''' - def changeUser(self, userId): - '''Logs out currently logged user and logs in p_loginName.''' - self.logout() - self.login(userId) - def getNonEmptySubModules(self, moduleName): '''Returns the list of sub-modules of p_app that are non-empty.''' res = [] @@ -53,16 +48,7 @@ class TestMixin: for arg in sys.argv: if arg.startswith('[coverage'): 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() - + return def _setup(self): pass # Functions executed before and after every test ------------------------------- @@ -74,7 +60,6 @@ def beforeTest(test): g['appFolder'] = cfg.diskFolder moduleOrClassName = g['test'].name # Not used yet. # Initialize the test - test.login('admin') g['t'] = g['test'] def afterTest(test): diff --git a/gen/mixins/__init__.py b/gen/mixins/__init__.py index 846080b..e136851 100644 --- a/gen/mixins/__init__.py +++ b/gen/mixins/__init__.py @@ -1201,6 +1201,11 @@ class BaseMixin: '''Gets, according to the workflow, the roles that are currently granted p_permission on this object.''' state = self.State(name=False) + if permission not in state.permissions: + wf = self.getWorkflow().__name__ + raise Exception('Permission "%s" not in permissions dict for ' \ + 'state %s.%s' % \ + (permission, wf, self.State(name=True))) roles = state.permissions[permission] if roles: return [role.name for role in roles] return () diff --git a/gen/ui/appy.css b/gen/ui/appy.css index 0989f2f..f229f9c 100644 --- a/gen/ui/appy.css +++ b/gen/ui/appy.css @@ -57,8 +57,8 @@ img { border: 0; vertical-align: middle } .userStrip a:visited { color: #e7e7e7 } .breadcrumb { font-size: 11pt; padding-bottom: 6px } .login { margin: 3px; color: black } -input.button { color: #666666; height: 20px; width: 130px; - cursor:pointer; font-size: 90%; padding: 1px 0 0 10px; +input.button { color: #666666; height: 20px; width: 130px; margin-bottom: 5px; + cursor:pointer; font-size: 90%; padding-left: 10px; background-color: white; background-repeat: no-repeat; background-position: 5% 25%; box-shadow: 2px 2px 2px #888888} input.buttonSmall { width: 100px !important; font-size: 85%; height: 18px; diff --git a/gen/wrappers/ToolWrapper.py b/gen/wrappers/ToolWrapper.py index 7f63a1b..71126c0 100644 --- a/gen/wrappers/ToolWrapper.py +++ b/gen/wrappers/ToolWrapper.py @@ -647,4 +647,8 @@ class ToolWrapper(AbstractWrapper): except Exception, e: failed.append(startObject) return nb, failed + + def _login(self, login): + '''Performs a login programmatically. Used by the test system.''' + self.request.user = self.search1('User', noSecurity=True, login=login) # ------------------------------------------------------------------------------ diff --git a/gen/wrappers/__init__.py b/gen/wrappers/__init__.py index 7db3da3..2fdedd6 100644 --- a/gen/wrappers/__init__.py +++ b/gen/wrappers/__init__.py @@ -730,7 +730,7 @@ class AbstractWrapper(object): return res def __repr__(self): - return '<%s appyobj at %s>' % (self.klass.__name__, id(self)) + return '<%s at %s>' % (self.klass.__name__, id(self)) def __cmp__(self, other): if other: return cmp(self.o, other.o)