Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,7 @@
hs_err_pid*

# Ignore everything in this directory
target
target

# Maven versions plugin backup files
*.versionsBackup
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public enum ActionType {
PRE_UPDATE_PASSWORD,
PRE_UPDATE_PROFILE,
AUTHENTICATION,
PRE_ISSUE_ID_TOKEN;
PRE_ISSUE_ID_TOKEN,
FLOW_EXTENSION;

public String getDisplayName() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,18 @@

package org.wso2.carbon.identity.action.execution.api.model;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* This class models the User.
Expand All @@ -30,9 +39,11 @@ public class User {

private final String id;
private final List<UserClaim> claims = new ArrayList<>();
private final Map<String, char[]> userCredentials = new HashMap<>();
private final List<String> groups = new ArrayList<>();
private final List<String> roles = new ArrayList<>();
private Organization organization;
private UserStore userStoreDomain;
/**
* Represents the user id when the user is shared across sub-organizations.
* This field differs from the regular user id ({@link #id}) in scenarios where a user is accessed in the context
Expand Down Expand Up @@ -65,15 +76,18 @@ public User(Builder builder) {

this.id = builder.id;
this.claims.addAll(builder.claims);
this.userCredentials.putAll(builder.userCredentials);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Defensively copy userCredentials to prevent mutable credential leakage.

Current putAll/unmodifiableMap handling is shallow. External code can still mutate the stored char[] values, and Builder.userCredentials(...) can NPE on null input.

🔧 Suggested fix
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@
-        this.userCredentials.putAll(builder.userCredentials);
+        builder.userCredentials.forEach((key, value) ->
+                this.userCredentials.put(key, value != null ? Arrays.copyOf(value, value.length) : null));
@@
     public Map<String, char[]> getUserCredentials() {
 
-        return Collections.unmodifiableMap(userCredentials);
+        Map<String, char[]> copy = new HashMap<>();
+        userCredentials.forEach((key, value) ->
+                copy.put(key, value != null ? Arrays.copyOf(value, value.length) : null));
+        return Collections.unmodifiableMap(copy);
     }
@@
         public Builder userCredentials(Map<String, char[]> userCredentials) {
 
-            this.userCredentials.putAll(userCredentials);
+            if (userCredentials == null) {
+                return this;
+            }
+            userCredentials.forEach((key, value) ->
+                    this.userCredentials.put(key, value != null ? Arrays.copyOf(value, value.length) : null));
             return this;
         }

Also applies to: 101-104, 224-227

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/api/model/User.java`
at line 79, The User class currently copies Builder.userCredentials via putAll
and wraps with Collections.unmodifiableMap but leaves the stored char[] mutable
and Builder.userCredentials(...) can NPE on null; fix by defensively copying the
map and each char[] value when setting Builder.userCredentials and when
constructing User (instead of plain putAll/unmodifiableMap), e.g., create a new
HashMap, for each entry copy the key and clone the char[] (or create a new
char[] and System.arraycopy) before putting it into the map, handle null input
in Builder.userCredentials by treating null as an empty map (or throwing a clear
IllegalArgumentException), and apply the same defensive-copy pattern in the
other affected spots (the builder setter and any other places around lines
referenced) so the internal userCredentials map and its char[] values are
immutable from external mutation.

this.groups.addAll(builder.groups);
this.roles.addAll(builder.roles);
this.organization = builder.organization;
this.sharedUserId = builder.sharedUserId;
this.userType = builder.userType;
this.federatedIdP = builder.federatedIdP;
this.accessingOrganization = builder.accessingOrganization;
this.userStoreDomain = builder.userStoreDomain;
}

@JsonInclude(JsonInclude.Include.NON_NULL)
public String getId() {

return id;
Expand All @@ -84,6 +98,11 @@ public List<UserClaim> getClaims() {
return Collections.unmodifiableList(claims);
}

public Map<String, char[]> getUserCredentials() {

return Collections.unmodifiableMap(userCredentials);
}

public List<String> getGroups() {

return Collections.unmodifiableList(groups);
Expand All @@ -99,6 +118,13 @@ public Organization getOrganization() {
return organization;
}

@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonSerialize(using = UserStoreNameSerializer.class)
public UserStore getUserStoreDomain() {

return userStoreDomain;
}

public String getSharedUserId() {

return sharedUserId;
Expand Down Expand Up @@ -126,9 +152,11 @@ public static class Builder {

private final String id;
private final List<UserClaim> claims = new ArrayList<>();
private final Map<String, char[]> userCredentials = new HashMap<>();
private final List<String> groups = new ArrayList<>();
private final List<String> roles = new ArrayList<>();
private Organization organization;
private UserStore userStoreDomain;
private String sharedUserId;
private String userType;
private String federatedIdP;
Expand Down Expand Up @@ -163,6 +191,12 @@ public Builder organization(Organization organization) {
return this;
}

public Builder userStoreDomain(UserStore userStoreDomain) {

this.userStoreDomain = userStoreDomain;
return this;
}

public Builder sharedUserId(String sharedUserId) {

this.sharedUserId = sharedUserId;
Expand All @@ -187,9 +221,24 @@ public Builder accessingOrganization(Organization accessingOrganization) {
return this;
}

public Builder userCredentials(Map<String, char[]> userCredentials) {

this.userCredentials.putAll(userCredentials);
return this;
}

public User build() {

return new User(this);
}
}

private static class UserStoreNameSerializer extends JsonSerializer<UserStore> {

@Override
public void serialize(UserStore value, JsonGenerator gen, SerializerProvider serializers) throws IOException {

gen.writeString(value.getName() != null ? value.getName() : "");
}
Comment on lines +238 to +242

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 1

Suggested change
@Override
public void serialize(UserStore value, JsonGenerator gen, SerializerProvider serializers) throws IOException {
gen.writeString(value.getName() != null ? value.getName() : "");
}
@Override
public void serialize(UserStore value, JsonGenerator gen, SerializerProvider serializers) throws IOException {
if (log.isDebugEnabled()) {
log.debug("Serializing UserStore domain for user.");
}
gen.writeString(value.getName() != null ? value.getName() : "");
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@
} catch (ActionExecutionRuntimeException e) {
LOG.debug("Skip executing action for action type: " + actionType.name(), e);
// Skip executing actions when no action available is considered as action execution being successful.
Action.ActionTypes.Category category = Action.ActionTypes.valueOf(actionType.toString()).getCategory();
if (Action.ActionTypes.Category.FLOW_EXTENSION.equals(category)) {
throw new ActionExecutionException(
"Failed to execute flow extension action with id: " + actionId
Comment on lines 154 to +160

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 2

Suggested change
} catch (ActionExecutionRuntimeException e) {
LOG.debug("Skip executing action for action type: " + actionType.name(), e);
// Skip executing actions when no action available is considered as action execution being successful.
Action.ActionTypes.Category category = Action.ActionTypes.valueOf(actionType.toString()).getCategory();
if (Action.ActionTypes.Category.FLOW_EXTENSION.equals(category)) {
throw new ActionExecutionException(
"Failed to execute flow extension action with id: " + actionId
} catch (ActionExecutionRuntimeException e) {
LOG.debug("Skip executing action for action type: " + actionType.name(), e);
// Skip executing actions when no action available is considered as action execution being successful.
Action.ActionTypes.Category category = Action.ActionTypes.valueOf(actionType.toString()).getCategory();
if (Action.ActionTypes.Category.FLOW_EXTENSION.equals(category)) {
LOG.error("Failed to execute flow extension action with id: " + actionId + " for action type: " + actionType.name() + ". Error: " + e.getMessage());
throw new ActionExecutionException(

+ " for action type: " + actionType.name(), e);

Check failure on line 161 in components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/internal/service/impl/ActionExecutorServiceImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal " for action type: " 5 times.

See more on https://sonarcloud.io/project/issues?id=wso2_carbon-identity-framework&issues=AZ5e46dXcewz58IZ6yJ-&open=AZ5e46dXcewz58IZ6yJ-&pullRequest=8112
}
return new SuccessStatus.Builder().setResponseContext(flowContext.getContextData()).build();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public boolean isExecutionForActionTypeEnabled(ActionType actionType) {
return isActionTypeEnabled(ActionTypeConfig.PRE_UPDATE_PROFILE.getActionTypeEnableProperty());
case PRE_ISSUE_ID_TOKEN:
return isActionTypeEnabled(ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getActionTypeEnableProperty());
case FLOW_EXTENSION:
return isActionTypeEnabled(ActionTypeConfig.FLOW_EXTENSION.getActionTypeEnableProperty());
default:
Comment on lines 89 to 92

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 3

Suggested change
return isActionTypeEnabled(ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getActionTypeEnableProperty());
case FLOW_EXTENSION:
return isActionTypeEnabled(ActionTypeConfig.FLOW_EXTENSION.getActionTypeEnableProperty());
default:
public boolean isExecutionForActionTypeEnabled(ActionType actionType) {
switch (actionType) {
case PRE_ISSUE_ACCESS_TOKEN:
return isActionTypeEnabled(ActionTypeConfig.PRE_ISSUE_ACCESS_TOKEN.getActionTypeEnableProperty());
case PRE_UPDATE_PASSWORD:
return isActionTypeEnabled(ActionTypeConfig.PRE_UPDATE_PASSWORD.getActionTypeEnableProperty());
case PRE_UPDATE_PROFILE:
return isActionTypeEnabled(ActionTypeConfig.PRE_UPDATE_PROFILE.getActionTypeEnableProperty());
case PRE_ISSUE_ID_TOKEN:
return isActionTypeEnabled(ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getActionTypeEnableProperty());
case FLOW_EXTENSION:
if (log.isDebugEnabled()) {
log.debug("Checking if FLOW_EXTENSION action type is enabled");
}
return isActionTypeEnabled(ActionTypeConfig.FLOW_EXTENSION.getActionTypeEnableProperty());

return false;
}
Expand Down Expand Up @@ -333,6 +335,8 @@ public String getRetiredUpToVersion(ActionType actionType) {
return getVersion(ActionTypeConfig.PRE_UPDATE_PROFILE.getRetiredUpToVersionProperty());
case PRE_ISSUE_ID_TOKEN:
return getVersion(ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getRetiredUpToVersionProperty());
case FLOW_EXTENSION:
return getVersion(ActionTypeConfig.FLOW_EXTENSION.getRetiredUpToVersionProperty());
default:
return null;
}
Expand Down Expand Up @@ -417,7 +421,13 @@ private enum ActionTypeConfig {
"Actions.Types.PreIssueIdToken.ActionRequest.ExcludedParameters.Parameter",
"Actions.Types.PreIssueIdToken.ActionRequest.AllowedHeaders.Header",
"Actions.Types.PreIssueIdToken.ActionRequest.AllowedParameters.Parameter",
"Actions.Types.PreIssueIdToken.Version.RetiredUpTo");
"Actions.Types.PreIssueIdToken.Version.RetiredUpTo"),
FLOW_EXTENSION("Actions.Types.FlowExtension.Enable",
"Actions.Types.FlowExtension.ActionRequest.ExcludedHeaders.Header",
"Actions.Types.FlowExtension.ActionRequest.ExcludedParameters.Parameter",
"Actions.Types.FlowExtension.ActionRequest.AllowedHeaders.Header",
"Actions.Types.FlowExtension.ActionRequest.AllowedParameters.Parameter",
"Actions.Types.FlowExtension.Version.RetiredUpTo");
Comment on lines +425 to +430

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

FLOW_EXTENSION request-filter config keys are defined but not applied.

ActionTypeConfig.FLOW_EXTENSION adds allowed/excluded header/parameter keys, but the readers (getExcludedHeadersInActionRequestForActionType, getExcludedParamsInActionRequestForActionType, getAllowedHeadersForActionType, getAllowedParamsForActionType) still only handle PRE_ISSUE_ACCESS_TOKEN, so FLOW_EXTENSION-specific filters never take effect.

🔧 Suggested fix
@@ public Set<String> getExcludedHeadersInActionRequestForActionType(ActionType actionType) {
         switch (actionType) {
             case PRE_ISSUE_ACCESS_TOKEN:
                 excludedHeadersPropertyValue = getPropertyValues(
                         ActionTypeConfig.PRE_ISSUE_ACCESS_TOKEN.getExcludedHeadersProperty());
                 break;
+            case FLOW_EXTENSION:
+                excludedHeadersPropertyValue = getPropertyValues(
+                        ActionTypeConfig.FLOW_EXTENSION.getExcludedHeadersProperty());
+                break;
             default:
                 break;
         }
@@ public Set<String> getExcludedParamsInActionRequestForActionType(ActionType actionType) {
         switch (actionType) {
             case PRE_ISSUE_ACCESS_TOKEN:
                 excludedParamsPropertyValue = getPropertyValues(
                         ActionTypeConfig.PRE_ISSUE_ACCESS_TOKEN.getExcludedParamsProperty());
 
                 break;
+            case FLOW_EXTENSION:
+                excludedParamsPropertyValue = getPropertyValues(
+                        ActionTypeConfig.FLOW_EXTENSION.getExcludedParamsProperty());
+                break;
             default:
                 break;
         }
@@ public Set<String> getAllowedHeadersForActionType(ActionType actionType) {
         switch (actionType) {
             case PRE_ISSUE_ACCESS_TOKEN:
                 allowedPropertyKey = ActionTypeConfig.PRE_ISSUE_ACCESS_TOKEN.getAllowedHeaderProperty();
                 break;
+            case FLOW_EXTENSION:
+                allowedPropertyKey = ActionTypeConfig.FLOW_EXTENSION.getAllowedHeaderProperty();
+                break;
             default:
                 break;
         }
@@ public Set<String> getAllowedParamsForActionType(ActionType actionType) {
         switch (actionType) {
             case PRE_ISSUE_ACCESS_TOKEN:
                 allowedPropertyKey = ActionTypeConfig.PRE_ISSUE_ACCESS_TOKEN.getAllowedParamsProperty();
                 break;
+            case FLOW_EXTENSION:
+                allowedPropertyKey = ActionTypeConfig.FLOW_EXTENSION.getAllowedParamsProperty();
+                break;
             default:
                 break;
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/internal/util/ActionExecutorConfig.java`
around lines 425 - 430, ActionTypeConfig.FLOW_EXTENSION defines allowed/excluded
header/parameter config keys but the reader methods
(getExcludedHeadersInActionRequestForActionType,
getExcludedParamsInActionRequestForActionType, getAllowedHeadersForActionType,
getAllowedParamsForActionType) only handle PRE_ISSUE_ACCESS_TOKEN, so
FLOW_EXTENSION filters are ignored; update each of those reader methods to
include a branch/case for ActionTypeConfig.FLOW_EXTENSION that reads the
corresponding keys from ActionExecutorConfig.FLOW_EXTENSION (e.g., the
AllowedHeaders.Header, AllowedParameters.Parameter, ExcludedHeaders.Header,
ExcludedParameters.Parameter entries) and returns those values just like the
PRE_ISSUE_ACCESS_TOKEN branch does.


private final String actionTypeEnableProperty;
private final String excludedHeadersProperty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ public enum ErrorMessage {
"The number of configured attributes: %s exceeds the maximum allowed limit: %s"),
ERROR_INVALID_ATTRIBUTES("60012", "Invalid attribute provided.",
"%s"),
ERROR_EMPTY_ATTRIBUTE_VALUE("60013", "Invalid attribute provided.",
"Each attribute must be a non-empty string."),
ERROR_UNSUPPORTED_ATTRIBUTE("60014", "Unsupported attribute provided.",
"The attribute %s is not supported to be shared with the extension."),
ERROR_ACTION_NAME_ALREADY_EXISTS("60015", "Action name already exists.",
"An action with the name '%s' already exists for the action type '%s'."),
ERROR_ACTION_NAME_BLANK("60016", "Invalid action name.",
"An action name must be a non-empty string."),

// Server errors.
ERROR_WHILE_ADDING_ACTION("65001", "Error while adding Action.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,13 @@ public enum ActionTypes {
"PRE_ISSUE_ID_TOKEN",
"Pre Issue ID Token",
"Configure an extension point for modifying ID token via a custom service.",
Category.PRE_POST);
Category.PRE_POST),
FLOW_EXTENSION(
"flowExtension",
"FLOW_EXTENSION",
"Flow Extension",
"Configure an extension point within any flow via a custom service.",
Category.FLOW_EXTENSION);

private final String pathParam;
private final String actionType;
Expand Down Expand Up @@ -131,7 +137,8 @@ public static ActionTypes[] filterByCategory(Category category) {
*/
public enum Category {
PRE_POST,
IN_FLOW
IN_FLOW,
FLOW_EXTENSION
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public static ActionDTOModelResolver getActionDTOModelResolver(Action.ActionType
return actionDTOModelResolvers.get(Action.ActionTypes.PRE_UPDATE_PASSWORD);
case PRE_ISSUE_ACCESS_TOKEN:
return actionDTOModelResolvers.get(Action.ActionTypes.PRE_ISSUE_ACCESS_TOKEN);
case FLOW_EXTENSION:
return actionDTOModelResolvers.get(Action.ActionTypes.FLOW_EXTENSION);
default:
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,7 @@ private ActionConverterFactory() {

public static ActionConverter getActionConverter(Action.ActionTypes actionType) {

switch (actionType) {
case PRE_UPDATE_PROFILE:
return actionConverters.get(Action.ActionTypes.PRE_UPDATE_PROFILE);
case PRE_UPDATE_PASSWORD:
return actionConverters.get(Action.ActionTypes.PRE_UPDATE_PASSWORD);
case PRE_ISSUE_ACCESS_TOKEN:
default:
return null;
}
return actionConverters.get(actionType);
}
Comment on lines 40 to 43

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 4

Suggested change
public static ActionConverter getActionConverter(Action.ActionTypes actionType) {
switch (actionType) {
case PRE_UPDATE_PROFILE:
return actionConverters.get(Action.ActionTypes.PRE_UPDATE_PROFILE);
case PRE_UPDATE_PASSWORD:
return actionConverters.get(Action.ActionTypes.PRE_UPDATE_PASSWORD);
case PRE_ISSUE_ACCESS_TOKEN:
default:
return null;
}
return actionConverters.get(actionType);
}
public static ActionConverter getActionConverter(Action.ActionTypes actionType) {
ActionConverter converter = actionConverters.get(actionType);
if (converter == null) {
log.debug("No ActionConverter found for action type: " + actionType);
}
return converter;


public static void registerActionConverter(ActionConverter actionConverter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.wso2.carbon.identity.action.management.api.exception.ActionMgtException;
import org.wso2.carbon.identity.action.management.api.exception.ActionMgtServerException;
import org.wso2.carbon.identity.action.management.api.model.Action;
import org.wso2.carbon.identity.action.management.api.model.Action.ActionTypes;
import org.wso2.carbon.identity.action.management.api.model.ActionDTO;
import org.wso2.carbon.identity.action.management.api.model.Authentication;
import org.wso2.carbon.identity.action.management.api.model.EndpointConfig;
Expand Down Expand Up @@ -76,6 +77,7 @@ public Action addAction(String actionType, Action action, String tenantDomain) t
Action.ActionTypes castedActionType = Action.ActionTypes.valueOf(resolvedActionType);
ActionValidatorFactory.getActionValidator(castedActionType).doPreAddActionValidations(
castedActionType, ActionManagementConfig.getInstance().getLatestVersion(castedActionType), action);
Comment on lines 77 to 79

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 5

Suggested change
Action.ActionTypes castedActionType = Action.ActionTypes.valueOf(resolvedActionType);
ActionValidatorFactory.getActionValidator(castedActionType).doPreAddActionValidations(
castedActionType, ActionManagementConfig.getInstance().getLatestVersion(castedActionType), action);
Action.ActionTypes castedActionType = Action.ActionTypes.valueOf(resolvedActionType);
log.info("Adding new action with type: " + castedActionType + " for tenant: " + tenantDomain);
ActionValidatorFactory.getActionValidator(castedActionType).doPreAddActionValidations(

validateActionNameUniqueness(action.getName(), null, castedActionType, tenantId);
// Check whether the maximum allowed actions per type is reached.
validateMaxActionsPerType(resolvedActionType, tenantDomain);
String generatedActionId = UUID.randomUUID().toString();
Expand Down Expand Up @@ -161,6 +163,7 @@ public Action updateAction(String actionType, String actionId, Action action, St
Action.ActionTypes castedActionType = Action.ActionTypes.valueOf(resolvedActionType);
ActionValidatorFactory.getActionValidator(castedActionType).doPreUpdateActionValidations(
castedActionType, resolveActionVersionAtUpdating(action, existingActionDTO), action);
validateActionNameUniqueness(action.getName(), actionId, castedActionType, tenantId);
ActionDTO updatingActionDTO = buildActionDTOForUpdate(resolvedActionType, actionId, action);

DAO_FACADE.updateAction(updatingActionDTO, existingActionDTO, tenantId);
Expand Down Expand Up @@ -317,8 +320,10 @@ private String getActionTypeFromPath(String actionType) throws ActionMgtClientEx
*/
private void validateMaxActionsPerType(String actionType, String tenantDomain) throws ActionMgtException {

// In-flow actions are not limited by the maximum actions per action type; eg: AUTHENTICATION action type.
if (Action.ActionTypes.Category.IN_FLOW.equals(Action.ActionTypes.valueOf(actionType).getCategory())) {
// In-flow and extension actions are not limited by the maximum actions per action type.
Action.ActionTypes.Category category = Action.ActionTypes.valueOf(actionType).getCategory();
if (Action.ActionTypes.Category.IN_FLOW.equals(category)
|| Action.ActionTypes.Category.FLOW_EXTENSION.equals(category)) {
return;
}
Map<String, Integer> actionsCountPerType = getActionsCountPerType(tenantDomain);
Expand Down Expand Up @@ -365,8 +370,13 @@ private ActionDTO buildActionDTOForCreation(String actionType, String actionId,
throws ActionMgtServerException {

Action.ActionTypes resolvedActionType = Action.ActionTypes.valueOf(actionType);
Action.Status resolvedStatus = resolvedActionType.getCategory() == Action.ActionTypes.Category.IN_FLOW ?
Action.Status.ACTIVE : Action.Status.INACTIVE;
// Only IN_FLOW and FLOW_EXTENSION category actions (e.g., AUTHENTICATION, FLOW_EXTENSION)
// start ACTIVE and can be used immediately. All other categories (e.g., PRE_POST) start
// INACTIVE and require explicit activation.
Action.ActionTypes.Category category = resolvedActionType.getCategory();
Action.Status resolvedStatus = (category == Action.ActionTypes.Category.IN_FLOW
|| category == Action.ActionTypes.Category.FLOW_EXTENSION)
? Action.Status.ACTIVE : Action.Status.INACTIVE;

String actionVersion = ActionManagementConfig.getInstance().getLatestVersion(resolvedActionType);

Expand Down Expand Up @@ -470,4 +480,25 @@ private Action buildAction(String actionType, ActionDTO actionDTO) {
.rule(actionDTO.getActionRule())
.build();
}

private void validateActionNameUniqueness(String name, String excludeId, ActionTypes actionType, int tenantId)
throws ActionMgtException {

if (!ActionTypes.FLOW_EXTENSION.equals(actionType)) {
return;
}
if (StringUtils.isBlank(name)) {
throw ActionManagementExceptionHandler.handleClientException(
ErrorMessage.ERROR_ACTION_NAME_BLANK);
}
Comment on lines +490 to +493

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

PATCH updates are incorrectly rejected when name is omitted for FLOW_EXTENSION.

Line 166 always invokes uniqueness validation, and Line 490 rejects blank names. That breaks the method’s PATCH contract (null/empty fields should be ignored) and can fail partial updates that do not include name.

Proposed fix
-        if (StringUtils.isBlank(name)) {
-            throw ActionManagementExceptionHandler.handleClientException(
-                    ErrorMessage.ERROR_ACTION_NAME_BLANK);
-        }
+        if (StringUtils.isBlank(name)) {
+            // PATCH update path: omitted/blank fields are ignored.
+            if (excludeId != null) {
+                return;
+            }
+            throw ActionManagementExceptionHandler.handleClientException(
+                    ErrorMessage.ERROR_ACTION_NAME_BLANK);
+        }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@components/action-mgt/org.wso2.carbon.identity.action.management/src/main/java/org/wso2/carbon/identity/action/management/internal/service/impl/ActionManagementServiceImpl.java`
around lines 490 - 493, The method in ActionManagementServiceImpl is rejecting
blank/missing name unconditionally (throwing
ErrorMessage.ERROR_ACTION_NAME_BLANK) which breaks PATCH semantics; change the
logic so blank or null name is only rejected for create/full-update flows, not
for PATCH/partial updates—i.e., guard the current blank-name check and the
uniqueness validation (the check invoked around the symbol referenced at line
~166) so they run only when the request explicitly supplies a non-blank name (or
when the operation is not a PATCH), and skip both validations when the name is
omitted in a PATCH. Ensure you reference the same method in
ActionManagementServiceImpl where the name and uniqueness checks are performed
and only validate uniqueness when name != null/blank.

List<ActionDTO> existingActions = DAO_FACADE.getActionsByActionType(actionType.getActionType(), tenantId);
boolean duplicateExists = existingActions.stream()
.filter(dto -> excludeId == null || !excludeId.equals(dto.getId()))
.anyMatch(dto -> name.equalsIgnoreCase(dto.getName()));

if (duplicateExists) {
throw ActionManagementExceptionHandler.handleClientException(
ErrorMessage.ERROR_ACTION_NAME_ALREADY_EXISTS, name, actionType.getActionType());
}
Comment on lines +500 to +502

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 6

Suggested change
throw ActionManagementExceptionHandler.handleClientException(
ErrorMessage.ERROR_ACTION_NAME_ALREADY_EXISTS, name, actionType.getActionType());
}
throw ActionManagementExceptionHandler.handleClientException(
ErrorMessage.ERROR_ACTION_NAME_ALREADY_EXISTS, name, actionType.getActionType());
log.error("Action name already exists: " + name + " for action type: " + actionType.getActionType());
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ public String getLatestVersion(ActionTypes actionType) throws ActionMgtServerExc
return getVersion(
ActionTypeConfig.PRE_UPDATE_PROFILE.getLatestVersionProperty(), actionType);
case PRE_ISSUE_ID_TOKEN:
return getVersion(ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getLatestVersionProperty(), actionType);
return getVersion(
ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getLatestVersionProperty(), actionType);
case FLOW_EXTENSION:
return getVersion(
ActionTypeConfig.FLOW_EXTENSION.getLatestVersionProperty(), actionType);
Comment on lines 96 to +101

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 7

Suggested change
case PRE_ISSUE_ID_TOKEN:
return getVersion(ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getLatestVersionProperty(), actionType);
return getVersion(
ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getLatestVersionProperty(), actionType);
case FLOW_EXTENSION:
return getVersion(
ActionTypeConfig.FLOW_EXTENSION.getLatestVersionProperty(), actionType);
return getVersion(
ActionTypeConfig.PRE_ISSUE_ID_TOKEN.getLatestVersionProperty(), actionType);
case FLOW_EXTENSION:
log.debug("Retrieving latest version for FLOW_EXTENSION action type");
return getVersion(
ActionTypeConfig.FLOW_EXTENSION.getLatestVersionProperty(), actionType);

default:
throw new ActionMgtServerException("Unsupported action type: " + actionType);
Comment on lines 102 to 103

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Log Improvement Suggestion No: 8

Suggested change
default:
throw new ActionMgtServerException("Unsupported action type: " + actionType);
default:
log.error("Unsupported action type encountered: " + actionType);
throw new ActionMgtServerException("Unsupported action type: " + actionType);

}
Expand Down Expand Up @@ -140,6 +144,11 @@ public enum ActionTypeConfig {
"Actions.Types.PreIssueIdToken.ActionRequest.ExcludedHeaders.Header",
"Actions.Types.PreIssueIdToken.ActionRequest.ExcludedParameters.Parameter",
"Actions.Types.PreIssueIdToken.Version.Latest"
),
FLOW_EXTENSION(
"Actions.Types.FlowExtension.ActionRequest.ExcludedHeaders.Header",
null,
"Actions.Types.FlowExtension.Version.Latest"
);

private final String excludedHeadersProperty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ public Object[][] actionTypesProvider() {
{Action.ActionTypes.PRE_ISSUE_ID_TOKEN, "preIssueIdToken", "PRE_ISSUE_ID_TOKEN",
"Pre Issue ID Token",
"Configure an extension point for modifying ID token via a custom service.",
Action.ActionTypes.Category.PRE_POST}
Action.ActionTypes.Category.PRE_POST},
{Action.ActionTypes.FLOW_EXTENSION, "flowExtension", "FLOW_EXTENSION",
"Flow Extension",
"Configure an extension point within any flow via a custom service.",
Action.ActionTypes.Category.FLOW_EXTENSION}
};
}

Expand All @@ -76,7 +80,9 @@ public Object[][] filterByCategoryProvider() {
new Action.ActionTypes[]{Action.ActionTypes.PRE_ISSUE_ACCESS_TOKEN,
Action.ActionTypes.PRE_UPDATE_PASSWORD, Action.ActionTypes.PRE_UPDATE_PROFILE,
Action.ActionTypes.PRE_REGISTRATION, Action.ActionTypes.PRE_ISSUE_ID_TOKEN}},
{Action.ActionTypes.Category.IN_FLOW, new Action.ActionTypes[]{Action.ActionTypes.AUTHENTICATION}}
{Action.ActionTypes.Category.IN_FLOW, new Action.ActionTypes[]{Action.ActionTypes.AUTHENTICATION}},
{Action.ActionTypes.Category.FLOW_EXTENSION,
new Action.ActionTypes[]{Action.ActionTypes.FLOW_EXTENSION}}
};
}

Expand Down
Loading
Loading