This concept describes the current state of permission-based configuration - or lack thereof - for enabling/disabling actions (or any other UI element) based on specific access restrictions.

Decisions are marked with a (tick) icon.

Concept is ready for implementation, both for 5.2.x and for a future major version.

 

Problem

One can restrict (action) availability based on user roles, but not based on user permissions at given workspace / path.

Concrete case

As of Magnolia 5.2.1, actions are not disabled if user has no permission to act on selected node. This is an issue with e.g. read-only pages, as captured by the following Jira ticket:

MGNLUI-2510 - Getting issue details... STATUS

Current configuration

Let's take the deactivation action as an example, with following base path /modules/pages/apps/pages/subApps/browser/actions.

  • actions
    • deactivate ( ActionDefinition )
      • availability ( AvailabilityDefinition )
        • access (
          $webResourceManager.requireResource("info.magnolia.sys.confluence.artifact-info-plugin:javadoc-resource-macro-resources") AccessDefinition
           )
          • roles
            • demo-publisher = demo-project-publisher
            • superuser = superuser
        • ruleClass = info.magnolia.ui.api.availability.IsNotDeletedRule ( AvailabilityRule )
      • class = info.magnolia.ui.framework.action.DeactivationActionDefinition
      • ...

As a side note, the AccessDefinition property is named access in the case of actions but it is generally named permissions, as in AppDescriptor or AppLauncherGroupDefinition.
This has been reported to be misleading, in particular in documentation, and should be inlined whenever is appropriate.

 

Decisions

1. Configuring permission checks in availability
  • Add a requiredPermissions property under AvailabilityDefinition or AccessDefinition
    • comma separated list of JCR permissions (aka action strings)
      • add_node, set_property, remove, read
      • we should rather use Magnolia permissions
      • (minus) doesn't fit for upcoming custom permissions
    • naming is debatable (permissions > requiredPermissions)
  • (tick) For 5.2.x we add a writePermissionRequired boolean property under AvailabilityDefinition
    • simply checks for Magnolia Permission.WRITE permission
    • we add this to the availability evaluation sequence
      • as of 5.2.2 this is in AbstractActionExecutor (action availability) and in BrowserSubApp (section availability)
      • use PermissionUtil when processing
    • for custom permissions, people need to implement AvailabilityRule
    • (warning) After another round of reviews, we ultimately decided against using voters for availability
      • We found important to instantiate whatever criteria (voters or rules) on subapp scope
      • In order to make it possible to inject components in these rules - which are then also resolved on subapp scope
        • as opposed to voters which are instantiated by n2b on global webapp startup
        • similarly as we have now ruleClass configured in ActionAvailability, which is instantiated on the fly by subapp's componentProvider.
      • We did not want to revise voters or introduce voter definitions at this time
      • We also chose the flag approach for 5.2.x so that we don't introduce any new mechanism and leave the door open to finalize the proposal for 5.3.
  • (lightbulb) For 5.3 we now aim at improving use and flexibility of the AvailabilityRules.
    • by configuring a collection of such rules in AvailabilityDefinition, instead of one single ruleClass, so that we can compose multiple rules
    • by introducing an AvailabilityRuleDefinition to make these rules configurable
    • yet to be decided
  • We unify availability's access, ruleClass and other criteria using voters, in a future major version
    • supports custom permissions (forum), even non-JCR based, using dedicated voters
    • (question) Do we keep availability's "shorthands"?
      • nodeTypes, root, properties...
      • yet update underlying implementation to work with voters
      • Proposal: how about maintaining all the shorthands we have and also providing a rather smooth mechanism of moving from old impl to the new one by implementing a custom Node2BeanTransformerImpl
        that would build voters based on the properties from AvailabilityDefinition (e.g. once the property name is nodes - we generate a corresponding voter)?
  • For 5.2.x, we introduce a delegating AvailabilityRule which helps us already start working with voters

    • getting well prepared for migrating to the next approach

