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

Webadmin 'Read-Write' perms #3783

Merged
merged 4 commits into from
May 25, 2015

Conversation

will-moore
Copy link
Member

This allows Admins to create 'Read-Write' groups in webadmin. See https://trac.openmicroscopy.org/ome/ticket/10751

To test:

  • Log in as Admin, go to webadmin, create a Group...
  • See that there is now a 4th "Read-Write" option for group perms.
  • Create a group with these perms.
  • Test new group behaves / appears as expected in web (or insight)
  • Edit the new group (check that perms are "Read-Write" before editing)
  • Edit permissions and check updated OK.

@joshmoore
Copy link
Member

NB: I'd propose this is one for a "space", i.e. something it's good to get in and start testing, but which realistically might not make it into 5.1.2 based on other constraints (like testing).

@jburel jburel added the develop label May 4, 2015
@joshmoore
Copy link
Member

@pwalczysko suggests that this should be marketed properly since this is a special type of group where there needs to be a lot of trust. cc: @hflynn

@joshmoore
Copy link
Member

General feeling that we should likely show a warning to sysadmins when creating such a group. cc: @jburel & @dominikl for insight; @sbesson for CLI.

@mtbc
Copy link
Member

mtbc commented May 4, 2015

Most likely server bug in r-w group is deleting others' data (e.g., annotation on image that is being deleted) in obviously wrong cases. (Less clear cases can probably wait for larger permissions review.)

@hflynn
Copy link
Member

hflynn commented May 4, 2015

I've stuck a card on the docs board for this - https://trello.com/c/dUmuMrkk/325-read-write-groups

@will-moore
Copy link
Member Author

Added a warning on selection of "Read-Write" group, using the same 'WARNING' dialog as when you select "Private" group. It's consistent but if this is too aggressive we can think again....?

screen shot 2015-05-04 at 15 46 59

@mtbc
Copy link
Member

mtbc commented May 4, 2015

I like it. Better warn ahead of time, as it's not like our deletes have an undo.

@hflynn
Copy link
Member

hflynn commented May 4, 2015

I agree, better a slightly aggressive warning message than angry users who have lost their data!

@pwalczysko
Copy link
Member

@will-moore : I suppose the warning will pop up even when just upgrading the permissions of an existing group (in some respect this is the more dangerous workflow)

@will-moore
Copy link
Member Author

Certainly that last commit shouldn't have caused any Travis failures.
Travis errors are:

[nomemorytestng] PASSED: testUsage on null(ome.services.graphs.GraphSpecUnitTest)
[nomemorytestng] FAILED: testCreateJPEG
[nomemorytestng] org.testng.internal.thread.ThreadTimeoutException: Method org.testng.internal.TestNGMethod.testCreateJPEG() didn't finish within the time-out 5000
[nomemorytestng]    at java.awt.geom.Area.pathToCurves(Area.java:193)
[nomemorytestng]    at java.awt.geom.Area.<init>(Area.java:126)

and

[nomemorytestng] 2015-05-04 18:03:35,590 WARN  [n.sf.ehcache.config.ConfigurationFactory] (      main) No configuration found. Configuring ehcache from ehcache-failsafe.xml  found in the classpath: jar:file:/home/travis/build/openmicroscopy/openmicroscopy/lib/cache/net.sf.ehcache/ehcache-core/jars/ehcache-core-2.1.0.jar!/ehcache-failsafe.xml
[nomemorytestng] 2015-05-04 18:03:46,093 ERROR [ o.services.sessions.stats.MethodCounter] (calStats()) ome.services.messages.stats.ObjectsReadStatsMessage[source=ome.services.sessions.stats.MethodCounter@243a0c0b] produced an error: java.lang.NullPointerException
[nomemorytestng] 2015-05-04 18:03:46,109 ERROR [ome.services.db.SelfCorrectingDataSource] (oRetries()) Failed to acquire connection after retries=0
[nomemorytestng] ome.server.utests.SelfCorrectingDatabaseUnitTest$MySQLException: null
[nomemorytestng]    at org.jmock.core.stub.ThrowStub.invoke(Unknown Source) ~[jmock-1.0.1.jar:na]

seem unrelated to this PR.

@joshmoore
Copy link
Member

Kicked travis.

@atarkowska
Copy link
Member

All seems to be working fine. No issues with adding RW group and updating users. Deletion works as expected.

firefoxscreensnapz034

Just noticed one issue with deleting Tag

<li class="jstree-last jstree-open" rel="tag-locked" id="tag-803"><ins class="jstree-icon">&nbsp;</ins><a href="#" class="jstree-clicked"><ins class="jstree-icon">&nbsp;</ins>by member</a><ul style="">
        <li class="jstree-last jstree-leaf" rel="dataset" id="dataset-154"><ins class="jstree-icon">&nbsp;</ins>
            <a href="#" class=""><ins class="jstree-icon">&nbsp;</ins>test</a>
        </li>
    </ul></li>

Cannot delete someone else tag because is locked "tag-locked" even Trash button is enabled. Data manager should be reviewed and fixed in the separate PR. This is good to go

@will-moore
Copy link
Member Author

Made a note at https://trello.com/c/ScEErwT9/222-5-1-2-follow-up to fix Delete tag-locked.

@jburel
Copy link
Member

jburel commented May 7, 2015

Since we turn on that functionality in a point release we should complete it on this PR. (release in few weeks), the "follow-up" PR might not be ready on time and we won't have the functionality completed.

@will-moore
Copy link
Member Author

@jburel Sorry - not sure I understand your last comment. Is there more that I need to do with this PR? Do you mean the delete of 'tag-locked'?

@jburel
Copy link
Member

jburel commented May 11, 2015

Discussion on RW (not only web related) scheduled on Wednesday morning when everybody is back.

@hflynn
Copy link
Member

hflynn commented May 13, 2015

I'm going to make a start on the permissions docs BTW, I want to get this documented before I go on holiday next week.

@will-moore
Copy link
Member Author

Disabled "Read-Write" for group owners:

screen shot 2015-05-14 at 10 47 34

@gusferguson
Copy link

@will-moore

Tested with trout merge user-6 and user-3.

The warning is showing up as expected for admin.
The grey-out of Read-write for group owners and tooltip explanation are good.

Good to merge.

@joshmoore
Copy link
Member

re: #3783 (comment) -- what's the status of the delete tag?

@will-moore
Copy link
Member Author

@joshmoore Handle delete of 'tag-locked' is on my todo list at https://trello.com/c/ScEErwT9/222-5-1-2-follow-up.

@will-moore
Copy link
Member Author

@joshmoore I'm assuming that this is awaiting a final decision to "support read-write for 5.1.2" and that there's nothing else I actually need to do on this PR?

@jburel
Copy link
Member

jburel commented May 21, 2015

When all the PR are ready, we should be good to go. gh-3827 will be reviewed again tomorrow

@joshmoore
Copy link
Member

Merging for 5.1.2.

joshmoore added a commit that referenced this pull request May 25, 2015
@joshmoore joshmoore merged commit f9f118e into ome:develop May 25, 2015
@will-moore will-moore deleted the read_write_groups_webadmin_10751 branch May 25, 2015 13:57
@joshmoore joshmoore added this to the 5.1.2 milestone May 27, 2015
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

8 participants