Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #12362, #12396: allow group owner to change permission level #2655

Merged

Conversation

mtbc
Copy link
Member

@mtbc mtbc commented Jun 18, 2014

Fixes http://trac.openmicroscopy.org.uk/ome/ticket/12362 and http://trac.openmicroscopy.org.uk/ome/ticket/12396 so that group owners can change group permissions in Insight and get the usual warning if they try to downgrade to Private.

--rebased-to #2752

@pwalczysko
Copy link
Member

Bug: It seems now that also some non-owners members are "allowed" to change the group permissions in UI, but in fact it will end up in error.

Workflow:

  • login to octopus as user-1
  • put group "Petr Test Mark's PR" to display
  • note that user-1 is an owner of another group (private-1) but not an owner of "Petr Test Mark's PR"
  • clcik on the group name and go to the right-hand pane, change the permissions level (not greyed out) and Save - not expected and additionally -> Error appears
java.lang.Exception: org.openmicroscopy.shoola.env.data.DSAccessException: Cannot access data. 
Cannot update the group. 
    at org.openmicroscopy.shoola.env.data.OMEROGateway.handleException(OMEROGateway.java:853)
    at org.openmicroscopy.shoola.env.data.OMEROGateway.updateGroup(OMEROGateway.java:4155)
    at org.openmicroscopy.shoola.env.data.AdminServiceImpl.updateGroup(AdminServiceImpl.java:380)
    at org.openmicroscopy.shoola.env.data.views.calls.AdminLoader$5.doCall(AdminLoader.java:192)
    at org.openmicroscopy.shoola.env.data.views.BatchCall.doStep(BatchCall.java:144)
    at org.openmicroscopy.shoola.util.concur.tasks.CompositeTask.doStep(CompositeTask.java:226)
    at org.openmicroscopy.shoola.env.data.views.CompositeBatchCall.doStep(CompositeBatchCall.java:126)
    at org.openmicroscopy.shoola.util.concur.tasks.ExecCommand.exec(ExecCommand.java:165)
    at org.openmicroscopy.shoola.util.concur.tasks.ExecCommand.run(ExecCommand.java:276)
    at org.openmicroscopy.shoola.util.concur.tasks.AsyncProcessor$Runner.run(AsyncProcessor.java:91)
    at java.lang.Thread.run(Thread.java:695)
