Skip to content

Commit

Permalink
#4048 - Document navigation options not visible to manager when viewi…
Browse files Browse the repository at this point in the history 
…ng other users document

- Centralize permission check for exporting documents into DocumentAccess class
- Permission check for exporting should consider the session owner, not the document owner
  • Loading branch information
@reckart
reckart committed Jun 1, 2023
1 parent 61dfc54 commit b8b0c9a
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 31 deletions.
4 changes: 4 additions & 0 deletions ...eption-documents/src/main/java/de/tudarmstadt/ukp/inception/documents/DocumentAccess.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
*/
package de.tudarmstadt.ukp.inception.documents;

import de.tudarmstadt.ukp.clarin.webanno.model.Project;
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;

/**
Expand All @@ -36,4 +38,6 @@ boolean canViewAnnotationDocument(String aUser, String aProjectId, long aDocumen

boolean canEditAnnotationDocument(String aUser, String aProjectId, long aDocumentId,
String aAnnotator);

boolean canExportAnnotationDocument(User aUser, Project aProject);
}
43 changes: 30 additions & 13 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 @@ -17,6 +17,7 @@
*/
package de.tudarmstadt.ukp.inception.documents;

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 org.apache.commons.collections4.CollectionUtils.containsAny;
Expand Down Expand Up @@ -72,15 +73,16 @@ public boolean canViewAnnotationDocument(String aProjectId, String aDocumentId,
}

@Override
public boolean canViewAnnotationDocument(String aUser, String aProjectId, long aDocumentId,
String aAnnotator)
public boolean canViewAnnotationDocument(String aSessionOwner, String aProjectId,
long aDocumentId, String aAnnotator)
{
log.trace(
"Permission check: canViewAnnotationDocument [user: {}] [project: {}] [document: {}] [annotator: {}]",
aUser, aProjectId, aDocumentId, aAnnotator);
"Permission check: canViewAnnotationDocument [aSessionOwner: {}] [project: {}] "
+ "[document: {}] [annotator: {}]",
aSessionOwner, aProjectId, aDocumentId, aAnnotator);

