Address comments

Thisara-Welmilla · Nov 18, 2024 · 1e7779f · 1e7779f
1 parent 75aedd8
commit 1e7779f
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 8 deletions.
diff --git a/...mgt/src/main/java/org/wso2/carbon/idp/mgt/AuthenticatorEndpointConfigServerException.java b/...mgt/src/main/java/org/wso2/carbon/idp/mgt/AuthenticatorEndpointConfigServerException.java
@@ -21,7 +21,7 @@
 /**
  * Exception class for user defined federated authenticator endpoint configurations related exceptions.
  */
-public class AuthenticatorEndpointConfigServerException extends IdentityProviderManagementServerException {
+public class AuthenticatorEndpointConfigServerException extends IdentityProviderManagementException {
 
         public AuthenticatorEndpointConfigServerException(String message) {
 

diff --git a/...rg.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java b/...rg.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java
@@ -68,9 +68,11 @@
 import org.wso2.carbon.utils.multitenancy.MultitenantConstants;
 
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.sql.Connection;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Base64;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -1501,8 +1503,8 @@ public IdentityProvider addIdPWithResourceId(IdentityProvider identityProvider,
             throws IdentityProviderManagementException {
 
         markConfidentialPropertiesUsingMetadata(identityProvider);
-        validateAddIdPInputValues(identityProvider.getIdentityProviderName(), tenantDomain);
         validateFederatedAuthenticatorConfigName(identityProvider.getFederatedAuthenticatorConfigs(), tenantDomain);
+        validateAddIdPInputValues(identityProvider.getIdentityProviderName(), tenantDomain);
         validateOutboundProvisioningRoles(identityProvider, tenantDomain);
 
         // Invoking the pre listeners.
@@ -1853,6 +1855,7 @@ public IdentityProvider updateIdPByResourceId(String resourceId, IdentityProvide
 
         newIdentityProvider.setTrustedTokenIssuer(isTrustedTokenIssuer(newIdentityProvider));
         validateUpdateIdPInputValues(currentIdentityProvider, resourceId, newIdentityProvider, tenantDomain);
+        validateFederatedAuthenticatorConfigName(newIdentityProvider.getFederatedAuthenticatorConfigs(), tenantDomain);
         updateIDP(currentIdentityProvider, newIdentityProvider, tenantId, tenantDomain);
 
         // Invoking the post listeners.
@@ -2201,7 +2204,8 @@ private void validateFederatedAuthenticatorConfigName(FederatedAuthenticatorConf
                 if (ApplicationAuthenticatorService.getInstance()
                         .getFederatedAuthenticatorByName(config.getName()) == null) {
                     throw IdPManagementUtil.handleClientException(IdPManagementConstants.ErrorMessage
-                            .ERROR_CODE_NO_SYSTEM_AUTHENTICATOR_FOUND, config.getName());
+                            .ERROR_CODE_NO_SYSTEM_AUTHENTICATOR_FOUND, new String(
+                            Base64.getEncoder().encode(config.getName().getBytes(StandardCharsets.UTF_8))));
                 }
             } else {
                 // Check if the given authenticator name is already taken.

diff --git a/...t/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java b/...t/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/IdPManagementDAO.java
@@ -117,6 +117,7 @@
 import static org.wso2.carbon.idp.mgt.util.IdPManagementConstants.MySQL;
 import static org.wso2.carbon.idp.mgt.util.IdPManagementConstants.RESET_PROVISIONING_ENTITIES_ON_CONFIG_UPDATE;
 import static org.wso2.carbon.idp.mgt.util.IdPManagementConstants.SCOPE_LIST_PLACEHOLDER;
+import static org.wso2.carbon.idp.mgt.util.IdPManagementConstants.SQLConstants.DEFINED_BY_COLUMN;
 import static org.wso2.carbon.idp.mgt.util.IdPManagementConstants.SQLQueries.GET_IDP_NAME_BY_RESOURCE_ID_SQL;
 import static org.wso2.carbon.idp.mgt.util.IdPManagementConstants.TEMPLATE_ID_IDP_PROPERTY_DISPLAY_NAME;
 import static org.wso2.carbon.idp.mgt.util.IdPManagementConstants.TEMPLATE_ID_IDP_PROPERTY_NAME;
@@ -1167,7 +1168,7 @@ private FederatedAuthenticatorConfig[] getFederatedAuthenticatorConfigs(
 
             while (rs.next()) {
                 FederatedAuthenticatorConfig authnConfig = createFederatedAuthenticatorConfig(DefinedByType.valueOf(
-                                rs.getString("DEFINED_BY")));
+                                rs.getString(DEFINED_BY_COLUMN)));
                 int authnId = rs.getInt("ID");
                 authnConfig.setName(rs.getString("NAME"));
 
@@ -3450,7 +3451,7 @@ public IdentityProvider getIdPByAuthenticatorPropertyValue(Connection dbConnecti
                 String roleClaimUri = rs.getString("ROLE_CLAIM_URI");
 
                 String defaultAuthenticatorName = rs.getString("DEFAULT_AUTHENTICATOR_NAME");
-                String defaultAuthenticatorDefinedByType = rs.getString("DEFINED_BY");
+                String defaultAuthenticatorDefinedByType = rs.getString(DEFINED_BY_COLUMN);
                 String defaultProvisioningConnectorConfigName = rs.getString("DEFAULT_PRO_CONNECTOR_NAME");
                 federatedIdp.setIdentityProviderDescription(rs.getString("DESCRIPTION"));
 

diff --git a/...so2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/util/IdPManagementConstants.java b/...so2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/util/IdPManagementConstants.java
@@ -137,6 +137,11 @@ public class IdPManagementConstants {
     public static final String EMAIL_USERNAME_RECOVERY_PROPERTY = "Recovery.Notification.Username.Email.Enable";
     public static final String SMS_USERNAME_RECOVERY_PROPERTY = "Recovery.Notification.Username.SMS.Enable";
 
+    public static class SQLConstants {
+
+        public static final String DEFINED_BY_COLUMN = "DEFINED_BY";
+    }
+
     public static class SQLQueries {
 
         public static final String GET_IDPS_SQL = "SELECT NAME, IS_PRIMARY, HOME_REALM_ID, DESCRIPTION, " +
@@ -618,7 +623,7 @@ public enum ErrorMessage {
                 "provisioning roles does not exist"),
         ERROR_CODE_INVALID_CONNECTOR_CONFIGURATION("IDP-60011", "Invalid connector configuration. %s"),
         ERROR_CODE_NO_SYSTEM_AUTHENTICATOR_FOUND("IDP-60012", "No system authenticator found for the " +
-                "provided authenticator name %s."),
+                "provided authenticator Id %s."),
         ERROR_CODE_AUTHENTICATOR_NAME_ALREADY_TAKEN("IDP-60013", "Federated authenticator name %s" +
                 "is already taken."),
         ERROR_INVALID_AUTHENTICATOR_NAME("IDP-60014", "Federated authenticator name does not match the" +

diff --git a/....idp.mgt/src/test/java/org/wso2/carbon/idp/mgt/IdentityProviderManagementServiceTest.java b/....idp.mgt/src/test/java/org/wso2/carbon/idp/mgt/IdentityProviderManagementServiceTest.java
@@ -165,7 +165,7 @@ public Object[][] addFederatedAuthenticatorData() {
 
         FederatedAuthenticatorConfig userDefinedAuthWithInvalidName = new UserDefinedFederatedAuthenticatorConfig();
         userDefinedAuthWithInvalidName.setDisplayName("DisplayName1");
-        userDefinedAuthWithInvalidName.setName("SAMLSSOAuthenticator");
+        userDefinedAuthWithInvalidName.setName("Invalid regex name");
         userDefinedAuthWithInvalidName.setEnabled(true);
         userDefinedAuthWithInvalidName.setDefinedByType(DefinedByType.USER);
 
@@ -177,14 +177,16 @@ public Object[][] addFederatedAuthenticatorData() {
     }
 
     @Test(dataProvider = "addFederatedAuthenticatorData")
-    public void testFederatedAuthenticatorNameValidation(FederatedAuthenticatorConfig config) {
+    public void testFederatedAuthenticatorNameValidation(FederatedAuthenticatorConfig config)
+            throws IdentityProviderManagementException {
 
         IdentityProvider identityProvider = new IdentityProvider();
         identityProvider.setDisplayName("testIdP1");
         identityProvider.setFederatedAuthenticatorConfigs(new FederatedAuthenticatorConfig[]{config});
 
         assertThrows(IdentityProviderManagementException.class, () ->
                 identityProviderManagementService.addIdP(identityProvider));
+        identityProviderManagementService.deleteIdP("testIdP1");
     }