Caused by: omero.SecurityViolation
    serverStackTrace = "ome.conditions.SecurityViolation: Current user is neither admin nor group-leader for the given user(s)/group(s)
                            at ome.logic.AdminImpl.throwNonAdminOrPi(AdminImpl.java:1402)
                            at ome.logic.AdminImpl.adminOrPiOfGroup(AdminImpl.java:1413)
                            at ome.logic.AdminImpl.updateGroup(AdminImpl.java:582)
                            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
                            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                            at java.lang.reflect.Method.invoke(Method.java:606)
                            at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:307)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:183)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:150)
                            at ome.security.basic.EventHandler.invoke(EventHandler.java:154)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at org.springframework.orm.hibernate3.HibernateInterceptor.invoke(HibernateInterceptor.java:111)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:108)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at ome.tools.hibernate.ProxyCleanupFilter$Interceptor.invoke(ProxyCleanupFilter.java:241)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at ome.services.util.ServiceHandler.invoke(ServiceHandler.java:116)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:202)
                            at com.sun.proxy.$Proxy80.updateGroup(Unknown Source)
                            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
                            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                            at java.lang.reflect.Method.invoke(Method.java:606)
                            at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:307)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:183)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:150)
                            at ome.security.basic.BasicSecurityWiring.invoke(BasicSecurityWiring.java:98)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at ome.services.blitz.fire.AopContextInitializer.invoke(AopContextInitializer.java:43)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:202)
                            at com.sun.proxy.$Proxy80.updateGroup(Unknown Source)
                            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
                            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                            at java.lang.reflect.Method.invoke(Method.java:606)
                            at ome.services.blitz.util.IceMethodInvoker.invoke(IceMethodInvoker.java:179)
                            at ome.services.throttling.Callback.run(Callback.java:56)
                            at ome.services.throttling.InThreadThrottlingStrategy.callInvokerOnRawArgs(InThreadThrottlingStrategy.java:56)
                            at ome.services.blitz.impl.AbstractAmdServant.callInvokerOnRawArgs(AbstractAmdServant.java:149)
                            at ome.services.blitz.impl.AdminI.updateGroup_async(AdminI.java:346)
                            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
                            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                            at java.lang.reflect.Method.invoke(Method.java:606)
                            at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:307)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:183)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:150)
                            at omero.cmd.CallContext.invoke(CallContext.java:78)
                            at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:172)
                            at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:202)
                            at com.sun.proxy.$Proxy81.updateGroup_async(Unknown Source)
                            at omero.api._IAdminTie.updateGroup_async(_IAdminTie.java:330)
                            at omero.api._IAdminDisp.___updateGroup(_IAdminDisp.java:751)
                            at omero.api._IAdminDisp.__dispatch(_IAdminDisp.java:1566)
                            at IceInternal.Incoming.invoke(Incoming.java:222)
                            at Ice.ConnectionI.invokeAll(ConnectionI.java:2482)
                            at Ice.ConnectionI.dispatch(ConnectionI.java:1258)
                            at Ice.ConnectionI.message(ConnectionI.java:1213)
                            at IceInternal.ThreadPool.run(ThreadPool.java:321)
                            at IceInternal.ThreadPool.access$300(ThreadPool.java:12)
                            at IceInternal.ThreadPool$EventHandlerThread.run(ThreadPool.java:693)
                            at java.lang.Thread.run(Thread.java:744)
                        "
    serverExceptionClass = "ome.conditions.SecurityViolation"
    message = "Current user is neither admin nor group-leader for the given user(s)/group(s)"
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
    at java.lang.reflect.Constructor.newInstance(Constructor.java:513)
    at java.lang.Class.newInstance0(Class.java:357)
    at java.lang.Class.newInstance(Class.java:310)
    at IceInternal.BasicStream$DynamicUserExceptionFactory.createAndThrow(BasicStream.java:2142)
    at IceInternal.BasicStream.throwException(BasicStream.java:1564)
    at IceInternal.Outgoing.throwUserException(Outgoing.java:443)
    at omero.api._IAdminDelM.updateGroup(_IAdminDelM.java:2222)
    at omero.api.IAdminPrxHelper.updateGroup(IAdminPrxHelper.java:7050)
    at omero.api.IAdminPrxHelper.updateGroup(IAdminPrxHelper.java:7023)
    at org.openmicroscopy.shoola.env.data.OMEROGateway.updateGroup(OMEROGateway.java:4132)
    ... 9 more

    at org.openmicroscopy.shoola.env.ui.UserNotifierImpl.showErrorDialog(UserNotifierImpl.java:191)
    at org.openmicroscopy.shoola.env.ui.UserNotifierImpl.notifyError(UserNotifierImpl.java:291)
    at org.openmicroscopy.shoola.env.ui.UserNotifierImpl.notifyError(UserNotifierImpl.java:259)
    at org.openmicroscopy.shoola.agents.metadata.MetadataLoader.handleException(MetadataLoader.java:112)
    at org.openmicroscopy.shoola.agents.metadata.MetadataLoader.handleException(MetadataLoader.java:183)
    at org.openmicroscopy.shoola.env.data.events.DSCallAdapter.eventFired(DSCallAdapter.java:84)
    at org.openmicroscopy.shoola.env.data.views.BatchCallMonitor$1.run(BatchCallMonitor.java:124)
    at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
    at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:715)
    at java.awt.EventQueue.access$400(EventQueue.java:82)
    at java.awt.EventQueue$2.run(EventQueue.java:676)
    at java.awt.EventQueue$2.run(EventQueue.java:674)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.AccessControlContext$1.doIntersectionPrivilege(AccessControlContext.java:86)
    at java.awt.EventQueue.dispatchEvent(EventQueue.java:685)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:296)
    at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:211)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:201)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:196)
    at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:188)
    at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)