try {
User user = getUser(aUser);
User user = getUser(aSessionOwner);
Project project = getProject(aProjectId);

List<PermissionLevel> permissionLevels = projectService.listRoles(project, user);
Expand All @@ -96,7 +98,7 @@ public boolean canViewAnnotationDocument(String aUser, String aProjectId, long a
}

// Annotators can only see their own documents
if (!aUser.equals(aAnnotator)) {
if (!aSessionOwner.equals(aAnnotator)) {
return false;
}

Expand All @@ -118,24 +120,25 @@ public boolean canViewAnnotationDocument(String aUser, String aProjectId, long a
}

@Override
public boolean canEditAnnotationDocument(String aUser, String aProjectId, long aDocumentId,
String aAnnotator)
public boolean canEditAnnotationDocument(String aSessionOwner, String aProjectId,
long aDocumentId, String aAnnotator)
{
log.trace(
"Permission check: canEditAnnotationDocument [user: {}] [project: {}] [document: {}] [annotator: {}]",
aUser, aProjectId, aDocumentId, aAnnotator);
"Permission check: canEditAnnotationDocument [aSessionOwner: {}] [project: {}] "
+ "[document: {}] [annotator: {}]",
aSessionOwner, aProjectId, aDocumentId, aAnnotator);

try {
User user = getUser(aUser);
User user = getUser(aSessionOwner);
Project project = getProject(aProjectId);

// Does the user have the permission to access the project at all?
if (!projectService.hasRole(user, project, PermissionLevel.ANNOTATOR)) {
if (!projectService.hasRole(user, project, ANNOTATOR)) {
return false;
}

// Users can edit their own annotations
if (!aUser.equals(aAnnotator)) {
if (!aSessionOwner.equals(aAnnotator)) {
return false;
}

Expand All @@ -156,6 +159,20 @@ public boolean canEditAnnotationDocument(String aUser, String aProjectId, long a
}
}

@Override
public boolean canExportAnnotationDocument(User aSessionOwner, Project aProject)
{
if (aProject == null) {
return false;
}

if (projectService.hasRole(aSessionOwner, aProject, MANAGER)) {
return true;
}

return !aProject.isDisableExport();
}

private Project getProject(String aProjectId)
{
try {
Expand Down
20 changes: 12 additions & 8 deletions ...a/de/tudarmstadt/ukp/clarin/webanno/ui/annotation/actionbar/docnav/DocumentNavigator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/
package de.tudarmstadt.ukp.clarin.webanno.ui.annotation.actionbar.docnav;

import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.MANAGER;
import static de.tudarmstadt.ukp.clarin.webanno.support.lambda.LambdaBehavior.visibleWhen;
import static wicket.contrib.input.events.EventType.click;
import static wicket.contrib.input.events.key.KeyType.Page_down;
Expand All @@ -32,10 +31,11 @@
import de.tudarmstadt.ukp.clarin.webanno.api.ProjectService;
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.clarin.webanno.support.lambda.LambdaAjaxLink;
import de.tudarmstadt.ukp.clarin.webanno.support.wicket.input.InputBehavior;
import de.tudarmstadt.ukp.clarin.webanno.ui.annotation.actionbar.open.OpenDocumentDialog;
import de.tudarmstadt.ukp.inception.rendering.editorstate.AnnotatorState;
import de.tudarmstadt.ukp.inception.documents.DocumentAccess;
import wicket.contrib.input.events.key.KeyType;

public class DocumentNavigator
Expand All @@ -44,6 +44,8 @@ public class DocumentNavigator
private static final long serialVersionUID = 7061696472939390003L;

private @SpringBean ProjectService projectService;
private @SpringBean UserDao userService;
private @SpringBean DocumentAccess documentAccess;

private AnnotationPageBase page;

Expand All @@ -64,12 +66,14 @@ public DocumentNavigator(String aId, AnnotationPageBase aPage)
add(new LambdaAjaxLink("showOpenDocumentDialog", this::actionShowOpenDocumentDialog));

add(exportDialog = new ExportDocumentDialog("exportDialog", page.getModel()));
add(new LambdaAjaxLink("showExportDialog", exportDialog::show).add(visibleWhen(() -> {
AnnotatorState state = page.getModelObject();
return state.getProject() != null
&& (projectService.hasRole(state.getUser(), state.getProject(), MANAGER)
|| !state.getProject().isDisableExport());
})));
add(new LambdaAjaxLink("showExportDialog", exportDialog::show)
.add(visibleWhen(this::isExportable)));
}

private boolean isExportable()
{
return documentAccess.canExportAnnotationDocument(userService.getCurrentUser(),
page.getModelObject().getProject());
}

/**
Expand Down
12 changes: 6 additions & 6 deletions ...va/de/tudarmstadt/ukp/clarin/webanno/ui/curation/actionbar/CurationDocumentNavigator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/
package de.tudarmstadt.ukp.clarin.webanno.ui.curation.actionbar;

import static de.tudarmstadt.ukp.clarin.webanno.model.PermissionLevel.MANAGER;
import static de.tudarmstadt.ukp.clarin.webanno.support.lambda.LambdaBehavior.visibleWhen;
import static wicket.contrib.input.events.EventType.click;
import static wicket.contrib.input.events.key.KeyType.Page_down;
Expand All @@ -32,9 +31,10 @@
import de.tudarmstadt.ukp.clarin.webanno.api.ProjectService;
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.clarin.webanno.support.lambda.LambdaAjaxLink;
import de.tudarmstadt.ukp.clarin.webanno.support.wicket.input.InputBehavior;
import de.tudarmstadt.ukp.inception.rendering.editorstate.AnnotatorState;
import de.tudarmstadt.ukp.inception.documents.DocumentAccess;
import de.tudarmstadt.ukp.inception.ui.curation.actionbar.opendocument.CurationOpenDocumentDialog;
import wicket.contrib.input.events.key.KeyType;

Expand All @@ -44,6 +44,8 @@ public class CurationDocumentNavigator
private static final long serialVersionUID = 7061696472939390003L;

private @SpringBean ProjectService projectService;
private @SpringBean DocumentAccess documentAccess;
private @SpringBean UserDao userService;

private AnnotationPageBase page;

Expand Down Expand Up @@ -71,10 +73,8 @@ public CurationDocumentNavigator(String aId, AnnotationPageBase aPage)

private boolean isExportable()
{
AnnotatorState state = page.getModelObject();
return state.getProject() != null
&& (projectService.hasRole(state.getUser(), state.getProject(), MANAGER)
|| !state.getProject().isDisableExport());
return documentAccess.canExportAnnotationDocument(userService.getCurrentUser(),
page.getModelObject().getProject());
}

/**
Expand Down
9 changes: 7 additions & 2 deletions ...eption/workload/matrix/annotation/MatrixWorkflowDocumentNavigationActionBarExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import de.tudarmstadt.ukp.clarin.webanno.api.annotation.actionbar.ActionBarExtension;
import de.tudarmstadt.ukp.clarin.webanno.api.annotation.page.AnnotationPageBase;
import de.tudarmstadt.ukp.clarin.webanno.model.Project;
import de.tudarmstadt.ukp.clarin.webanno.security.UserDao;
import de.tudarmstadt.ukp.inception.workload.matrix.MatrixWorkloadExtension;
import de.tudarmstadt.ukp.inception.workload.matrix.config.MatrixWorkloadManagerAutoConfiguration;
import de.tudarmstadt.ukp.inception.workload.model.WorkloadManagementService;
Expand All @@ -52,18 +53,21 @@ public class MatrixWorkflowDocumentNavigationActionBarExtension
private final WorkloadManagementService workloadManagementService;
private final MatrixWorkloadExtension matrixWorkloadExtension;
private final ProjectService projectService;
private final UserDao userService;

// SpringBeans
private @SpringBean EntityManager entityManager;

@Autowired
public MatrixWorkflowDocumentNavigationActionBarExtension(DocumentService aDocumentService,
WorkloadManagementService aWorkloadManagementService,
MatrixWorkloadExtension aMatrixWorkloadExtension, ProjectService aProjectService)
MatrixWorkloadExtension aMatrixWorkloadExtension, ProjectService aProjectService,
UserDao aUserService)
{
workloadManagementService = aWorkloadManagementService;
matrixWorkloadExtension = aMatrixWorkloadExtension;
projectService = aProjectService;
userService = aUserService;
}

@Override
Expand Down Expand Up @@ -92,7 +96,8 @@ public boolean accepts(AnnotationPageBase aPage)
return false;
}

if (projectService.hasRole(aPage.getModelObject().getUser(), project, MANAGER)) {
var sessionOwner = userService.getCurrentUser();
if (projectService.hasRole(sessionOwner, project, MANAGER)) {
return false;
}

Expand Down
6 changes: 4 additions & 2 deletions ...armstadt/ukp/inception/workload/matrix/config/MatrixWorkloadManagerAutoConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,11 @@ public MatrixWorkflowActionBarExtension matrixWorkflowActionBarExtension()
@Bean
public MatrixWorkflowDocumentNavigationActionBarExtension matrixWorkflowDocumentNavigationActionBarExtension(
DocumentService aDocumentService, WorkloadManagementService aWorkloadManagementService,
MatrixWorkloadExtension aMatrixWorkloadExtension, ProjectService aProjectService)
MatrixWorkloadExtension aMatrixWorkloadExtension, ProjectService aProjectService,
UserDao aUserService)
{
return new MatrixWorkflowDocumentNavigationActionBarExtension(aDocumentService,
aWorkloadManagementService, aMatrixWorkloadExtension, aProjectService);
aWorkloadManagementService, aMatrixWorkloadExtension, aProjectService,
aUserService);
}
}

0 comments on commit b8b0c9a

Please sign in to comment.