[MAGNOLIA-5870] MgnlUser class hardcodes 'users' workspace for all users - errors on content publishing Created: 05/Aug/14  Updated: 16/Aug/18  Resolved: 19/Sep/14

Status: Closed
Project: Magnolia
Component/s: security
Affects Version/s: 5.2.8, 5.3.1
Fix Version/s: 5.2.9, 5.3.4

Type: Bug Priority: Major
Reporter: Christian Ringele Assignee: Federico Grilli
Resolution: Fixed Votes: 0
Labels: support
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
causality
dependency
is depended upon by MGNLPUR-179 Split PUR module to separated workspa... Closed
relation
is related to MAGNOLIA-5866 MgnlUserManager#changePassword wrongl... Closed
is related to MAGNOLIA-5920 Deprecate methods in User interface w... 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)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

Magnolia assumes that the user is always existing in the users workspace.
But in some situations it is beneficial, if (public) users are stored in a different place: For example clustering the public users between all public nodes.

Solution/Improvement:
The workspace of the user should be providable by the (custom) UserManager.

From the customer:
The MgnlUser class (incorrectly) assumes that all users are always stored in the 'users' JCR workspace. Including public users. This is hardcoded in the 'private boolean hasAny' method:

final Collection<String> groupsOrRoles = MgnlContext.doInSystemContext(new SilentSessionOp<Collection<String>>(RepositoryConstants.USERS) {

However in the customers case they store our public users in a separate custom 'public users' workspace.

You will come across this once you try to publish content because then all users in the system are looped to see if they are in the 'publishers' group. As soon as the loop comes across one of our public users an error occurs since the code above attempts to retrieve the user from the 'users' workspace and not our 'public users' workspace:

2014-08-05 14:13:46,652 ERROR info.magnolia.cms.security.JCRSessionOp           : Failed to execute info.magnolia.cms.security.MgnlUser$1@17a9735a session operation with /public/sample-public-users/test-public-user
javax.jcr.PathNotFoundException: /public/sample-public-users/test-public-user
	at org.apache.jackrabbit.core.ItemManager.getNode(ItemManager.java:577)
	at org.apache.jackrabbit.core.session.SessionItemOperation$6.perform(SessionItemOperation.java:129)
	at org.apache.jackrabbit.core.session.SessionItemOperation$6.perform(SessionItemOperation.java:125)
	at org.apache.jackrabbit.core.session.SessionItemOperation.perform(SessionItemOperation.java:187)
	at org.apache.jackrabbit.core.session.SessionState.perform(SessionState.java:216)
	at org.apache.jackrabbit.core.SessionImpl.perform(SessionImpl.java:361)
	at org.apache.jackrabbit.core.SessionImpl.getNode(SessionImpl.java:1111)
	at info.magnolia.jcr.wrapper.DelegateSessionWrapper.getNode(DelegateSessionWrapper.java:177)
	at info.magnolia.jcr.wrapper.DelegateSessionWrapper.getNode(DelegateSessionWrapper.java:177)
	at info.magnolia.jcr.wrapper.NodeWrappingDelegateSessionWrapper.getNode(NodeWrappingDelegateSessionWrapper.java:53)
	at info.magnolia.jcr.wrapper.DelegateSessionWrapper.getNode(DelegateSessionWrapper.java:177)
	at info.magnolia.jcr.decoration.ContentDecoratorSessionWrapper.getNode(ContentDecoratorSessionWrapper.java:120)
	at info.magnolia.jcr.wrapper.DelegateSessionWrapper.getNode(DelegateSessionWrapper.java:177)
	at info.magnolia.jcr.decoration.ContentDecoratorSessionWrapper.getNode(ContentDecoratorSessionWrapper.java:120)
	at info.magnolia.cms.security.MgnlUser$1.doExec(MgnlUser.java:197)
	at info.magnolia.cms.security.MgnlUser$1.doExec(MgnlUser.java:192)

The user is then null but the code goes on until a null pointer occurs later on. See attached log file excerpt.

The fix is that the code should obtain the specific JCR workspace from the UserManager class in use. That is what the user manager is for. We have defined a custom user manager class that uses our 'public users' workspace by overriding the MgnluserManager#getRepositoryName() method (PS: this method should really be defined in the UserManager interface I think?).

The content publication does work by the way but of course we do not want these errors in our logs. Another thing is that the Pulse does not work properly after this has happens (the task counter is never updated) but I am not yet sure if that is due to this issue (may well be).



 Comments   
Comment by Federico Grilli [ 19/Sep/14 ]

On second thoughts, I actually realised I misunderstood the comments in here and my solution is not going in the direction of getting rid of those extraneous methods which belong to Managers. To fix the support-related issue it should be enough to use a workaround I already found in the code, i.e.

SecuritySupport.Factory.getInstance().getUserManager(getRealm()).hasAny(getName(), groupName, NODE_GROUPS);

No need to inject UserManager which would actually create a new ctor to be deprecated soon. In the next major version then we will likely deprecate all the various methods in the User interface which belong more to Managers and eventually get rid of them.

Generated at Mon Feb 12 04:09:10 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.