@pwalczysko
Copy link
Member

Note that the above bug is specific for users which are owners of another group. A member which is not an owner of another group will get a greyed out menu in right hand pane as expected

@pwalczysko
Copy link
Member

If you have images annotated by other users in Read-annotate and then the owner downgrades the group to Private, the action of the downgrade fails silently. This is acceptable for this PR I suppose, we wanted to have a warning though. What is surprising is that even after the owner of the group deletes and unlinks these links, the downgrade to Private still fails silently. -> Not expected.

@pwalczysko
Copy link
Member

Note that the downgrade to Private works fine if the images are not annotated.

@mtbc
Copy link
Member Author

mtbc commented Jun 19, 2014

I'm glad you noticed this issue with your "Petr Test Mark's PR" group: it should be fixed in the next merge build. Yes, definitely make sure the absent warning for failed downgrade is ticketed somewhere if not already, as well as the wrongly failed downgrades.

@pwalczysko
Copy link
Member

Created http://trac.openmicroscopy.org.uk/ome/ticket/12395 for the failed downgrade.

@pwalczysko
Copy link
Member

Created http://trac.openmicroscopy.org.uk/ome/ticket/12396?pane=edit for the missing warning dialog.

@mtbc
Copy link
Member Author

mtbc commented Jun 19, 2014

Now @pwalczysko shows me that it's possible to adjust groups from the Project browser as well as the Administration one! So, I will have to look at that.

@mtbc
Copy link
Member Author

mtbc commented Jun 20, 2014

@pwalczysko: it seems to be working this morning, perhaps when we tried before the new one hadn't deployed or something: please re-test.

@atarkowska
Copy link
Member

@mtbc @pwalczysko is that functionality only for admins? What about group owners, in Web is allowed

@mtbc
Copy link
Member Author

mtbc commented Jun 20, 2014

Yes, also group owners.

@atarkowska
Copy link
Member

ohh sorry I had to look at the wrong group.

@atarkowska
Copy link
Member

OK, after testing I thing web does better job of handling group management then Insight. Basically there are cases where you are not allowed to downgrade it and Web notify you about it. Insight silently skip it. After clicking Save button there is no information that this action is not possible.
omero insightscreensnapz002
Web says:
google chromescreensnapz002
then when you decide to do it it will say:
google chromescreensnapz003
I will be closing web ticket https://trac.openmicroscopy.org.uk/ome/ticket/12363 as irrelevant. Perhaps Insight should do the same as Web

@pwalczysko
Copy link
Member

@aleksandra-tarkowska : I am afraid that in Web the ticket is describing a situation where you are logged in as a group owner (= NOT admin). This is not irrelevant, at least during Paris meeting it was not.

@atarkowska
Copy link
Member

@pwalczysko because of annotation links it is not allowed.

@pwalczysko
Copy link
Member

@mtbc @aleksandra-tarkowska
Yes, it is greyed out now for user-1. As discussed with @aleksandra-tarkowska , there is still a discrepancy in web between admin and group owner (the downgrade to Private is blocked for owner also in cases when it is allowed for Admin -> we want both Gr Owner and Admin to have the same experience (including warning dialogs).

Ready to merge.

@mtbc mtbc added the dev_5_0 label Jun 23, 2014
@mtbc mtbc changed the title fix #12362: allow group owner to change permission level fix #12362, #12396: allow group owner to change permission level Jun 23, 2014
@mtbc
Copy link
Member Author

mtbc commented Jun 23, 2014

Missing warning dialog of #2655 (comment) now fixed.

@pwalczysko
Copy link
Member

Works as expected. Ready to merge.

joshmoore added a commit that referenced this pull request Jul 4, 2014
…issions

fix #12362, #12396: allow group owner to change permission level
@joshmoore joshmoore merged commit 1a99dd8 into ome:dev_5_0 Jul 4, 2014
@mtbc mtbc deleted the trac-12362-group-owner-change-permissions branch July 4, 2014 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants