Skip to content

Commit

Permalink
Issue 12678 fixing workflow roles permission issue (#13526)
Browse files Browse the repository at this point in the history 
* #12678 fixing permission issues with workflow special roles

* #12678 solving issue adding new action

* #12678 renaming findAction methods with permissionables

* #12678 renaming findAction methods with permissionables and removing with spaces

* #12678 add integration tests

* #12678 remove unnecessary import

* #12678 add debug log to catch

* #12678 add debug log to catch

* #12678 removing try and catch
  • Loading branch information
@jgambarios
Oswaldo Gallango authored and jgambarios committed Feb 1, 2018
1 parent 0b677ab commit 1dc8e22
Show file tree
Hide file tree
Showing 6 changed files with 329 additions and 42 deletions.
228 changes: 227 additions & 1 deletion ...c/integration-test/java/com/dotmarketing/portlets/workflows/business/WorkflowAPITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,22 @@ public class WorkflowAPITest extends IntegrationTestBase {

private static String contentTypeName;
private static String contentTypeName2;
private static String contentTypeName3;
private static String workflowSchemeName1;
private static String workflowSchemeName2;
private static String workflowSchemeName3;
private static String workflowSchemeName4;
private static String workflowSchemeName5;

private static ContentType contentType;
private static Structure contentTypeStructure;

private static ContentType contentType2;
private static Structure contentTypeStructure2;

private static ContentType contentType3;
private static Structure contentTypeStructure3;

/* Workflow Scheme 1 */
private static WorkflowScheme workflowScheme1;

Expand Down Expand Up @@ -156,6 +161,16 @@ public class WorkflowAPITest extends IntegrationTestBase {
private static WorkflowAction workflowScheme4Step3ActionPublisher;
private static String workflowScheme4Step3ActionPublisherName;

/* Workflow Scheme 5 */
private static WorkflowScheme workflowScheme5;

private static WorkflowStep workflowScheme5Step1;
private static String workflowScheme5Step1Name;
private static WorkflowAction workflowScheme5Step1Action1;
private static String workflowScheme5Step1ActionPublishName;
private static String workflowScheme5Step1Action1SubAction1Name;
private static WorkflowActionClass workflowScheme5Step1Action1SubAction1;

/* Roles */
private static Role reviewer;
private static Role contributor;
Expand Down Expand Up @@ -314,7 +329,7 @@ public static void prepare() throws Exception {
workflowScheme4Step3ActionPublisherName = "WorkflowScheme4Step3ActionPublisher_" + time;

/**
* Generate ContentType
* Generate ContentType 2
*/
contentType2 = insertContentType(contentTypeName2, BaseContentType.CONTENT);
contentTypeStructure2 = new StructureTransformer(ContentType.class.cast(contentType2))
Expand Down Expand Up @@ -385,6 +400,35 @@ public static void prepare() throws Exception {
workflowScheme4.getId());


/**
* Generate ContentType 3
*/
contentTypeName3 = "WorkflowTesting3_" + time;
contentType3 = insertContentType(contentTypeName3, BaseContentType.CONTENT);
contentTypeStructure3 = new StructureTransformer(ContentType.class.cast(contentType3))
.asStructure();


/* Workflow */
workflowSchemeName5 = "WorkflowSchemeTest5" + time;
workflowScheme5Step1Name = "WorkflowScheme5Step1_" + time;
workflowScheme5Step1ActionPublishName = "WorkflowScheme5Step1ActionPublish_" + time;
workflowScheme5Step1Action1SubAction1Name="Publish content";

workflowScheme5 = addWorkflowScheme(workflowSchemeName5, false);

/* Generate scheme steps */
workflowScheme5Step1 = addWorkflowStep(workflowScheme5Step1Name, 1, false, false,
workflowScheme5.getId());

workflowScheme5Step1Action1 = addWorkflowAction(workflowScheme5Step1ActionPublishName, 1,
workflowScheme5Step1.getId(), true, workflowScheme5Step1.getId(), anyWhoView, workflowScheme5.getId());

workflowScheme5Step1Action1SubAction1 = addSubActionClass(workflowScheme5Step1Action1SubAction1Name,
workflowScheme5Step1Action1.getId(),
com.dotmarketing.portlets.workflows.actionlet.PublishContentActionlet.class,1);


}

/**
Expand Down Expand Up @@ -611,6 +655,7 @@ public void findAvailableActions() throws DotDataException, DotSecurityException
testContentlet.setLanguageId(1);
testContentlet.setStringProperty(FIELD_VAR_NAME, "WorkflowContentTest_" + time);
testContentlet.setContentTypeId(contentType.id());
testContentlet.setHost(defaultHost.getIdentifier());
testContentlet = contentletAPI.checkin(testContentlet, user, false);

contentletAPI.isInodeIndexed(testContentlet.getInode());
Expand Down Expand Up @@ -694,6 +739,177 @@ public void findAvailableActions() throws DotDataException, DotSecurityException

}

/**
* Test the find findActionRespectingPermissions methods
*/
@Test
public void findActionRespectingPermissions() throws DotDataException, DotSecurityException {

//Users
final User billIntranet = APILocator.getUserAPI().loadUserById("dotcms.org.2806");
final User chrisPublisher = APILocator.getUserAPI().loadUserById("dotcms.org.2795");


Contentlet testContentlet = new Contentlet();
try {

//Set Workflow on contentType3
List<WorkflowScheme> worflowSchemes = new ArrayList<>();
worflowSchemes.add(workflowScheme5);

/* Associate the schemas to the content type */
workflowAPI.saveSchemesForStruct(contentTypeStructure3, worflowSchemes);

long time = System.currentTimeMillis();
testContentlet.setLanguageId(1);
testContentlet.setStringProperty(FIELD_VAR_NAME, "Workflow5ContentTest_" + time);
testContentlet.setContentTypeId(contentType3.id());
testContentlet.setHost(defaultHost.getIdentifier());
testContentlet = contentletAPI.checkin(testContentlet, APILocator.getPermissionAPI().getPermissions(testContentlet, false, true), user, false);

contentletAPI.isInodeIndexed(testContentlet.getInode());

//Adding permissions to limited user on the contentType3
List<Permission> permissions = new ArrayList<>();
Permission p1 = new Permission(
testContentlet.getPermissionId(),
APILocator.getRoleAPI().getUserRole(chrisPublisher).getId(),
(PermissionAPI.PERMISSION_READ | PermissionAPI.PERMISSION_EDIT
| PermissionAPI.PERMISSION_WRITE
| PermissionAPI.PERMISSION_PUBLISH
| PermissionAPI.PERMISSION_EDIT_PERMISSIONS),
true);
permissions.add(p1);

APILocator.getPermissionAPI().save(permissions, testContentlet, user, false);


//Validate the saved permissions
List<Permission> foundContentletPermissions = APILocator.getPermissionAPI()
.getPermissions(testContentlet);
assertNotNull(foundContentletPermissions);
assertFalse(foundContentletPermissions.isEmpty());

WorkflowAction action = APILocator.getWorkflowAPI().findActionRespectingPermissions(workflowScheme5Step1Action1.getId(),testContentlet,chrisPublisher);
assertNotNull(action);
assertEquals(action.getName(), workflowScheme5Step1Action1.getName());

//This should throw a DotSecurityException
try {
action = APILocator.getWorkflowAPI()
.findActionRespectingPermissions(workflowScheme5Step1Action1.getId(),
testContentlet, billIntranet);
}catch (Exception e){
assertTrue(e instanceof DotSecurityException);
}


action = APILocator.getWorkflowAPI().findActionRespectingPermissions(workflowScheme5Step1Action1.getId(),workflowScheme5Step1.getId(),testContentlet,chrisPublisher);
assertNotNull(action);
assertEquals(action.getName(), workflowScheme5Step1Action1.getName());

//This should throw a DotSecurityException
try {
action = APILocator.getWorkflowAPI()
.findActionRespectingPermissions(workflowScheme5Step1Action1.getId(),
workflowScheme5Step1.getId(), testContentlet, billIntranet);
}catch (Exception e){
assertTrue(e instanceof DotSecurityException);
}

} finally {
contentletAPI.archive(testContentlet, user, false);
contentletAPI.delete(testContentlet, user, false);
}

}

/**
* Test the find findAction methods
*/
@Test
public void findAction() throws DotDataException, DotSecurityException {

//Users
final User billIntranet = APILocator.getUserAPI().loadUserById("dotcms.org.2806");
final User chrisPublisher = APILocator.getUserAPI().loadUserById("dotcms.org.2795");

Contentlet testContentlet = new Contentlet();
try {

//Set Workflow on contentType3
List<WorkflowScheme> worflowSchemes = new ArrayList<>();
worflowSchemes.add(workflowScheme5);

/* Associate the schemas to the content type */
workflowAPI.saveSchemesForStruct(contentTypeStructure3, worflowSchemes);

long time = System.currentTimeMillis();
testContentlet.setLanguageId(1);
testContentlet.setStringProperty(FIELD_VAR_NAME, "Workflow5ContentTest_" + time);
testContentlet.setContentTypeId(contentType3.id());
testContentlet.setHost(defaultHost.getIdentifier());
testContentlet = contentletAPI.checkin(testContentlet, APILocator.getPermissionAPI().getPermissions(testContentlet, false, true), user, false);

contentletAPI.isInodeIndexed(testContentlet.getInode());

//Adding permissions to limited user on the contentType3
List<Permission> permissions = new ArrayList<>();
Permission p1 = new Permission(
testContentlet.getPermissionId(),
APILocator.getRoleAPI().getUserRole(chrisPublisher).getId(),
(PermissionAPI.PERMISSION_READ | PermissionAPI.PERMISSION_EDIT
| PermissionAPI.PERMISSION_WRITE
| PermissionAPI.PERMISSION_PUBLISH
| PermissionAPI.PERMISSION_EDIT_PERMISSIONS),
true);
permissions.add(p1);

APILocator.getPermissionAPI().save(permissions, testContentlet, user, false);


//Validate the saved permissions
List<Permission> foundContentletPermissions = APILocator.getPermissionAPI()
.getPermissions(testContentlet);
assertNotNull(foundContentletPermissions);
assertFalse(foundContentletPermissions.isEmpty());

WorkflowAction action = APILocator.getWorkflowAPI().findAction(workflowScheme5Step1Action1.getId(),
workflowScheme5Step1.getId(),chrisPublisher);
assertNotNull(action);
assertEquals(action.getName(), workflowScheme5Step1Action1.getName());

//This should throw a DotSecurityException
try {
action = APILocator.getWorkflowAPI()
.findAction(workflowScheme5Step1Action1.getId(),
workflowScheme5Step1.getId(), billIntranet);
}catch (Exception e){
assertTrue(e instanceof DotSecurityException);
}


action = APILocator.getWorkflowAPI().findAction(workflowScheme5Step1Action1.getId(),
workflowScheme5Step1.getId(),chrisPublisher);
assertNotNull(action);
assertEquals(action.getName(), workflowScheme5Step1Action1.getName());

//This should throw a DotSecurityException
try {
action = APILocator.getWorkflowAPI()
.findAction(workflowScheme5Step1Action1.getId(),
workflowScheme5Step1.getId(), billIntranet);
}catch (Exception e){
assertTrue(e instanceof DotSecurityException);
}

} finally {
contentletAPI.archive(testContentlet, user, false);
contentletAPI.delete(testContentlet, user, false);
}

}

/**
* Validate if the scheme is present in the list of schemes
*
Expand Down Expand Up @@ -874,6 +1090,8 @@ protected static WorkflowActionClass addSubActionClass(final String name, final
public static void cleanup() throws DotDataException, DotSecurityException {

contentTypeAPI.delete(contentType);
contentTypeAPI.delete(contentType2);
//contentTypeAPI.delete(contentType3);
try {
//Deleting workflow 1
workflowAPI.deleteAction(workflowScheme1Step1Action1);
Expand Down Expand Up @@ -935,6 +1153,14 @@ public static void cleanup() throws DotDataException, DotSecurityException {
workflowAPI.saveScheme(workflowScheme4);
workflowAPI.deleteScheme(workflowScheme4);

//Deleting workflow 5
workflowAPI.deleteAction(workflowScheme5Step1Action1);
workflowAPI.deleteStep(workflowScheme5Step1);

workflowScheme5.setArchived(true);
workflowAPI.saveScheme(workflowScheme5);
workflowAPI.deleteScheme(workflowScheme5);

}catch (AlreadyExistException e){

}
Expand Down
2 changes: 1 addition & 1 deletion dotCMS/src/main/java/com/dotcms/rest/WorkflowResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public Response fireWorkflow(@Context HttpServletRequest request,
WorkflowAPI wapi = APILocator.getWorkflowAPI();
WorkflowAction action = null;
try {
action = wapi.findAction(wfAction, user);
action = wapi.findActionRespectingPermissions(wfAction, contentlet, user);
if (action == null) {
throw new ServletException("No such workflow action");
}
Expand Down
8 changes: 2 additions & 6 deletions ...src/main/java/com/dotmarketing/portlets/contentlet/business/web/ContentletWebAPIImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,12 +376,8 @@ private void _saveWebAsset(Map<String, Object> contentletFormData,
String wfActionId = (String) contentletFormData.get("wfActionId");
if(UtilMethods.isSet(wfActionId)) {

WorkflowAction action = null;
try {
action = APILocator.getWorkflowAPI().findAction(wfActionId, user);
} catch(Exception e){
WorkflowAction action = APILocator.getWorkflowAPI().findActionRespectingPermissions(wfActionId, currentContentlet, user);

}
if(action != null
&& ! action.requiresCheckout() // no modifies the db
&& APILocator.getContentletAPI().canLock(currentContentlet, user)){
Expand Down Expand Up @@ -1088,7 +1084,7 @@ public void run() {
/**
* Returns the relationships associated to the current contentlet
*
* @param req ActionRequest.
* @param contentletFormData Contentlet form map.
* @param user User.
* @return ContentletRelationships.
*/
Expand Down
36 changes: 35 additions & 1 deletion dotCMS/src/main/java/com/dotmarketing/portlets/workflows/business/WorkflowAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,31 @@ public void reorderAction(final WorkflowAction action,



/**
* Finds an action by Id and checking the user permissions over the workflow portlet.
* The action will be validated against the user permissions.
* @param id String action id
* @param user User the user that makes the request
* @return WorkflowAction
* @throws DotDataException
* @throws DotSecurityException
*/
public WorkflowAction findAction(String id, User user) throws DotDataException, DotSecurityException;

/**
* Finds an action associated to the steps.
* Finds an action by Id and checking the user permissions over the permissionable.
* The action will be validated against the user permissions.
* @param id String action id
* @param permissionable Permissionable Content/Content Type against who is going to be validated the permissions
* @param user User the user that makes the request
* @return WorkflowAction
* @throws DotDataException
* @throws DotSecurityException
*/
public WorkflowAction findActionRespectingPermissions(String id, Permissionable permissionable, User user) throws DotDataException, DotSecurityException;

/**
* Finds an action associated to the steps and user permissions over the workflow portlet.
* The action will be validated against the user permissions.
* @param actionId String action id
* @param stepId String step id
Expand All @@ -256,6 +277,19 @@ public void reorderAction(final WorkflowAction action,
*/
public WorkflowAction findAction(String actionId, String stepId, User user) throws DotDataException, DotSecurityException;

/**
* Finds an action associated to the steps and the user permissions over the permissionable.
* The action will be validated against the user permissions.
* @param actionId String action id
* @param stepId String step id
* @param permissionable Permissionable Content/Content Type against who is going to be validated the permissions
* @param user User the user that makes the request
* @return WorkflowAction
* @throws DotDataException
* @throws DotSecurityException
*/
public WorkflowAction findActionRespectingPermissions(String actionId, String stepId, Permissionable permissionable, User user) throws DotDataException, DotSecurityException;

/**
* Finds the available {@link WorkflowAction} for the contentlet to a user on any give
* piece of content, based on how and who has the content locked and what workflow step the content
Expand Down
Loading

0 comments on commit 1dc8e22

Please sign in to comment.