Skip to content

Commit

Permalink
Fix updating action adding updating or removing rule.
Browse files Browse the repository at this point in the history
  • Loading branch information
@malithie
malithie committed Jan 8, 2025
1 parent f8f90f2 commit 0d58c41
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 36 deletions.
87 changes: 59 additions & 28 deletions ...n/java/org/wso2/carbon/identity/action/management/dao/impl/ActionManagementDAOFacade.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ public void addAction(ActionDTO actionDTO, Integer tenantId) throws ActionMgtExc
// Since exceptions thrown are wrapped with TransactionException, extracting the actual cause.
handleActionPropertyResolverClientException(e.getCause());
LOG.debug("Error while creating the Action of Action Type: " + actionDTO.getType().getDisplayName() +
" in Tenant Domain: " + IdentityTenantUtil.getTenantDomain(tenantId) +
". Rolling back created action information, authentication secrets and action properties.");
" in Tenant Domain: " + IdentityTenantUtil.getTenantDomain(tenantId) +
". Rolling back created action information, authentication secrets and action properties.");
throw ActionManagementExceptionHandler.handleServerException(ErrorMessage.ERROR_WHILE_ADDING_ACTION, e);
}
}
Expand Down Expand Up @@ -289,8 +289,8 @@ private void deleteAuthenticationSecrets(ActionDTO deletingActionDTO) throws Act
* Add the encrypted authentication secrets and replace the input authentication properties in the ActionDTOBuilder
* object.
*
* @param actionDTOBuilder ActionDTOBuilder object.
* @param encryptedProperties List of encrypted AuthProperty objects.
* @param actionDTOBuilder ActionDTOBuilder object.
* @param encryptedProperties List of encrypted AuthProperty objects.
*/
private void addEncryptedAuthSecretsToBuilder(ActionDTOBuilder actionDTOBuilder,
List<AuthProperty> encryptedProperties) {
Expand All @@ -299,17 +299,17 @@ private void addEncryptedAuthSecretsToBuilder(ActionDTOBuilder actionDTOBuilder,
.collect(Collectors.toMap(AuthProperty::getName, AuthProperty::getValue));

actionDTOBuilder.endpoint(new EndpointConfig.EndpointConfigBuilder()
.uri(actionDTOBuilder.getEndpoint().getUri())
.authentication(new Authentication.AuthenticationBuilder()
.type(actionDTOBuilder.getEndpoint().getAuthentication().getType())
.properties(encryptedPropertyMap)
.build())
.build());
.uri(actionDTOBuilder.getEndpoint().getUri())
.authentication(new Authentication.AuthenticationBuilder()
.type(actionDTOBuilder.getEndpoint().getAuthentication().getType())
.properties(encryptedPropertyMap)
.build())
.build());
}

