Merge pull request #4258 from joshmoore/trac13018-inactive-share

trac 13018: disabling inactive share access

components/blitz/src/ome/services/blitz/fire/PermissionsVerifierI.java

@@ -11,6 +11,7 @@
 import java.util.List;
 
 import ome.api.IQuery;
+import ome.conditions.SecurityViolation;
 import ome.conditions.SessionException;
 import ome.model.meta.Experimenter;
 import ome.services.sessions.SessionManager;
@@ -166,6 +167,9 @@ public Object doWork(Session session, ServiceFactory sf) {
                 return false;
             }
 
+        } catch (SecurityViolation sv) {
+            reason.value = sv.getMessage();
+            return false;
         } catch (Throwable t) {
             reason.value = "Internal error. Please contact your administrator:\n"
                     + t.getMessage();

components/server/src/ome/security/sharing/SharingACLVoter.java

@@ -19,6 +19,7 @@
 import ome.security.basic.CurrentDetails;
 import ome.security.basic.TokenHolder;
 import ome.services.sharing.ShareStore;
+import ome.services.sharing.data.ShareData;
 
 import org.hibernate.Session;
 import org.slf4j.Logger;
@@ -73,7 +74,11 @@ public boolean allowLoad(Session session, Class<? extends IObject> klass, Detail
             return true;
         }
         long sessionID = cd.getCurrentEventContext().getCurrentShareId();
-        return store.contains(sessionID, klass, id);
+        ShareData data = store.get(sessionID);
+        if (data.enabled) {
+            return store.contains(sessionID, klass, id);
+        }
+        return false;
     }
 
     public void throwLoadViolation(IObject iObject) throws SecurityViolation {

components/server/src/ome/services/sessions/SessionManagerImpl.java

@@ -519,9 +519,38 @@ protected SessionContext createSessionContext(List<?> list, SessionContext previ
 
     public Session find(String uuid) {
         SessionContext sessionContext = cache.getSessionContext(uuid);
+        checkIfShare(sessionContext);
         return (sessionContext == null) ? null : sessionContext.getSession();
     }
 
+    private void checkIfShare(SessionContext sessionContext) {
+        if (sessionContext.getSession() instanceof Share) {
+            final Long id = sessionContext.getSession().getId();
+            final String uuid = sessionContext.getSession().getUuid();
+            final String prefix = String.format("Share:%s (%s)", id, uuid);
+
+            List<Object[]> rv = executeProjection(
+                    "select s.active, s.timeToLive, s.started from Share s where s.id = :id",
+                    new Parameters().addId(sessionContext.getSession().getId()));
+
+            if (rv.size() != 1) {
+                throw new RuntimeException(prefix + " could not be found!");
+            }
+
+            Object[] items = rv.get(0);
+            Boolean active = (Boolean) items[0];
+            Long timeToLive = (Long) items[1];
+            Timestamp started = (Timestamp) items[2];
+
+            if (Boolean.FALSE.equals(active)) {
+               throw new SecurityViolation(prefix + " is inactive");
+            } else if ((System.currentTimeMillis() - started.getTime()) > timeToLive) {
+                String msg = String.format("%s has expired: %s, timeToLive=%s",
+                        prefix, started, timeToLive);
+                throw new SecurityViolation(msg);
+            }
+        }
+    }
     private final static String findBy1 =
         "select s.id, s.uuid from Session s " +
         "join s.owner o where " +

components/tools/OmeroPy/test/integration/test_ishare.py

@@ -13,6 +13,8 @@
 import library as lib
 import pytest
 import omero
+import Glacier2
+
 from omero.rtypes import rtime, rlong, rlist
 from omero.gateway import BlitzGateway
 
@@ -983,6 +985,56 @@ def test8704(self):
         finally:
             user_client.__del__()
 
+    def test13018(self):
+        """
+        Test that image in share is unavailable when share
+        is inactive or expired
+        """
+        owner = self.new_client()
+        member, mobj = self.new_client_and_user()
+
+        createTestImage(owner.sf)
+        image = owner.sf.getQueryService().findAll("Image", None)[0]
+
+        o_share = owner.sf.getShareService()
+        sid = o_share.createShare("", None, [image], [mobj], [], True)
+
+        m_share = member.sf.getShareService()
+        m_share.activate(sid)
+
+        o_share.setActive(sid, False)
+        with pytest.raises(omero.ValidationException):
+            m_share.activate(sid)
+
+        with pytest.raises(omero.ValidationException):
+            obj = omero.model.ShareI(sid, False)
+            member.sf.setSecurityContext(obj)
+
+        # test inactive share, if member has no access to the image
+        s = o_share.getShare(sid)
+        with pytest.raises(Glacier2.PermissionDeniedException):
+            self.new_client(session=s.uuid)
+
+        # activate again
+        o_share.setActive(sid, True)
+
+        # test that the image is now loadable again.
+        t_conn = self.new_client(session=s.uuid)
+        t_conn.sf.getQueryService().find("Image", image.id.val)
+
+        # test expired share, if member has no access to the image
+        expiration = long(time.time() * 1000) + 500
+        o_share.setExpiration(sid, rtime(expiration))
+        self.assert_expiration(expiration, o_share.getShare(sid))
+        time.sleep(0.5)
+
+        # Forced closing
+        o_session = owner.sf.getSessionService()
+        o_session.closeSession(o_share.getShare(sid))
+
+        with pytest.raises(Glacier2.PermissionDeniedException):
+            self.new_client(session=s.uuid)
+
     # Helpers
 
     def assert_access(self, client, sid, success=True):