Clean up ClientSecurityUtil class to fix sonar flagged issues and code duplication (#4301)

catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java

@@ -62,6 +62,7 @@ public final class Entity {
   public static final String FIELD_FOLLOWERS = "followers";
   public static final String FIELD_TAGS = "tags";
   public static final String FIELD_DELETED = "deleted";
+  public static final String FIELD_PIPELINE_STATUSES = "pipelineStatuses";
 
   //
   // Services

catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/services/ingestionpipelines/IngestionPipelineResource.java

@@ -14,6 +14,7 @@
 package org.openmetadata.catalog.resources.services.ingestionpipelines;
 
 import static org.openmetadata.catalog.Entity.FIELD_OWNER;
+import static org.openmetadata.catalog.Entity.FIELD_PIPELINE_STATUSES;
 import static org.openmetadata.catalog.security.SecurityUtil.ADMIN;
 import static org.openmetadata.catalog.security.SecurityUtil.BOT;
 import static org.openmetadata.catalog.security.SecurityUtil.OWNER;
@@ -169,7 +170,7 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
     ListFilter filter = new ListFilter(include).addQueryParam("service", serviceParam);
     ResultList<IngestionPipeline> ingestionPipelines =
         super.listInternal(uriInfo, securityContext, fieldsParam, filter, limitParam, before, after);
-    if (fieldsParam != null && fieldsParam.contains("pipelineStatuses")) {
+    if (fieldsParam != null && fieldsParam.contains(FIELD_PIPELINE_STATUSES)) {
       addStatus(ingestionPipelines.getData());
     }
     return ingestionPipelines;
@@ -226,7 +227,7 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
           Include include)
       throws IOException {
     IngestionPipeline ingestionPipeline = getInternal(uriInfo, securityContext, id, fieldsParam, include);
-    if (fieldsParam != null && fieldsParam.contains("pipelineStatuses")) {
+    if (fieldsParam != null && fieldsParam.contains(FIELD_PIPELINE_STATUSES)) {
       ingestionPipeline = addStatus(ingestionPipeline);
     }
     return ingestionPipeline;
@@ -292,7 +293,7 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
           Include include)
       throws IOException {
     IngestionPipeline ingestionPipeline = getByNameInternal(uriInfo, securityContext, fqn, fieldsParam, include);
-    if (fieldsParam != null && fieldsParam.contains("pipelineStatuses")) {
+    if (fieldsParam != null && fieldsParam.contains(FIELD_PIPELINE_STATUSES)) {
       ingestionPipeline = addStatus(ingestionPipeline);
     }
     return ingestionPipeline;
@@ -409,8 +410,7 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
   public Response testIngestion(
       @Context UriInfo uriInfo,
       @Context SecurityContext securityContext,
-      @Valid TestServiceConnection testServiceConnection)
+      @Valid TestServiceConnection testServiceConnection) {
-      throws IOException {
     HttpResponse<String> response = airflowRESTClient.testConnection(testServiceConnection);
     return Response.status(200, response.body()).build();
   }

catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java

@@ -68,7 +68,7 @@ public final class EntityUtil {
 
   // Note ordering is same as server side ordering by ID as string to ensure PATCH operations work
   public static final Comparator<EntityReference> compareEntityReference =
-      Comparator.comparing(entityReference -> entityReference.getName());
+      Comparator.comparing(EntityReference::getName);
   public static final Comparator<EntityVersionPair> compareVersion =
       Comparator.comparing(EntityVersionPair::getVersion);
   public static final Comparator<TagLabel> compareTagLabel = Comparator.comparing(TagLabel::getTagFQN);
@@ -76,8 +76,7 @@ public final class EntityUtil {
   public static final Comparator<TableConstraint> compareTableConstraint =
       Comparator.comparing(TableConstraint::getConstraintType);
   public static final Comparator<ChangeEvent> compareChangeEvent = Comparator.comparing(ChangeEvent::getTimestamp);
-  public static final Comparator<GlossaryTerm> compareGlossaryTerm =
+  public static final Comparator<GlossaryTerm> compareGlossaryTerm = Comparator.comparing(GlossaryTerm::getName);
-      Comparator.comparing(glossaryTerm -> glossaryTerm.getName());
 
   //
   // Matchers used for matching two items in a list

catalog-rest-service/src/main/java/org/openmetadata/catalog/util/OpenMetadataClientSecurityUtil.java

@@ -17,12 +17,14 @@ import org.openmetadata.catalog.services.connections.metadata.OpenMetadataServer
 
 @Slf4j
 public final class OpenMetadataClientSecurityUtil {
+  public static final String CLIENT_ID = "clientId";
   public static final String AUDIENCE = "audience";
   public static final String DOMAIN = "domain";
   public static final String EMAIL = "email";
   public static final String SCOPES = "scopes";
   public static final String AUTHORITY = "authority";
   public static final String CLIENT_SECRET = "clientSecret";
+  public static final String SECRET_KEY = "secretKey";
 
   private OpenMetadataClientSecurityUtil() {
     /* Utility class with private constructor */
@@ -34,61 +36,49 @@ public final class OpenMetadataClientSecurityUtil {
     AuthConfiguration authConfig = airflowConfiguration.getAuthConfig();
     OpenMetadataServerConnection openMetadataServerConnection = new OpenMetadataServerConnection();
     openMetadataServerConnection.setAuthProvider(authProvider);
+    if (authProvider != AuthProvider.NO_AUTH && authConfig == null) {
+      throw new OpenMetadataClientSecurityConfigException(
+          String.format("%s SSO client config requires authConfig section", authProvider));
+    }
     switch (authProvider) {
       case GOOGLE:
-        validateAuthConfigs(authConfig, authProvider);
+        GoogleSSOClientConfig googleSSOClientConfig = authConfig.getGoogle();
-        GoogleSSOClientConfig googleSSOClientConfig =
+        checkAuthConfig(googleSSOClientConfig, authProvider);
-            new GoogleSSOClientConfig()
+        checkRequiredField(SECRET_KEY, googleSSOClientConfig.getSecretKey(), authProvider);
-                .withSecretKey(authConfig.getGoogle().getSecretKey())
-                .withAudience(authConfig.getGoogle().getAudience());
         openMetadataServerConnection.setSecurityConfig(googleSSOClientConfig);
         break;
       case AUTH_0:
-        validateAuthConfigs(authConfig, authProvider);
+        Auth0SSOClientConfig auth0SSOClientConfig = authConfig.getAuth0();
-        Auth0SSOClientConfig auth0SSOClientConfig =
+        checkAuthConfig(auth0SSOClientConfig, authProvider);
-            new Auth0SSOClientConfig()
+        checkRequiredField(CLIENT_ID, auth0SSOClientConfig.getClientId(), authProvider);
-                .withClientId(authConfig.getAuth0().getClientId())
+        checkRequiredField(SECRET_KEY, auth0SSOClientConfig.getSecretKey(), authProvider);
-                .withSecretKey(authConfig.getAuth0().getSecretKey())
+        checkRequiredField(DOMAIN, auth0SSOClientConfig.getDomain(), authProvider);
-                .withDomain(authConfig.getAuth0().getDomain());
         openMetadataServerConnection.setSecurityConfig(auth0SSOClientConfig);
         break;
       case OKTA:
-        validateAuthConfigs(authConfig, authProvider);
+        OktaSSOClientConfig oktaSSOClientConfig = authConfig.getOkta();
-        OktaSSOClientConfig oktaSSOClientConfig =
+        checkAuthConfig(oktaSSOClientConfig, authProvider);
-            new OktaSSOClientConfig()
+        checkRequiredField(CLIENT_ID, oktaSSOClientConfig.getClientId(), authProvider);
-                .withClientId(authConfig.getOkta().getClientId())
+        checkRequiredField("privateKey", oktaSSOClientConfig.getPrivateKey(), authProvider);
-                .withEmail(authConfig.getOkta().getEmail())
+        checkRequiredField(EMAIL, oktaSSOClientConfig.getEmail(), authProvider);
-                .withOrgURL(authConfig.getOkta().getOrgURL())
+        checkRequiredField("orgUrl", oktaSSOClientConfig.getOrgURL(), authProvider);
-                .withPrivateKey(authConfig.getOkta().getPrivateKey());
-
-        List<String> oktaScopesList = authConfig.getOkta().getScopes();
-        if (!oktaScopesList.isEmpty()) {
-          oktaSSOClientConfig.setScopes(oktaScopesList);
-        }
-
         openMetadataServerConnection.setSecurityConfig(oktaSSOClientConfig);
         break;
       case AZURE:
-        validateAuthConfigs(authConfig, authProvider);
+        AzureSSOClientConfig azureSSOClientConfig = authConfig.getAzure();
-        AzureSSOClientConfig azureSSOClientConfig =
+        checkAuthConfig(azureSSOClientConfig, authProvider);
-            new AzureSSOClientConfig()
+        checkRequiredField(CLIENT_ID, azureSSOClientConfig.getClientId(), authProvider);
-                .withClientId(authConfig.getAzure().getClientId())
+        checkRequiredField(CLIENT_SECRET, azureSSOClientConfig.getClientSecret(), authProvider);
-                .withClientSecret(authConfig.getAzure().getClientSecret())
+        checkRequiredField(AUTHORITY, azureSSOClientConfig.getAuthority(), authProvider);
-                .withAuthority(authConfig.getAzure().getAuthority());
+        checkRequiredField(SCOPES, azureSSOClientConfig.getScopes(), authProvider);
-        List<String> scopesList = authConfig.getAzure().getScopes();
-        if (!scopesList.isEmpty()) {
-          azureSSOClientConfig.setScopes(scopesList);
-        }
-
         openMetadataServerConnection.setSecurityConfig(azureSSOClientConfig);
         break;
       case CUSTOM_OIDC:
-        validateAuthConfigs(authConfig, authProvider);
+        CustomOIDCSSOClientConfig customOIDCSSOClientConfig = authConfig.getCustomOidc();
-        CustomOIDCSSOClientConfig customOIDCSSOClientConfig =
+        checkAuthConfig(customOIDCSSOClientConfig, authProvider);
-            new CustomOIDCSSOClientConfig()
+        checkRequiredField(CLIENT_ID, customOIDCSSOClientConfig.getClientId(), authProvider);
-                .withClientId(authConfig.getCustomOidc().getClientId())
+        checkRequiredField(SECRET_KEY, customOIDCSSOClientConfig.getSecretKey(), authProvider);
-                .withSecretKey(authConfig.getCustomOidc().getSecretKey())
+        checkRequiredField("tokenEndpoint", customOIDCSSOClientConfig.getTokenEndpoint(), authProvider);
-                .withTokenEndpoint(authConfig.getCustomOidc().getTokenEndpoint());
         openMetadataServerConnection.setSecurityConfig(customOIDCSSOClientConfig);
         break;
       case NO_AUTH:
@@ -100,127 +90,28 @@ public final class OpenMetadataClientSecurityUtil {
     return openMetadataServerConnection;
   }
 
-  public static void validateAuthConfigs(AuthConfiguration authConfig, AuthProvider authProvider)
+  public static void checkAuthConfig(Object authConfig, AuthProvider authProvider) {
-      throws OpenMetadataClientSecurityConfigException {
     if (authConfig == null) {
-      throw new OpenMetadataClientSecurityConfigException(
-          String.format("%s SSO client config requires authConfig section", authProvider));
-    }
-    switch (authProvider) {
-      case NO_AUTH:
-        // No auth doesn't require auth configs
-        break;
-      case AZURE:
-        if (authConfig.getAzure() == null) {
       throw new OpenMetadataClientSecurityConfigException(
           String.format("%s SSO client config requires authConfig.%s section", authProvider, authProvider));
-        } else {
-          AzureSSOClientConfig azureSSOClientConfig = authConfig.getAzure();
-          if (azureSSOClientConfig.getClientId() == null || azureSSOClientConfig.getClientId().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires clientId", authProvider));
-          }
-          if (azureSSOClientConfig.getClientSecret() == null || azureSSOClientConfig.getClientSecret().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires clientSecret", authProvider));
-          }
-          if (azureSSOClientConfig.getAuthority() == null || azureSSOClientConfig.getAuthority().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires authority", authProvider));
-          }
-          if (azureSSOClientConfig.getScopes() == null || azureSSOClientConfig.getScopes().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires scopes", authProvider));
     }
   }
-        break;
-      case GOOGLE:
-        if (authConfig.getGoogle() == null) {
-          throw new OpenMetadataClientSecurityConfigException(
-              String.format("%s SSO client config requires authConfig.%s section", authProvider, authProvider));
-        } else {
-          GoogleSSOClientConfig googleSSOClientConfig = authConfig.getGoogle();
-          if (googleSSOClientConfig.getSecretKey() == null || googleSSOClientConfig.getSecretKey().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires secretKey", authProvider));
-          }
-        }
-        break;
-      case OKTA:
-        if (authConfig.getOkta() == null) {
-          throw new OpenMetadataClientSecurityConfigException(
-              String.format("%s SSO client config requires authConfig.%s section", authProvider, authProvider));
-        } else {
-          OktaSSOClientConfig oktaSSOClientConfig = authConfig.getOkta();
-          if (oktaSSOClientConfig.getClientId() == null || oktaSSOClientConfig.getClientId().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires clientId", authProvider));
-          }
-          if (oktaSSOClientConfig.getPrivateKey() == null || oktaSSOClientConfig.getPrivateKey().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires privateKey", authProvider));
-          }
-          if (oktaSSOClientConfig.getEmail() == null || oktaSSOClientConfig.getEmail().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires email", authProvider));
-          }
-          if (oktaSSOClientConfig.getOrgURL() == null || oktaSSOClientConfig.getOrgURL().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires orgUrl", authProvider));
-          }
-        }
-        break;
-      case AUTH_0:
-        if (authConfig.getAuth0() == null) {
-          throw new OpenMetadataClientSecurityConfigException(
-              String.format("%s SSO client config requires authConfig.%s section", authProvider, authProvider));
-        } else {
-          Auth0SSOClientConfig auth0SSOClientConfig = authConfig.getAuth0();
-          if (auth0SSOClientConfig.getClientId() == null || auth0SSOClientConfig.getClientId().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires clientId", authProvider));
-          }
-          if (auth0SSOClientConfig.getSecretKey() == null || auth0SSOClientConfig.getSecretKey().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires secretKey", authProvider));
-          }
-          if (auth0SSOClientConfig.getDomain() == null || auth0SSOClientConfig.getDomain().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires domain", authProvider));
-          }
-        }
-        break;
-      case CUSTOM_OIDC:
-        if (authConfig.getCustomOidc() == null) {
-          throw new OpenMetadataClientSecurityConfigException(
-              String.format("%s SSO client config requires authConfig.%s section", authProvider, authProvider));
-        } else {
-          CustomOIDCSSOClientConfig customOIDCSSOClientConfig = authConfig.getCustomOidc();
-          if (customOIDCSSOClientConfig.getClientId() == null || customOIDCSSOClientConfig.getClientId().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires clientId", authProvider));
-          }
-          if (customOIDCSSOClientConfig.getSecretKey() == null || customOIDCSSOClientConfig.getSecretKey().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires secretKey", authProvider));
-          }
-          if (customOIDCSSOClientConfig.getTokenEndpoint() == null
-              || customOIDCSSOClientConfig.getTokenEndpoint().isEmpty()) {
-            throw new OpenMetadataClientSecurityConfigException(
-                String.format("%s SSO client config requires tokenEndpoint", authProvider));
-          }
-        }
-        break;
 
-      default:
+  public static void checkRequiredField(String fieldName, String fieldValue, AuthProvider authProvider) {
-        throw new IllegalStateException("Unexpected value: " + authProvider);
+    if (fieldValue == null || fieldValue.isEmpty()) {
+      throw new OpenMetadataClientSecurityConfigException(
+          String.format("%s SSO client config requires %s", authProvider, fieldName));
+    }
+  }
+
+  public static void checkRequiredField(String fieldName, List<?> fieldValue, AuthProvider authProvider) {
+    if (fieldValue == null || fieldValue.isEmpty()) {
+      throw new OpenMetadataClientSecurityConfigException(
+          String.format("%s SSO client config requires %s", authProvider, fieldName));
     }
   }
 
   public static List<String> getSecurityScopes(String scopes) {
-    if (scopes != null && !scopes.isEmpty()) {
+    return scopes != null && !scopes.isEmpty() ? Arrays.asList(scopes.split("\\s*,\\s*")) : Collections.emptyList();
-      return Arrays.asList(scopes.split("\\s*,\\s*"));
-    }
-    return Collections.emptyList();
   }
 }