Skip to content

Commit 31351a8

Browse files
david-leifkerdavid-leifker
committed
wip access controls
TODO: * cache keys - need to be context aware to prevent incorrect results * ownership migration upgrade step * complete unit tests for access controls * restricted entity hydration and graphql response (chris)
1 parent 50a8d8a commit 31351a8

File tree

21 files changed

+232
-69
lines changed

21 files changed

+232
-69
lines changed

‎datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/health/EntityHealthResolver.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ private Health computeIncidentsHealthForAsset(
138138
final Filter filter = buildIncidentsEntityFilter(entityUrn, IncidentState.ACTIVE.toString());
139139
final SearchResult searchResult =
140140
_entityClient.filter(
141-
Constants.INCIDENT_ENTITY_NAME, filter, null, 0, 1, context.getAuthentication());
141+
context.getOperationContext(), Constants.INCIDENT_ENTITY_NAME, filter, null, 0, 1);
142142
final Integer activeIncidentCount = searchResult.getNumEntities();
143143
if (activeIncidentCount > 0) {
144144
// There are active incidents.

‎datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/incident/EntityIncidentsResolver.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,12 @@ public CompletableFuture<EntityIncidentsResult> get(DataFetchingEnvironment envi
6262
final SortCriterion sortCriterion = buildIncidentsSortCriterion();
6363
final SearchResult searchResult =
6464
_entityClient.filter(
65+
context.getOperationContext(),
6566
Constants.INCIDENT_ENTITY_NAME,
6667
filter,
6768
sortCriterion,
6869
start,
69-
count,
70-
context.getAuthentication());
70+
count);
7171

7272
final List<Urn> incidentUrns =
7373
searchResult.getEntities().stream()

‎datahub-graphql-core/src/test/java/com/linkedin/datahub/graphql/resolvers/incident/EntityIncidentsResolverTest.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.linkedin.datahub.graphql.resolvers.incident;
22

33
import static com.linkedin.datahub.graphql.resolvers.incident.EntityIncidentsResolver.*;
4+
import static org.mockito.Mockito.mock;
45
import static org.testng.Assert.*;
56

67
import com.datahub.authentication.Authentication;
@@ -34,6 +35,7 @@
3435
import com.linkedin.metadata.search.SearchResult;
3536
import com.linkedin.metadata.search.utils.QueryUtils;
3637
import graphql.schema.DataFetchingEnvironment;
38+
import io.datahubproject.metadata.context.OperationContext;
3739
import java.util.HashMap;
3840
import java.util.Map;
3941
import org.mockito.Mockito;
@@ -86,12 +88,12 @@ public void testGetSuccess() throws Exception {
8688

8789
Mockito.when(
8890
mockClient.filter(
91+
Mockito.any(OperationContext.class),
8992
Mockito.eq(Constants.INCIDENT_ENTITY_NAME),
9093
Mockito.eq(expectedFilter),
9194
Mockito.eq(expectedSort),
9295
Mockito.eq(0),
93-
Mockito.eq(10),
94-
Mockito.any(Authentication.class)))
96+
Mockito.eq(10)))
9597
.thenReturn(
9698
new SearchResult()
9799
.setFrom(0)
@@ -120,6 +122,7 @@ public void testGetSuccess() throws Exception {
120122
// Execute resolver
121123
QueryContext mockContext = Mockito.mock(QueryContext.class);
122124
Mockito.when(mockContext.getAuthentication()).thenReturn(Mockito.mock(Authentication.class));
125+
Mockito.when(mockContext.getOperationContext()).thenReturn(mock(OperationContext.class));
123126
DataFetchingEnvironment mockEnv = Mockito.mock(DataFetchingEnvironment.class);
124127

125128
Mockito.when(mockEnv.getArgumentOrDefault(Mockito.eq("start"), Mockito.eq(0))).thenReturn(0);

‎entity-registry/src/main/java/com/linkedin/metadata/aspect/hooks/OwnerTypeMap.java

+35-10
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import com.linkedin.metadata.models.EntitySpec;
1919
import com.linkedin.mxe.SystemMetadata;
2020
import com.linkedin.util.Pair;
21+
import java.util.Collections;
22+
import java.util.HashSet;
2123
import java.util.Map;
2224
import java.util.Set;
2325
import java.util.stream.Collectors;
@@ -43,32 +45,55 @@ protected void mutate(
4345
@Nonnull AspectRetriever aspectRetriever) {
4446
if (OWNERSHIP_ASPECT_NAME.equals(aspectSpec.getName()) && newAspectValue != null) {
4547
Ownership ownership = new Ownership(newAspectValue.data());
46-
if (!ownership.getOwners().isEmpty()) {
4748

48-
Map<Urn, Set<Owner>> ownerTypes =
49-
ownership.getOwners().stream()
50-
.collect(Collectors.groupingBy(Owner::getOwner, Collectors.toSet()));
49+
final Map<Urn, Set<Owner>> oldOwnerTypes = groupByOwner(oldAspectValue);
50+
final Map<Urn, Set<Owner>> newOwnerTypes = groupByOwner(newAspectValue);
51+
52+
if (!oldOwnerTypes.isEmpty() || !newOwnerTypes.isEmpty()) {
53+
Set<Urn> changedOwners = new HashSet<>(newOwnerTypes.keySet());
54+
changedOwners.addAll(oldOwnerTypes.keySet());
5155

5256
ownership.setOwnerTypes(
5357
new UrnArrayMap(
54-
ownerTypes.entrySet().stream()
58+
changedOwners.stream()
5559
.map(
56-
entry ->
57-
Pair.of(
58-
encodeFieldName(entry.getKey().toString()),
60+
ownerUrn -> {
61+
if (!newOwnerTypes.containsKey(ownerUrn)
62+
&& oldOwnerTypes.containsKey(ownerUrn)) {
63+
// removed
64+
return Pair.of(encodeFieldName(ownerUrn.toString()), new UrnArray());
65+
} else {
66+
return Pair.of(
67+
encodeFieldName(ownerUrn.toString()),
5968
new UrnArray(
60-
entry.getValue().stream()
69+
newOwnerTypes
70+
.getOrDefault(ownerUrn, Collections.emptySet())
71+
.stream()
6172
.map(
6273
owner ->
6374
owner.getTypeUrn() != null
6475
? owner.getTypeUrn()
6576
: DEFAULT_OWNERSHIP_TYPE_URN)
66-
.collect(Collectors.toSet()))))
77+
.collect(Collectors.toSet())));
78+
}
79+
})
6780
.collect(Collectors.toMap(Pair::getKey, Pair::getValue))));
6881
}
6982
}
7083
}
7184

85+
private static Map<Urn, Set<Owner>> groupByOwner(
86+
@Nullable RecordTemplate ownershipRecordTemplate) {
87+
if (ownershipRecordTemplate != null) {
88+
Ownership ownership = new Ownership(ownershipRecordTemplate.data());
89+
if (!ownership.getOwners().isEmpty()) {
90+
return ownership.getOwners().stream()
91+
.collect(Collectors.groupingBy(Owner::getOwner, Collectors.toSet()));
92+
}
93+
}
94+
return Collections.emptyMap();
95+
}
96+
7297
public static String encodeFieldName(String value) {
7398
return value.replaceAll("[.]", "%2E");
7499
}

‎entity-registry/src/main/java/com/linkedin/metadata/models/annotation/SearchableAnnotation.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ public class SearchableAnnotation {
2020

2121
public static final String FIELD_NAME_ALIASES = "fieldNameAliases";
2222
public static final String ANNOTATION_NAME = "Searchable";
23+
public static final Set<FieldType> OBJECT_FIELD_TYPES =
24+
ImmutableSet.of(FieldType.OBJECT, FieldType.MAP_ARRAY);
2325
private static final Set<FieldType> DEFAULT_QUERY_FIELD_TYPES =
2426
ImmutableSet.of(
2527
FieldType.TEXT,
@@ -71,7 +73,8 @@ public enum FieldType {
7173
OBJECT,
7274
BROWSE_PATH_V2,
7375
WORD_GRAM,
74-
DOUBLE
76+
DOUBLE,
77+
MAP_ARRAY
7578
}
7679

7780
@Nonnull

‎metadata-auth/auth-api/src/main/java/com/datahub/authorization/config/SearchAuthorizationConfiguration.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
package com.datahub.authorization.config;
22

3+
import lombok.AccessLevel;
4+
import lombok.AllArgsConstructor;
35
import lombok.Builder;
46
import lombok.Data;
7+
import lombok.NoArgsConstructor;
58

6-
@Builder
9+
@Builder(toBuilder = true)
710
@Data
11+
@AllArgsConstructor(access = AccessLevel.PACKAGE)
12+
@NoArgsConstructor(access = AccessLevel.PACKAGE)
813
public class SearchAuthorizationConfiguration {
914
private boolean enabled;
1015
}

‎metadata-io/src/main/java/com/linkedin/metadata/search/elasticsearch/ElasticSearchService.java

+34-9
Original file line numberDiff line numberDiff line change
@@ -182,16 +182,18 @@ public SearchResult filter(
182182
@Nonnull OperationContext opContext,
183183
@Nonnull String entityName,
184184
@Nullable Filter filters,
185-
@Nonnull SearchFlags searchFlags,
185+
@Nullable SearchFlags searchFlags,
186186
@Nullable SortCriterion sortCriterion,
187187
int from,
188188
int size) {
189189
log.debug(
190190
String.format(
191191
"Filtering Search documents entityName: %s, filters: %s, sortCriterion: %s, from: %s, size: %s",
192192
entityName, filters, sortCriterion, from, size));
193+
SearchFlags finalSearchFlags =
194+
applyDefaultSearchFlags(searchFlags, null, DEFAULT_SERVICE_SEARCH_FLAGS);
193195
return esSearchDAO.filter(
194-
opContext, entityName, filters, searchFlags, sortCriterion, from, size);
196+
opContext, entityName, filters, finalSearchFlags, sortCriterion, from, size);
195197
}
196198

197199
@Nonnull
@@ -309,10 +311,19 @@ public ScrollResult fullTextScroll(
309311
String.format(
310312
"Scrolling Structured Search documents entities: %s, input: %s, postFilters: %s, sortCriterion: %s, scrollId: %s, size: %s",
311313
entities, input, postFilters, sortCriterion, scrollId, size));
312-
SearchFlags flags = Optional.ofNullable(searchFlags).orElse(new SearchFlags());
313-
flags.setFulltext(true);
314+
SearchFlags finalSearchFlags =
315+
applyDefaultSearchFlags(searchFlags, null, DEFAULT_SERVICE_SEARCH_FLAGS);
316+
finalSearchFlags.setFulltext(true);
314317
return esSearchDAO.scroll(
315-
opContext, entities, input, postFilters, sortCriterion, scrollId, keepAlive, size, flags);
318+
opContext,
319+
entities,
320+
input,
321+
postFilters,
322+
sortCriterion,
323+
scrollId,
324+
keepAlive,
325+
size,
326+
finalSearchFlags);
316327
}
317328

