search

ome / omero-ms-queue

GNU General Public License v3.0
3 stars 5 forks source link

Imports into a group that is not defined as the default fail #19

Open c0c0n3 opened 5 years ago

c0c0n3 commented 5 years ago

Just moving across this issue from the old Smuggler repo. Here's the issue as reported by @juliomateoslangerak: https://github.com/c0c0n3/ome-smuggler/issues/16

Note. When resolved, we should close it in the Smuggler repo too.

It looks like we might be able to add the group option available in the OMERO CLI import command to the cli component in this repo and then make it available through the server component's import REST API as well as the soon-to-be-released Insight with integrated offline import functionality. This is doable as long as the OMERO Java import lib that the cli component relies on actually supports that group option too.

c0c0n3 commented 5 years ago

The Issue?

So I've done some digging and it looks like the import failure @juliomateoslangerak reported boils down to the fact that

files get imported into the current session's group even if you specify a -g argument with a different group than the session's.

This rules out my suggestion of adding a group argument to Smuggler's web import interface.

Here's how to reproduce the issue using vanilla omero import.

My set up

Plans o' mice and men

Use omero CLI to log in as master_of_disaster, grab the login session key and then import an image into d2 using that key as import credentials while also explicitly specifying readonly as a group.

Chronicle of a doomed import

Version check:

$ omero version
5.4.9-ice36-b101

Log me in, Scotty!

$ omero login
Server: [localhost:4064]
Username: [andrea]master_of_disaster
Password:
Created session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: private

Note it says "Current group: private". Now grab that juicy session key...

$ ls ~/omero/sessions/localhost/master_of_disaster/
7302cb0c-a762-4941-96ff-1214eec4b141  ._LASTSESS_

...and try your luck with the import, note the -g readonly argument to say we want to import with the readonly group:

$ omero import -s localhost -p 4064 -g readonly -k 7302cb0c-a762-4941-96ff-1214eec4b141 -d 2 2.jpg
Ignoring group since session key set
Joined session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: private

Oh, oh, what's that "Ignoring group since session key set"?! Spill the beans!

2019-04-22 18:07:29,069 330        [      main] INFO          ome.formats.importer.ImportConfig - OMERO Version: 5.4.9-ice36-b101
2019-04-22 18:07:29,084 345        [      main] INFO          ome.formats.importer.ImportConfig - Bioformats version: 5.9.2 revision: 9fc607f85b8900be786813296f1eee75cc1ed883 date: 31 August 2018                                                              
2019-04-22 18:07:29,138 399        [      main] INFO   formats.importer.cli.CommandLineImporter - Log levels -- Bio-Formats: ERROR OMERO.importer: INFO                                                                                                          
2019-04-22 18:07:29,542 803        [      main] INFO      ome.formats.importer.ImportCandidates - Depth: 4 Metadata Level: MINIMUM                                                                                                                               
2019-04-22 18:07:29,799 1060       [      main] INFO      ome.formats.importer.ImportCandidates - 1 file(s) parsed into 1 group(s) with 1 call(s) to setId in 250ms. (256ms total) [0 unknowns]                                                                  
...
2019-04-22 18:07:31,724 2985       [      main] INFO          ome.formats.importer.ImportConfig - Using import target: Dataset:2
2019-04-22 18:07:31,750 3011       [2-thread-1] ERROR        ome.formats.importer.ImportLibrary - Could not load target: ome.formats.importer.targets.ModelImportTarget@3507e02d                                                                                 
2019-04-22 18:07:31,756 3017       [2-thread-1] ERROR        ome.formats.importer.ImportLibrary - Error on import
java.lang.RuntimeException: Failed to load target
        at ome.formats.importer.ImportLibrary$1.call(ImportLibrary.java:303) [blitz.jar:na]
        ...
Caused by: omero.SecurityViolation: null
        ...
        at omero.api.IQueryPrxHelper.end_get(IQueryPrxHelper.java:2000) ~[blitz.jar:na]
        at omero.api.IQueryPrxHelper.get(IQueryPrxHelper.java:1872) ~[blitz.jar:na]
        at omero.api.IQueryPrxHelper.get(IQueryPrxHelper.java:1859) ~[blitz.jar:na]
        at ome.formats.importer.targets.ModelImportTarget.load(ModelImportTarget.java:154) ~[blitz.jar:na]
        at ome.formats.importer.ImportLibrary$1.call(ImportLibrary.java:292) [blitz.jar:na]
        ... 7 common frames omitted
2019-04-22 18:07:31,757 3018       [2-thread-1] INFO         ome.formats.importer.ImportLibrary - Exiting on error

==> Summary
0 files uploaded, 0 filesets created, 0 images imported, 0 errors in 0:00:00.396
code 2
c0c0n3 commented 5 years ago

Just to add a bit more detail, if you acquire a session for the group you want to import with, then everything works smoothly:

$ omero login -u master_of_disaster -g readonly                                                                                  
Server: [localhost:4064]
Password:
Created session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: readonly

Note this session's current group is the readonly group. Now you can get the session key:

$ ls ~/omero/sessions/localhost/master_of_disaster/
79519cd4-d0ba-4552-b6b4-b3e71950fd3d

and plonk it in:

$ omero import -s localhost -p 4064 -k 79519cd4-d0ba-4552-b6b4-b3e71950fd3d -d 2 2.jpg
Joined session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: readonly
...
2019-04-22 19:45:05,370 2781       [      main] INFO          ome.formats.importer.ImportConfig - Using import target: Dataset:2
...
==> Summary
1 file uploaded, 1 fileset created, 1 image imported, 0 errors in 0:00:02.626

