From d8a7cedde6c54a658b9eda94fca912cee30188cb Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Wed, 20 Apr 2022 17:07:17 -0700 Subject: [PATCH] Clean up ClientSecurityUtil class to fix sonar flagged issues and code duplication (#4301) --- .../java/org/openmetadata/catalog/Entity.java | 1 + .../IngestionPipelineResource.java | 10 +- .../openmetadata/catalog/util/EntityUtil.java | 5 +- .../util/OpenMetadataClientSecurityUtil.java | 201 ++++-------------- 4 files changed, 54 insertions(+), 163 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java index e68a6f64aff..657b028f98d 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/Entity.java +++ b/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 diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/services/ingestionpipelines/IngestionPipelineResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/services/ingestionpipelines/IngestionPipelineResource.java index 4af55adcab3..641683526fd 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/services/ingestionpipelines/IngestionPipelineResource.java +++ b/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 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 response = airflowRESTClient.testConnection(testServiceConnection); return Response.status(200, response.body()).build(); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java index fff6a051632..42f841abc36 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java +++ b/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 compareEntityReference = - Comparator.comparing(entityReference -> entityReference.getName()); + Comparator.comparing(EntityReference::getName); public static final Comparator compareVersion = Comparator.comparing(EntityVersionPair::getVersion); public static final Comparator compareTagLabel = Comparator.comparing(TagLabel::getTagFQN); @@ -76,8 +76,7 @@ public final class EntityUtil { public static final Comparator compareTableConstraint = Comparator.comparing(TableConstraint::getConstraintType); public static final Comparator compareChangeEvent = Comparator.comparing(ChangeEvent::getTimestamp); - public static final Comparator compareGlossaryTerm = - Comparator.comparing(glossaryTerm -> glossaryTerm.getName()); + public static final Comparator compareGlossaryTerm = Comparator.comparing(GlossaryTerm::getName); // // Matchers used for matching two items in a list diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/OpenMetadataClientSecurityUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/OpenMetadataClientSecurityUtil.java index 8803bd96a43..b9e6953856f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/OpenMetadataClientSecurityUtil.java +++ b/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 = - new GoogleSSOClientConfig() - .withSecretKey(authConfig.getGoogle().getSecretKey()) - .withAudience(authConfig.getGoogle().getAudience()); + GoogleSSOClientConfig googleSSOClientConfig = authConfig.getGoogle(); + checkAuthConfig(googleSSOClientConfig, authProvider); + checkRequiredField(SECRET_KEY, googleSSOClientConfig.getSecretKey(), authProvider); openMetadataServerConnection.setSecurityConfig(googleSSOClientConfig); break; case AUTH_0: - validateAuthConfigs(authConfig, authProvider); - Auth0SSOClientConfig auth0SSOClientConfig = - new Auth0SSOClientConfig() - .withClientId(authConfig.getAuth0().getClientId()) - .withSecretKey(authConfig.getAuth0().getSecretKey()) - .withDomain(authConfig.getAuth0().getDomain()); + Auth0SSOClientConfig auth0SSOClientConfig = authConfig.getAuth0(); + checkAuthConfig(auth0SSOClientConfig, authProvider); + checkRequiredField(CLIENT_ID, auth0SSOClientConfig.getClientId(), authProvider); + checkRequiredField(SECRET_KEY, auth0SSOClientConfig.getSecretKey(), authProvider); + checkRequiredField(DOMAIN, auth0SSOClientConfig.getDomain(), authProvider); openMetadataServerConnection.setSecurityConfig(auth0SSOClientConfig); break; case OKTA: - validateAuthConfigs(authConfig, authProvider); - OktaSSOClientConfig oktaSSOClientConfig = - new OktaSSOClientConfig() - .withClientId(authConfig.getOkta().getClientId()) - .withEmail(authConfig.getOkta().getEmail()) - .withOrgURL(authConfig.getOkta().getOrgURL()) - .withPrivateKey(authConfig.getOkta().getPrivateKey()); - - List oktaScopesList = authConfig.getOkta().getScopes(); - if (!oktaScopesList.isEmpty()) { - oktaSSOClientConfig.setScopes(oktaScopesList); - } - + OktaSSOClientConfig oktaSSOClientConfig = authConfig.getOkta(); + checkAuthConfig(oktaSSOClientConfig, authProvider); + checkRequiredField(CLIENT_ID, oktaSSOClientConfig.getClientId(), authProvider); + checkRequiredField("privateKey", oktaSSOClientConfig.getPrivateKey(), authProvider); + checkRequiredField(EMAIL, oktaSSOClientConfig.getEmail(), authProvider); + checkRequiredField("orgUrl", oktaSSOClientConfig.getOrgURL(), authProvider); openMetadataServerConnection.setSecurityConfig(oktaSSOClientConfig); break; case AZURE: - validateAuthConfigs(authConfig, authProvider); - AzureSSOClientConfig azureSSOClientConfig = - new AzureSSOClientConfig() - .withClientId(authConfig.getAzure().getClientId()) - .withClientSecret(authConfig.getAzure().getClientSecret()) - .withAuthority(authConfig.getAzure().getAuthority()); - List scopesList = authConfig.getAzure().getScopes(); - if (!scopesList.isEmpty()) { - azureSSOClientConfig.setScopes(scopesList); - } - + AzureSSOClientConfig azureSSOClientConfig = authConfig.getAzure(); + checkAuthConfig(azureSSOClientConfig, authProvider); + checkRequiredField(CLIENT_ID, azureSSOClientConfig.getClientId(), authProvider); + checkRequiredField(CLIENT_SECRET, azureSSOClientConfig.getClientSecret(), authProvider); + checkRequiredField(AUTHORITY, azureSSOClientConfig.getAuthority(), authProvider); + checkRequiredField(SCOPES, azureSSOClientConfig.getScopes(), authProvider); openMetadataServerConnection.setSecurityConfig(azureSSOClientConfig); break; case CUSTOM_OIDC: - validateAuthConfigs(authConfig, authProvider); - CustomOIDCSSOClientConfig customOIDCSSOClientConfig = - new CustomOIDCSSOClientConfig() - .withClientId(authConfig.getCustomOidc().getClientId()) - .withSecretKey(authConfig.getCustomOidc().getSecretKey()) - .withTokenEndpoint(authConfig.getCustomOidc().getTokenEndpoint()); + CustomOIDCSSOClientConfig customOIDCSSOClientConfig = authConfig.getCustomOidc(); + checkAuthConfig(customOIDCSSOClientConfig, authProvider); + checkRequiredField(CLIENT_ID, customOIDCSSOClientConfig.getClientId(), authProvider); + checkRequiredField(SECRET_KEY, customOIDCSSOClientConfig.getSecretKey(), authProvider); + checkRequiredField("tokenEndpoint", customOIDCSSOClientConfig.getTokenEndpoint(), authProvider); 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) - throws OpenMetadataClientSecurityConfigException { + public static void checkAuthConfig(Object authConfig, AuthProvider authProvider) { if (authConfig == null) { throw new OpenMetadataClientSecurityConfigException( - String.format("%s SSO client config requires authConfig section", authProvider)); + String.format("%s SSO client config requires authConfig.%s section", authProvider, 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: - throw new IllegalStateException("Unexpected value: " + authProvider); + public static void checkRequiredField(String fieldName, String fieldValue, AuthProvider 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 getSecurityScopes(String scopes) { - if (scopes != null && !scopes.isEmpty()) { - return Arrays.asList(scopes.split("\\s*,\\s*")); - } - return Collections.emptyList(); + return scopes != null && !scopes.isEmpty() ? Arrays.asList(scopes.split("\\s*,\\s*")) : Collections.emptyList(); } }