Skip to content

Commit

Permalink
#4235 - Direct access-by-URL to sidebar curation mode sometimes does …
Browse files Browse the repository at this point in the history 
…not work

- More detailed trace logging related to curation sidebar and access control
  • Loading branch information
@reckart
reckart committed Oct 26, 2023
1 parent a75cb5c commit e6b7825
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 59 deletions.
26 changes: 18 additions & 8 deletions ...on-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccessImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,7 +33,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;
Expand Down Expand Up @@ -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;
}
Expand Down
39 changes: 32 additions & 7 deletions ...on-project/src/main/java/de/tudarmstadt/ukp/clarin/webanno/project/ProjectAccessImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
121 changes: 77 additions & 44 deletions ...c/main/java/de/tudarmstadt/ukp/inception/ui/curation/sidebar/CurationSidebarBehavior.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@

import static de.tudarmstadt.ukp.clarin.webanno.support.WebAnnoConst.CURATION_USER;
import static de.tudarmstadt.ukp.clarin.webanno.ui.core.page.ProjectPageBase.setProjectPageParameter;
import static java.lang.invoke.MethodHandles.lookup;
import static java.util.Arrays.asList;

import java.lang.invoke.MethodHandles;
import static org.slf4j.LoggerFactory.getLogger;

