[MAGNOLIA-3147] MgnlUser, MgnlGroup, MgnlRole and their managers should not save implicitely OR it should be possible to give them a specific hierarchy manager OR Created: 18/Mar/10  Updated: 04/Nov/15  Resolved: 04/Nov/15

Status: Closed
Project: Magnolia
Component/s: core, modulemechanism, security
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major
Reporter: Magnolia International Assignee: Philipp Bärfuss
Resolution: Won't Do Votes: 0
Labels: installation, security
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File user_task_does_not_save.patch    
Issue Links:
supersession
supersedes MGNLSTK-608 installation: avoid harmless but alar... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:

 Description   

Several approaches to consider, not all of them necessarily exclusive

  • module mechanism could swap the context for the entire process such that only read-only HierarchyManager are available
  • InstallContext.getHierarchyManager and .get*Node() should provide read-only HMs and nodes
  • MgnlGroup and associate classes could use the current context (they currently use, in part the system context, see notes below)
  • the various addRole*, addGroup, addPermission tasks could also work around this (see attached patch for a draft/idea)

The classes of i.m.cms.security use a mixture of system and current context. They currently use this for 2 reasons: 1) bypass security; it's probably "ok" for reading, but not such a good idea for write method. In cases where this is needed, we now have the doInSystemContext approach which could help. 2) checking for "duplicates": when adding roles to groups or users, we do {{ String newName = Path.getUniqueLabel(sysHM, node.getHandle(), "0");}} to name the property into which the added group or role's uuid is saved and avoid confict. One possibility for this would be to simply use the uuid as the property name as well (would definitely avoid conflicts).



 Comments   
Comment by Jan Haderka [ 04/Dec/13 ]

I believe at least the title of this issue is outdated. Mgnl* classes no longer save anything even if they are backed by the node. It's the respective managers only that perform repo update. And indeed if this is undesirable, those managers are configurable and can be easily replaced by other implementations whenever necessary. so rather then being about those managers this issue is about update mechanism needing to replace default impls of said classes w/ something delegating session.save() op upon finishing the installation of module.

Comment by Michael Mühlebach [ 04/Nov/15 ]

Given the thousands of other issues we have open that are more highly requested, we won't be able to address this issue in the foreseeable future. Instead we will focus on issues with a higher impact, and more votes.
Thanks for taking the time to raise this issue. As you are no doubt aware this issue has been on our backlog for some time now with very little movement.
I'm going to close this to set expectations so the issue doesn't stay open for years with few updates. If the issue is still relevant please feel free to reopen it or create a new issue.

Generated at Mon Feb 12 03:43:39 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.