2. The add_node permission with subnodes-only ACLs
  • with /A readonly and /A/B/* read-write, add_node is not granted on /A/B
  • JCR spec is a bit unclear as to what absPath means in that case
    • adding a node at absPath VS. adding a node under absPath
  • 4.5 behaves the same in similar subnodes-only permissions
  • (info) Current behavior is actually correct against JCR permissions
    • add, move, reorder all require write permission on parent node
3. ActionExecutor is responsible for availability checks
  • Currently hooking in AbstractActionExecutor#isAvailableForItem
  • (warning) item is null when root is selected, no way to assess permissions then
  • #isAvailable() is (the sole) JCR Item dependent api in ActionExecutor interface and doesn't belong here
  • (info) We keep this as a separate topic, not for 5.2.x anyway
    • We may cover that for 5.3 (by e.g. having a single AvailabilityChecker component).

 

Forward thinking
  • Availability / AccessDefinition is a broad concept meant to be reused across several UI components (e.g. fields, tabs, templates).
  • Can one configure custom permissions for an action? e.g. forum moderator can only perform moderation at specific path
  • Can one plug basic permission rules for non-JCR datasources (no ACLs)?

  • ActionExecutor is probably not where availability / permission checks belong.

 

 

 

 

  • No labels

6 Comments

  1. Jan Haderka

    • with /A readonly and /A/B/* read-write, add_node is not granted on /A/B

    This is how JCR permission resolving works. Adding, reordering or removing a child is an operation on a parent node. In order to be able to add a child to /A/B, you need to have write permission on /A/B not just on /A/B/*.

  2. Jan Haderka

    as opposed to voters which are instantiated by n2b on global webapp startup

    This is not true. What is stopping you from actually calling n2b yourself to obtain voters from your voter chain and placing it in local scope? There is no global voter registry anywhere and there is no one global chain. There's completely independent configurations of voters in filter chain and for example in cache configuration and other places.

    • by configuring a collection of such rules in AvailabilityDefinition, instead of one single ruleClass, so that we can compose multiple rules

    So you are effectively reimplementing functionality of voters, just in limited scope and calling them differently? Apart from work to reimplement all this, you are also creating additional effort to document all of this and to train users to use it. I don't think that it is very effective solution.

    1. Mikaël Geljić

      This is not true. What is stopping you from actually calling n2b yourself to obtain voters from your voter chain and placing it in local scope? There is no global voter registry anywhere and there is no one global chain. There's completely independent configurations of voters in filter chain and for example in cache configuration and other places.

      Well that was actually our proposal when we presented to architecture group last Thursday. We also wanted to ditch the AvailabilityRule completely in favor of voters - along the lines of what we had discussed several weeks ago.

      Then it probably seemed voters would be harder to grasp and require more work ahead of 5.3, I might not relate the exact details of the discussion, but Philipp on the other hand was keen on using similar approach as for ActionDefs / Actions - that is injecting params / components in action constructor and call a no-arg evaluation. That may well be the part where our voter-based approach didn't seem like the right fit.

      Nevertheless, for 5.2.x we chose not to introduce a new mechanism (hence the writePermissionRequired flag) so that we keep flexibility to define/refine availability for 5.3 - although we also need to finalize our sprint this Thursday in a production-ready fashion.

  3. Jan Haderka

    I'm not per se against implementing something else and I can see how the initialisation of voters can be an issue for injection, just pointing out that is will require us to teach users yet another voting/rules mechanism and in order for this feature to satisfy ee clients it needs the flexibility of voters, so we will ultimately end up reimplementing them with just different initialisation (we need those rules to be able to vote on given param, but we also need to be able to group them and we need to be able to weight them so there is clear way on how they override each other). And on top of that, those rules should be configurable via admin interface, w/o having to write java class and/or modify the module descriptor for majority of the cases.

    Just few examples I recall of what our clients wants:

    • action X should be available to users (groups/roles) from Department A and Department B, but not if they are in group/role Temporary Employee
    • action X should be available to users (groups/roles) from Dept A and Dept B or to Power Users and Admins but only over the weekends
    • action X should be available to users (groups/roles) from Dept A and Dept B and to Temporary Employees, but to those only in "high season" (pre-Xmass, pre-easter, ...)

    etc. I can search my mailbox for few more use cases if you need more examples.

  4. Mikaël Geljić

    It's definitely good to have concrete and more advanced use cases in mind. I'm also starting to reconsider whether we should mix permissions vs. UI state (tree selection) here... Anyway I can suggest having a 'config workshop session' with Philipp and you, where we just focus on creating an 'ideal configuration'; this has worked well with Tobias on a couple occasions.

  5. Jan Haderka

    One more thing that just occurred to me - we are rewriting voters anyway because we are using them for personalisation so the issues constructor should be void as of 5.3