yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-07-24 09:50:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixes #6851 Backend : Roles patch API is not working for policies (#6890)

Browse Source
This commit is contained in:
Suresh Srinivas 2022-08-23 15:31:42 -07:00 committed by GitHub
parent ca39b9f603
commit 605871db2e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 153 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java
View File

@ -25,6 +25,8 @@ public final class CatalogExceptionMessage {
public static final String FIELD_NOT_TOKENIZED = "Field is not tokenized";
public static final String FIELD_ALREADY_TOKENIZED = "Field is already tokenized";
public static final String INVALID_ENTITY_LINK = "Entity link must have both {arrayFieldName} and {arrayFieldValue}";
public static final String EMPTY_POLICIES_IN_ROLE = "At least one policy is required in a role";
public static final String EMPTY_RULES_IN_POLICY = "At least one rule is required in a policy";
private CatalogExceptionMessage() {}

22
catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java
View File

@ -17,10 +17,14 @@ import static org.openmetadata.catalog.Entity.FIELD_OWNER;
import static org.openmetadata.catalog.Entity.LOCATION;
import static org.openmetadata.catalog.Entity.POLICY;
import static org.openmetadata.catalog.util.EntityUtil.entityReferenceMatch;
import static org.openmetadata.catalog.util.EntityUtil.resolveRules;
import static org.openmetadata.catalog.util.EntityUtil.ruleMatch;
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.sqlobject.transaction.Transaction;
@ -33,6 +37,7 @@ import org.openmetadata.catalog.jdbi3.CollectionDAO.EntityRelationshipRecord;
import org.openmetadata.catalog.resources.policies.PolicyResource;
import org.openmetadata.catalog.security.policyevaluator.CompiledRule;
import org.openmetadata.catalog.type.EntityReference;
import org.openmetadata.catalog.type.MetadataOperation;
import org.openmetadata.catalog.type.PolicyType;
import org.openmetadata.catalog.type.Relationship;
import org.openmetadata.catalog.util.EntityUtil;
@ -140,11 +145,18 @@ public class PolicyRepository extends EntityRepository<Policy> {
// Resolve JSON blobs into Rule object and perform schema based validation
List<Rule> rules = EntityUtil.resolveRules(policy.getRules());
if (listOrEmpty(rules).isEmpty()) {
throw new IllegalArgumentException(CatalogExceptionMessage.EMPTY_RULES_IN_POLICY);
}
System.out.println("XXX rules count " + rules.size());
// Validate all the expressions in the rule
for (Rule rule : rules) {
CompiledRule.validateExpression(rule.getCondition(), Boolean.class);
rule.getResources().sort(String.CASE_INSENSITIVE_ORDER);
rule.getOperations().sort(Comparator.comparing(MetadataOperation::value));
}
rules.sort(Comparator.comparing(Rule::getName));
}
public List<Policy> getAccessControlPolicies() throws IOException {
@ -182,8 +194,8 @@ public class PolicyRepository extends EntityRepository<Policy> {
throw new IllegalArgumentException(CatalogExceptionMessage.readOnlyAttribute(POLICY, "policyType"));
}
recordChange(ENABLED, original.getEnabled(), updated.getEnabled());
recordChange("rules", original.getRules(), updated.getRules());
updateLocation(original, updated);
updateRules(original.getRules(), updated.getRules());
}
private void updateLocation(Policy origPolicy, Policy updatedPolicy) throws IOException {
@ -203,5 +215,13 @@ public class PolicyRepository extends EntityRepository<Policy> {
}
recordChange("location", origPolicy.getLocation(), updatedPolicy.getLocation(), true, entityReferenceMatch);
}
private void updateRules(List<Object> origRules, List<Object> updatedRules) throws IOException {
// Record change description
List<Rule> deletedRules = new ArrayList<>();
List<Rule> addedRules = new ArrayList<>();
recordListChange(
"rules", resolveRules(origRules), resolveRules(updatedRules), addedRules, deletedRules, ruleMatch);
}
}
}

25
catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java
View File