private void addActionRule(ActionDTOBuilder actionDTOBuilder, String tenantDomain) throws ActionMgtException {

if (actionDTOBuilder.getActionRule() == null) {
if (actionDTOBuilder.getActionRule() == null || actionDTOBuilder.getActionRule().getRule() == null) {
return;
}

Expand Down Expand Up @@ -341,25 +341,56 @@ private void loadActionRule(ActionDTOBuilder actionDTOBuilder, String tenantDoma
}

private void updateActionRule(ActionDTOBuilder updatingActionDTOBuilder, ActionDTO existingActionDTO,
String tenantDomain)
throws ActionMgtException {

if (existingActionDTO.getActionRule() == null && updatingActionDTOBuilder.getActionRule() != null) {
// This means a new action rule is added when updating the action.
String tenantDomain) throws ActionMgtException {

/*
When updating an action, the action rule can be added, removed or updated.
When action rule is added, the Rule object is added to the ActionRule of the ActionDTO.
When action rule is removed, the Rule object is set as null in the ActionRule of the ActionDTO.
This happens as the API accepts the removal of the rule in an action update via an empty rule JSON object.
e.g., rule: {}. If rule is not present in the payload that means rule is not updated.
When action rule is updated, the Rule object is updated in the ActionRule of the ActionDTO.
*/
if (isAddingNewActionRule(updatingActionDTOBuilder, existingActionDTO)) {
addActionRule(updatingActionDTOBuilder, tenantDomain);
} else if (existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() == null) {
// This means the existing action rule is removed when updating the action.
} else if (isRemovingExistingActionRule(updatingActionDTOBuilder, existingActionDTO)) {
deleteActionRule(existingActionDTO, tenantDomain);
} else if (existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() !=
null) { // This means the existing action rule is updated when updating the action.
try {
updatingActionDTOBuilder.getActionRule().getRule().setId(existingActionDTO.getActionRule().getId());
ActionMgtServiceComponentHolder.getInstance()
.getRuleManagementService()
.updateRule(updatingActionDTOBuilder.getActionRule().getRule(), tenantDomain);
} catch (RuleManagementException e) {
throw new ActionMgtServerException("Error while updating the Rule associated with the Action.", e);
}
} else if (isUpdatingExistingActionRule(updatingActionDTOBuilder, existingActionDTO)) {
updateExistingActionRule(updatingActionDTOBuilder, existingActionDTO, tenantDomain);
}
}

private boolean isAddingNewActionRule(ActionDTOBuilder updatingActionDTOBuilder, ActionDTO existingActionDTO)
throws ActionMgtException {

return existingActionDTO.getActionRule() == null && updatingActionDTOBuilder.getActionRule() != null &&
updatingActionDTOBuilder.getActionRule().getRule() != null;
}

private boolean isRemovingExistingActionRule(ActionDTOBuilder updatingActionDTOBuilder,
ActionDTO existingActionDTO) throws ActionMgtException {

return existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() != null &&
updatingActionDTOBuilder.getActionRule().getRule() == null;
}

private boolean isUpdatingExistingActionRule(ActionDTOBuilder updatingActionDTOBuilder,
ActionDTO existingActionDTO) throws ActionMgtException {

return existingActionDTO.getActionRule() != null && updatingActionDTOBuilder.getActionRule() != null &&
updatingActionDTOBuilder.getActionRule().getRule() != null;
}

private void updateExistingActionRule(ActionDTOBuilder updatingActionDTOBuilder, ActionDTO existingActionDTO,
String tenantDomain) throws ActionMgtException {

try {
updatingActionDTOBuilder.getActionRule().getRule().setId(existingActionDTO.getActionRule().getId());
ActionMgtServiceComponentHolder.getInstance()
.getRuleManagementService()
.updateRule(updatingActionDTOBuilder.getActionRule().getRule(), tenantDomain);
} catch (RuleManagementException e) {
throw new ActionMgtServerException("Error while updating the Rule associated with the Action.", e);
}
}

Expand Down
18 changes: 12 additions & 6 deletions ...ain/java/org/wso2/carbon/identity/action/management/dao/impl/ActionManagementDAOImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,12 +481,18 @@ private void addRuleReference(ActionDTO actionDTO, Integer tenantId) throws Acti
private void updateRuleReference(ActionDTO updatingActionDTO, ActionDTO existingActionDTO, Integer tenantId)
throws ActionMgtServerException {

if (existingActionDTO.getActionRule() == null && updatingActionDTO.getActionRule() != null) {
// This means a new action rule is added when updating the action. Add the rule reference.
addRuleReference(updatingActionDTO, tenantId);
} else if (existingActionDTO.getActionRule() != null && updatingActionDTO.getActionRule() == null) {
// This means the existing action rule is removed when updating the action. Remove the rule reference.
deleteRuleReference(updatingActionDTO, tenantId);
try {
if (existingActionDTO.getActionRule() == null && updatingActionDTO.getActionRule() != null &&
updatingActionDTO.getActionRule().getRule() != null) {
// This means a new action rule is added when updating the action. Add the rule reference.
addRuleReference(updatingActionDTO, tenantId);
} else if (existingActionDTO.getActionRule() != null && updatingActionDTO.getActionRule() != null &&
updatingActionDTO.getActionRule().getRule() == null) {
// This means the existing action rule is removed when updating the action. Remove the rule reference.
deleteRuleReference(updatingActionDTO, tenantId);
}
} catch (ActionMgtException e) {
throw new ActionMgtServerException("Error while updating the reference for the Rule in Action.", e);
}
}

Expand Down
10 changes: 10 additions & 0 deletions ...management/src/main/java/org/wso2/carbon/identity/action/management/model/ActionRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ action status (active/inactive), returning state of the prior action in the chai

private Rule getRuleFromRuleManagementService() throws ActionMgtServerException {

if (StringUtils.isBlank(id) || StringUtils.isBlank(tenantDomain)) {
/*
This could happen if the ActionRule is created without the Rule object.
That means the rule value is explicitly set to null.
In such cases, loading the Rule from the Rule Management Service is not expected.
This scenario is used to let the user remove a Rule from an Action via the Action update API.
*/
return null;
}

try {
return ActionMgtServiceComponentHolder.getInstance()
.getRuleManagementService()
Expand Down
21 changes: 20 additions & 1 deletion ...st/java/org/wso2/carbon/identity/action/management/dao/ActionManagementDAOFacadeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,8 @@ public void testFailureInUpdateActionUpdatingRuleAtRuleManagementException() thr
@Test(priority = 18)
public void testUpdateActionRemovingRule() throws Exception {

ActionDTO updatingActionDTOWithoutRule = createActionDTOForPasswordUpdateAction();
// In order to remove the rule from ActionRule create an ActionRule with a null Rule reference.
ActionDTO updatingActionDTOWithoutRule = createActionDTOForPasswordUpdateActionWithRule(null);
mockActionPropertyResolver(testActionPropertyResolver);

daoFacade.updateAction(updatingActionDTOWithoutRule, actionDTORetrieved, TENANT_ID);
Expand Down Expand Up @@ -507,6 +508,24 @@ public void testUpdateActionAddingRule() throws Exception {
}

@Test(priority = 21)
public void testUpdateActionWithRuleWithoutUpdatingRule() throws Exception {

Rule rule = mockRule();
mockRuleManagementService(rule);
ActionDTO updatingActionDTO = createActionDTOForPasswordUpdateAction();
ActionDTO expectedActionDTO = createActionDTOForPasswordUpdateActionWithRule(rule);
mockActionPropertyResolver(testActionPropertyResolver);

daoFacade.updateAction(updatingActionDTO, actionDTORetrieved, TENANT_ID);

actionDTORetrieved = daoFacade.getActionByActionId(PRE_UPDATE_PASSWORD_TYPE, PRE_UPDATE_PASSWORD_ACTION_ID,
TENANT_ID);

verifyActionDTO(actionDTORetrieved, expectedActionDTO);
verifyActionDTORule(actionDTORetrieved, expectedActionDTO);
}

@Test(priority = 22)
public void testDeleteActionWithRule() throws ActionMgtException {

mockActionPropertyResolver(testActionPropertyResolver);
Expand Down
2 changes: 1 addition & 1 deletion ...a/org/wso2/carbon/identity/action/management/service/ActionManagementServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ public void testGetActionsWithRulesByActionType() throws ActionMgtException {
}

@Test(priority = 17)
public void testUpdateActionWithRule() throws ActionMgtException, SecretManagementException {
public void testUpdateActionUpdatingRule() throws ActionMgtException, SecretManagementException {

Action updatingAction = TestUtil.buildMockActionWithRule(
TEST_ACTION_NAME_UPDATED,
Expand Down

0 comments on commit 0d58c41

Please sign in to comment.