Skip to content

Commit

Permalink
feat(search): search access controls unit tests (#9925)
Browse files Browse the repository at this point in the history
  • Loading branch information
david-leifker committed Feb 28, 2024
1 parent b3ff422 commit c47db24
Show file tree
Hide file tree
Showing 23 changed files with 1,228 additions and 47 deletions.
7 changes: 1 addition & 6 deletions datahub-frontend/app/auth/AuthModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.linkedin.entity.client.SystemEntityClient;
import com.linkedin.entity.client.SystemRestliEntityClient;
import com.linkedin.metadata.restli.DefaultRestliClientFactory;
import com.linkedin.metadata.utils.elasticsearch.IndexConventionImpl;
import com.linkedin.parseq.retry.backoff.ExponentialBackoff;
import com.linkedin.util.Configuration;
import config.ConfigurationProvider;
Expand All @@ -42,15 +41,11 @@
import org.pac4j.play.store.PlayCookieSessionStore;
import org.pac4j.play.store.PlaySessionStore;
import org.pac4j.play.store.ShiroAesDataEncrypter;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import play.Environment;
import play.cache.SyncCacheApi;
import utils.ConfigUtil;

import javax.annotation.Nonnull;

/** Responsible for configuring, validating, and providing authentication related components. */
@Slf4j
public class AuthModule extends AbstractModule {
Expand Down Expand Up @@ -173,7 +168,7 @@ protected OperationContext provideOperationContext(final Authentication systemAu
final ConfigurationProvider configurationProvider) {
ActorContext systemActorContext =
ActorContext.builder()
.systemAuthentication(true)
.allowSystemAuth(true)
.authentication(systemAuthentication)
.build();
OperationContextConfig systemConfig = OperationContextConfig.builder()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
package com.linkedin.datahub.upgrade.config;

import com.linkedin.datahub.upgrade.system.entity.steps.BackfillPolicyFields;
import com.linkedin.datahub.upgrade.system.policyfields.BackfillPolicyFields;
import com.linkedin.metadata.entity.EntityService;
import com.linkedin.metadata.search.SearchService;
import io.datahubproject.metadata.context.OperationContext;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Conditional;
import org.springframework.context.annotation.Configuration;

@Configuration
@Conditional(SystemUpdateCondition.NonBlockingSystemUpdateCondition.class)
public class BackfillPolicyFieldsConfig {

@Bean
public BackfillPolicyFields backfillPolicyFields(
final OperationContext opContext,
EntityService<?> entityService,
SearchService searchService,
@Value("${systemUpdate.policyFields.enabled}") final boolean enabled,
@Value("${systemUpdate.policyFields.reprocess.enabled}") final boolean reprocessEnabled,
@Value("${systemUpdate.policyFields.batchSize}") final Integer batchSize) {
return new BackfillPolicyFields(
entityService, searchService, enabled, reprocessEnabled, batchSize);
opContext, entityService, searchService, enabled, reprocessEnabled, batchSize);
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
package com.linkedin.datahub.upgrade.system.entity.steps;
package com.linkedin.datahub.upgrade.system.policyfields;

import com.google.common.collect.ImmutableList;
import com.linkedin.datahub.upgrade.Upgrade;
import com.linkedin.datahub.upgrade.UpgradeStep;
import com.linkedin.datahub.upgrade.system.NonBlockingSystemUpgrade;
import com.linkedin.metadata.entity.EntityService;
import com.linkedin.metadata.search.SearchService;
import io.datahubproject.metadata.context.OperationContext;
import java.util.List;

public class BackfillPolicyFields implements Upgrade {
public class BackfillPolicyFields implements NonBlockingSystemUpgrade {
private final List<UpgradeStep> _steps;

public BackfillPolicyFields(
OperationContext opContext,
EntityService<?> entityService,
SearchService searchService,
boolean enabled,
Expand All @@ -20,7 +22,7 @@ public BackfillPolicyFields(
_steps =
ImmutableList.of(
new BackfillPolicyFieldsStep(
entityService, searchService, reprocessEnabled, batchSize));
opContext, entityService, searchService, reprocessEnabled, batchSize));
} else {
_steps = ImmutableList.of();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.linkedin.datahub.upgrade.system.entity.steps;
package com.linkedin.datahub.upgrade.system.policyfields;

import static com.linkedin.metadata.Constants.*;

Expand Down Expand Up @@ -30,6 +30,7 @@
import com.linkedin.mxe.MetadataChangeProposal;
import com.linkedin.mxe.SystemMetadata;
import com.linkedin.policy.DataHubPolicyInfo;
import io.datahubproject.metadata.context.OperationContext;
import java.net.URISyntaxException;
import java.util.Collections;
import java.util.function.Function;
Expand All @@ -44,16 +45,20 @@
public class BackfillPolicyFieldsStep implements UpgradeStep {
private static final String UPGRADE_ID = "BackfillPolicyFieldsStep";
private static final Urn UPGRADE_ID_URN = BootstrapStep.getUpgradeUrn(UPGRADE_ID);

private final OperationContext opContext;
private final boolean reprocessEnabled;
private final Integer batchSize;
private final EntityService<?> entityService;
private final SearchService _searchService;

public BackfillPolicyFieldsStep(
OperationContext opContext,
EntityService<?> entityService,
SearchService searchService,
boolean reprocessEnabled,
Integer batchSize) {
this.opContext = opContext;
this.entityService = entityService;
this._searchService = searchService;
this.reprocessEnabled = reprocessEnabled;
Expand Down Expand Up @@ -108,7 +113,8 @@ public boolean skip(UpgradeContext context) {
return false;
}

boolean previouslyRun = entityService.exists(UPGRADE_ID_URN, true);
boolean previouslyRun =
entityService.exists(UPGRADE_ID_URN, DATA_HUB_UPGRADE_RESULT_ASPECT_NAME, true);
if (previouslyRun) {
log.info("{} was already run. Skipping.", id());
}
Expand All @@ -120,6 +126,7 @@ private String backfillPolicies(AuditStamp auditStamp, String scrollId) {
final Filter filter = backfillPolicyFieldFilter();
final ScrollResult scrollResult =
_searchService.scrollAcrossEntities(
opContext,
ImmutableList.of(Constants.POLICY_ENTITY_NAME),
"*",
filter,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
package com.linkedin.metadata.aspect.hooks;

import static com.linkedin.metadata.Constants.DEFAULT_OWNERSHIP_TYPE_URN;
import static org.mockito.Mockito.mock;
import static org.testng.Assert.assertEquals;

import com.linkedin.common.Owner;
import com.linkedin.common.OwnerArray;
import com.linkedin.common.Ownership;
import com.linkedin.common.UrnArray;
import com.linkedin.common.UrnArrayMap;
import com.linkedin.common.urn.Urn;
import com.linkedin.common.urn.UrnUtils;
import com.linkedin.metadata.aspect.AspectRetriever;
import com.linkedin.metadata.aspect.batch.ChangeMCP;
import com.linkedin.metadata.aspect.plugins.config.AspectPluginConfig;
import com.linkedin.metadata.models.registry.EntityRegistry;
import com.linkedin.test.metadata.aspect.TestEntityRegistry;
import com.linkedin.test.metadata.aspect.batch.TestMCP;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.testng.annotations.Test;

public class OwnerTypeMapTest {
private static final AspectRetriever ASPECT_RETRIEVER = mock(AspectRetriever.class);
private static final EntityRegistry ENTITY_REGISTRY = new TestEntityRegistry();
private static final AspectPluginConfig ASPECT_PLUGIN_CONFIG =
AspectPluginConfig.builder()
.className("some class")
.enabled(true)
.supportedEntityAspectNames(
List.of(
AspectPluginConfig.EntityAspectName.builder()
.entityName("*")
.aspectName("ownership")
.build()))
.build();
private static final Urn TEST_ENTITY_URN =
UrnUtils.getUrn(
"urn:li:dataset:(urn:li:dataPlatform:bigquery,calm-pagoda-323403.jaffle_shop.orders,PROD)");
private static final Urn TEST_USER_A = UrnUtils.getUrn("urn:li:corpUser:a");
private static final Urn TEST_USER_B = UrnUtils.getUrn("urn:li:corpUser:b");
private static final Urn TEST_GROUP_A = UrnUtils.getUrn("urn:li:corpGroup:a");
private static final Urn TEST_GROUP_B = UrnUtils.getUrn("urn:li:corpGroup:b");
private static final Urn TECH_OWNER =
UrnUtils.getUrn("urn:li:ownershipType:__system__technical_owner");
private static final Urn BUS_OWNER =
UrnUtils.getUrn("urn:li:ownershipType:__system__business_owner");

@Test
public void ownershipTypeMutationNoneType() {
OwnerTypeMap testHook = new OwnerTypeMap(ASPECT_PLUGIN_CONFIG);
Ownership ownership = buildOwnership(Map.of(TEST_USER_A, List.of(), TEST_GROUP_A, List.of()));
testHook.writeMutation(buildMCP(null, ownership), ASPECT_RETRIEVER);

assertEquals(
ownership.getOwnerTypes(),
new UrnArrayMap(
Map.of(
DEFAULT_OWNERSHIP_TYPE_URN.toString(),
new UrnArray(List.of(TEST_USER_A, TEST_GROUP_A)))),
"Expected generic owners to be grouped by `none` ownership type.");
}

@Test
public void ownershipTypeMutationNoneTypeAdd() {
OwnerTypeMap testHook = new OwnerTypeMap(ASPECT_PLUGIN_CONFIG);
Ownership oldOwnership = buildOwnership(Map.of(TEST_USER_A, List.of()));
Ownership newOwnership =
buildOwnership(Map.of(TEST_USER_A, List.of(), TEST_GROUP_A, List.of()));
testHook.writeMutation(buildMCP(oldOwnership, newOwnership), ASPECT_RETRIEVER);

assertEquals(
newOwnership.getOwnerTypes(),
new UrnArrayMap(
Map.of(
DEFAULT_OWNERSHIP_TYPE_URN.toString(),
new UrnArray(List.of(TEST_USER_A, TEST_GROUP_A)))),
"Expected generic owners to be grouped by `none` ownership type.");
}

@Test
public void ownershipTypeMutationNoneTypeRemove() {
OwnerTypeMap testHook = new OwnerTypeMap(ASPECT_PLUGIN_CONFIG);
Ownership oldOwnership =
buildOwnership(Map.of(TEST_USER_A, List.of(), TEST_GROUP_A, List.of()));
Ownership newOwnership = buildOwnership(Map.of(TEST_USER_A, List.of()));
testHook.writeMutation(buildMCP(oldOwnership, newOwnership), ASPECT_RETRIEVER);

assertEquals(
newOwnership.getOwnerTypes(),
new UrnArrayMap(
Map.of(DEFAULT_OWNERSHIP_TYPE_URN.toString(), new UrnArray(List.of(TEST_USER_A)))),
"Expected generic owners to be grouped by `none` ownership type.");
}

@Test
public void ownershipTypeMutationMixedType() {
OwnerTypeMap testHook = new OwnerTypeMap(ASPECT_PLUGIN_CONFIG);
Ownership ownership =
buildOwnership(
Map.of(
TEST_USER_A,
List.of(),
TEST_GROUP_A,
List.of(),
TEST_USER_B,
List.of(BUS_OWNER),
TEST_GROUP_B,
List.of(TECH_OWNER)));
testHook.writeMutation(buildMCP(null, ownership), ASPECT_RETRIEVER);

assertEquals(
ownership.getOwnerTypes(),
new UrnArrayMap(
Map.of(
DEFAULT_OWNERSHIP_TYPE_URN.toString(),
new UrnArray(List.of(TEST_USER_A, TEST_GROUP_A)),
BUS_OWNER.toString(),
new UrnArray(List.of(TEST_USER_B)),
TECH_OWNER.toString(),
new UrnArray(List.of(TEST_GROUP_B)))),
"Expected generic owners to be grouped by `none` ownership type as well as specified types.");
}

@Test
public void ownershipTypeMutationMixedTypeAdd() {
OwnerTypeMap testHook = new OwnerTypeMap(ASPECT_PLUGIN_CONFIG);
Ownership oldOwnership =
buildOwnership(Map.of(TEST_USER_A, List.of(), TEST_USER_B, List.of(BUS_OWNER)));
Ownership newOwnership =
buildOwnership(
Map.of(
TEST_USER_A,
List.of(),
TEST_GROUP_A,
List.of(),
TEST_USER_B,
List.of(BUS_OWNER),
TEST_GROUP_B,
List.of(TECH_OWNER)));
testHook.writeMutation(buildMCP(oldOwnership, newOwnership), ASPECT_RETRIEVER);

assertEquals(
newOwnership.getOwnerTypes(),
new UrnArrayMap(
Map.of(
DEFAULT_OWNERSHIP_TYPE_URN.toString(),
new UrnArray(List.of(TEST_USER_A, TEST_GROUP_A)),
BUS_OWNER.toString(),
new UrnArray(List.of(TEST_USER_B)),
TECH_OWNER.toString(),
new UrnArray(List.of(TEST_GROUP_B)))),
"Expected generic owners to be grouped by `none` ownership type as well as specified types.");
}

@Test
public void ownershipTypeMutationMixedTypeRemove() {
OwnerTypeMap testHook = new OwnerTypeMap(ASPECT_PLUGIN_CONFIG);
Ownership oldOwnership =
buildOwnership(
Map.of(
TEST_USER_A,
List.of(),
TEST_GROUP_A,
List.of(),
TEST_USER_B,
List.of(BUS_OWNER),
TEST_GROUP_B,
List.of(TECH_OWNER)));
Ownership newOwnership =
buildOwnership(Map.of(TEST_GROUP_A, List.of(), TEST_GROUP_B, List.of(TECH_OWNER)));
testHook.writeMutation(buildMCP(oldOwnership, newOwnership), ASPECT_RETRIEVER);

assertEquals(
newOwnership.getOwnerTypes(),
new UrnArrayMap(
Map.of(
DEFAULT_OWNERSHIP_TYPE_URN.toString(),
new UrnArray(List.of(TEST_GROUP_A)),
BUS_OWNER.toString(),
new UrnArray(),
TECH_OWNER.toString(),
new UrnArray(List.of(TEST_GROUP_B)))),
"Expected generic owners to be grouped by `none` ownership type as well as specified types.");
}

private static Ownership buildOwnership(Map<Urn, List<Urn>> ownershipTypes) {
Ownership ownership = new Ownership();
ownership.setOwners(
ownershipTypes.entrySet().stream()
.flatMap(
entry -> {
if (entry.getValue().isEmpty()) {
Owner owner = new Owner();
owner.setOwner(entry.getKey());
return Stream.of(owner);
} else {
return entry.getValue().stream()
.map(
typeUrn -> {
Owner owner = new Owner();
owner.setOwner(entry.getKey());
owner.setTypeUrn(typeUrn);
return owner;
});
}
})
.collect(Collectors.toCollection(OwnerArray::new)));
return ownership;
}

private static Set<ChangeMCP> buildMCP(@Nullable Ownership oldOwnership, Ownership newOwnership) {
return TestMCP.ofOneMCP(TEST_ENTITY_URN, oldOwnership, newOwnership, ENTITY_REGISTRY);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.datahub.plugins.Plugin;
import com.linkedin.common.urn.Urn;
import com.linkedin.policy.DataHubPolicyInfo;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -60,7 +61,7 @@ default Set<DataHubPolicyInfo> getActorPolicies(@Nonnull Urn actorUrn) {
}

/** Given the actor's urn retrieve the actor's groups */
default Set<Urn> getActorGroups(@Nonnull Urn actorUrn) {
return Collections.emptySet();
default Collection<Urn> getActorGroups(@Nonnull Urn actorUrn) {
return Collections.emptyList();
}
}
Loading

0 comments on commit c47db24

Please sign in to comment.