Skip to content

Commit 3687d43

Browse files
authored
Share permission lists across core roles (#7847)
## Rationale Core roles (RestrictedReaderRole -> ReaderRole -> AuthorRole -> EditorWithoutDeleteRole -> EditorRole), share common permissions but their actual permission lists are duplicated in each class. This can easily lead to mistakes. A better approach is for the roles to share permission lists, each role adding just the permissions it needs to add. This is similar to the way we build the permission lists for admin roles.
1 parent dd6832a commit 3687d43

6 files changed

Lines changed: 88 additions & 94 deletions

File tree

api/src/org/labkey/api/security/roles/ApplicationAdminRole.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import java.util.Collection;
3131

3232
/**
33-
* A step down from site admins, app admins have broad access but don't get to control native resources on the server.
33+
* A step-down from site admins, app admins have broad access but don't get to control native resources on the server.
3434
*/
3535
public class ApplicationAdminRole extends AbstractRootContainerRole implements AdminRoleListener
3636
{

api/src/org/labkey/api/security/roles/AuthorRole.java

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,27 @@
1818
import org.labkey.api.pipeline.PipeRoot;
1919
import org.labkey.api.reports.permissions.ShareReportPermission;
2020
import org.labkey.api.security.SecurableResource;
21-
import org.labkey.api.security.permissions.AssayReadPermission;
22-
import org.labkey.api.security.permissions.DataClassReadPermission;
2321
import org.labkey.api.security.permissions.InsertPermission;
24-
import org.labkey.api.security.permissions.MediaReadPermission;
25-
import org.labkey.api.security.permissions.NotebookReadPermission;
26-
import org.labkey.api.security.permissions.ReadPermission;
27-
import org.labkey.api.security.permissions.ReadSomePermission;
22+
import org.labkey.api.security.permissions.Permission;
2823
import org.labkey.api.study.Dataset;
2924

30-
/*
31-
* User: Dave
32-
* Date: Apr 27, 2009
33-
*/
25+
import java.util.Collection;
26+
import java.util.stream.Stream;
27+
3428
public class AuthorRole extends AbstractRole
3529
{
30+
static final Collection<Class<? extends Permission>> PERMISSIONS = Stream.concat(
31+
ReaderRole.PERMISSIONS.stream(),
32+
Stream.of(
33+
InsertPermission.class,
34+
ShareReportPermission.class
35+
)
36+
).toList();
37+
3638
public AuthorRole()
3739
{
3840
super("Author", "Authors may read and add some information. They can also update and delete some information they added.",
39-
ReadPermission.class,
40-
ReadSomePermission.class,
41-
AssayReadPermission.class,
42-
DataClassReadPermission.class,
43-
MediaReadPermission.class,
44-
NotebookReadPermission.class,
45-
InsertPermission.class,
46-
ShareReportPermission.class
41+
PERMISSIONS
4742
);
4843
}
4944

api/src/org/labkey/api/security/roles/EditorRole.java

Lines changed: 10 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -15,70 +15,25 @@
1515
*/
1616
package org.labkey.api.security.roles;
1717

18-
import org.labkey.api.lists.permissions.ManagePicklistsPermission;
19-
import org.labkey.api.pipeline.PipeRoot;
20-
import org.labkey.api.reports.permissions.EditSharedReportPermission;
21-
import org.labkey.api.reports.permissions.ShareReportPermission;
22-
import org.labkey.api.security.SecurableResource;
23-
import org.labkey.api.security.permissions.AssayReadPermission;
24-
import org.labkey.api.security.permissions.DataClassReadPermission;
2518
import org.labkey.api.security.permissions.DeletePermission;
26-
import org.labkey.api.security.permissions.EditSharedViewPermission;
27-
import org.labkey.api.security.permissions.InsertPermission;
28-
import org.labkey.api.security.permissions.MediaReadPermission;
29-
import org.labkey.api.security.permissions.MoveEntitiesPermission;
30-
import org.labkey.api.security.permissions.NotebookReadPermission;
3119
import org.labkey.api.security.permissions.Permission;
32-
import org.labkey.api.security.permissions.ReadPermission;
33-
import org.labkey.api.security.permissions.ReadSomePermission;
3420
import org.labkey.api.security.permissions.SampleWorkflowDeletePermission;
35-
import org.labkey.api.security.permissions.SampleWorkflowJobPermission;
36-
import org.labkey.api.security.permissions.UpdatePermission;
37-
import org.labkey.api.study.Dataset;
38-
import org.labkey.api.study.Study;
39-
import org.labkey.api.study.permissions.SharedParticipantGroupPermission;
4021

41-
import java.util.Arrays;
4222
import java.util.Collection;
23+
import java.util.stream.Stream;
4324

44-
/*
45-
* User: Dave
46-
* Date: Apr 27, 2009
47-
* Time: 1:22:17 PM
48-
*/
49-
public class EditorRole extends AbstractRole
25+
public class EditorRole extends EditorWithoutDeleteRole
5026
{
51-
protected static Collection<Class<? extends Permission>> BASE_EDITOR_PERMISSIONS = Arrays.asList(
52-
ReadPermission.class,
53-
ReadSomePermission.class,
54-
AssayReadPermission.class,
55-
DataClassReadPermission.class,
56-
MediaReadPermission.class,
57-
NotebookReadPermission.class,
58-
InsertPermission.class,
59-
MoveEntitiesPermission.class,
60-
UpdatePermission.class,
61-
EditSharedViewPermission.class,
62-
ShareReportPermission.class,
63-
EditSharedReportPermission.class,
64-
SharedParticipantGroupPermission.class,
65-
ManagePicklistsPermission.class,
66-
SampleWorkflowJobPermission.class
67-
);
27+
protected static Collection<Class<? extends Permission>> PERMISSIONS = Stream.concat(
28+
EditorWithoutDeleteRole.PERMISSIONS.stream(),
29+
Stream.of(
30+
DeletePermission.class,
31+
SampleWorkflowDeletePermission.class
32+
)
33+
).toList();
6834

6935
public EditorRole()
7036
{
71-
this("Editor", "Editors may read, add, update and delete information.", BASE_EDITOR_PERMISSIONS, Arrays.asList(DeletePermission.class, SampleWorkflowDeletePermission.class));
72-
}
73-
74-
public EditorRole(String name, String description, Collection<Class<? extends Permission>>... permCollections)
75-
{
76-
super(name, description, permCollections);
77-
}
78-
79-
@Override
80-
public boolean isApplicable(SecurableResource resource)
81-
{
82-
return super.isApplicable(resource) || resource instanceof PipeRoot || resource instanceof Study || resource instanceof Dataset;
37+
super("Editor", "Editors may read, add, update and delete information.", PERMISSIONS);
8338
}
8439
}

api/src/org/labkey/api/security/roles/EditorWithoutDeleteRole.java

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,50 @@
1515
*/
1616
package org.labkey.api.security.roles;
1717

18-
public class EditorWithoutDeleteRole extends EditorRole
18+
import org.labkey.api.lists.permissions.ManagePicklistsPermission;
19+
import org.labkey.api.pipeline.PipeRoot;
20+
import org.labkey.api.reports.permissions.EditSharedReportPermission;
21+
import org.labkey.api.security.SecurableResource;
22+
import org.labkey.api.security.permissions.EditSharedViewPermission;
23+
import org.labkey.api.security.permissions.MoveEntitiesPermission;
24+
import org.labkey.api.security.permissions.Permission;
25+
import org.labkey.api.security.permissions.SampleWorkflowJobPermission;
26+
import org.labkey.api.security.permissions.UpdatePermission;
27+
import org.labkey.api.study.Dataset;
28+
import org.labkey.api.study.Study;
29+
import org.labkey.api.study.permissions.SharedParticipantGroupPermission;
30+
31+
import java.util.Collection;
32+
import java.util.stream.Stream;
33+
34+
public class EditorWithoutDeleteRole extends AbstractRole
1935
{
36+
protected static Collection<Class<? extends Permission>> PERMISSIONS = Stream.concat(
37+
AuthorRole.PERMISSIONS.stream(),
38+
Stream.of(
39+
EditSharedReportPermission.class,
40+
EditSharedViewPermission.class,
41+
ManagePicklistsPermission.class,
42+
MoveEntitiesPermission.class,
43+
SampleWorkflowJobPermission.class,
44+
SharedParticipantGroupPermission.class,
45+
UpdatePermission.class
46+
)
47+
).toList();
48+
2049
public EditorWithoutDeleteRole()
2150
{
22-
super("Editor without Delete", "Editors in this role may read, add, and update information but not delete.", BASE_EDITOR_PERMISSIONS);
51+
super("Editor without Delete", "Editors in this role may read, add, and update information but not delete.", PERMISSIONS);
52+
}
53+
54+
protected EditorWithoutDeleteRole(String name, String description, Iterable<Class<? extends Permission>>... permCollections)
55+
{
56+
super(name, description, permCollections);
57+
}
58+
59+
@Override
60+
public boolean isApplicable(SecurableResource resource)
61+
{
62+
return super.isApplicable(resource) || resource instanceof PipeRoot || resource instanceof Study || resource instanceof Dataset;
2363
}
2464
}

api/src/org/labkey/api/security/roles/ReaderRole.java

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,34 +23,30 @@
2323
import org.labkey.api.security.permissions.NotebookReadPermission;
2424
import org.labkey.api.security.permissions.Permission;
2525
import org.labkey.api.security.permissions.ReadPermission;
26-
import org.labkey.api.security.permissions.ReadSomePermission;
2726

2827
import java.util.Collection;
28+
import java.util.stream.Stream;
2929

30-
/*
31-
* User: Dave
32-
* Date: Apr 27, 2009
33-
* Time: 1:22:04 PM
34-
*/
3530
public class ReaderRole extends AbstractRole
3631
{
32+
static final Collection<Class<? extends Permission>> PERMISSIONS = Stream.concat(
33+
RestrictedReaderRole.PERMISSIONS.stream(),
34+
Stream.of(
35+
AssayReadPermission.class,
36+
DataClassReadPermission.class,
37+
MediaReadPermission.class,
38+
NotebookReadPermission.class,
39+
ReadPermission.class
40+
)
41+
).toList();
42+
3743
public ReaderRole()
3844
{
3945
super("Reader", "Readers may read information but may not change anything.",
40-
ReadPermission.class,
41-
ReadSomePermission.class,
42-
AssayReadPermission.class,
43-
DataClassReadPermission.class,
44-
NotebookReadPermission.class,
45-
MediaReadPermission.class
46+
PERMISSIONS
4647
);
4748
}
4849

49-
public ReaderRole(String name, String description, Collection<Class<? extends Permission>>... permCollections)
50-
{
51-
super(name, description, permCollections);
52-
}
53-
5450
@Override
5551
public boolean isApplicable(SecurableResource resource)
5652
{

api/src/org/labkey/api/security/roles/RestrictedReaderRole.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,27 @@
1616
package org.labkey.api.security.roles;
1717

1818
import org.labkey.api.security.SecurableResource;
19+
import org.labkey.api.security.permissions.Permission;
1920
import org.labkey.api.security.permissions.ReadSomePermission;
2021
import org.labkey.api.study.Study;
2122

23+
import java.util.Collection;
24+
import java.util.List;
25+
2226
/**
2327
* Used exclusively in dataset security, as a marker in the study policy to indicate a group has per-dataset permissions.
2428
* As a result, we don't directly surface this role anywhere in the product. See #42682.
2529
*/
2630
public class RestrictedReaderRole extends AbstractRole
2731
{
32+
static final Collection<Class<? extends Permission>> PERMISSIONS = List.of(
33+
ReadSomePermission.class
34+
);
35+
2836
public RestrictedReaderRole()
2937
{
3038
super("Restricted Reader", "Restricted Readers may read some information, but not all.",
31-
ReadSomePermission.class);
39+
PERMISSIONS);
3240
}
3341

3442
@Override

0 commit comments

Comments
 (0)