Skip to content

Commit

Permalink
Simplify policy filtering
Browse files Browse the repository at this point in the history
  • Loading branch information
atanasdinov committed Jan 10, 2023
1 parent 4b691d4 commit 11774ab
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import com.ft.up.apipolicy.transformer.BodyProcessingFieldTransformer;
import com.ft.up.apipolicy.transformer.BodyProcessingFieldTransformerFactory;
import io.dropwizard.setup.Environment;
import java.util.Arrays;
import javax.ws.rs.core.MultivaluedHashMap;
import javax.ws.rs.core.MultivaluedMap;

Expand Down Expand Up @@ -112,8 +111,7 @@ public void initFilters(ApiPolicyConfiguration configuration, Environment enviro
addSyndication = new AddSyndication(jsonTweaker);
brandFilter = new AddBrandFilterParameters(jsonTweaker, resolver);
linkValidationFilter = new LinkedContentValidationFilter();
contentNotificationsFilter =
new NotificationsTypeFilter(jsonTweaker, INTERNAL_UNSTABLE, EXTENDED_PULL_NOTIFICATIONS);
contentNotificationsFilter = new NotificationsTypeFilter(jsonTweaker);
accessLevelPropertyFilter =
new RemoveJsonPropertiesUnlessPolicyPresentFilter(
jsonTweaker, INTERNAL_UNSTABLE, ACCESS_LEVEL_JSON_PROPERTY);
Expand All @@ -140,10 +138,10 @@ public void initFilters(ApiPolicyConfiguration configuration, Environment enviro
}

public ApiFilter notificationsFilter() {
return new PolicyBasedJsonFilter(getNotificationsFilters(null));
return new PolicyBasedJsonFilter(getNotificationsFilters(false));
}

private MultivaluedMap<String, Policy> getNotificationsFilters(Policy... contentTypePolices) {
private MultivaluedMap<String, Policy> getNotificationsFilters(boolean includeContentType) {
MultivaluedMap<String, Policy> notificationsJsonFilters = new MultivaluedHashMap<>();
// whitelisted (no policy required)
notificationsJsonFilters.put("$.requestUrl", null);
Expand All @@ -157,9 +155,9 @@ private MultivaluedMap<String, Policy> getNotificationsFilters(Policy... content
notificationsJsonFilters.add("$.notifications[*].notificationDate", INCLUDE_LAST_MODIFIED_DATE);
notificationsJsonFilters.add("$.notifications[*].publishReference", INCLUDE_PROVENANCE);

if (contentTypePolices != null) {
notificationsJsonFilters.put(
"$.notifications[*].contentType", Arrays.asList(contentTypePolices));
if (includeContentType) {
notificationsJsonFilters.add("$.notifications[*].contentType", INTERNAL_UNSTABLE);
notificationsJsonFilters.add("$.notifications[*].contentType", EXTENDED_PULL_NOTIFICATIONS);
}

return notificationsJsonFilters;
Expand Down Expand Up @@ -249,8 +247,7 @@ public ApiFilter[] contentNotificationsFilters() {
return new ApiFilter[] {
contentNotificationsFilter,
brandFilter,
new PolicyBasedJsonFilter(
getNotificationsFilters(INTERNAL_UNSTABLE, EXTENDED_PULL_NOTIFICATIONS))
new PolicyBasedJsonFilter(getNotificationsFilters(true))
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.ft.up.apipolicy.filters;

import static com.ft.up.apipolicy.configuration.Policy.EXTENDED_PULL_NOTIFICATIONS;
import static com.ft.up.apipolicy.configuration.Policy.INTERNAL_UNSTABLE;

import com.ft.up.apipolicy.JsonConverter;
import com.ft.up.apipolicy.configuration.Policy;
import com.ft.up.apipolicy.pipeline.ApiFilter;
import com.ft.up.apipolicy.pipeline.HttpPipelineChain;
import com.ft.up.apipolicy.pipeline.MutableRequest;
Expand All @@ -24,14 +26,9 @@ public class NotificationsTypeFilter implements ApiFilter {
private static final String LIVE_BLOG_PACKAGE_CONTENT_TYPE = "LiveBlogPackage";

private final JsonConverter converter;
private final Policy allTypesPolicy;
private final Policy specificTypesPolicy;

public NotificationsTypeFilter(
JsonConverter converter, Policy allTypesPolicy, Policy specificTypesPolicy) {
public NotificationsTypeFilter(JsonConverter converter) {
this.converter = converter;
this.allTypesPolicy = allTypesPolicy;
this.specificTypesPolicy = specificTypesPolicy;
}

@Override
Expand Down Expand Up @@ -69,10 +66,10 @@ private void addQueryParams(MutableRequest request) {
List<String> typeParams = new ArrayList<>();
List<String> monitorParams = new ArrayList<>();

if (request.policyIs(allTypesPolicy)) {
if (request.policyIs(INTERNAL_UNSTABLE)) {
typeParams.add(ALL_CONTENT_TYPES);
monitorParams.add(Boolean.TRUE.toString());
} else if (request.policyIs(specificTypesPolicy)) {
} else if (request.policyIs(EXTENDED_PULL_NOTIFICATIONS)) {
typeParams.add(ARTICLE_CONTENT_TYPE);
typeParams.add(LIVE_BLOG_PACKAGE_CONTENT_TYPE);
typeParams.add(LIVE_BLOG_POST_CONTENT_TYPE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.ws.rs.core.MultivaluedHashMap;
import javax.ws.rs.core.MultivaluedMap;

public class PolicyBasedJsonFilter implements ApiFilter {

Expand All @@ -38,7 +36,7 @@ private static Pattern jsonPathToRegex(String jsonPath) {
return Pattern.compile(regex);
}

private final MultivaluedMap<Pattern, String> policyFilters;
private final Map<Pattern, List<String>> policyFilters;
private final JsonConverter jsonConverter;

/**
Expand All @@ -47,13 +45,15 @@ private static Pattern jsonPathToRegex(String jsonPath) {
* @param filters a map of JSONPath to required policy. A mapping to a null policy indicates that
* the path is returned in all requests (whitelisted).
*/
public PolicyBasedJsonFilter(MultivaluedMap<String, Policy> filters) {
this.policyFilters = new MultivaluedHashMap<>();
public PolicyBasedJsonFilter(Map<String, List<Policy>> filters) {
this.policyFilters = new HashMap<>();
filters.forEach(
(k, v) ->
this.policyFilters.put(
jsonPathToRegex(k),
(v == null) ? null : v.stream().map(Enum::toString).collect(Collectors.toList())));
Optional.ofNullable(v)
.map(f -> f.stream().map(Enum::toString).collect(Collectors.toList()))
.orElse(Collections.emptyList())));
this.jsonConverter = new JsonConverter(new ObjectMapper());
}

Expand All @@ -74,25 +74,30 @@ private boolean isSuccess(int httpStatus) {
return (httpStatus / 100) == 2;
}

private boolean isAllowedPath(String path, Set<String> policies) {
for (Map.Entry<Pattern, List<String>> entry : policyFilters.entrySet()) {
if (entry.getKey().matcher(path).matches()) {
List<String> pathPolicies = entry.getValue();
if (pathPolicies == null || pathPolicies.isEmpty()) {
return true;
}
private boolean isAllowedPath(String path, Set<String> requestPolicies) {
Optional<Pattern> pathKey =
policyFilters.keySet().stream()
.filter(pattern -> pattern.matcher(path).matches())
.findAny();

if (pathKey.isPresent()) {
List<String> pathPolicies = policyFilters.get(pathKey.get());
if (pathPolicies.isEmpty()) {
// There's no policy requirement for the path.
return true;
}

for (String pathPolicy : pathPolicies) {
if (policies.contains(pathPolicy)) {
return true;
}
for (String pathPolicy : pathPolicies) {
if (requestPolicies.contains(pathPolicy)) {
return true;
}
}
}

FluentLoggingBuilder.getNewInstance(CLASS_NAME, "isAllowedPath")
.withTransactionId(get("transaction_id"))
.withField(MESSAGE, path + " is not allowed by any policies among " + policies.toString())
.withField(
MESSAGE, path + " is not allowed by any policies among " + requestPolicies.toString())
.build()
.logDebug();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ public class NotificationsTypeFilterTest {

private final JsonConverter jsonConverter = JsonConverter.testConverter();

private final NotificationsTypeFilter filter =
new NotificationsTypeFilter(
jsonConverter, Policy.INTERNAL_UNSTABLE, Policy.EXTENDED_PULL_NOTIFICATIONS);
private final NotificationsTypeFilter filter = new NotificationsTypeFilter(jsonConverter);
private final MutableRequest request = mock(MutableRequest.class);
private final HttpPipelineChain chain = mock(HttpPipelineChain.class);
private final MultivaluedMap<String, Object> headers = new MultivaluedHashMap<>();
Expand Down

0 comments on commit 11774ab

Please sign in to comment.