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

CLI: admin_only decorator #3158

Merged
merged 5 commits into from
Nov 6, 2014
Merged

CLI: admin_only decorator #3158

merged 5 commits into from
Nov 6, 2014

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Oct 30, 2014

Addresses https://trac.openmicroscopy.org.uk/ome/ticket/5137

This PR should extend the admin_only decorator and implement it across all CLI plugins instead of using try/except blocks. Integration tests are also extended to test the admin check.
The only place where the decorator is not used is fs sets --check since the check is conditional to the argument being passed.

/cc @bpindelski


--no-rebase

@sbesson sbesson changed the title 5137 admin only CLI: admin_only decorator Oct 30, 2014
@bpindelski
Copy link

Tests pass. I did a git grep SecurityViolation in components/tools/OmeroPy/src/omero/plugins, which gave me some places that might need checking:

  • hql.py:231,
  • script.py:655,
  • sessions.py:395,
  • user.py:371

Besides that, code looks good.

@jburel jburel added the develop label Oct 31, 2014
@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build OME-5.1-merge-push#550. See the console output for more details.

@sbesson
Copy link
Member Author

sbesson commented Nov 5, 2014

Thanks @bpindelski. Reviewed the other plugins and 5710abf should add the admin_only decorator to the group add subcommand. with a corresponding test.

joshmoore added a commit that referenced this pull request Nov 6, 2014
@joshmoore joshmoore merged commit e167cba into ome:develop Nov 6, 2014
@sbesson sbesson deleted the 5137_admin_only branch November 6, 2014 10:21
@sbesson sbesson added this to the 5.1.0-m2 milestone Nov 26, 2014
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

5 participants