Improve authentication name validation.

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/ApplicationAuthenticatorService.java

@@ -38,7 +38,6 @@
 
 import static org.wso2.carbon.identity.application.common.util.AuthenticatorMgtExceptionBuilder.buildClientException;
 import static org.wso2.carbon.identity.application.common.util.AuthenticatorMgtExceptionBuilder.buildRuntimeServerException;
-import static org.wso2.carbon.identity.application.common.util.IdentityApplicationConstants.Authenticator.DISPLAY_NAME;
 
 /**
  * Application authenticator service.
@@ -207,13 +206,12 @@ public UserDefinedLocalAuthenticatorConfig addUserDefinedLocalAuthenticator(
             UserDefinedLocalAuthenticatorConfig authenticatorConfig, String tenantDomain)
             throws AuthenticatorMgtException {
 
-        LocalAuthenticatorConfig config = getLocalAuthenticatorByName(authenticatorConfig.getName(), tenantDomain);
-        if (config != null) {
+        if (isExistingAuthenticatorName(authenticatorConfig.getName(), tenantDomain)) {
             throw buildClientException(AuthenticatorMgtError.ERROR_AUTHENTICATOR_ALREADY_EXIST,
                     authenticatorConfig.getName());
         }
         authenticatorValidator.validateAuthenticatorName(authenticatorConfig.getName());
-        authenticatorValidator.validateForBlank(DISPLAY_NAME, authenticatorConfig.getDisplayName());
+        authenticatorValidator.validateDisplayName(authenticatorConfig.getDisplayName());
 
         return dao.addUserDefinedLocalAuthenticator(
                 authenticatorConfig, IdentityTenantUtil.getTenantId(tenantDomain));
@@ -238,7 +236,7 @@ public UserDefinedLocalAuthenticatorConfig updateUserDefinedLocalAuthenticator(
                     authenticatorConfig.getName());
         }
 
-        authenticatorValidator.validateForBlank(DISPLAY_NAME, authenticatorConfig.getDisplayName());
+        authenticatorValidator.validateDisplayName(authenticatorConfig.getDisplayName());
 
         return dao.updateUserDefinedLocalAuthenticator(
                 existingConfig, authenticatorConfig, IdentityTenantUtil.getTenantId(tenantDomain));
@@ -279,6 +277,40 @@ public UserDefinedLocalAuthenticatorConfig getUserDefinedLocalAuthenticator(Stri
                 authenticatorName, IdentityTenantUtil.getTenantId(tenantDomain));
     }
 
+    /**
+     * Check whether an any local of federated authenticator configuration with the given name exists.
+     *
+     * @param authenticatorName Name of the authenticator.
+     * @param tenantDomain      Tenant domain.
+     * @return True if an authenticator with the given name exists.
+     * @throws AuthenticatorMgtException If an error occurs while checking the existence of the authenticator.
+     */
+    public boolean isExistingAuthenticatorName(String authenticatorName, String tenantDomain)
+            throws AuthenticatorMgtException {
+
+        // Check whether an authenticator with the given name exists in the database.
+        if (dao.isExistingAuthenticatorName(authenticatorName, IdentityTenantUtil.getTenantId(tenantDomain))) {
+            return true;
+        }
+
+        /* Check whether an authenticator with the given name exists in the system defined authenticators
+        which are not saved in database. */
+        for (LocalAuthenticatorConfig localAuthenticator : localAuthenticators) {
+            if (localAuthenticator.getName().equals(authenticatorName)) {
+                return true;
+            }
+        }
+
+        /* Check whether an authenticator with the given name exists in the federated defined authenticators
+        which are not saved in database. */
+        for (FederatedAuthenticatorConfig federatedAuthenticator : federatedAuthenticators) {
+            if (federatedAuthenticator.getName().equals(authenticatorName)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private UserDefinedLocalAuthenticatorConfig resolveExistingAuthenticator(String authenticatorName,
             String tenantDomain) throws AuthenticatorMgtException {
 

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/constant/AuthenticatorMgtSQLConstants.java

@@ -64,6 +64,8 @@ public static class Query {
                 ":IS_ENABLED;, DISPLAY_NAME = :DISPLAY_NAME; WHERE NAME = :NAME; AND TENANT_ID = :TENANT_ID;";
         public static final String GET_AUTHENTICATOR_SQL = "SELECT * FROM IDP_AUTHENTICATOR " +
                 "WHERE DEFINED_BY = :DEFINED_BY; AND NAME = :NAME; AND TENANT_ID = :TENANT_ID;";
+        public static final String IS_AUTHENTICATOR_EXISTS_BY_NAME_SQL = "SELECT * FROM IDP_AUTHENTICATOR " +
+                "WHERE NAME = :NAME; AND TENANT_ID = :TENANT_ID;";
         public static final String GET_ALL_USER_DEFINED_AUTHENTICATOR_SQL = "SELECT * FROM IDP_AUTHENTICATOR " +
                 "WHERE DEFINED_BY = :DEFINED_BY; AND TENANT_ID = :TENANT_ID;";
         public static final String DELETE_AUTHENTICATOR_SQL = "DELETE FROM IDP_AUTHENTICATOR WHERE NAME = :NAME; " +

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/dao/AuthenticatorManagementDAO.java

@@ -83,4 +83,14 @@ List<UserDefinedLocalAuthenticatorConfig> getAllUserDefinedLocalAuthenticators(i
      */
     void deleteUserDefinedLocalAuthenticator(String authenticatorConfigName, UserDefinedLocalAuthenticatorConfig
             authenticatorConfig, int tenantId) throws AuthenticatorMgtException;
+
+    /**
+     * Check whether an any local of federated authenticator configuration with the given name exists.
+     *
+     * @param authenticatorName Name of the authenticator.
+     * @param tenantId          Tenant Id.
+     * @return True if an authenticator with the given name exists.
+     * @throws AuthenticatorMgtException If an error occurs while checking the existence of the authenticator.
+     */
+    boolean isExistingAuthenticatorName(String authenticatorName, int tenantId) throws AuthenticatorMgtException;
 }

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/dao/impl/AuthenticatorManagementDAOImpl.java

@@ -32,6 +32,7 @@
 import org.wso2.carbon.identity.base.AuthenticatorPropertyConstants.DefinedByType;
 import org.wso2.carbon.identity.core.util.IdentityDatabaseUtil;
 
+import java.sql.ResultSet;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -168,6 +169,25 @@ public void deleteUserDefinedLocalAuthenticator(String authenticatorConfigName,
         }
     }
 
+    public boolean isExistingAuthenticatorName(String authenticatorName, int tenantId)
+            throws AuthenticatorMgtException {
+
+        NamedJdbcTemplate jdbcTemplate = new NamedJdbcTemplate(IdentityDatabaseUtil.getDataSource());
+        try {
+            ResultSet results = jdbcTemplate.withTransaction(template ->
+                template.fetchSingleRecord(Query.IS_AUTHENTICATOR_EXISTS_BY_NAME_SQL,
+                        (resultSet, rowNumber) -> resultSet,
+                        statement -> {
+                            statement.setString(Column.NAME, authenticatorName);
+                            statement.setInt(Column.TENANT_ID, tenantId);
+                        }));
+            return results != null;
+        } catch (TransactionException e) {
+            throw buildServerException(AuthenticatorMgtError.ERROR_WHILE_CHECKING_FOR_EXISTING_AUTHENTICATOR_BY_NAME, e,
+                    authenticatorName);
+        }
+    }
+
     private UserDefinedLocalAuthenticatorConfig getUserDefinedLocalAuthenticatorByName(String authenticatorConfigName,
             int tenantId) throws TransactionException {
 

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/dao/impl/AuthenticatorManagementFacade.java

@@ -189,6 +189,20 @@ public void deleteUserDefinedLocalAuthenticator(String authenticatorConfigName,
         }
     }
 
+    @Override
+    public boolean isExistingAuthenticatorName(String authenticatorName, int tenantId)
+            throws AuthenticatorMgtException {
+
+        NamedJdbcTemplate jdbcTemplate = new NamedJdbcTemplate(IdentityDatabaseUtil.getDataSource());
+        try {
+            return jdbcTemplate.withTransaction(
+                    template -> dao.isExistingAuthenticatorName(authenticatorName, tenantId));
+        } catch (TransactionException e) {
+            throw handleAuthenticatorMgtException(AuthenticatorMgtError
+                            .ERROR_WHILE_CHECKING_FOR_EXISTING_AUTHENTICATOR_BY_NAME, e, authenticatorName);
+        }
+    }
+
     /**
      * Handle the authenticator management client exception.
      *

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/dao/impl/CacheBackedAuthenticatorMgtDAO.java

@@ -123,4 +123,11 @@ public void deleteUserDefinedLocalAuthenticator(String authenticatorConfigName,
         authenticatorMgtFacade.deleteUserDefinedLocalAuthenticator(authenticatorConfigName, authenticatorConfig,
                 tenantId);
     }
+
+    @Override
+    public boolean isExistingAuthenticatorName(String authenticatorConfigName, int tenantId)
+            throws AuthenticatorMgtException {
+
+        return authenticatorMgtFacade.isExistingAuthenticatorName(authenticatorConfigName, tenantId);
+    }
 }

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/util/AuthenticatorMgtExceptionBuilder.java

@@ -90,8 +90,8 @@ public enum AuthenticatorMgtError {
                 "The authenticator already exists for the given name: %s."),
         ERROR_INVALID_AUTHENTICATOR_NAME("60014", "Authenticator name is invalid.",
                 "The provided authenticator name %s is not in the expected format %s."),
-        ERROR_BLANK_FIELD_VALUE("60015", "Invalid empty or blank value.",
-                "Value for %s should not be empty or blank."),
+        ERROR_INVALID_DISPLAY_NAME("60015", "Authenticator display name is invalid.",
+                "The provided authenticator name %s is not in the expected format %s."),
 
         // Server errors.
         ERROR_WHILE_ADDING_AUTHENTICATOR("65001", "Error while adding authenticator.",
@@ -119,7 +119,9 @@ public enum AuthenticatorMgtError {
         ERROR_CODE_DELETING_ENDPOINT_CONFIG("65012", "Error while managing endpoint configurations.",
                 "Error while managing endpoint configurations for the user defined local authenticator %s."),
         ERROR_CODE_HAVING_MULTIPLE_PROP("65013", "Multiple properties found", "Only actionId " +
-                "property is allowed for authenticator: %s.");
+                "property is allowed for authenticator: %s."),
+        ERROR_WHILE_CHECKING_FOR_EXISTING_AUTHENTICATOR_BY_NAME("65014", "Error while retrieving " +
+                "authenticator.", "Error while check any authenticator exists by given name: %s.");
 
         private final String code;
         private final String message;

components/application-mgt/org.wso2.carbon.identity.application.common/src/main/java/org/wso2/carbon/identity/application/common/util/UserDefinedLocalAuthenticatorValidator.java

@@ -18,7 +18,6 @@
 
 package org.wso2.carbon.identity.application.common.util;
 
-import org.apache.commons.lang.StringUtils;
 import org.wso2.carbon.identity.application.common.exception.AuthenticatorMgtClientException;
 import org.wso2.carbon.identity.application.common.util.AuthenticatorMgtExceptionBuilder.AuthenticatorMgtError;
 
@@ -31,20 +30,24 @@
  */
 public class UserDefinedLocalAuthenticatorValidator {
 
-    private static final String AUTHENTICATOR_NAME_REGEX = "^[a-zA-Z0-9][a-zA-Z0-9-_]*$";
+    private static final String AUTHENTICATOR_NAME_REGEX = "^[custom_][a-zA-Z0-9-_]{3,}$";
     private final Pattern authenticatorNameRegexPattern = Pattern.compile(AUTHENTICATOR_NAME_REGEX);
 
+    private static final String DISPLAY_NAME_REGEX = "^.{3,}$";
+    private final Pattern disaplayNameRegexPattern = Pattern.compile(DISPLAY_NAME_REGEX);
+
     /**
-     * Validate whether required fields exist.
+     * Validate the user defined local authenticator display name.
      *
-     * @param fieldName     Field name.
-     * @param fieldValue    Field value.
-     * @throws AuthenticatorMgtClientException if the provided field is empty.
+     * @param displayName   The display name.
+     * @throws AuthenticatorMgtClientException   if the display name is not valid.
      */
-    public void validateForBlank(String fieldName, String fieldValue) throws AuthenticatorMgtClientException {
+    public void validateDisplayName(String displayName) throws AuthenticatorMgtClientException {
 
-        if (StringUtils.isBlank(fieldValue)) {
-            throw buildClientException(AuthenticatorMgtError.ERROR_BLANK_FIELD_VALUE, fieldName);
+        boolean isValidDisplayName = disaplayNameRegexPattern.matcher(displayName).matches();
+        if (!isValidDisplayName) {
+            throw buildClientException(AuthenticatorMgtError.ERROR_INVALID_DISPLAY_NAME,
+                    displayName, DISPLAY_NAME_REGEX);
         }
     }
 

components/application-mgt/org.wso2.carbon.identity.application.common/src/test/java/org/wso2/carbon/identity/application/common/model/test/ApplicationAuthenticatorServiceTest.java

@@ -86,10 +86,10 @@ public class ApplicationAuthenticatorServiceTest {
     private static EndpointConfig endpointConfig;
     private static EndpointConfig endpointConfigToBeUpdated;
 
-    private static final String AUTHENTICATOR1_NAME = "auth1";
-    private static final String AUTHENTICATOR2_NAME = "auth2";
-    private static final String AUTHENTICATOR_CONFIG_FOR_EXCEPTION_NAME = "exception_auth";
-    private static final String NON_EXIST_AUTHENTICATOR_NAME = "non_exist_auth";
+    private static final String AUTHENTICATOR1_NAME = "custom_auth1";
+    private static final String AUTHENTICATOR2_NAME = "custom_auth2";
+    private static final String AUTHENTICATOR_CONFIG_FOR_EXCEPTION_NAME = "custom_exception_auth";
+    private static final String NON_EXIST_AUTHENTICATOR_NAME = "custom_non_exist_auth";
     private static final String SYSTEM_AUTHENTICATOR_NAME = "system_auth";
 
     @BeforeClass
@@ -169,11 +169,11 @@ public void testCreateUserDefinedLocalAuthenticatorWithExistingAuthenticator(
     }
 
     @Test(priority = 3, expectedExceptions = AuthenticatorMgtException.class,
-            expectedExceptionsMessageRegExp = "Invalid empty or blank value.")
+            expectedExceptionsMessageRegExp = "Authenticator display name is invalid.")
     public void testCreateUserDefinedLocalAuthenticatorWithBlankDisplayName() throws AuthenticatorMgtException {
 
-        UserDefinedLocalAuthenticatorConfig config = createUserDefinedAuthenticatorConfig("withBlankDisplayName",
-                AuthenticationType.IDENTIFICATION);
+        UserDefinedLocalAuthenticatorConfig config = createUserDefinedAuthenticatorConfig(
+                "custom_withBlankDisplayName", AuthenticationType.IDENTIFICATION);
         config.setDisplayName("");
         ApplicationCommonServiceDataHolder.getInstance().getApplicationAuthenticatorService()
                 .addUserDefinedLocalAuthenticator(config, tenantDomain);
@@ -347,6 +347,22 @@ public void testGetUserDefinedLocalAuthenticator(UserDefinedLocalAuthenticatorCo
     }
 
     @Test(priority = 16)
+    public void testIsExistingAuthenticatorName() throws AuthenticatorMgtException {
+
+        Assert.assertTrue(ApplicationCommonServiceDataHolder.getInstance().
+                getApplicationAuthenticatorService().isExistingAuthenticatorName(
+                authenticatorConfig1.getName(), tenantDomain));
+    }
+
+    @Test(priority = 17)
+    public void testIsExistingAuthenticatorNameForNonExistName() throws AuthenticatorMgtException {
+
+        Assert.assertFalse(ApplicationCommonServiceDataHolder.getInstance().
+                getApplicationAuthenticatorService().isExistingAuthenticatorName(
+                NON_EXIST_AUTHENTICATOR_NAME, tenantDomain));
+    }
+
+    @Test(priority = 18)
     public void testDeleteUserDefinedLocalAuthenticatorWithActionException() throws Exception {
 
         ActionManagementService actionManagementServiceForException = mock(ActionManagementService.class);
@@ -364,7 +380,7 @@ public void testDeleteUserDefinedLocalAuthenticatorWithActionException() throws
                         authenticatorConfigForException.getName(), tenantDomain));
     }
 
-    @Test(priority = 17, dataProvider = "authenticatorConfigToModify")
+    @Test(priority = 19, dataProvider = "authenticatorConfigToModify")
     public void testDeleteUserDefinedLocalAuthenticator(UserDefinedLocalAuthenticatorConfig config)
             throws AuthenticatorMgtException {
 
@@ -374,7 +390,7 @@ public void testDeleteUserDefinedLocalAuthenticator(UserDefinedLocalAuthenticato
                 .getLocalAuthenticatorByName(config.getName()));
     }
 
-    @Test(priority = 18)
+    @Test(priority = 20)
     public void testDeleteUserDefinedLocalAuthenticatorWithNonExistingAuthenticator() throws AuthenticatorMgtException {
 
         // Assert that no exception is thrown when trying to delete a non-existing authenticator.

components/application-mgt/org.wso2.carbon.identity.application.common/src/test/java/org/wso2/carbon/identity/application/common/model/test/dao/AuthenticatorManagementDAOImplTest.java

@@ -50,8 +50,8 @@ public class AuthenticatorManagementDAOImplTest {
     private UserDefinedLocalAuthenticatorConfig authenticatorForUpdate;
     private UserDefinedLocalAuthenticatorConfig authenticatorForUpdateForException;
 
-    private static final String AUTHENTICATOR1_NAME = "auth1";
-    private static final String AUTHENTICATOR2_NAME = "auth2";
+    private static final String AUTHENTICATOR1_NAME = "custom_auth1";
+    private static final String AUTHENTICATOR2_NAME = "custom_auth2";
     private static final String AUTHENTICATOR_CONFIG_FOR_EXCEPTION_NAME = "exception_auth";
     private static final String NON_EXIST_AUTHENTICATOR_NAME = "non_exist_auth";
 
@@ -166,7 +166,21 @@ public void testGetUserDefinedLocalAuthenticatorForNonExist() throws Authenticat
                 NON_EXIST_AUTHENTICATOR_NAME, tenantId));
     }
 
-    @Test(dataProvider = "authenticatorConfig", priority = 9)
+    @Test(priority = 9)
+    public void testIsExistingAuthenticatorName() throws AuthenticatorMgtException {
+
+        Assert.assertTrue(authenticatorManagementDAO.isExistingAuthenticatorName(
+                authenticatorConfig1.getName(), tenantId));
+    }
+
+    @Test(priority = 10)
+    public void testIsExistingAuthenticatorNameForNonExistName() throws AuthenticatorMgtException {
+
+        Assert.assertFalse(authenticatorManagementDAO.isExistingAuthenticatorName(
+                NON_EXIST_AUTHENTICATOR_NAME, tenantId));
+    }
+
+    @Test(dataProvider = "authenticatorConfig", priority = 11)
     public void testDeleteUserDefinedLocalAuthenticator(UserDefinedLocalAuthenticatorConfig config)
             throws AuthenticatorMgtException {
 

components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java

@@ -26,6 +26,7 @@
 import org.apache.commons.logging.LogFactory;
 import org.wso2.carbon.identity.application.common.ApplicationAuthenticatorService;
 import org.wso2.carbon.identity.application.common.ProvisioningConnectorService;
+import org.wso2.carbon.identity.application.common.exception.AuthenticatorMgtException;
 import org.wso2.carbon.identity.application.common.model.ClaimConfig;
 import org.wso2.carbon.identity.application.common.model.ClaimMapping;
 import org.wso2.carbon.identity.application.common.model.FederatedAuthenticatorConfig;
@@ -2234,9 +2235,15 @@ private void validateFederatedAuthenticatorConfigName(FederatedAuthenticatorConf
                 }
             } else {
                 // Check if the given authenticator name is already taken.
-                if (getFederatedAuthenticatorByName(config.getName(), tenantDomain) != null) {
-                    throw IdPManagementUtil.handleClientException(IdPManagementConstants.ErrorMessage
-                            .ERROR_CODE_AUTHENTICATOR_NAME_ALREADY_TAKEN, config.getName());
+                try {
+                    if (ApplicationAuthenticatorService.getInstance()
+                            .isExistingAuthenticatorName(config.getName(), tenantDomain)) {
+                        throw IdPManagementUtil.handleClientException(IdPManagementConstants.ErrorMessage
+                                .ERROR_CODE_AUTHENTICATOR_NAME_ALREADY_TAKEN, config.getName());
+                    }
+                } catch (AuthenticatorMgtException e) {
+                    throw IdPManagementUtil.handleServerException(IdPManagementConstants.ErrorMessage
+                            .ERROR_CODE_ADDING_FEDERATED_AUTHENTICATOR, config.getName());
                 }
                 // Check if the given authenticator name matches the regex pattern.
                 if (!userDefinedAuthNameRegexPattern.matcher(config.getName()).matches()) {