Merge branch 'release/29.x'

* release/29.x: #4235 - Direct access-by-URL to sidebar curation mode sometimes does not work #4235 - Direct access-by-URL to sidebar curation mode sometimes does not work #4235 - Direct access-by-URL to sidebar curation mode sometimes does not work
inception-project · Oct 31, 2023 · 1990c16 · 1990c16
2 parents 5382d57 + a71ae8e
commit 1990c16
Show file tree

Hide file tree

Showing 13 changed files with 409 additions and 170 deletions.
diff --git a/...c/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/page/AnnotationPageBase.java b/...c/main/java/de/tudarmstadt/ukp/clarin/webanno/api/annotation/page/AnnotationPageBase.java
@@ -42,7 +42,6 @@
 import org.apache.wicket.request.IRequestParameters;
 import org.apache.wicket.request.RequestHandlerExecutor.ReplaceHandlerException;
 import org.apache.wicket.request.Url;
-import org.apache.wicket.request.cycle.RequestCycle;
 import org.apache.wicket.request.flow.RedirectToUrlException;
 import org.apache.wicket.request.mapper.parameter.PageParameters;
 import org.apache.wicket.spring.injection.annot.SpringBean;
@@ -101,31 +100,34 @@ protected AnnotationPageBase(PageParameters aParameters)
         super(aParameters);
 
         var params = getPageParameters();
-        StringValue documentParameter = params.get(PAGE_PARAM_DOCUMENT);
-        StringValue userParameter = params.get(PAGE_PARAM_DATA_OWNER);
+        var documentParameter = params.get(PAGE_PARAM_DOCUMENT);
+        var userParameter = params.get(PAGE_PARAM_DATA_OWNER);
 
         // If the page was accessed using an URL form ending in a document ID, let's move
         // the document ID into the fragment and redirect to the form without the document ID.
         // This ensures that any links on the page do not carry the document ID, so that we can
         // happily switch between documents using AJAX without having to worry about links with
         // a document ID potentially sending us back to a specific document.
         if (!documentParameter.isEmpty()) {
-            RequestCycle requestCycle = getRequestCycle();
-            Url clientUrl = requestCycle.getRequest().getClientUrl();
-            clientUrl.resolveRelative(Url.parse("./"));
-            List<String> fragmentParams = new ArrayList<>();
+            var requestCycle = getRequestCycle();
+
+            var fragmentParams = new ArrayList<String>();
             fragmentParams.add(format("%s=%s", PAGE_PARAM_DOCUMENT, documentParameter.toString()));
             params.remove(PAGE_PARAM_DOCUMENT);
+
             if (!userParameter.isEmpty()) {
-                fragmentParams.add(format("%s=%s", PAGE_PARAM_DATA_OWNER, userParameter.toString()));
+                fragmentParams
+                        .add(format("%s=%s", PAGE_PARAM_DATA_OWNER, userParameter.toString()));
                 params.remove(PAGE_PARAM_DATA_OWNER);
             }
-            for (var namedParam : params.getAllNamed()) {
-                clientUrl.setQueryParameter(namedParam.getKey(), namedParam.getValue());
-            }
-            clientUrl.setFragment("!" + fragmentParams.stream().collect(joining("&")));
-            String url = requestCycle.getUrlRenderer().renderRelativeUrl(clientUrl);
-            throw new RedirectToUrlException(url.toString());
+
+            var url = Url.parse(requestCycle.urlFor(this.getClass(), params));
+            url.setFragment("!" + fragmentParams.stream().collect(joining("&")));
+            var finalUrl = requestCycle.getUrlRenderer().renderFullUrl(url);
+            LOG.trace(
+                    "Pushing parameter for document [{}] and user [{}] into fragment: {} (URL redirect)",
+                    documentParameter, userParameter, finalUrl);
+            throw new RedirectToUrlException(finalUrl.toString());
         }
     }
 
@@ -197,18 +199,18 @@ protected UrlParametersReceivingBehavior createUrlFragmentBehavior()
             protected void onParameterArrival(IRequestParameters aRequestParameters,
                     AjaxRequestTarget aTarget)
             {
-                StringValue document = aRequestParameters.getParameterValue(PAGE_PARAM_DOCUMENT);
-                StringValue focus = aRequestParameters.getParameterValue(PAGE_PARAM_FOCUS);
-                StringValue user = aRequestParameters.getParameterValue(PAGE_PARAM_DATA_OWNER);
+                var document = aRequestParameters.getParameterValue(PAGE_PARAM_DOCUMENT);
+                var focus = aRequestParameters.getParameterValue(PAGE_PARAM_FOCUS);
+                var user = aRequestParameters.getParameterValue(PAGE_PARAM_DATA_OWNER);
 
-                // nothing changed, do not check for project, because inception always opens
-                // on a project
                 if (document.isEmpty() && focus.isEmpty()) {
                     return;
                 }
 
-                SourceDocument previousDoc = getModelObject().getDocument();
-                User aPreviousUser = getModelObject().getUser();
+                LOG.trace("URL fragment update: {}@{} focus {}", user, document, focus);
+
+                var previousDoc = getModelObject().getDocument();
+                var aPreviousUser = getModelObject().getUser();
                 handleParameters(document, focus, user);
 
                 updateDocumentView(aTarget, previousDoc, aPreviousUser, focus);
@@ -219,6 +221,20 @@ protected void onParameterArrival(IRequestParameters aRequestParameters,
     protected abstract void handleParameters(StringValue aDocumentParameter,
             StringValue aFocusParameter, StringValue aUser);
 
+    /**
+     * Switch between documents. Note that the document and data owner to switch to are obtained
+     * from the {@link AnnotatorState}. The parameters indicate the the old document and data owner
+     * before the switch!
+     * 
+     * @param aTarget
+     *            a request target.
+     * @param aPreviousDocument
+     *            the document before the switch.
+     * @param aPreviousUser
+     *            the data owner before the switch.
+     * @param aFocusParameter
+     *            the focus before the switch.
+     */
     protected abstract void updateDocumentView(AjaxRequestTarget aTarget,
             SourceDocument aPreviousDocument, User aPreviousUser, StringValue aFocusParameter);
 
@@ -511,14 +527,14 @@ private class UrlFragmentUpdateListener
         @Override
         public void onTargetRespond(AjaxRequestTarget aTarget)
         {
-            AnnotatorState state = getModelObject();
+            var state = getModelObject();
 
             if (state.getDocument() == null) {
                 return;
             }
 
-            Long currentDocumentId = state.getDocument().getId();
-            int currentFocusUnitIndex = state.getFocusUnitIndex();
+            var currentDocumentId = state.getDocument().getId();
+            var currentFocusUnitIndex = state.getFocusUnitIndex();
 
             // Check if the relevant parameters have actually changed since the URL parameters were
             // last set - if this is not the case, then let's not set the parameters because that
@@ -532,7 +548,7 @@ public void onTargetRespond(AjaxRequestTarget aTarget)
             urlFragmentLastDocumentId = currentDocumentId;
             urlFragmentLastFocusUnitIndex = currentFocusUnitIndex;
 
-            UrlFragment fragment = new UrlFragment(aTarget);
+            var fragment = new UrlFragment(aTarget);
 
             fragment.putParameter(PAGE_PARAM_DOCUMENT, currentDocumentId);
 

diff --git a/...ain/java/de/tudarmstadt/ukp/inception/annotation/events/PreparingToOpenDocumentEvent.java b/...ain/java/de/tudarmstadt/ukp/inception/annotation/events/PreparingToOpenDocumentEvent.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Technische Universität Darmstadt under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The Technische Universität Darmstadt 
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.
+ *  
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package de.tudarmstadt.ukp.inception.annotation.events;
+
+import org.springframework.context.ApplicationEvent;
+
+import de.tudarmstadt.ukp.clarin.webanno.api.annotation.page.AnnotationPageBase;
+import de.tudarmstadt.ukp.clarin.webanno.model.SourceDocument;
+import de.tudarmstadt.ukp.clarin.webanno.support.wicket.event.HybridApplicationUIEvent;
+import de.tudarmstadt.ukp.inception.rendering.editorstate.AnnotatorState;
+
+/**
+ * Fire this event to give listeners a chance to look at or even modify the {@link AnnotatorState}
+ * before actually loading the data.
+ */
+public class PreparingToOpenDocumentEvent
+    extends ApplicationEvent
+    implements HybridApplicationUIEvent
+{
+    private static final long serialVersionUID = -5971290341142438144L;
+
+    private final SourceDocument document;
+    // user who owns/annotates the opened document
+    private final String documentOwner;
+    // user who opened the document
+    private final String sessionOwner;
+
+    public PreparingToOpenDocumentEvent(AnnotationPageBase aSource, SourceDocument aDocument,
+            String aDocumentOwner, String aSessionOwner)
+    {
+        super(aSource);
+        document = aDocument;
+        documentOwner = aDocumentOwner;
+        sessionOwner = aSessionOwner;
+    }
+
+    public SourceDocument getDocument()
+    {
+        return document;
+    }
+
+    public String getSessionOwner()
+    {
+        return sessionOwner;
+    }
+
+    public String getDocumentOwner()
+    {
+        return documentOwner;
+    }
+
+    @Override
+    public AnnotationPageBase getSource()
+    {
+        return (AnnotationPageBase) super.getSource();
+    }
+}
diff --git a/...on-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccessImpl.java b/...on-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccessImpl.java
@@ -22,8 +22,6 @@
 import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.MANAGER;
 import static org.apache.commons.collections4.CollectionUtils.containsAny;
 
-import java.util.List;
-
 import javax.persistence.NoResultException;
 
 import org.apache.commons.lang3.StringUtils;
@@ -34,7 +32,6 @@
 import de.tudarmstadt.ukp.clarin.webanno.api.ProjectService;
 import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationDocument;
 import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationDocumentState;
-import de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel;
 import de.tudarmstadt.ukp.clarin.webanno.model.Project;
 import de.tudarmstadt.ukp.clarin.webanno.model.SourceDocument;
 import de.tudarmstadt.ukp.clarin.webanno.security.UserDao;
@@ -82,38 +79,51 @@ public boolean canViewAnnotationDocument(String aSessionOwner, String aProjectId
                 aSessionOwner, aProjectId, aDocumentId, aAnnotator);
 
         try {
-            User user = getUser(aSessionOwner);
-            Project project = getProject(aProjectId);
+            var user = getUser(aSessionOwner);
+            var project = getProject(aProjectId);
 
-            List<PermissionLevel> permissionLevels = projectService.listRoles(project, user);
+            var permissionLevels = projectService.listRoles(project, user);
 
             // Does the user have the permission to access the project at all?
             if (permissionLevels.isEmpty()) {
+                log.trace("Access denied: User {} has no acccess to project {}", user, project);
                 return false;
             }
 
             // Managers and curators can see anything
             if (containsAny(permissionLevels, MANAGER, CURATOR)) {
+                log.trace("Access granted: User {} can view annotations [{}] as MANGER or CURATOR",
+                        user, aDocumentId);
                 return true;
             }
 
             // Annotators can only see their own documents
             if (!aSessionOwner.equals(aAnnotator)) {
+                log.trace(
+                        "Access denied: User {} tries to see annotations from [{}] but can only see own annotations",
+                        user, aAnnotator);
                 return false;
             }
 
             // Annotators cannot view blocked documents
-            SourceDocument doc = documentService.getSourceDocument(project.getId(), aDocumentId);
+            var doc = documentService.getSourceDocument(project.getId(), aDocumentId);
             if (documentService.existsAnnotationDocument(doc, aAnnotator)) {
-                AnnotationDocument aDoc = documentService.getAnnotationDocument(doc, aAnnotator);
+                var aDoc = documentService.getAnnotationDocument(doc, aAnnotator);
                 if (aDoc.getState() == AnnotationDocumentState.IGNORE) {
+                    log.trace("Access denied: Document {} is locked (IGNORE) for user {}", aDoc,
+                            aAnnotator);
                     return false;
                 }
             }
 
+            log.trace(
+                    "Access granted: canViewAnnotationDocument [aSessionOwner: {}] [project: {}] "
+                            + "[document: {}] [annotator: {}]",
+                    aSessionOwner, aProjectId, aDocumentId, aAnnotator);
             return true;
         }
         catch (NoResultException | AccessDeniedException e) {
+            log.trace("Access denied: prerequisites not met", e);
             // If any object does not exist, the user cannot view
             return false;
         }

diff --git a/...log/src/main/java/de/tudarmstadt/ukp/inception/log/config/EventLoggingPropertiesImpl.java b/...log/src/main/java/de/tudarmstadt/ukp/inception/log/config/EventLoggingPropertiesImpl.java
@@ -24,6 +24,7 @@
 
 import de.tudarmstadt.ukp.clarin.webanno.api.event.AfterCasWrittenEvent;
 import de.tudarmstadt.ukp.inception.annotation.events.BeforeDocumentOpenedEvent;
+import de.tudarmstadt.ukp.inception.annotation.events.PreparingToOpenDocumentEvent;
 
 @ConfigurationProperties("event-logging")
 public class EventLoggingPropertiesImpl
@@ -37,6 +38,7 @@ public class EventLoggingPropertiesImpl
             AvailabilityChangeEvent.class.getSimpleName(), //
             "RecommenderTaskNotificationEvent", //
             BeforeDocumentOpenedEvent.class.getSimpleName(), //
+            PreparingToOpenDocumentEvent.class.getSimpleName(), //
             "BrokerAvailabilityEvent", //
             "ShutdownDialogAvailableEvent");
 

diff --git a/...on-project/src/main/java/de/tudarmstadt/ukp/clarin/webanno/project/ProjectAccessImpl.java b/...on-project/src/main/java/de/tudarmstadt/ukp/clarin/webanno/project/ProjectAccessImpl.java
@@ -62,12 +62,25 @@ public boolean canAccessProject(String aUser, String aProjectId)
         log.trace("Permission check: canAccessProject [user: {}] [project: {}]", aUser, aProjectId);
 
         try {
-            User user = getUser(aUser);
-            Project project = getProject(aProjectId);
+            var user = getUser(aUser);
+            var project = getProject(aProjectId);
 
-            return userService.isAdministrator(user) || projectService.hasAnyRole(user, project);
+            if (userService.isAdministrator(user)) {
+                log.trace("Access granted: User {} can access project {} as administrator", user,
+                        project);
+                return true;
+            }
+
+            if (projectService.hasAnyRole(user, project)) {
+                log.trace("Access granted: User {} can access project {} as project member", user,
+                        project);
+                return true;
+            }
+
+            return false;
         }
         catch (NoResultException | AccessDeniedException e) {
+            log.trace("Access denied: prerequisites not met", e);
             // If any object does not exist, the user cannot view
             return false;
         }
@@ -84,13 +97,25 @@ public boolean canManageProject(String aUser, String aProjectId)
         log.trace("Permission check: canManageProject [user: {}] [project: {}]", aUser, aProjectId);
 
         try {
-            User user = getUser(aUser);
-            Project project = getProject(aProjectId);
+            var user = getUser(aUser);
+            var project = getProject(aProjectId);
 
-            return userService.isAdministrator(user)
-                    || projectService.hasRole(user, project, PermissionLevel.MANAGER);
+            if (userService.isAdministrator(user)) {
+                log.trace("Access granted: User {} can manage project {} as administrator", user,
+                        project);
+                return true;
+            }
+
+            if (projectService.hasRole(user, project, PermissionLevel.MANAGER)) {
+                log.trace("Access granted: User {} can manage project {} as manager", user,
+                        project);
+                return true;
+            }
+
+            return false;
         }
         catch (NoResultException | AccessDeniedException e) {
+            log.trace("Access denied: prerequisites not met", e);
             // If any object does not exist, the user cannot view
             return false;
         }