From e7d92e830e444f91611072d226e9208af1dbd3f9 Mon Sep 17 00:00:00 2001 From: Richard Eckart de Castilho Date: Thu, 7 Mar 2024 20:57:28 +0100 Subject: [PATCH] #4603 - Bulk actions in search sidebar should not be greyed-out when in sidebar curation mode - Use DocumentAccess to check if document is editable - Also use DocumentAccess in the AnnotationPage to check if document is editable --- .../annotation/page/AnnotationPageBase.java | 36 +++------ .../documents/api}/DocumentAccess.java | 17 +++-- .../documents/DocumentAccessImpl.java | 76 ++++++++++++++----- .../DocumentServiceAutoConfiguration.java | 2 +- .../webanno/ui/annotation/AnnotationPage.java | 2 +- .../actionbar/docnav/DocumentNavigator.java | 2 +- .../actionbar/CurationDocumentNavigator.java | 2 +- .../sidebar/SearchAnnotationSidebar.java | 28 +++++-- 8 files changed, 100 insertions(+), 65 deletions(-) rename inception/{inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents => inception-documents-api/src/main/java/de/tudarmstadt/ukp/inception/documents/api}/DocumentAccess.java (81%) diff --git a/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/page/AnnotationPageBase.java b/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/page/AnnotationPageBase.java index 605e69792fc..fe39e4d86b7 100644 --- a/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/page/AnnotationPageBase.java +++ b/inception/inception-api-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/page/AnnotationPageBase.java @@ -17,9 +17,6 @@ */ package de.tudarmstadt.ukp.clarin.webanno.api.annotation.page; -import static de.tudarmstadt.ukp.clarin.webanno.model.Mode.CURATION; -import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.ANNOTATOR; -import static de.tudarmstadt.ukp.clarin.webanno.model.SourceDocumentState.CURATION_FINISHED; import static de.tudarmstadt.ukp.inception.rendering.selection.FocusPosition.CENTERED; import static de.tudarmstadt.ukp.inception.support.WebAnnoConst.CURATION_USER; import static java.lang.String.format; @@ -49,6 +46,7 @@ import org.apache.wicket.util.string.StringValueConversionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.security.access.AccessDeniedException; import org.wicketstuff.urlfragment.UrlFragment; import org.wicketstuff.urlfragment.UrlParametersReceivingBehavior; @@ -62,6 +60,7 @@ import de.tudarmstadt.ukp.clarin.webanno.security.UserDao; import de.tudarmstadt.ukp.clarin.webanno.security.model.User; import de.tudarmstadt.ukp.clarin.webanno.ui.core.page.ProjectPageBase; +import de.tudarmstadt.ukp.inception.documents.api.DocumentAccess; import de.tudarmstadt.ukp.inception.documents.api.DocumentService; import de.tudarmstadt.ukp.inception.editor.action.AnnotationActionHandler; import de.tudarmstadt.ukp.inception.project.api.ProjectService; @@ -88,6 +87,7 @@ public abstract class AnnotationPageBase private @SpringBean AnnotationSchemaService annotationService; private @SpringBean DocumentService documentService; + private @SpringBean DocumentAccess documentAccess; private @SpringBean UserPreferencesService userPreferenceService; private @SpringBean UserDao userRepository; private @SpringBean ProjectService projectService; @@ -446,36 +446,20 @@ protected void loadPreferences() throws IOException public void ensureIsEditable() throws NotEditableException { - AnnotatorState state = getModelObject(); + var state = getModelObject(); if (state.getDocument() == null) { throw new NotEditableException("No document selected"); } - // If curating (check mode for curation page and user for curation sidebar), - // then it is editable unless the curation is finished - if (state.getMode() == CURATION || CURATION_USER.equals(state.getUser().getUsername())) { - if (state.getDocument().getState().equals(CURATION_FINISHED)) { - throw new NotEditableException("Curation is already finished. You can put it back " - + "into progress via the monitoring page."); - } - - return; - } - - if (getModelObject().isUserViewingOthersWork(userRepository.getCurrentUsername())) { - throw new NotEditableException( - "Viewing another users annotations - document is read-only!"); - } + var sessionOwner = userRepository.getCurrentUser(); - if (isAnnotationFinished()) { - throw new NotEditableException("This document is already closed for user [" - + state.getUser().getUsername() + "]. Please ask your " - + "project manager to re-open it via the monitoring page."); + try { + documentAccess.assertCanEditAnnotationDocument(sessionOwner, state.getDocument(), + state.getUser().getUsername()); } - - if (!projectService.hasRole(userRepository.getCurrentUsername(), getProject(), ANNOTATOR)) { - throw new NotEditableException("You are not an annotator in this project."); + catch (AccessDeniedException e) { + throw new NotEditableException(e.getMessage()); } } diff --git a/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccess.java b/inception/inception-documents-api/src/main/java/de/tudarmstadt/ukp/inception/documents/api/DocumentAccess.java similarity index 81% rename from inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccess.java rename to inception/inception-documents-api/src/main/java/de/tudarmstadt/ukp/inception/documents/api/DocumentAccess.java index 353ea22808f..3199102b50e 100644 --- a/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccess.java +++ b/inception/inception-documents-api/src/main/java/de/tudarmstadt/ukp/inception/documents/api/DocumentAccess.java @@ -15,19 +15,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package de.tudarmstadt.ukp.inception.documents; +package de.tudarmstadt.ukp.inception.documents.api; + +import org.springframework.security.access.AccessDeniedException; import de.tudarmstadt.ukp.clarin.webanno.model.Project; +import de.tudarmstadt.ukp.clarin.webanno.model.SourceDocument; import de.tudarmstadt.ukp.clarin.webanno.security.AccessCheckingBean; import de.tudarmstadt.ukp.clarin.webanno.security.model.User; -import de.tudarmstadt.ukp.inception.documents.config.DocumentServiceAutoConfiguration; -/** - *