import org.apache.wicket.Component;
import org.apache.wicket.RestartResponseException;
Expand All @@ -30,7 +30,6 @@
import org.apache.wicket.request.mapper.parameter.PageParameters;
import org.apache.wicket.spring.injection.annot.SpringBean;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import de.tudarmstadt.ukp.clarin.webanno.api.annotation.page.AnnotationPageBase;
import de.tudarmstadt.ukp.clarin.webanno.model.Project;
Expand All @@ -44,7 +43,7 @@ public class CurationSidebarBehavior
{
private static final long serialVersionUID = -6224298395673360592L;

private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private static final Logger LOG = getLogger(lookup().lookupClass());

private static final String STAY = "stay";
private static final String OFF = "off";
Expand All @@ -60,6 +59,13 @@ public class CurationSidebarBehavior
public void onEvent(Component aComponent, IEvent<?> aEvent)
{
if (!(aEvent.getPayload() instanceof PreparingToOpenDocumentEvent)) {
if (aEvent.getPayload() != null) {
LOG.trace("Event not relevant to curation sidebar: {} / {}", aEvent.getClass(),
aEvent.getPayload().getClass());
}
else {
LOG.trace("Event not relevant to curation sidebar: {}", aEvent.getClass());
}
return;
}

Expand All @@ -69,6 +75,9 @@ public void onEvent(Component aComponent, IEvent<?> aEvent)

if (!(page instanceof AnnotationPage)) {
// Only applies to the AnnotationPage - not to the CurationPage!
LOG.trace(
"Curation sidebar is not deployed on AnnotationPage but rather [{}] - ignoring event [{}]",
page.getClass(), event.getClass());
return;
}

Expand All @@ -79,6 +88,9 @@ public void onEvent(Component aComponent, IEvent<?> aEvent)
var project = doc.getProject();
var dataOwner = event.getDocumentOwner();

LOG.trace("Curation sidebar reacting to [{}]@{} being opened by [{}]", dataOwner, doc,
sessionOwner);

handleSessionActivation(page, params, doc, sessionOwner);

ensureDataOwnerMatchesCurationTarget(page, project, sessionOwner, dataOwner);
Expand All @@ -87,17 +99,29 @@ public void onEvent(Component aComponent, IEvent<?> aEvent)
private void ensureDataOwnerMatchesCurationTarget(AnnotationPageBase aPage, Project aProject,
String aSessionOwner, String aDataOwner)
{
// Are we curating?
if (isViewingPotentialCurationTarget(aDataOwner) && isSessionActive(aProject)) {
// If the curation target user is different from the data owner set in the annotation
// state, then we need to update the state and reload.
var curationTarget = curationSidebarService.getCurationTargetUser(aSessionOwner,
aProject.getId());
if (!aDataOwner.equals(curationTarget.getUsername())) {
LOG.trace("Data owner [{}] should match curation target {} - changing to {}",
curationTarget, aDataOwner, curationTarget);
aPage.getModelObject().setUser(curationTarget);
}
if (!isSessionActive(aProject)) {
LOG.trace(
"No curation session active - no need to adjust data owner to curation target");
return;
}

if (!isViewingPotentialCurationTarget(aDataOwner)) {
return;
}

// If the curation target user is different from the data owner set in the annotation
// state, then we need to update the state and reload.
var curationTarget = curationSidebarService.getCurationTargetUser(aSessionOwner,
aProject.getId());

if (!aDataOwner.equals(curationTarget.getUsername())) {
LOG.trace("Data owner [{}] should match curation target {} - changing to {}",
curationTarget, aDataOwner, curationTarget);
aPage.getModelObject().setUser(curationTarget);
}
else {
LOG.trace("Data owner [{}] alredy matches curation target {}", curationTarget,
aDataOwner);
}
}

Expand All @@ -106,43 +130,52 @@ private void handleSessionActivation(AnnotationPageBase aPage, PageParameters aP
{
var project = aDoc.getProject();
var curationSessionParameterValue = aParams.get(PARAM_CURATION_SESSION);
if (!curationSessionParameterValue.isEmpty()) {
switch (curationSessionParameterValue.toString(STAY)) {
case ON:
LOG.trace("Checking if to start curation session");
// Start a new session or switch to new curation target
var curationTargetOwnParameterValue = aParams.get(PARAM_CURATION_TARGET_OWN);
if (!isSessionActive(project) || !curationTargetOwnParameterValue.isEmpty()) {
curationSidebarService.startSession(aSessionOwner, project,
curationTargetOwnParameterValue.toBoolean(false));
}
break;
case OFF:
LOG.trace("Checking if to stop curation session");
if (isSessionActive(project)) {
curationSidebarService.closeSession(aSessionOwner, project.getId());
}
break;
default:
// Ignore
}
if (curationSessionParameterValue.isEmpty()) {
return;
}

LOG.trace("Removing session control parameters and reloading (redirect)");
aParams.remove(PARAM_CURATION_TARGET_OWN);
aParams.remove(PARAM_CURATION_SESSION);
setProjectPageParameter(aParams, project);
aParams.set(AnnotationPage.PAGE_PARAM_DOCUMENT, aDoc.getId());
// We need to do a redirect here to discard the arguments from the URL.
// This also discards the page state.
throw new RestartResponseException(aPage.getClass(), aParams);
switch (curationSessionParameterValue.toString(STAY)) {
case ON:
LOG.trace("Checking if to start curation session");
// Start a new session or switch to new curation target
var curationTargetOwnParameterValue = aParams.get(PARAM_CURATION_TARGET_OWN);
if (!isSessionActive(project) || !curationTargetOwnParameterValue.isEmpty()) {
curationSidebarService.startSession(aSessionOwner, project,
curationTargetOwnParameterValue.toBoolean(false));
}
break;
case OFF:
LOG.trace("Checking if to stop curation session");
if (isSessionActive(project)) {
curationSidebarService.closeSession(aSessionOwner, project.getId());
}
break;
default:
// Ignore
LOG.trace("No change in curation session state requested [{}]",
curationSessionParameterValue);
}

LOG.trace("Removing session control parameters and reloading (redirect)");
aParams.remove(PARAM_CURATION_TARGET_OWN);
aParams.remove(PARAM_CURATION_SESSION);
setProjectPageParameter(aParams, project);
aParams.set(AnnotationPage.PAGE_PARAM_DOCUMENT, aDoc.getId());
// We need to do a redirect here to discard the arguments from the URL.
// This also discards the page state.
throw new RestartResponseException(aPage.getClass(), aParams);
}

private boolean isViewingPotentialCurationTarget(String aDataOwner)
{
// Curation sidebar is not allowed when viewing another users annotations
var sessionOwner = userService.getCurrentUsername();
return asList(CURATION_USER, sessionOwner).contains(aDataOwner);
var candidates = asList(CURATION_USER, sessionOwner);
var result = candidates.contains(aDataOwner);
if (!result) {
LOG.trace("Data ownwer [{}] is not in curation candidates {}", aDataOwner, candidates);
}
return result;
}

private boolean isSessionActive(Project aProject)
Expand Down
10 changes: 10 additions & 0 deletions ...rc/main/java/de/tudarmstadt/ukp/inception/ui/curation/sidebar/CurationSidebarFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
package de.tudarmstadt.ukp.inception.ui.curation.sidebar;

import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.CURATOR;
import static java.lang.invoke.MethodHandles.lookup;
import static org.slf4j.LoggerFactory.getLogger;

import org.apache.wicket.Component;
import org.apache.wicket.model.IModel;
import org.slf4j.Logger;

import de.tudarmstadt.ukp.clarin.webanno.api.CasProvider;
import de.tudarmstadt.ukp.clarin.webanno.api.ProjectService;
Expand All @@ -41,6 +44,8 @@
public class CurationSidebarFactory
extends AnnotationSidebarFactory_ImplBase
{
private static final Logger LOG = getLogger(lookup().lookupClass());

private final ProjectService projectService;
private final UserDao userService;

Expand Down Expand Up @@ -75,9 +80,14 @@ public AnnotationSidebar_ImplBase create(String aId, IModel<AnnotatorState> aMod
{
var sidebar = new CurationSidebar(aId, aModel, aActionHandler, aCasProvider,
aAnnotationPage);

if (aAnnotationPage.getBehaviors(CurationSidebarBehavior.class).isEmpty()) {
LOG.trace("Installing CurationSidebarBehavior");
aAnnotationPage.add(new CurationSidebarBehavior());
}
else {
LOG.trace("CurationSidebarBehavior is already installed");
}

return sidebar;
}
Expand Down

0 comments on commit e6b7825

Please sign in to comment.