Commit 8efdd9f5 authored by Kazuhiko Shiozaki's avatar Kazuhiko Shiozaki

erp5_base: fix conditino of create_user_action.

This transition is now available when Person has no validated Login document.
Person's reference is now set only when it is not yet set.
parent caefbddd
kwargs = state_change['kwargs']
person = state_change['object']
person.edit(reference=kwargs['reference'])
if not person.hasReference():
person.edit(reference=kwargs['reference'])
person.newContent(
portal_type='ERP5 Login',
password=kwargs['password'],
......
......@@ -88,7 +88,7 @@
<dictionary>
<item>
<key> <string>text</string> </key>
<value> <string>python:not here.getReference()</string> </value>
<value> <string>python:not [x for x in here.objectValues(portal_type=here.getPortalObject().getPortalLoginTypeList()) if x.getValidationState() == \'validated\']</string> </value>
</item>
</dictionary>
</pickle>
......
  • @jerome, @vpelletier wIth the latter change, I see the following in the log in my project :

      Module Products.PageTemplates.ZRPythonExpr, line 48, in __call__
       - __traceback_info__: here.Base_filterDuplicateActions(portal.portal_actions.listFilteredActionsFor(action_context))
        return eval(self._code, vars, {})
      Module PythonExpr, line 1, in <expression>
    
      Module Products.ERP5Type.patches.ActionsTool, line 72, in listFilteredActionsFor
        actions.extend( provider.listActionInfos(object=object) )
      Module Products.CMFCore.ActionProviderBase, line 96, in listActionInfos
        actions = self.listActions(object=object)
      Module Products.ERP5Type.patches.WorkflowTool, line 472, in WorkflowTool_listActions
        a = wf.listObjectActions(info)
      Module Products.ERP5Type.patches.DCWorkflow, line 237, in DCWorkflowDefinition_listObjectActions
        tdef.actbox_name and self._checkTransitionGuard(tdef, ob):
      Module Products.DCWorkflow.DCWorkflow, line 445, in _checkTransitionGuard
        if guard.check(getSecurityManager(), self, ob, **kw):
      Module Products.ERP5Type.patches.DCWorkflow, line 831, in Guard_check
        res = expr(econtext)
      Module Products.CMFCore.Expression, line 47, in __call__
        res = compiled(econtext)
      Module Products.PageTemplates.ZRPythonExpr, line 48, in __call__
       - __traceback_info__: not [x for x in here.objectValues(portal_type=here.getPortalObject().getPortalLoginTypeList()) if x.getValidationState() == 'validated']
        return eval(self._code, vars, {})
      Module PythonExpr, line 1, in <expression>
    
      Module AccessControl.ZopeGuards, line 197, in next
        guard(self.container, ob)
      Module AccessControl.ZopeGuards, line 227, in guard
        if getSecurityManager().validate(container, container, index, value):
    Unauthorized: You are not allowed to access '3' in this context

    Here, the user is Auditor on Person but has no role on ERP5 Login. What do you think the right way to cover this ?

    • make a proxy role script to check this TALES content and add proxy roles there ?
    • remove TALES condition so that we can always create a new Login with this action ?
    • any idea else ?
  • Here, the user is Auditor on Person but has no role on ERP5 Login. What do you think the right way to cover this ?

    Can't we just use contentValues(checked_permission='Access contents infomation') in that condition ?

    If user cannot access existing login, the role condition on this transition should prevent from activating login.

  • Is it a feature that one having Auditor on Person does not have on ERP5 Login (I can userstand why it would be, I just want confirmation it is really the intention) ?

    About the preferred way to solve this, I would say removing the TALES expression seems the most consistent fix here, as I think the intent is to really create a login, not a user.

    BTW, this change makes the action incompatible with single-login code. Is this intended ? Maybe we need:

    • a way to tell if site supports multi-login (python script checking whether a ERP5 Login User Manager is active in some plugin "personalities")

    • two mutually-exclusive actions, or maybe a single one which would check both old and new conditions ?

  • Is it a feature that one having Auditor on Person does not have on ERP5 Login

    Generally speaking, enabling 'acquire role' on ERP5 Login should be wrong, I believe. But in reality, we propably need quite similar configuration on Person and ERP5 Login.

    About the preferred way to solve this, I would say removing the TALES expression seems the most consistent fix here, as I think the intent is to really create a login, not a user.

    I prefer this idea too.

    For compatibility, all places except ERP5UserManager itself are now implemented for ERP5LoginUserManager only, like PasswordTool, erp5_credential etc. Do we need to care the compatibility on this action ?

  • Do we need to care the compatibility on this action ?

    I would prefer to have compatibility. I think without it the change will either be rejected or will come back to bite us each time one upgrade their production and get (maybe subtle) issues.

  • BTW, this change makes the action incompatible with single-login code

    My understanding is that in this merge request we do not try to keep compatibility with old style login directly with person.

Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment