[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: |
|
||||||||||||||||||||||||||||
| 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. Solution/Improvement: From the customer: 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. |