Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 12678 fixing workflow roles permission issue #13526

Merged
merged 10 commits into from
Feb 1, 2018
Merged

Issue 12678 fixing workflow roles permission issue #13526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f4716b6
#12678 fixing permission issues with workflow special roles
Jan 30, 2018
fe9c0a3
#12678 solving issue adding new action
Jan 30, 2018
e0ed5d2
#12678 renaming findAction methods with permissionables
Jan 30, 2018
7cc31fb
#12678 renaming findAction methods with permissionables and removing …
Jan 30, 2018
5d714f2
#12678 add integration tests
Feb 1, 2018
ceff036
#12678 remove unnecessary import
Feb 1, 2018
cb8dea8
#12678 updated with latest master branch
Feb 1, 2018
d9218e3
#12678 add debug log to catch
Feb 1, 2018
2c8f658
#12678 add debug log to catch
Feb 1, 2018
32b5dfe
#12678 removing try and catch
Feb 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 227 additions & 1 deletion ...c/integration-test/java/com/dotmarketing/portlets/workflows/business/WorkflowAPITest.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR Split this 154 characters long line (which is greater than 150 authorized). rule


/**
* 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;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MINOR Split this 175 characters long line (which is greater than 150 authorized). rule

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't be bad to add an integration test for this new API method

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test included


/**
* 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