Opened 8 years ago

Closed 8 years ago

#2033 closed defect (fixed)

Permissions for annotating items may be incorrectly implemented

Reported by: Nicklas Nordborg Owned by: everyone
Priority: major Milestone: BASE 3.10
Component: core Version:
Keywords: Cc:

Description

I might be wrong but I have always though that in order to be able to annotate an item the logged in user must have WRITE permission on the item and USE permission on the annotation type.

However there seems to be nothing at all in the current implementation that checks the permission on the annotation type implying that READ permission on the annotation type is enough. Setting up a test case confirm this.

The only exception to this is the new Annotation Batcher API introduced in BASE 3.8 (see #2000) which actually has a check for USE permission on the annotation type (AnnotationBatcher line 430).

It might well be that it is the batcher API that is incorrect, but in any case there is an inconsistency between the regular API and the batch API. I think the regular API should be fixed, though this may require a lot of work to make sure that other related things (web interface, caching, etc.) continue to work since I think only the WRITE permission on the item is checked in most cases.

Change History (8)

comment:1 by Nicklas Nordborg, 8 years ago

One other way to test this is to use the annotation importer plug-in. Since BASE 3.8 it uses the Batch API. But it still presents the user with a list of all annotation types with READ permission. Mapping a column to an annotation with only READ permission causes a runtime exception:

net.sf.basedb.core.PermissionDeniedException: Permission denied: 
Not allowed to use Annotation type[id=22175090; name=Enum#1] 
  at net.sf.basedb.core.BasicItem.checkPermission(BasicItem.java:112) 
  at net.sf.basedb.core.AnnotationBatcher.addAnnotationTypes(AnnotationBatcher.java:430) 
  at net.sf.basedb.plugins.AnnotationFlatFileImporter.beginData(AnnotationFlatFileImporter.java:764) 
  at net.sf.basedb.plugins.AbstractFlatFileImporter.doImport(AbstractFlatFileImporter.java:677) 
  at net.sf.basedb.plugins.AnnotationFlatFileImporter.doImport(AnnotationFlatFileImporter.java:651) 
  at net.sf.basedb.plugins.AbstractFlatFileImporter.run(AbstractFlatFileImporter.java:451) 
  at net.sf.basedb.core.PluginExecutionRequest.invoke(PluginExecutionRequest.java:116) 
....

Adding the annotation manually works.

Last edited 8 years ago by Nicklas Nordborg (previous) (diff)

comment:2 by Nicklas Nordborg, 8 years ago

(In [7204]) References #2033: Permissions for annotating items may be incorrectly implemented

Fixed the annotation importer so that it only return annotation types that the logged in user has permission to USE.

comment:3 by Nicklas Nordborg, 8 years ago

(In [7205]) References #2033: Permissions for annotating items may be incorrectly implemented

Check for USE permission on the annotation type before creating, modifying or deleting an annotation.

comment:4 by Nicklas Nordborg, 8 years ago

(In [7206]) References #2033: Permissions for annotating items may be incorrectly implemented

Changes in the web interface (Annotations tab) so that the "edit" icon is only visible if the user has permission to modify or create an annotation.

The edit dialog will still list all annotations but editing is disabled for read-only annotations.

comment:5 by Nicklas Nordborg, 8 years ago

(In [7207]) References #2033: Permissions for annotating items may be incorrectly implemented

Changes in the web interface (Overview tab) so that the "edit" icon is only visible if the user has permission to modify an annotation.

comment:6 by Nicklas Nordborg, 8 years ago

(In [7210]) References #2033: Permissions for annotating items may be incorrectly implemented

Changes in the web interface (Experimental factors in various places) so that the "edit" icon is only visible if the user has permission to modify an annotation.

comment:7 by Nicklas Nordborg, 8 years ago

(In [7221]) References #2033 and #2034. Added a note about the changes to the list of update warnings and incompatible changes.

comment:8 by Nicklas Nordborg, 8 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.