Task #8685 (closed)
Making {omero.share:<sid>} enough to activate share context
Reported by: | atarkowska | Owned by: | atarkowska |
---|---|---|---|
Priority: | blocker | Milestone: | OMERO-4.4 |
Component: | Web | Version: | n.a. |
Keywords: | n.a. | Cc: | web-team@… |
Resources: | n.a. | Referenced By: | n.a. |
References: | n.a. | Remaining Time: | 0.0d |
Sprint: | 2012-05-22 (15) |
Description (last modified by atarkowska)
Now, in order to switch to share context, connection needs to call activate/deactivate on the security context. That will cause concurrency issues in multitudinous environment.
As discussed previously setting {"omero.share": sid} in query should be enough to get "shared" data by members of the share.
Action to be taken:
- utest adjustment (Ola)
- security context implementation (Josh)
See #8687 as well
Change History (14)
comment:1 Changed 12 years ago by atarkowska
- Description modified (diff)
comment:2 Changed 12 years ago by atarkowska
comment:3 Changed 12 years ago by atarkowska
- Cc web-team@… added; cxallan wmoore jmoore removed
- Owner changed from atarkowska to jmoore
How do you think Josh?
comment:4 Changed 12 years ago by atarkowska
comment:5 Changed 12 years ago by jmoore
The intent is certainly not that "omero.group" and "omero.share" be set at the same time. It would probably be good to specify what that even means. What I was expecting clients to do was to set omero.share if the share is active, and otherwise use whatever omero.group the implementation needs (like that from the group, etc). In other words, for a share invocation, omero.share should suffice, and if you want to try to set omero.group too we can make that work but you should definitely not use just the group for an object in the share, since that doesn't tell the server to switch to the "share" implementation.
comment:6 Changed 12 years ago by jmoore
Ola, this test is passing for me on my chmod branch. Any thoughts on how we could go about testing?
comment:7 Changed 12 years ago by atarkowska
The test was written against 8610_mergin it means we have to merge both branches together then
comment:8 Changed 12 years ago by wmoore
These branches are already merged in my chmod_web_8434 branch.
comment:9 Changed 12 years ago by jburel
- Sprint changed from 2012-05-08 (14) to 2012-05-22 (15)
Moved from sprint 2012-05-08 (14)
comment:10 Changed 12 years ago by Aleksandra Tarkowska <A.Tarkowska@…>
(In [2e08771d8967edc3f9758e39c828bda3ea052b8b/ome.git] on branch develop) adjusting integration test ishare.test8513, see #8685 and #8687 for comments
comment:11 Changed 12 years ago by jmoore
- Owner changed from jmoore to atarkowska
- Remaining Time changed from 0.5 to 0.1
Ola, I've committed what I needed to as part of #8608:
commit 9235f74342b150a13bcd93a920c62ab4306f055a Author: jmoore <josh@glencoesoftware.com> Date: Thu May 10 11:55:07 2012 +0200 Ignoring omero.group/user when omero.share set (Fix #8608) diff --git a/components/server/src/ome/security/basic/BasicEventContext.java b/components/server/src/ome/security/basic/BasicEventContext.java index 2b3009b..a17933e 100644 --- a/components/server/src/ome/security/basic/BasicEventContext.java +++ b/components/server/src/ome/security/basic/BasicEventContext.java @@ -96,7 +96,23 @@ class BasicEventContext extends SimpleEventContext { // Now re-apply values. final List<String> toPrint = new ArrayList<String>(); - + + final Long sid = parseId(callContext, "omero.share"); + if (sid != null) { + if (!isAdmin) { + // If the user is not an admin then we need to verify that + // s/he is a valid member of the share. + ShareData data = store.getShareIfAccessible(sid, isAdmin, this.cuId); + if (data == null) { + throw new SecurityViolation(String.format( + "User %s cannot access share %s", this.cuId, sid)); + } + } + setShareId(sid); + toPrint.add("share="+sid); + return; // IGNORE all other settings for share (#8608) + }
Passing off to you. Close if you think it's all done.
comment:12 Changed 12 years ago by atarkowska
- Status changed from new to accepted
comment:13 Changed 12 years ago by atarkowska
- Remaining Time changed from 0.1 to 0
- Resolution set to fixed
- Status changed from accepted to closed
Great works now.
comment:14 Changed 12 years ago by Aleksandra Tarkowska <A.Tarkowska@…>
(In [23de52ab460415ccaea71c5494a7135cbdbe8da9/ome.git] on branch develop) final clean up of sharing context and activation, see #8685
It looks like SecurityViolation? is raised because omero.gateway.ImageWrapper?._prepareTB sets 'SERVICE_OPTS' based on the image details (group id of the image owner) rather then group id from the event context (group id of the member of the share accessing image. In that case security system does not know what to do. Either we define the hierarchy of parameters or we change the implementation. Please refer to the #8687 because more issues like that appeared in the log.