@ -14,8 +14,10 @@
package org.openmetadata.catalog.jdbi3;
import static org.openmetadata.catalog.Entity.POLICIES;
import static org.openmetadata.catalog.util.EntityUtil.entityReferenceMatch;
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import com.fasterxml.jackson.core.JsonProcessingException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
@ -26,6 +28,7 @@ import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.sqlobject.transaction.Transaction;
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.entity.teams.Role;
import org.openmetadata.catalog.exception.CatalogExceptionMessage;
import org.openmetadata.catalog.jdbi3.CollectionDAO.EntityRelationshipRecord;
import org.openmetadata.catalog.resources.teams.RoleResource;
import org.openmetadata.catalog.type.EntityReference;
@ -77,6 +80,9 @@ public class RoleRepository extends EntityRepository<Role> {
@Override
public void prepare(Role role) throws IOException {
setFullyQualifiedName(role);
if (listOrEmpty(role.getPolicies()).isEmpty()) {
throw new IllegalArgumentException(CatalogExceptionMessage.EMPTY_POLICIES_IN_ROLE);
}
EntityUtil.populateEntityReferences(role.getPolicies());
}
@ -136,6 +142,25 @@ public class RoleRepository extends EntityRepository<Role> {
@Override
public void entitySpecificUpdate() throws IOException {
updateDefault(original, updated);
updatePolicies(listOrEmpty(original.getPolicies()), listOrEmpty(updated.getPolicies()));
}
private void updatePolicies(List<EntityReference> origPolicies, List<EntityReference> updatedPolicies)
throws JsonProcessingException {
// Record change description
List<EntityReference> deletedPolicies = new ArrayList<>();
List<EntityReference> addedPolicies = new ArrayList<>();
boolean changed =
recordListChange(
"policies", origPolicies, updatedPolicies, addedPolicies, deletedPolicies, entityReferenceMatch);
if (changed) {
// Remove all the Role to policy relationships
deleteFrom(original.getId(), Entity.ROLE, Relationship.HAS, Entity.POLICY);
// Add Role to policy relationships back based on Updated entity
storeRelationships(updated);
}
}
private void updateDefault(Role origRole, Role updatedRole) throws IOException {

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

@ -57,6 +57,7 @@ import org.openmetadata.catalog.type.EventFilter;
import org.openmetadata.catalog.type.EventType;
import org.openmetadata.catalog.type.FailureDetails;
import org.openmetadata.catalog.type.FieldChange;
import org.openmetadata.catalog.type.MetadataOperation;
import org.openmetadata.catalog.type.MlFeature;
import org.openmetadata.catalog.type.MlHyperParameter;
import org.openmetadata.catalog.type.Schedule;
@ -87,6 +88,7 @@ public final class EntityUtil {
public static final Comparator<ChangeEvent> compareChangeEvent = Comparator.comparing(ChangeEvent::getTimestamp);
public static final Comparator<GlossaryTerm> compareGlossaryTerm = Comparator.comparing(GlossaryTerm::getName);
public static final Comparator<CustomProperty> compareCustomProperty = Comparator.comparing(CustomProperty::getName);
public static final Comparator<MetadataOperation> compareOperation = Comparator.comparing(MetadataOperation::value);
//
// Matchers used for matching two items in a list
@ -137,6 +139,14 @@ public final class EntityUtil {
public static final BiPredicate<CustomProperty, CustomProperty> customFieldMatch =
(ref1, ref2) -> ref1.getName().equals(ref2.getName());
public static final BiPredicate<Rule, Rule> ruleMatch =
(ref1, ref2) ->
ref1.getName().equals(ref2.getName())
&& ref1.getOperations().equals(ref2.getOperations())
&& ref1.getResources().equals(ref2.getResources())
&& ref1.getEffect().equals(ref2.getEffect())
&& Objects.equals(ref1.getCondition(), ref2.getCondition());
private EntityUtil() {}
/** Validate Ingestion Schedule */

2
catalog-rest-service/src/test/java/org/openmetadata/catalog/pipelineService/PipelineServiceClientTest.java
View File

@ -3,7 +3,7 @@ package org.openmetadata.catalog.pipelineService;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.openmetadata.catalog.exception.PipelineServiceVersionException;
public class PipelineServiceClientTest {

50
catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java
View File

@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.openmetadata.catalog.entity.policies.accessControl.Rule.Effect.ALLOW;
import static org.openmetadata.catalog.entity.policies.accessControl.Rule.Effect.DENY;
import static org.openmetadata.catalog.util.EntityUtil.resolveRules;
import static org.openmetadata.catalog.util.TestUtils.ADMIN_AUTH_HEADERS;
import static org.openmetadata.catalog.util.TestUtils.UpdateType.MINOR_UPDATE;
import static org.openmetadata.catalog.util.TestUtils.assertListNotNull;
@ -88,13 +89,15 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
}
public void setupPolicies() throws HttpResponseException {
POLICY1 = createEntity(createRequest("policy1"), ADMIN_AUTH_HEADERS);
POLICY2 = createEntity(createRequest("policy2"), ADMIN_AUTH_HEADERS);
POLICY1 = createEntity(createRequest("policy1").withOwner(null), ADMIN_AUTH_HEADERS);
POLICY2 = createEntity(createRequest("policy2").withOwner(null), ADMIN_AUTH_HEADERS);
}
@Override
public CreatePolicy createRequest(String name) {
return new CreatePolicy().withName(name).withPolicyType(PolicyType.Lifecycle);
List<Rule> rules = new ArrayList<>();
rules.add(accessControlRule("rule1", List.of("all"), List.of(MetadataOperation.EDIT_DESCRIPTION), ALLOW));
return createAccessControlPolicyWithRules(name, rules);
}
@Override
@ -104,7 +107,7 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
if (createRequest.getLocation() != null) {
assertEquals(createRequest.getLocation(), policy.getLocation().getId());
}
assertEquals(createRequest.getRules(), EntityUtil.resolveRules(policy.getRules()));
assertEquals(createRequest.getRules(), resolveRules(policy.getRules()));
}
@Override
@ -123,6 +126,10 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
EntityReference expectedLocation = (EntityReference) expected;
EntityReference actualLocation = JsonUtils.readValue(actual.toString(), EntityReference.class);
assertEquals(expectedLocation.getId(), actualLocation.getId());
} else if (fieldName.equals("rules")) {
List<Rule> expectedRule = (List<Rule>) expected;
List<Rule> actualRule = resolveRules(JsonUtils.readObjects(actual.toString(), Object.class));
assertEquals(expectedRule, actualRule);
} else {
assertCommonFieldChange(fieldName, expected, actual);
}
@ -253,6 +260,36 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
}
@Test
void patch_PolicyRules(TestInfo test) throws IOException {
Rule rule1 = accessControlRule("rule1", List.of("all"), List.of(MetadataOperation.VIEW_ALL), ALLOW);
Policy policy = createAndCheckEntity(createRequest(test).withRules(List.of(rule1)), ADMIN_AUTH_HEADERS);
// Change an existing rule1
String origJson = JsonUtils.pojoToJson(policy);
Rule updatedRule1 = accessControlRule("rule1", List.of("all"), List.of(MetadataOperation.ALL), ALLOW);
policy.setRules(List.of(updatedRule1));
ChangeDescription change = getChangeDescription(policy.getVersion());
change.getFieldsDeleted().add(new FieldChange().withName("rules").withOldValue(List.of(rule1)));
change.getFieldsAdded().add(new FieldChange().withName("rules").withNewValue(List.of(updatedRule1)));
policy = patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Add a new rule
origJson = JsonUtils.pojoToJson(policy);
Rule newRule = accessControlRule("newRule", List.of("all"), List.of(MetadataOperation.EDIT_DESCRIPTION), ALLOW);
policy.getRules().add(newRule);
change = getChangeDescription(policy.getVersion());
change.getFieldsAdded().add(new FieldChange().withName("rules").withNewValue(List.of(newRule)));
policy = patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Delete newRule1 rule
origJson = JsonUtils.pojoToJson(policy);
policy.setRules(List.of(newRule));
change = getChangeDescription(policy.getVersion());
change.getFieldsDeleted().add(new FieldChange().withName("rules").withOldValue(List.of(updatedRule1)));
patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
}
@Test
void get_policyResources() throws HttpResponseException {
// Get list of policy resources and make sure it has all the entities and other resources
@ -364,6 +401,11 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
private static Rule accessControlRule(List<String> resources, List<MetadataOperation> operations, Effect effect) {
String name = "rule" + new Random().nextInt(21);
return accessControlRule(name, resources, operations, effect);
}
private static Rule accessControlRule(
String name, List<String> resources, List<MetadataOperation> operations, Effect effect) {
return new Rule().withName(name).withResources(resources).withOperations(operations).withEffect(effect);
}
}

49
catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java
View File

@ -13,6 +13,7 @@
package org.openmetadata.catalog.resources.teams;
import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static javax.ws.rs.core.Response.Status.FORBIDDEN;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
@ -43,9 +44,11 @@ import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.api.teams.CreateRole;
import org.openmetadata.catalog.entity.teams.Role;
import org.openmetadata.catalog.entity.teams.User;
import org.openmetadata.catalog.exception.CatalogExceptionMessage;
import org.openmetadata.catalog.resources.EntityResourceTest;
import org.openmetadata.catalog.resources.teams.RoleResource.RoleList;
import org.openmetadata.catalog.type.ChangeDescription;
import org.openmetadata.catalog.type.EntityReference;
import org.openmetadata.catalog.type.FieldChange;
import org.openmetadata.catalog.type.MetadataOperation;
import org.openmetadata.catalog.util.JsonUtils;
@ -166,6 +169,41 @@ public class RoleResourceTest extends EntityResourceTest<Role, CreateRole> {
permissionNotAllowed(TEST_USER_NAME, List.of(MetadataOperation.EDIT_DISPLAY_NAME)));
}
@Test
void patch_rolePolicies(TestInfo test) throws IOException {
Role role = createEntity(createRequest(test), ADMIN_AUTH_HEADERS);
// Add new DATA_STEWARD_POLICY to role in addition to DATA_CONSUMER_POLICY
String originalJson = JsonUtils.pojoToJson(role);
role.getPolicies().addAll(DATA_STEWARD_ROLE.getPolicies());
ChangeDescription change = getChangeDescription(role.getVersion());
change.getFieldsAdded().add(new FieldChange().withName("policies").withNewValue(DATA_STEWARD_ROLE.getPolicies()));
role = patchEntityAndCheck(role, originalJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Remove new DATA_CONSUMER_POLICY
originalJson = JsonUtils.pojoToJson(role);
role.setPolicies(DATA_STEWARD_ROLE.getPolicies());
change = getChangeDescription(role.getVersion());
change
.getFieldsDeleted()
.add(new FieldChange().withName("policies").withOldValue(DATA_CONSUMER_ROLE.getPolicies()));
role = patchEntityAndCheck(role, originalJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Remove all the policies. It should be disallowed
final String originalJson1 = JsonUtils.pojoToJson(role);
final UUID id = role.getId();
final Role role1 = role;
role1.setPolicies(null);
change = getChangeDescription(role1.getVersion());
change
.getFieldsDeleted()
.add(new FieldChange().withName("policies").withOldValue(DATA_CONSUMER_ROLE.getPolicies()));
assertResponse(
() -> patchEntity(id, originalJson1, role1, ADMIN_AUTH_HEADERS),
BAD_REQUEST,
CatalogExceptionMessage.EMPTY_POLICIES_IN_ROLE);
}
private static void validateRole(
Role role, String expectedDescription, String expectedDisplayName, String expectedUpdatedBy) {
assertListNotNull(role.getId(), role.getHref());
@ -240,6 +278,15 @@ public class RoleResourceTest extends EntityResourceTest<Role, CreateRole> {
@Override
public void assertFieldChange(String fieldName, Object expected, Object actual) throws IOException {
assertCommonFieldChange(fieldName, expected, actual);
if (expected == actual) {
return;
}
if (fieldName.equals("policies")) {
List<EntityReference> expectedRefs = (List<EntityReference>) expected;
List<EntityReference> actualRefs = JsonUtils.readObjects(actual.toString(), EntityReference.class);
assertEntityReferences(expectedRefs, actualRefs);
} else {
assertCommonFieldChange(fieldName, expected, actual);
}
}
}