- * This class is exposed as a Spring Component via - * {@link DocumentServiceAutoConfiguration#documentAccess}. - *

- */ public interface DocumentAccess extends AccessCheckingBean { @@ -39,5 +35,10 @@ boolean canViewAnnotationDocument(String aUser, String aProjectId, long aDocumen boolean canEditAnnotationDocument(String aUser, String aProjectId, long aDocumentId, String aAnnotator); + void assertCanEditAnnotationDocument(User aSessionOwner, + SourceDocument aDocument, String aDataOwner) + throws AccessDeniedException; + boolean canExportAnnotationDocument(User aUser, Project aProject); + } diff --git a/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccessImpl.java b/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccessImpl.java index 90171a33a9a..a2be0c96b2f 100644 --- a/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccessImpl.java +++ b/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccessImpl.java @@ -20,6 +20,8 @@ import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.ANNOTATOR; import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.CURATOR; import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.MANAGER; +import static de.tudarmstadt.ukp.clarin.webanno.model.SourceDocumentState.CURATION_FINISHED; +import static de.tudarmstadt.ukp.inception.support.WebAnnoConst.CURATION_USER; import static org.apache.commons.collections4.CollectionUtils.containsAny; import javax.persistence.NoResultException; @@ -29,12 +31,12 @@ import org.slf4j.LoggerFactory; import org.springframework.security.access.AccessDeniedException; -import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationDocument; import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationDocumentState; import de.tudarmstadt.ukp.clarin.webanno.model.Project; import de.tudarmstadt.ukp.clarin.webanno.model.SourceDocument; import de.tudarmstadt.ukp.clarin.webanno.security.UserDao; import de.tudarmstadt.ukp.clarin.webanno.security.model.User; +import de.tudarmstadt.ukp.inception.documents.api.DocumentAccess; import de.tudarmstadt.ukp.inception.documents.api.DocumentService; import de.tudarmstadt.ukp.inception.documents.config.DocumentServiceAutoConfiguration; import de.tudarmstadt.ukp.inception.project.api.ProjectService; @@ -139,34 +141,70 @@ public boolean canEditAnnotationDocument(String aSessionOwner, String aProjectId aSessionOwner, aProjectId, aDocumentId, aAnnotator); try { - User user = getUser(aSessionOwner); - Project project = getProject(aProjectId); + var sessionOwner = getUser(aSessionOwner); + var project = getProject(aProjectId); + var doc = documentService.getSourceDocument(project.getId(), aDocumentId); - // Does the user have the permission to access the project at all? - if (!projectService.hasRole(user, project, ANNOTATOR)) { - return false; + assertCanEditAnnotationDocument(sessionOwner, doc, aAnnotator); + + return true; + } + catch (NoResultException | AccessDeniedException e) { + // If any object does not exist, the user cannot edit + return false; + } + } + + @Override + public void assertCanEditAnnotationDocument(User aSessionOwner, SourceDocument aDocument, + String aDataOwner) + { + var project = aDocument.getProject(); + + // Is the user a curator? + if (projectService.hasRole(aSessionOwner, project, CURATOR)) { + // If curation is already done, document is no longer editable + if (CURATION_USER.equals(aDataOwner)) { + if (aDocument.getState() == CURATION_FINISHED) { + throw new AccessDeniedException( + "Curation is already finished. You can put it back " + + "into progress via the monitoring page."); + } + + return; // Access granted } - // Users can edit their own annotations - if (!aSessionOwner.equals(aAnnotator)) { - return false; + // Fall-through - user may still be an annotator + } + + // Is the user an annotator? + if (projectService.hasRole(aSessionOwner, project, ANNOTATOR)) { + // Annotators can edit their own annotations + if (!aSessionOwner.getUsername().equals(aDataOwner)) { + throw new AccessDeniedException( + "Viewing another users annotations - document is read-only!"); } - // Blocked documents cannot be edited - SourceDocument doc = documentService.getSourceDocument(project.getId(), aDocumentId); - if (documentService.existsAnnotationDocument(doc, aAnnotator)) { - AnnotationDocument aDoc = documentService.getAnnotationDocument(doc, aAnnotator); + // Blocked or finished documents cannot be edited + if (documentService.existsAnnotationDocument(aDocument, aDataOwner)) { + var aDoc = documentService.getAnnotationDocument(aDocument, aDataOwner); + if (aDoc.getState() == AnnotationDocumentState.FINISHED) { + throw new AccessDeniedException("This document is already closed for user [" + + aDataOwner + "]. Please ask your " + + "project manager to re-open it via the monitoring page."); + } + if (aDoc.getState() == AnnotationDocumentState.IGNORE) { - return false; + throw new AccessDeniedException("This document is blocked for user [" + + aDataOwner + "]. Please ask your " + + "project manager if you believe this is wrong."); } } - return true; - } - catch (NoResultException | AccessDeniedException e) { - // If any object does not exist, the user cannot edit - return false; + return; // Access granted } + + throw new AccessDeniedException("You have no permission to edit this document"); } @Override diff --git a/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/config/DocumentServiceAutoConfiguration.java b/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/config/DocumentServiceAutoConfiguration.java index 7ad89d5ec02..569e33a6031 100644 --- a/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/config/DocumentServiceAutoConfiguration.java +++ b/inception/inception-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/config/DocumentServiceAutoConfiguration.java @@ -27,9 +27,9 @@ import de.tudarmstadt.ukp.clarin.webanno.api.casstorage.CasStorageService; import de.tudarmstadt.ukp.clarin.webanno.api.export.DocumentImportExportService; import de.tudarmstadt.ukp.clarin.webanno.security.UserDao; -import de.tudarmstadt.ukp.inception.documents.DocumentAccess; import de.tudarmstadt.ukp.inception.documents.DocumentAccessImpl; import de.tudarmstadt.ukp.inception.documents.DocumentServiceImpl; +import de.tudarmstadt.ukp.inception.documents.api.DocumentAccess; import de.tudarmstadt.ukp.inception.documents.api.DocumentService; import de.tudarmstadt.ukp.inception.documents.api.RepositoryProperties; import de.tudarmstadt.ukp.inception.documents.exporters.SourceDocumentExporter; diff --git a/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/AnnotationPage.java b/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/AnnotationPage.java index df18cba5c9e..80adf72e237 100755 --- a/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/AnnotationPage.java +++ b/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/AnnotationPage.java @@ -82,7 +82,7 @@ import de.tudarmstadt.ukp.inception.annotation.events.FeatureValueUpdatedEvent; import de.tudarmstadt.ukp.inception.annotation.events.PreparingToOpenDocumentEvent; import de.tudarmstadt.ukp.inception.annotation.layer.span.SpanLayerSupport; -import de.tudarmstadt.ukp.inception.documents.DocumentAccess; +import de.tudarmstadt.ukp.inception.documents.api.DocumentAccess; import de.tudarmstadt.ukp.inception.documents.api.DocumentService; import de.tudarmstadt.ukp.inception.editor.AnnotationEditorBase; import de.tudarmstadt.ukp.inception.editor.AnnotationEditorExtensionRegistry; diff --git a/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/actionbar/docnav/DocumentNavigator.java b/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/actionbar/docnav/DocumentNavigator.java index e170952b46a..0bfc6c79cd9 100644 --- a/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/actionbar/docnav/DocumentNavigator.java +++ b/inception/inception-ui-annotation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/actionbar/docnav/DocumentNavigator.java @@ -32,7 +32,7 @@ import de.tudarmstadt.ukp.clarin.webanno.api.annotation.page.AnnotationPageBase; import de.tudarmstadt.ukp.clarin.webanno.security.UserDao; import de.tudarmstadt.ukp.clarin.webanno.ui.annotation.actionbar.open.OpenDocumentDialog; -import de.tudarmstadt.ukp.inception.documents.DocumentAccess; +import de.tudarmstadt.ukp.inception.documents.api.DocumentAccess; import de.tudarmstadt.ukp.inception.project.api.ProjectService; import de.tudarmstadt.ukp.inception.support.lambda.LambdaAjaxLink; import de.tudarmstadt.ukp.inception.support.wicket.input.InputBehavior; diff --git a/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/curation/actionbar/CurationDocumentNavigator.java b/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/curation/actionbar/CurationDocumentNavigator.java index 9a693d96872..0c3a1d5a97c 100644 --- a/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/curation/actionbar/CurationDocumentNavigator.java +++ b/inception/inception-ui-curation/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/curation/actionbar/CurationDocumentNavigator.java @@ -31,7 +31,7 @@ import de.tudarmstadt.ukp.clarin.webanno.api.annotation.actionbar.export.ExportDocumentDialog; import de.tudarmstadt.ukp.clarin.webanno.api.annotation.page.AnnotationPageBase; import de.tudarmstadt.ukp.clarin.webanno.security.UserDao; -import de.tudarmstadt.ukp.inception.documents.DocumentAccess; +import de.tudarmstadt.ukp.inception.documents.api.DocumentAccess; import de.tudarmstadt.ukp.inception.project.api.ProjectService; import de.tudarmstadt.ukp.inception.support.lambda.LambdaAjaxLink; import de.tudarmstadt.ukp.inception.support.wicket.input.InputBehavior; diff --git a/inception/inception-ui-search/src/main/java/de/tudarmstadt/ukp/inception/app/ui/search/sidebar/SearchAnnotationSidebar.java b/inception/inception-ui-search/src/main/java/de/tudarmstadt/ukp/inception/app/ui/search/sidebar/SearchAnnotationSidebar.java index ea892e72853..963308c7081 100644 --- a/inception/inception-ui-search/src/main/java/de/tudarmstadt/ukp/inception/app/ui/search/sidebar/SearchAnnotationSidebar.java +++ b/inception/inception-ui-search/src/main/java/de/tudarmstadt/ukp/inception/app/ui/search/sidebar/SearchAnnotationSidebar.java @@ -71,6 +71,7 @@ import org.apache.wicket.util.resource.ResourceStreamNotFoundException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.security.access.AccessDeniedException; import org.wicketstuff.event.annotation.OnEvent; import de.agilecoders.wicket.core.markup.html.bootstrap.navigation.BootstrapPagingNavigator.Size; @@ -82,6 +83,7 @@ import de.tudarmstadt.ukp.clarin.webanno.model.LinkMode; import de.tudarmstadt.ukp.clarin.webanno.model.SourceDocument; import de.tudarmstadt.ukp.clarin.webanno.security.UserDao; +import de.tudarmstadt.ukp.clarin.webanno.security.model.User; import de.tudarmstadt.ukp.clarin.webanno.ui.annotation.AnnotationPage; import de.tudarmstadt.ukp.clarin.webanno.ui.annotation.sidebar.AnnotationSidebar_ImplBase; import de.tudarmstadt.ukp.inception.annotation.events.BulkAnnotationEvent; @@ -90,6 +92,7 @@ import de.tudarmstadt.ukp.inception.app.ui.search.sidebar.options.DeleteAnnotationsOptions; import de.tudarmstadt.ukp.inception.app.ui.search.sidebar.options.SearchOptions; import de.tudarmstadt.ukp.inception.bootstrap.IconToggleBox; +import de.tudarmstadt.ukp.inception.documents.api.DocumentAccess; import de.tudarmstadt.ukp.inception.documents.api.DocumentService; import de.tudarmstadt.ukp.inception.editor.action.AnnotationActionHandler; import de.tudarmstadt.ukp.inception.rendering.editorstate.AnnotatorState; @@ -144,6 +147,7 @@ public class SearchAnnotationSidebar private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private @SpringBean DocumentService documentService; + private @SpringBean DocumentAccess documentAccess; private @SpringBean AnnotationSchemaService annotationService; private @SpringBean SearchService searchService; private @SpringBean UserDao userRepository; @@ -607,6 +611,7 @@ public void actionApplyToSelectedResults(AjaxRequestTarget aTarget, Operation aC return; } + var sessionOwner = userRepository.getCurrentUser(); var dataOwner = getAnnotationPage().getModelObject().getUser(); var layer = getModelObject().getSelectedAnnotationLayer(); try { @@ -628,17 +633,12 @@ public void actionApplyToSelectedResults(AjaxRequestTarget aTarget, Operation aC var sourceDoc = documentService.getSourceDocument(state.getProject().getId(), documentId); - var annoDoc = documentService.createOrGetAnnotationDocument(sourceDoc, dataOwner); - - switch (annoDoc.getState()) { - case FINISHED: // fall-through - case IGNORE: - // Skip processing any documents which are finished or ignored + if (!canAccessDocument(sessionOwner, sourceDoc, dataOwner)) { continue; - default: - // Do nothing } + var annoDoc = documentService.createOrGetAnnotationDocument(sourceDoc, dataOwner); + // Holder for lazily-loaded CAS Optional cas = Optional.empty(); @@ -695,6 +695,18 @@ public void actionApplyToSelectedResults(AjaxRequestTarget aTarget, Operation aC getAnnotationPage().actionRefreshDocument(aTarget); } + private boolean canAccessDocument(User sessionOwner, SourceDocument sourceDoc, User dataOwner) + { + try { + documentAccess.assertCanEditAnnotationDocument(sessionOwner, sourceDoc, + dataOwner.getUsername()); + return true; + } + catch (AccessDeniedException e) { + return false; + } + } + private void createAnnotationAtSearchResult(SourceDocument aDocument, CAS aCas, SpanAdapter aAdapter, SearchResult aSearchResult, BulkOperationResult aBulkResult) throws AnnotationException