318329
@Nonnull
@@ -331,10 +342,19 @@ public ScrollResult structuredScroll(
331342
String.format(
332343
"Scrolling FullText Search documents entities: %s, input: %s, postFilters: %s, sortCriterion: %s, scrollId: %s, size: %s",
333344
entities, input, postFilters, sortCriterion, scrollId, size));
334-
SearchFlags flags = Optional.ofNullable(searchFlags).orElse(new SearchFlags());
335-
flags.setFulltext(false);
345+
SearchFlags finalSearchFlags =
346+
applyDefaultSearchFlags(searchFlags, null, DEFAULT_SERVICE_SEARCH_FLAGS);
347+
finalSearchFlags.setFulltext(false);
336348
return esSearchDAO.scroll(
337-
opContext, entities, input, postFilters, sortCriterion, scrollId, keepAlive, size, flags);
349+
opContext,
350+
entities,
351+
input,
352+
postFilters,
353+
sortCriterion,
354+
scrollId,
355+
keepAlive,
356+
size,
357+
finalSearchFlags);
338358
}
339359

340360
public Optional<SearchResponse> raw(@Nonnull String indexName, @Nullable String jsonQuery) {
@@ -348,6 +368,7 @@ public int maxResultSize() {
348368

349369
@Override
350370
public ExplainResponse explain(
371+
@Nonnull OperationContext opContext,
351372
@Nonnull String query,
352373
@Nonnull String documentId,
353374
@Nonnull String entityName,
@@ -358,13 +379,17 @@ public ExplainResponse explain(
358379
@Nullable String keepAlive,
359380
int size,
360381
@Nullable List<String> facets) {
382+
SearchFlags finalSearchFlags =
383+
applyDefaultSearchFlags(searchFlags, null, DEFAULT_SERVICE_SEARCH_FLAGS);
384+
361385
return esSearchDAO.explain(
386+
opContext,
362387
query,
363388
documentId,
364389
entityName,
365390
postFilters,
366391
sortCriterion,
367-
searchFlags,
392+
finalSearchFlags,
368393
scrollId,
369394
keepAlive,
370395
size,

‎metadata-io/src/main/java/com/linkedin/metadata/search/elasticsearch/indexbuilder/MappingsBuilder.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static com.linkedin.metadata.Constants.ENTITY_TYPE_URN_PREFIX;
44
import static com.linkedin.metadata.Constants.STRUCTURED_PROPERTY_MAPPING_FIELD;
55
import static com.linkedin.metadata.models.StructuredPropertyUtils.sanitizeStructuredPropertyFQN;
6+
import static com.linkedin.metadata.models.annotation.SearchableAnnotation.OBJECT_FIELD_TYPES;
67
import static com.linkedin.metadata.search.elasticsearch.indexbuilder.SettingsBuilder.*;
78

89
import com.google.common.collect.ImmutableMap;
@@ -53,6 +54,7 @@ public static Map<String, String> getPartialNgramConfigWithOverrides(
5354
public static final String PATH = "path";
5455

5556
public static final String PROPERTIES = "properties";
57+
public static final String DYNAMIC_TEMPLATES = "dynamic_templates";
5658

5759
private MappingsBuilder() {}
5860

@@ -100,6 +102,7 @@ public static Map<String, Object> getMappings(
100102
return merged.isEmpty() ? null : merged;
101103
});
102104
}
105+
103106
return mappings;
104107
}
105108

@@ -221,7 +224,7 @@ private static Map<String, Object> getMappingsForField(
221224
mappingForField.put(TYPE, ESUtils.LONG_FIELD_TYPE);
222225
} else if (fieldType == FieldType.DATETIME) {
223226
mappingForField.put(TYPE, ESUtils.DATE_FIELD_TYPE);
224-
} else if (fieldType == FieldType.OBJECT) {
227+
} else if (OBJECT_FIELD_TYPES.contains(fieldType)) {
225228
mappingForField.put(TYPE, ESUtils.OBJECT_FIELD_TYPE);
226229
} else if (fieldType == FieldType.DOUBLE) {
227230
mappingForField.put(TYPE, ESUtils.DOUBLE_FIELD_TYPE);

‎metadata-io/src/main/java/com/linkedin/metadata/search/elasticsearch/indexbuilder/ReindexConfig.java

+48-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package com.linkedin.metadata.search.elasticsearch.indexbuilder;
22

33
import static com.linkedin.metadata.Constants.*;
4+
import static com.linkedin.metadata.search.elasticsearch.indexbuilder.MappingsBuilder.PROPERTIES;
5+
import static com.linkedin.metadata.search.elasticsearch.indexbuilder.SettingsBuilder.TYPE;
46

57
import com.fasterxml.jackson.core.JsonProcessingException;
68
import com.fasterxml.jackson.core.StreamReadConstraints;
79
import com.fasterxml.jackson.databind.ObjectMapper;
810
import com.google.common.collect.ImmutableList;
911
import com.google.common.collect.MapDifference;
1012
import com.google.common.collect.Maps;
13+
import com.linkedin.metadata.search.utils.ESUtils;
1114
import java.util.List;
1215
import java.util.Map;
1316
import java.util.Objects;
@@ -146,9 +149,10 @@ public ReindexConfig build() {
146149
if (super.exists) {
147150
/* Consider mapping changes */
148151
MapDifference<String, Object> mappingsDiff =
149-
Maps.difference(
150-
getOrDefault(super.currentMappings, List.of("properties")),
151-
getOrDefault(super.targetMappings, List.of("properties")));
152+
calculateMapDifference(
153+
getOrDefault(super.currentMappings, List.of(PROPERTIES)),
154+
getOrDefault(super.targetMappings, List.of(PROPERTIES)));
155+
152156
super.requiresApplyMappings =
153157
!mappingsDiff.entriesDiffering().isEmpty()
154158
|| !mappingsDiff.entriesOnlyOnRight().isEmpty();
@@ -298,6 +302,47 @@ private boolean isSettingsReindexRequired() {
298302
(Map<String, Object>) indexSettings.get("analysis"),
299303
super.currentSettings.getByPrefix("index.analysis."));
300304
}
305+
306+
/**
307+
* Dynamic fields should not be considered as part of the difference. This might need to be
308+
* improved in the future for nested object fields.
309+
*
310+
* @param currentMappings current mappings
311+
* @param targetMappings target mappings
312+
* @return difference map
313+
*/
314+
private static MapDifference<String, Object> calculateMapDifference(
315+
Map<String, Object> currentMappings, Map<String, Object> targetMappings) {
316+
317+
// Identify dynamic object fields in target
318+
Set<String> targetObjectFields =
319+
targetMappings.entrySet().stream()
320+
.filter(
321+
entry ->
322+
((Map<String, Object>) entry.getValue()).containsKey(TYPE)
323+
&& ((Map<String, Object>) entry.getValue())
324+
.get(TYPE)
325+
.equals(ESUtils.OBJECT_FIELD_TYPE))
326+
.map(Map.Entry::getKey)
327+
.collect(Collectors.toSet());
328+
329+
if (!targetObjectFields.isEmpty()) {
330+
log.info("Object fields filtered from comparison: {}", targetObjectFields);
331+
Map<String, Object> filteredCurrentMappings =
332+
removeKeys(currentMappings, targetObjectFields);
333+
Map<String, Object> filteredTargetMappings = removeKeys(targetMappings, targetObjectFields);
334+
return Maps.difference(filteredCurrentMappings, filteredTargetMappings);
335+
}
336+
337+
return Maps.difference(currentMappings, targetMappings);
338+
}
339+
}
340+
341+
private static Map<String, Object> removeKeys(
342+
Map<String, Object> mapObject, Set<String> keysToRemove) {
343+
return mapObject.entrySet().stream()
344+
.filter(entry -> !keysToRemove.contains(entry.getKey()))
345+
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
301346
}
302347

303348
private static boolean equalsGroup(Map<String, Object> newSettings, Settings oldSettings) {

0 commit comments

Comments
 (0)