From b27bdcd68081ad8a9d72cd80d5770199c49f5ac2 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 2 Nov 2023 21:02:35 +0530 Subject: [PATCH 01/13] PLT-302 Optimize Persona ES alias --- .../store/aliasstore/ESAliasStore.java | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 2b01f293993..67dde2e1708 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -38,6 +38,7 @@ import javax.inject.Inject; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -165,6 +166,7 @@ private Map getFilterForPurpose(AtlasEntity purpose) throws Atla private void personaPolicyToESDslClauses(List policies, List> allowClauseList) throws AtlasBaseException { + Set terms = new HashSet<>(); for (AtlasEntity policy: policies) { if (policy.getStatus() == null || AtlasEntity.Status.ACTIVE.equals(policy.getStatus())) { @@ -178,19 +180,23 @@ private void personaPolicyToESDslClauses(List policies, } for (String asset : assets) { - addPersonaMetadataFilterClauses(asset, allowClauseList); + terms.add(asset); + allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); } - addPersonaMetadataFilterConnectionClause(connectionQName, allowClauseList); + terms.add(connectionQName); } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { for (String glossaryQName : assets) { - addPersonaGlossaryFilterClauses(glossaryQName, allowClauseList); + terms.add(glossaryQName); + allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + glossaryQName))); } } } } } + + allowClauseList.add(mapOf("terms", mapOf(QUALIFIED_NAME, terms))); } private Map esClausesToFilter(List> allowClauseList) { @@ -208,22 +214,6 @@ private String getAliasName(AtlasEntity entity) { return getESAliasName(entity); } - private void addPersonaMetadataFilterClauses(String asset, List> clauseList) { - clauseList.add(mapOf("term", mapOf(QUALIFIED_NAME, asset))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset))); - } - - private void addPersonaGlossaryFilterClauses(String asset, List> clauseList) { - clauseList.add(mapOf("term", mapOf(QUALIFIED_NAME, asset))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); - clauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + asset))); - } - - private void addPersonaMetadataFilterConnectionClause(String connection, List> clauseList) { - clauseList.add(mapOf("term", mapOf(QUALIFIED_NAME, connection))); - } - private void addPurposeMetadataFilterClauses(List tags, List> clauseList) { clauseList.add(mapOf("terms", mapOf(TRAIT_NAMES_PROPERTY_KEY, tags))); clauseList.add(mapOf("terms", mapOf(PROPAGATED_TRAIT_NAMES_PROPERTY_KEY, tags))); From c07cb54332098be15114be899c5aa72578673984 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Fri, 3 Nov 2023 12:36:59 +0530 Subject: [PATCH 02/13] Remove redundant logging --- .../java/org/apache/atlas/plugin/util/KeycloakUserStore.java | 2 -- .../atlas/policytransformer/CachePolicyTransformerImpl.java | 4 +--- .../apache/atlas/repository/audit/ESBasedAuditRepository.java | 1 - .../main/java/org/apache/atlas/tasks/TaskQueueWatcher.java | 3 +-- webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java | 1 - 5 files changed, 2 insertions(+), 9 deletions(-) diff --git a/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java b/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java index 06fd614673a..178b4f88d32 100644 --- a/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java +++ b/auth-agents-common/src/main/java/org/apache/atlas/plugin/util/KeycloakUserStore.java @@ -151,7 +151,6 @@ public RangerRoles loadRolesIfUpdated(long lastUpdatedTime) throws AtlasBaseExce boolean isKeycloakUpdated = isKeycloakSubjectsStoreUpdated(lastUpdatedTime); if (!isKeycloakUpdated) { - LOG.info("loadRolesIfUpdated: Skipping as no update found"); return null; } @@ -268,7 +267,6 @@ public RangerUserStore loadUserStoreIfUpdated(long lastUpdatedTime) throws Atlas boolean isKeycloakUpdated = isKeycloakSubjectsStoreUpdated(lastUpdatedTime); if (!isKeycloakUpdated) { - LOG.info("loadUserStoreIfUpdated: Skipping as no update found"); return null; } diff --git a/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java b/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java index 502412f8042..4a81129a3c6 100644 --- a/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java +++ b/auth-agents-common/src/main/java/org/apache/atlas/policytransformer/CachePolicyTransformerImpl.java @@ -188,9 +188,7 @@ public ServicePolicies getPolicies(String serviceName, String pluginId, Long las RequestContext.get().endMetricRecord(recorderFilterPolicies); - if (LOG.isDebugEnabled()) { - LOG.debug("Found {} policies", servicePolicies.getPolicies().size()); - } + LOG.info("Found {} policies", servicePolicies.getPolicies().size()); } } catch (Exception e) { diff --git a/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java b/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java index 3de1728a7a7..cbab135606d 100644 --- a/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java +++ b/repository/src/main/java/org/apache/atlas/repository/audit/ESBasedAuditRepository.java @@ -206,7 +206,6 @@ public List listEventsV2(String entityId, EntityAuditEventV2 @Override public EntityAuditSearchResult searchEvents(String queryString) throws AtlasBaseException { - LOG.info("Hitting ES query to fetch audits: {}", queryString); try { String response = performSearchOnIndex(queryString); return getResultFromResponse(response); diff --git a/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java b/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java index 1d8762d3285..25478e9a5ad 100644 --- a/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java +++ b/repository/src/main/java/org/apache/atlas/tasks/TaskQueueWatcher.java @@ -98,9 +98,9 @@ public void run() { if (CollectionUtils.isNotEmpty(tasks)) { final CountDownLatch latch = new CountDownLatch(tasks.size()); submitAll(tasks, latch); + LOG.info("Submitted {} tasks to the queue", tasks.size()); waitForTasksToComplete(latch); } else { - LOG.info("No tasks to queue, sleeping for {} ms", pollInterval); redisService.releaseDistributedLock(ATLAS_TASK_LOCK); } Thread.sleep(pollInterval); @@ -153,7 +153,6 @@ public void run() { if (LOG.isDebugEnabled()) { LOG.debug("TasksFetcher: Fetching tasks for queuing"); } - LOG.info("TasksFetcher: Fetching tasks for queuing"); this.tasks = registry.getTasksForReQueue(); } diff --git a/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java b/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java index ac23824c3c6..c5868cbfa30 100644 --- a/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java +++ b/webapp/src/main/java/org/apache/atlas/web/rest/AuthREST.java @@ -209,7 +209,6 @@ private boolean isPolicyUpdated(String serviceName, long lastUpdatedTime) { EntityAuditSearchResult result = auditRepository.searchEvents(parameters.getQueryString()); if (result == null || CollectionUtils.isEmpty(result.getEntityAudits())) { - LOG.info("getPoliciesIfUpdated: Skipping as no update found"); return false; } } catch (AtlasBaseException e) { From 0f1e3aca74f7a61cda3818522671afccd40b9df9 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Wed, 8 Nov 2023 15:45:33 +0530 Subject: [PATCH 03/13] PLT-302 Limit Persona policies assets --- .../main/java/org/apache/atlas/AtlasConfiguration.java | 4 +++- .../src/main/java/org/apache/atlas/AtlasErrorCode.java | 4 +++- .../repository/store/aliasstore/ESAliasStore.java | 10 +++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java b/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java index 5cd42da2e3b..7a0b2c835f8 100644 --- a/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java +++ b/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java @@ -104,7 +104,9 @@ public enum AtlasConfiguration { INDEX_CLIENT_CONNECTION_TIMEOUT("atlas.index.client.connection.timeout.ms", 900000), INDEX_CLIENT_SOCKET_TIMEOUT("atlas.index.client.socket.timeout.ms", 900000), ENABLE_SEARCH_LOGGER("atlas.enable.search.logger", true), - SEARCH_LOGGER_MAX_THREADS("atlas.enable.search.logger.max.threads", 20); + SEARCH_LOGGER_MAX_THREADS("atlas.enable.search.logger.max.threads", 20), + + PERSONA_POLICY_ASSET_MAX_LIMIT("atlas.persona.policy.asset.maxlimit", 1000); private static final Configuration APPLICATION_PROPERTIES; diff --git a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java index 11120c466bc..04d72bdfb2d 100644 --- a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java +++ b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java @@ -282,7 +282,9 @@ public enum AtlasErrorCode { JSON_ERROR(400, "ATLAS-400-00-109", "Error occurred putting object into JSONObject: {0}"), DISABLED_OPERATION(400, "ATLAS-400-00-110", "This operation is temporarily disabled as it is under maintenance."), TASK_INVALID_PARAMETERS(400, "ATLAS-400-00-111", "Invalid parameters for task {0}"), - TASK_TYPE_NOT_SUPPORTED(400, "ATLAS-400-00-112", "Task type {0} is not supported"); + TASK_TYPE_NOT_SUPPORTED(400, "ATLAS-400-00-112", "Task type {0} is not supported"), + + PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED(400, "ATLAS-400-00-113", "Exceeded limit of maximum allowed assets across policies for a Persona: Limit: {0}, assets: {1}"); private String errorCode; diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 67dde2e1708..cd595998ee2 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -17,6 +17,8 @@ */ package org.apache.atlas.repository.store.aliasstore; +import org.apache.atlas.AtlasConfiguration; +import org.apache.atlas.AtlasErrorCode; import org.apache.atlas.ESAliasRequestBuilder; import org.apache.atlas.ESAliasRequestBuilder.AliasAction; import org.apache.atlas.exception.AtlasBaseException; @@ -69,6 +71,8 @@ public class ESAliasStore implements IndexAliasStore { private final AtlasGraph graph; private final EntityGraphRetriever entityRetriever; + private final int assetsMaxLimit = AtlasConfiguration.PERSONA_POLICY_ASSET_MAX_LIMIT.getInt(); + @Inject public ESAliasStore(AtlasGraph graph, EntityGraphRetriever entityRetriever) { @@ -166,7 +170,7 @@ private Map getFilterForPurpose(AtlasEntity purpose) throws Atla private void personaPolicyToESDslClauses(List policies, List> allowClauseList) throws AtlasBaseException { - Set terms = new HashSet<>(); + List terms = new ArrayList<>(); for (AtlasEntity policy: policies) { if (policy.getStatus() == null || AtlasEntity.Status.ACTIVE.equals(policy.getStatus())) { @@ -196,6 +200,10 @@ private void personaPolicyToESDslClauses(List policies, } } + if (terms.size() > assetsMaxLimit) { + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + } + allowClauseList.add(mapOf("terms", mapOf(QUALIFIED_NAME, terms))); } From 8da1cd4e8726a88d839ff5d129db92151bb59cf4 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 9 Nov 2023 12:28:55 +0530 Subject: [PATCH 04/13] PLT-302 Address review comment --- .../repository/store/aliasstore/ESAliasStore.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index cd595998ee2..87e59174ec3 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -178,6 +178,11 @@ private void personaPolicyToESDslClauses(List policies, if (getIsAllowPolicy(policy)) { if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { + + if (terms.size() + assets.size() + 1 > assetsMaxLimit) { + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + } + String connectionQName = getPolicyConnectionQN(policy); if (StringUtils.isEmpty(connectionQName)) { connectionQName = getConnectionQualifiedNameFromPolicyAssets(entityRetriever, assets); @@ -191,6 +196,10 @@ private void personaPolicyToESDslClauses(List policies, terms.add(connectionQName); } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { + if (terms.size() + assets.size() > assetsMaxLimit) { + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + } + for (String glossaryQName : assets) { terms.add(glossaryQName); allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + glossaryQName))); @@ -200,10 +209,6 @@ private void personaPolicyToESDslClauses(List policies, } } - if (terms.size() > assetsMaxLimit) { - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); - } - allowClauseList.add(mapOf("terms", mapOf(QUALIFIED_NAME, terms))); } From 08be3166d542ed660e8d7d0c2f6a2b809532b3c4 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 9 Nov 2023 12:52:23 +0530 Subject: [PATCH 05/13] PLT-302 Address review comment --- .../apache/atlas/repository/store/aliasstore/ESAliasStore.java | 1 + 1 file changed, 1 insertion(+) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 87e59174ec3..955f801b758 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -180,6 +180,7 @@ private void personaPolicyToESDslClauses(List policies, if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { if (terms.size() + assets.size() + 1 > assetsMaxLimit) { + // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence omparing with "assets.size() + 1" throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); } From 4d31bf51220f17217f9dbdc63c9bd93cfbeec86f Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Thu, 9 Nov 2023 12:55:16 +0530 Subject: [PATCH 06/13] PLT-302 Address review comment --- .../apache/atlas/repository/store/aliasstore/ESAliasStore.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 955f801b758..e6ff8f69d3f 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -180,7 +180,7 @@ private void personaPolicyToESDslClauses(List policies, if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { if (terms.size() + assets.size() + 1 > assetsMaxLimit) { - // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence omparing with "assets.size() + 1" + // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence comparing with "assets.size() + 1" throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); } From 1e7bde66af17e036f2c12c8564528c62dd86e9b0 Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Fri, 10 Nov 2023 23:35:48 +0530 Subject: [PATCH 07/13] PLT-302 Fix assets count in error message --- .../atlas/repository/store/aliasstore/ESAliasStore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index e6ff8f69d3f..7bd0546c81c 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -181,7 +181,7 @@ private void personaPolicyToESDslClauses(List policies, if (terms.size() + assets.size() + 1 > assetsMaxLimit) { // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence comparing with "assets.size() + 1" - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size() + assets.size() + 1)); } String connectionQName = getPolicyConnectionQN(policy); @@ -198,7 +198,7 @@ private void personaPolicyToESDslClauses(List policies, } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { if (terms.size() + assets.size() > assetsMaxLimit) { - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size() + assets.size())); } for (String glossaryQName : assets) { From b276fc1a731a19a7e91a1c41f5b7298aeb702afb Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Fri, 10 Nov 2023 23:53:55 +0530 Subject: [PATCH 08/13] PLT-302 Fix logging --- .../atlas/repository/store/aliasstore/ESAliasStore.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 7bd0546c81c..f4c073d7254 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -179,7 +179,8 @@ private void personaPolicyToESDslClauses(List policies, if (getIsAllowPolicy(policy)) { if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { - if (terms.size() + assets.size() + 1 > assetsMaxLimit) { + int assetSize = terms.size() + assets.size() + 1; + if (assetSize > assetsMaxLimit) { // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence comparing with "assets.size() + 1" throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size() + assets.size() + 1)); } @@ -197,8 +198,10 @@ private void personaPolicyToESDslClauses(List policies, terms.add(connectionQName); } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { + + int assetSize = terms.size() + assets.size(); if (terms.size() + assets.size() > assetsMaxLimit) { - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size() + assets.size())); + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(assetSize)); } for (String glossaryQName : assets) { From 1ea33b43f6bcc101e7beeb20f352d57b7cbc103e Mon Sep 17 00:00:00 2001 From: Nikhil P Bonte Date: Sat, 11 Nov 2023 00:13:14 +0530 Subject: [PATCH 09/13] PLT-302 Fix logging --- .../apache/atlas/repository/store/aliasstore/ESAliasStore.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index f4c073d7254..475d8561978 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -182,7 +182,7 @@ private void personaPolicyToESDslClauses(List policies, int assetSize = terms.size() + assets.size() + 1; if (assetSize > assetsMaxLimit) { // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence comparing with "assets.size() + 1" - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size() + assets.size() + 1)); + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(assetSize)); } String connectionQName = getPolicyConnectionQN(policy); From ac5d5e125e1d7e388edc2130d3a03ca5ba684de0 Mon Sep 17 00:00:00 2001 From: ektavarma10 Date: Tue, 14 Nov 2023 14:45:17 +0530 Subject: [PATCH 10/13] Add metric counter when authenticating a request from a service-account --- .../web/security/AtlasKeycloakAuthenticationProvider.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java b/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java index 367839b82aa..40b9fba612e 100644 --- a/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java +++ b/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java @@ -16,6 +16,8 @@ */ package org.apache.atlas.web.security; +import io.micrometer.core.instrument.Counter; +import org.apache.atlas.service.metrics.MetricUtils; import org.apache.atlas.ApplicationProperties; import org.apache.commons.configuration.Configuration; import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationProvider; @@ -66,6 +68,11 @@ public Authentication authenticate(Authentication authentication) { } } + if(authentication.getName().startsWith("service-account-apikey")) { + // Increment the counter when the authentication is for a service account. + Counter.builder("service_account_apikey_request_counter").register(MetricUtils.getMeterRegistry()).increment(); + } + return authentication; } From b82b610b20c95291d42342503bf0b8a31febf119 Mon Sep 17 00:00:00 2001 From: Aayush Sarva Date: Thu, 16 Nov 2023 18:54:28 +0530 Subject: [PATCH 11/13] Simplify assets max limit reached check --- .../store/aliasstore/ESAliasStore.java | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java index 475d8561978..7f9d16768b3 100644 --- a/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java +++ b/repository/src/main/java/org/apache/atlas/repository/store/aliasstore/ESAliasStore.java @@ -171,46 +171,42 @@ private Map getFilterForPurpose(AtlasEntity purpose) throws Atla private void personaPolicyToESDslClauses(List policies, List> allowClauseList) throws AtlasBaseException { List terms = new ArrayList<>(); + for (AtlasEntity policy: policies) { if (policy.getStatus() == null || AtlasEntity.Status.ACTIVE.equals(policy.getStatus())) { List assets = getPolicyAssets(policy); - if (getIsAllowPolicy(policy)) { - if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { - - int assetSize = terms.size() + assets.size() + 1; - if (assetSize > assetsMaxLimit) { - // For Metadata policies, along with assets we add 1 more clause for connection qualifiedName hence comparing with "assets.size() + 1" - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(assetSize)); - } - - String connectionQName = getPolicyConnectionQN(policy); - if (StringUtils.isEmpty(connectionQName)) { - connectionQName = getConnectionQualifiedNameFromPolicyAssets(entityRetriever, assets); - } + if (!getIsAllowPolicy(policy)) { + continue; + } + + if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_METADATA)) { - for (String asset : assets) { - terms.add(asset); - allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); - } + String connectionQName = getPolicyConnectionQN(policy); + if (StringUtils.isEmpty(connectionQName)) { + connectionQName = getConnectionQualifiedNameFromPolicyAssets(entityRetriever, assets); + } - terms.add(connectionQName); + for (String asset : assets) { + terms.add(asset); + allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, asset + "/*"))); + } - } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { + terms.add(connectionQName); - int assetSize = terms.size() + assets.size(); - if (terms.size() + assets.size() > assetsMaxLimit) { - throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(assetSize)); - } + } else if (getPolicyActions(policy).contains(ACCESS_READ_PERSONA_GLOSSARY)) { - for (String glossaryQName : assets) { - terms.add(glossaryQName); - allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + glossaryQName))); - } + for (String glossaryQName : assets) { + terms.add(glossaryQName); + allowClauseList.add(mapOf("wildcard", mapOf(QUALIFIED_NAME, "*@" + glossaryQName))); } } } + + if (terms.size() > assetsMaxLimit) { + throw new AtlasBaseException(AtlasErrorCode.PERSONA_POLICY_ASSETS_LIMIT_EXCEEDED, String.valueOf(assetsMaxLimit), String.valueOf(terms.size())); + } } allowClauseList.add(mapOf("terms", mapOf(QUALIFIED_NAME, terms))); From 20d3bdd8d9557344c2357783d62749cd3ae2ca0c Mon Sep 17 00:00:00 2001 From: ektavarma10 Date: Fri, 17 Nov 2023 23:20:27 +0530 Subject: [PATCH 12/13] Add keycloak online introspection for requests coming from service-accounts-apikey --- .../keycloak/client/AtlasKeycloakClient.java | 5 +++ .../keycloak/client/KeycloakRestClient.java | 18 +++++++++ .../client/RetrofitKeycloakClient.java | 5 +++ .../security/AtlasAuthenticationProvider.java | 2 + .../AtlasKeycloakAuthenticationProvider.java | 37 ++++++++++++++++++- .../KeycloakAuthenticationException.java | 13 +++++++ 6 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 webapp/src/main/java/org/apache/atlas/web/security/KeycloakAuthenticationException.java diff --git a/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/AtlasKeycloakClient.java b/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/AtlasKeycloakClient.java index 5eec02b7c68..fe723bbce1b 100644 --- a/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/AtlasKeycloakClient.java +++ b/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/AtlasKeycloakClient.java @@ -9,6 +9,7 @@ import org.codehaus.jettison.json.JSONException; import org.codehaus.jettison.json.JSONObject; import org.keycloak.representations.idm.*; +import org.keycloak.representations.oidc.TokenMetadataRepresentation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -174,6 +175,10 @@ public List getEvents(List type, String client, Str return KEYCLOAK.getEvents(type, client, user, dateFrom, dateTo, ipAddress, first, max).body(); } + public TokenMetadataRepresentation introspectToken(String token) throws AtlasBaseException { + return KEYCLOAK.introspectToken(token).body(); + } + public static AtlasKeycloakClient getKeycloakClient() throws AtlasBaseException { if (Objects.isNull(KEYCLOAK_CLIENT)) { LOG.info("Initializing Keycloak client.."); diff --git a/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/KeycloakRestClient.java b/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/KeycloakRestClient.java index 73c8c207a9b..7cc1eea22f6 100644 --- a/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/KeycloakRestClient.java +++ b/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/KeycloakRestClient.java @@ -1,8 +1,11 @@ package org.apache.atlas.keycloak.client; +import okhttp3.FormBody; +import okhttp3.RequestBody; import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.keycloak.client.config.KeycloakConfig; import org.keycloak.representations.idm.*; +import org.keycloak.representations.oidc.TokenMetadataRepresentation; import retrofit2.Response; import java.util.List; @@ -13,6 +16,9 @@ */ public final class KeycloakRestClient extends AbstractKeycloakClient { + private static final String TOKEN = "token"; + private static final String CLIENT_ID = "client_id"; + private static final String CLIENT_SECRET = "client_secret"; public KeycloakRestClient(final KeycloakConfig keycloakConfig) { super(keycloakConfig); } @@ -121,4 +127,16 @@ public Response> getEvents(List type, String c String dateTo, String ipAddress, Integer first, Integer max) throws AtlasBaseException { return processResponse(this.retrofit.getEvents(this.keycloakConfig.getRealmId(), type, client, user, dateFrom, dateTo, ipAddress, first, max)); } + + public Response introspectToken(String token) throws AtlasBaseException { + return processResponse(this.retrofit.introspectToken(this.keycloakConfig.getRealmId(), getIntrospectTokenRequest(token))); + } + + private RequestBody getIntrospectTokenRequest(String token) { + return new FormBody.Builder() + .addEncoded(TOKEN, token) + .addEncoded(CLIENT_ID, this.keycloakConfig.getClientId()) + .addEncoded(CLIENT_SECRET, this.keycloakConfig.getClientSecret()) + .build(); + } } diff --git a/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/RetrofitKeycloakClient.java b/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/RetrofitKeycloakClient.java index cb813c00959..c396c9368dc 100644 --- a/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/RetrofitKeycloakClient.java +++ b/client-keycloak/src/main/java/org/apache/atlas/keycloak/client/RetrofitKeycloakClient.java @@ -3,6 +3,7 @@ import okhttp3.RequestBody; import org.keycloak.representations.AccessTokenResponse; import org.keycloak.representations.idm.*; +import org.keycloak.representations.oidc.TokenMetadataRepresentation; import retrofit2.Call; import retrofit2.http.*; @@ -142,4 +143,8 @@ Call> getEvents(@Path("realmId") String realmId, @Quer @POST("realms/{realmId}/protocol/openid-connect/token") Call grantToken(@Path("realmId") String realmId, @Body RequestBody request); + @Headers({"Accept: application/json", "Content-Type: application/x-www-form-urlencoded", "Cache-Control: no-store", "Cache-Control: no-cache"}) + @POST("realms/{realmId}/protocol/openid-connect/token/introspect") + Call introspectToken(@Path("realmId") String realmId, @Body RequestBody request); + } diff --git a/webapp/src/main/java/org/apache/atlas/web/security/AtlasAuthenticationProvider.java b/webapp/src/main/java/org/apache/atlas/web/security/AtlasAuthenticationProvider.java index dff3d8d31c3..17ca1864fe7 100644 --- a/webapp/src/main/java/org/apache/atlas/web/security/AtlasAuthenticationProvider.java +++ b/webapp/src/main/java/org/apache/atlas/web/security/AtlasAuthenticationProvider.java @@ -130,6 +130,8 @@ public Authentication authenticate(Authentication authentication) } else if (keycloakAuthenticationEnabled) { try { authentication = atlasKeycloakAuthenticationProvider.authenticate(authentication); + } catch (KeycloakAuthenticationException ex) { + throw new AtlasAuthenticationException("Authentication failed."); } catch (Exception ex) { LOG.error("Error while Keycloak authentication", ex); } diff --git a/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java b/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java index 40b9fba612e..8469572806a 100644 --- a/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java +++ b/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java @@ -17,11 +17,15 @@ package org.apache.atlas.web.security; import io.micrometer.core.instrument.Counter; +import org.apache.atlas.keycloak.client.AtlasKeycloakClient; import org.apache.atlas.service.metrics.MetricUtils; import org.apache.atlas.ApplicationProperties; import org.apache.commons.configuration.Configuration; import org.keycloak.adapters.springsecurity.authentication.KeycloakAuthenticationProvider; import org.keycloak.adapters.springsecurity.token.KeycloakAuthenticationToken; +import org.keycloak.representations.oidc.TokenMetadataRepresentation; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; @@ -30,18 +34,25 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; @Component public class AtlasKeycloakAuthenticationProvider extends AtlasAbstractAuthenticationProvider { private final boolean groupsFromUGI; private final String groupsClaim; + private final boolean isTokenIntrospectionEnabled; private final KeycloakAuthenticationProvider keycloakAuthenticationProvider; + private final AtlasKeycloakClient atlasKeycloakClient; + private static final Logger LOG = LoggerFactory.getLogger(AtlasKeycloakAuthenticationProvider.class); public AtlasKeycloakAuthenticationProvider() throws Exception { this.keycloakAuthenticationProvider = new KeycloakAuthenticationProvider(); + this.atlasKeycloakClient = AtlasKeycloakClient.getKeycloakClient(); Configuration configuration = ApplicationProperties.get(); + + this.isTokenIntrospectionEnabled = configuration.getBoolean("atlas.canary.keycloak.token-introspection", false); this.groupsFromUGI = configuration.getBoolean("atlas.authentication.method.keycloak.ugi-groups", true); this.groupsClaim = configuration.getString("atlas.authentication.method.keycloak.groups_claim"); } @@ -68,9 +79,27 @@ public Authentication authenticate(Authentication authentication) { } } - if(authentication.getName().startsWith("service-account-apikey")) { + if (authentication.getName().startsWith("service-account-apikey")) { // Increment the counter when the authentication is for a service account. Counter.builder("service_account_apikey_request_counter").register(MetricUtils.getMeterRegistry()).increment(); + + // Validate the token online with keycloak server if token introspection is enabled + LOG.info("isTokenIntrospectionEnabled: {}", isTokenIntrospectionEnabled); + if (isTokenIntrospectionEnabled) { + LOG.info("Validating request for clientId: {}", authentication.getName().substring("service-account-".length())); + try { + KeycloakAuthenticationToken keycloakToken = (KeycloakAuthenticationToken) authentication; + String bearerToken = keycloakToken.getAccount().getKeycloakSecurityContext().getTokenString(); + TokenMetadataRepresentation introspectToken = atlasKeycloakClient.introspectToken(bearerToken); + if (Objects.nonNull(introspectToken) && introspectToken.isActive()) { + authentication.setAuthenticated(true); + } else { + handleInvalidApiKey(authentication); + } + } catch (Exception e) { + throw new KeycloakAuthenticationException("Keycloak Authentication failed", e.getCause()); + } + } } return authentication; @@ -80,4 +109,10 @@ public Authentication authenticate(Authentication authentication) { public boolean supports(Class aClass) { return keycloakAuthenticationProvider.supports(aClass); } + + private void handleInvalidApiKey(Authentication authentication) { + authentication.setAuthenticated(false); + LOG.error("Invalid clientId: {}", authentication.getName().substring("service-account-".length())); + throw new KeycloakAuthenticationException("Invalid ClientId"); + } } \ No newline at end of file diff --git a/webapp/src/main/java/org/apache/atlas/web/security/KeycloakAuthenticationException.java b/webapp/src/main/java/org/apache/atlas/web/security/KeycloakAuthenticationException.java new file mode 100644 index 00000000000..01480015036 --- /dev/null +++ b/webapp/src/main/java/org/apache/atlas/web/security/KeycloakAuthenticationException.java @@ -0,0 +1,13 @@ +package org.apache.atlas.web.security; + +import org.springframework.security.core.AuthenticationException; + +public class KeycloakAuthenticationException extends AuthenticationException { + public KeycloakAuthenticationException(String msg, Throwable cause) { + super(msg, cause); + } + + public KeycloakAuthenticationException(String msg) { + super(msg); + } +} From 5b4c62058129af46968f62602aa814b937f500fc Mon Sep 17 00:00:00 2001 From: ektavarma10 Date: Mon, 20 Nov 2023 08:56:34 +0530 Subject: [PATCH 13/13] Add keycloak.token-introspection in Atlas Configuration --- intg/src/main/java/org/apache/atlas/AtlasConfiguration.java | 3 ++- .../web/security/AtlasKeycloakAuthenticationProvider.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java b/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java index 7a0b2c835f8..8c019d16d9b 100644 --- a/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java +++ b/intg/src/main/java/org/apache/atlas/AtlasConfiguration.java @@ -106,7 +106,8 @@ public enum AtlasConfiguration { ENABLE_SEARCH_LOGGER("atlas.enable.search.logger", true), SEARCH_LOGGER_MAX_THREADS("atlas.enable.search.logger.max.threads", 20), - PERSONA_POLICY_ASSET_MAX_LIMIT("atlas.persona.policy.asset.maxlimit", 1000); + PERSONA_POLICY_ASSET_MAX_LIMIT("atlas.persona.policy.asset.maxlimit", 1000), + ENABLE_KEYCLOAK_TOKEN_INTROSPECTION("atlas.canary.keycloak.token-introspection", false); private static final Configuration APPLICATION_PROPERTIES; diff --git a/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java b/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java index 8469572806a..88523998cda 100644 --- a/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java +++ b/webapp/src/main/java/org/apache/atlas/web/security/AtlasKeycloakAuthenticationProvider.java @@ -17,6 +17,7 @@ package org.apache.atlas.web.security; import io.micrometer.core.instrument.Counter; +import org.apache.atlas.AtlasConfiguration; import org.apache.atlas.keycloak.client.AtlasKeycloakClient; import org.apache.atlas.service.metrics.MetricUtils; import org.apache.atlas.ApplicationProperties; @@ -52,7 +53,7 @@ public AtlasKeycloakAuthenticationProvider() throws Exception { Configuration configuration = ApplicationProperties.get(); - this.isTokenIntrospectionEnabled = configuration.getBoolean("atlas.canary.keycloak.token-introspection", false); + this.isTokenIntrospectionEnabled = AtlasConfiguration.ENABLE_KEYCLOAK_TOKEN_INTROSPECTION.getBoolean(); this.groupsFromUGI = configuration.getBoolean("atlas.authentication.method.keycloak.ugi-groups", true); this.groupsClaim = configuration.getString("atlas.authentication.method.keycloak.groups_claim"); }