and the file gets shovelled down OMERO's mouth.

c0c0n3 commented 5 years ago

Where does that leave us? Given that Smuggler gets given a session key to do an import, it looks like the easiest way to work around the issue is not to do anything at all in Smuggler :-)

In fact, it's probably easier to require import clients to pass along a suitable session key. In other words, if Insight (or any other client) wants to import into a dataset belonging to a group other than the user's default, it should get a session for that group. Incidentally, this is what omero import does:

$ omero login
Server: [localhost:4064]
Username: [andrea]master_of_disaster
Password:
Created session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: private

$ omero import -s localhost -p 4064 -g readonly -d 2 2.jpg
Previously logged in to localhost:4064 as master_of_disaster
Username: [master_of_disaster]
Skipped session 34a1959f-f77f-4624-8c38-e62c97f30df2 due to property conflicts: omero.group: None!=readonly
Password:
Created session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: readonly
...
==> Summary
1 file uploaded, 1 fileset created, 1 image imported, 0 errors in 0:00:02.405

@jburel, @joshmoore you agree this is the easiest? Or is there a better way of doing this? On another note, why isn't the -g argument honoured if you have a valid session key and you're a member of the specified group?

joshmoore commented 5 years ago

Did you try passing -g $GID rather than the name?

c0c0n3 commented 5 years ago

Just tried using the group ID but it makes no difference, I get the same error. It looks like the -g option is ignored when using session keys.

For the record, here's what I've done. As before, I logged in and grabbed my session key:

$ omero login
...
Created session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: private

$ ls ~/omero/sessions/localhost/master_of_disaster/
34d5eca1-b2d4-4e24-ba8f-d1cc1da4cdc1

Then I tried importing 2.jpg into dataset d2 (dataset ID: 2) which is available to members of the readonly group (group ID: 4), note -g 4 instead of -g readonly in the command line below:

$ omero import -s localhost -p 4064 -g 4 -k 34d5eca1-b2d4-4e24-ba8f-d1cc1da4cdc1 -d 2 2.jpg 
Ignoring group since session key set
Joined session for master_of_disaster@localhost:4064. Idle timeout: 10 min. Current group: private
... 
2019-05-11 11:35:37,889 3999       [2-thread-1] ERROR        ome.formats.importer.ImportLibrary - Error on import
java.lang.RuntimeException: Failed to load target
...
Caused by: omero.SecurityViolation: null
...
Exiting on error
==> Summary
0 files uploaded, 0 filesets created, 0 images imported, 0 errors in 0:00:00.710

Kaboom! :-)

c0c0n3 commented 5 years ago

@jburel, @joshmoore what should we do? I can see two options here:

  1. OMERO should honour the -g argument; or
  2. Clients should acquire a session for the group under which data is to be imported---i.e. what I suggested earlier.

Any other option you can think of?

c0c0n3 commented 5 years ago

@jburel @joshmoore @dominikl it looks like this issue is related to:

is that right?

joshmoore commented 5 years ago

it looks like this issue is related to:

It's the same type of problem, but adding omero.group to the gateway won't fix your issue since import doesn't use the gateway.

It looks like the -g option is ignored when using session keys.

@c0c0n3 : is the session shared between multiple processes? If so, -g can't do what it wants which is to change the group on the session since that would be a race condition.

Essentially what you want is an application-wide setting, but that doesn't currently exist. (I don't imagine it would be too hard to add into omero.client and possibly omero.formats.OMEROMetadataStoreClient since Ice does support setting a default property ({"omero.group": $GID}) on all services.

c0c0n3 commented 5 years ago

@joshmoore thanks for clarifying, mucho appreciato!

is the session shared between multiple processes

Nope, in my tests above, the only process using the session was the OMERO cli.

joshmoore commented 5 years ago

Ah, ok. Then you should be able to use bin/omero sessions group to set the group on the session.

c0c0n3 commented 5 years ago

Awe! I should try that! But isn't the -g param supposed to work too? In other words, is this a bug:

$ omero import -s localhost -p 4064 -g 4 -k 34d5eca1-b2d4-4e24-ba8f-d1cc1da4cdc1 -d 2 2.jpg 
Ignoring group since session key set

Assuming the specified session isn't shared as you pointed out.

joshmoore commented 5 years ago

I'm sure it can be debated, but I think the tension here is "What do you do if the user passed -g 4 to bin/omero login _as well?" Which one should take precedence? I'd try either setting on bin/omero login or as a follow up with bin/omero sessions and see where you get.

c0c0n3 commented 5 years ago

Yep, I see your point and agree with you for the login case. But in my opinion authorisation shouldn't be tied to the group the user logged in with. For example, say I'm a member of two groups: g1 and g2. As a member of g1 I can write to dataset d1 and likewise as a member of g2 I have write perms on dataset d2. After logging in, I'd expect to be able to import an image in either dataset. But I suppose this is a moot point :-)

Instead for practical purposes, I'll try using the session service to change my group before importing, thank you so much for the suggestion!!! 👍

juliomateoslangerak commented 5 years ago

I'm not sure to get into the technical details. If the user moves into the group where he wants to import before launching the import, does that solve the issue? Otherwise said, the session that is used is a copy of a session that is associated to the one used by the main insight interface or does it come from somewhere else?