From 73a7424115d993d9315053467803f833219c3499 Mon Sep 17 00:00:00 2001 From: Alex Karmanov Date: Tue, 16 Jan 2024 07:32:04 -0500 Subject: [PATCH] BACKLOG-22157 Update api, add dry run property, add tests (#6) * BACKLOG-22157 Update api, add dry run property, add tests * BACKLOG-22157 Fix test --- .../richtext/HtmlFilteringInterceptor.java | 19 +++++ .../RichTextConfigurationInterface.java | 2 + .../configuration/RichTextConfiguration.java | 11 +++ .../graphql/models/GqlRichTextConfig.java | 10 ++- .../models/GqlRichTextDisallowedConfig.java | 39 ++++++++++ .../models/RichTextConfigInterface.java | 11 +++ .../htmlFiltering/GqlHtmlFilteringQuery.java | 24 ++++-- ....jahia.modules.richtext.config-default.yml | 1 + tests/cypress/e2e/filteringAPI.cy.ts | 73 +++++++++++++++++++ .../fixtures/filteringAPI/config.graphql | 26 +++++++ .../fixtures/filteringAPI/preview.graphql | 17 +++++ 11 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextDisallowedConfig.java create mode 100644 src/main/java/org/jahia/modules/richtext/graphql/models/RichTextConfigInterface.java create mode 100644 tests/cypress/e2e/filteringAPI.cy.ts create mode 100644 tests/cypress/fixtures/filteringAPI/config.graphql create mode 100644 tests/cypress/fixtures/filteringAPI/preview.graphql diff --git a/src/main/java/org/jahia/modules/richtext/HtmlFilteringInterceptor.java b/src/main/java/org/jahia/modules/richtext/HtmlFilteringInterceptor.java index d44f5f3..44a83a3 100644 --- a/src/main/java/org/jahia/modules/richtext/HtmlFilteringInterceptor.java +++ b/src/main/java/org/jahia/modules/richtext/HtmlFilteringInterceptor.java @@ -53,10 +53,12 @@ import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Deactivate; import org.osgi.service.component.annotations.Reference; +import org.owasp.html.HtmlChangeListener; import org.owasp.html.PolicyFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Nullable; import javax.jcr.RepositoryException; import javax.jcr.Value; import javax.jcr.ValueFormatException; @@ -129,6 +131,23 @@ public Value beforeSetValue(JCRNodeWrapper node, String name, } } + if (filteringConfig.htmlSanitizerDryRun(resolveSite.getSiteKey())) { + logger.info(String.format("Dry run: Skipping Sanitization of [%s]", node.getProperty("text").getRealProperty().getPath())); + policyFactory.sanitize(content, new HtmlChangeListener() { + @Override + public void discardedTag(@Nullable Object o, String tag) { + logger.info(String.format("Removed tag: %s", tag)); + } + + @Override + public void discardedAttributes(@Nullable Object o, String tag, String... strings) { + logger.info(String.format("Removed attributes %s for tag %s", String.join(", ", strings), tag)); + } + }, null); + + return originalValue; + } + String result = policyFactory.sanitize(content); if (result != content && !result.equals(content)) { diff --git a/src/main/java/org/jahia/modules/richtext/RichTextConfigurationInterface.java b/src/main/java/org/jahia/modules/richtext/RichTextConfigurationInterface.java index d70a6f2..a625df8 100644 --- a/src/main/java/org/jahia/modules/richtext/RichTextConfigurationInterface.java +++ b/src/main/java/org/jahia/modules/richtext/RichTextConfigurationInterface.java @@ -20,4 +20,6 @@ public interface RichTextConfigurationInterface { public JSONObject getMergedJSONPolicy(String... siteKeys); public boolean configExists(String siteKey); + + public boolean htmlSanitizerDryRun(String siteKey); } diff --git a/src/main/java/org/jahia/modules/richtext/configuration/RichTextConfiguration.java b/src/main/java/org/jahia/modules/richtext/configuration/RichTextConfiguration.java index 2e3d003..e4be084 100644 --- a/src/main/java/org/jahia/modules/richtext/configuration/RichTextConfiguration.java +++ b/src/main/java/org/jahia/modules/richtext/configuration/RichTextConfiguration.java @@ -137,6 +137,17 @@ public boolean configExists(String siteKey) { return false; } + @Override + public boolean htmlSanitizerDryRun(String siteKey) { + if (configExists(siteKey)) { + JSONObject f = configs.get(siteKeyToPid.get(siteKey)).getJSONObject("htmlFiltering"); + + return f.has("htmlSanitizerDryRun") && f.getBoolean("htmlSanitizerDryRun"); + } + + return false; + } + private void mergeJsonObject(JSONObject target, JSONObject source) { for (String key : source.keySet()) { if (source.get(key) instanceof JSONObject) { diff --git a/src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextConfig.java b/src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextConfig.java index d894a2a..5ed7858 100644 --- a/src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextConfig.java +++ b/src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextConfig.java @@ -10,11 +10,12 @@ import java.util.Set; @GraphQLDescription("Model for richtext configuration") -public class GqlRichTextConfig { +public class GqlRichTextConfig implements RichTextConfigInterface { private Set protocols = new HashSet<>(); private Set elements = new HashSet<>(); private List attributes = new ArrayList<>(); + private GqlRichTextDisallowedConfig disallow = new GqlRichTextDisallowedConfig(); @GraphQLField @GraphQLName("protocols") @@ -36,4 +37,11 @@ public Set getElements() { public List getAttributes() { return attributes; } + + @GraphQLField + @GraphQLName("disallow") + @GraphQLDescription("Disallowed html elements and attributes") + public GqlRichTextDisallowedConfig getDisallow() { + return disallow; + } } diff --git a/src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextDisallowedConfig.java b/src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextDisallowedConfig.java new file mode 100644 index 0000000..871b10a --- /dev/null +++ b/src/main/java/org/jahia/modules/richtext/graphql/models/GqlRichTextDisallowedConfig.java @@ -0,0 +1,39 @@ +package org.jahia.modules.richtext.graphql.models; + +import graphql.annotations.annotationTypes.GraphQLDescription; +import graphql.annotations.annotationTypes.GraphQLField; +import graphql.annotations.annotationTypes.GraphQLName; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +@GraphQLDescription("Model for disallowed richtext configuration") +public class GqlRichTextDisallowedConfig implements RichTextConfigInterface { + + private Set protocols = new HashSet<>(); + private Set elements = new HashSet<>(); + private List attributes = new ArrayList<>(); + + @GraphQLField + @GraphQLName("protocols") + @GraphQLDescription("Protocols") + public Set getProtocols() { + return protocols; + } + + @GraphQLField + @GraphQLName("elements") + @GraphQLDescription("HTML elements") + public Set getElements() { + return elements; + } + + @GraphQLField + @GraphQLName("attributes") + @GraphQLDescription("HTML attributes") + public List getAttributes() { + return attributes; + } +} diff --git a/src/main/java/org/jahia/modules/richtext/graphql/models/RichTextConfigInterface.java b/src/main/java/org/jahia/modules/richtext/graphql/models/RichTextConfigInterface.java new file mode 100644 index 0000000..42f10f6 --- /dev/null +++ b/src/main/java/org/jahia/modules/richtext/graphql/models/RichTextConfigInterface.java @@ -0,0 +1,11 @@ +package org.jahia.modules.richtext.graphql.models; + +import java.util.List; +import java.util.Set; + +public interface RichTextConfigInterface { + + public Set getProtocols(); + public Set getElements(); + public List getAttributes(); +} diff --git a/src/main/java/org/jahia/modules/richtext/graphql/query/impl/htmlFiltering/GqlHtmlFilteringQuery.java b/src/main/java/org/jahia/modules/richtext/graphql/query/impl/htmlFiltering/GqlHtmlFilteringQuery.java index cd7ecf1..cbdd15d 100644 --- a/src/main/java/org/jahia/modules/richtext/graphql/query/impl/htmlFiltering/GqlHtmlFilteringQuery.java +++ b/src/main/java/org/jahia/modules/richtext/graphql/query/impl/htmlFiltering/GqlHtmlFilteringQuery.java @@ -76,7 +76,7 @@ public List getSitesWithActiveFiltering() { } @GraphQLField - @GraphQLName("richTextConfiguration") + @GraphQLName("richtextConfiguration") @GraphQLDescription("RichText filtering configuration for a given site") public GqlRichTextConfig getRichTextConfiguration(@GraphQLNonNull @GraphQLName("siteKey") @GraphQLDescription("Site key for the affected site") String siteKey) { RichTextConfigurationInterface filteringConfig = BundleUtils.getOsgiService(RichTextConfigurationInterface.class, null); @@ -93,24 +93,38 @@ public GqlRichTextConfig getRichTextConfiguration(@GraphQLNonNull @GraphQLName(" getElements(config, gqlRichTextConfig); getAttributes(config, gqlRichTextConfig); + if (config.has("disallow")) { + RichTextConfigInterface disallow = gqlRichTextConfig.getDisallow(); + config = config.getJSONObject("disallow"); + getProtocols(config, disallow); + getElements(config, disallow); + getAttributes(config, disallow); + } + return gqlRichTextConfig; } - private void getProtocols(JSONObject config, GqlRichTextConfig gqlRichTextConfig) { + private void getProtocols(JSONObject config, RichTextConfigInterface gqlRichTextConfig) { if (config.has("protocols")) { JSONArray protocols = config.getJSONArray("protocols"); protocols.forEach(p -> gqlRichTextConfig.getProtocols().add((String) p)); } } - private void getElements(JSONObject config, GqlRichTextConfig gqlRichTextConfig) { + private void getElements(JSONObject config, RichTextConfigInterface gqlRichTextConfig) { if (config.has("elements")) { JSONArray elements = config.getJSONArray("elements"); - elements.forEach(e -> ((JSONObject)e).getJSONArray("name").forEach(el -> gqlRichTextConfig.getElements().add((String) el))); + elements.forEach(e -> { + if (((JSONObject)e).get("name") instanceof JSONArray) { + ((JSONObject)e).getJSONArray("name").forEach(el -> gqlRichTextConfig.getElements().add((String) el)); + } else { + gqlRichTextConfig.getElements().add(((JSONObject)e).getString("name")); + } + }); } } - private void getAttributes(JSONObject config, GqlRichTextConfig gqlRichTextConfig) { + private void getAttributes(JSONObject config, RichTextConfigInterface gqlRichTextConfig) { if (config.has("attributes")) { JSONArray attributes = config.getJSONArray("attributes"); List a = gqlRichTextConfig.getAttributes(); diff --git a/src/main/resources/META-INF/configuration-default/org.jahia.modules.richtext.config-default.yml b/src/main/resources/META-INF/configuration-default/org.jahia.modules.richtext.config-default.yml index f2be321..7124bf3 100644 --- a/src/main/resources/META-INF/configuration-default/org.jahia.modules.richtext.config-default.yml +++ b/src/main/resources/META-INF/configuration-default/org.jahia.modules.richtext.config-default.yml @@ -1,4 +1,5 @@ htmlFiltering: + htmlSanitizerDryRun: false protocols: - http - https diff --git a/tests/cypress/e2e/filteringAPI.cy.ts b/tests/cypress/e2e/filteringAPI.cy.ts new file mode 100644 index 0000000..9121f58 --- /dev/null +++ b/tests/cypress/e2e/filteringAPI.cy.ts @@ -0,0 +1,73 @@ +import {createSite, deleteSite} from '@jahia/cypress'; +import {DocumentNode} from 'graphql'; +import {enableHtmlFiltering, installConfig} from '../fixtures/utils'; + +describe('HTML rich text filtering API', () => { + const siteKey = 'filteringSite'; + const text = '
Testing

Testing

Testing

'; + let previewMutation: DocumentNode; + let configQuery: DocumentNode; + + before(() => { + createSite(siteKey); + installConfig('configs/org.jahia.modules.richtext.config-filteringSite.yml'); + enableHtmlFiltering(siteKey); + previewMutation = require('graphql-tag/loader!../fixtures/filteringAPI/preview.graphql'); + configQuery = require('graphql-tag/loader!../fixtures/filteringAPI/config.graphql'); + }); + + after(() => { + deleteSite(siteKey); + }); + + it('filters via API and reports on removed elements/attributes', () => { + cy.apollo({ + mutation: previewMutation, + variables: { + text: text, + siteKey: siteKey + } + }).then(response => { + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.removedAttributes).length(1); + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.removedAttributes[0].attributes[0]).to.equal('removed-attribute'); + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.removedAttributes[0].element).to.equal('div'); + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.removedElements).length(1); + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.removedElements).contain('strong'); + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.html).contain('role="myRole"'); + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.html).contain('id="myId"'); + expect(response.data.richtextConfiguration.htmlFiltering.testFiltering.html).contain('

Testing

'); + }); + }); + + it('returns a list of configured elements and attributes', () => { + cy.apollo({ + query: configQuery, + variables: { + siteKey: siteKey + } + }).then(response => { + expect(response.data.richtextConfiguration.htmlFiltering.richtextConfiguration.attributes).length(37); + expect(response.data.richtextConfiguration.htmlFiltering.richtextConfiguration.attributes.find(a => a.attribute === 'class')).to.deep.equal({ + attribute: 'class', + elements: [], + isGlobal: true, + pattern: '(myclass1|myclass2)', + __typename: 'GqlRichTextConfigAttribute' + }); + expect(response.data.richtextConfiguration.htmlFiltering.richtextConfiguration.attributes.find(a => a.attribute === 'autoplay')).to.deep.equal({ + attribute: 'autoplay', + elements: [ + 'audio', + 'video' + ], + isGlobal: false, + pattern: null, + __typename: 'GqlRichTextConfigAttribute' + }); + expect(response.data.richtextConfiguration.htmlFiltering.richtextConfiguration.elements).length(70); + expect(response.data.richtextConfiguration.htmlFiltering.richtextConfiguration.protocols).length(3); + expect(response.data.richtextConfiguration.htmlFiltering.richtextConfiguration.disallow.elements).length(1); + expect(response.data.richtextConfiguration.htmlFiltering.richtextConfiguration.disallow.elements).contain('strong'); + }); + }); +}); diff --git a/tests/cypress/fixtures/filteringAPI/config.graphql b/tests/cypress/fixtures/filteringAPI/config.graphql new file mode 100644 index 0000000..94b5d21 --- /dev/null +++ b/tests/cypress/fixtures/filteringAPI/config.graphql @@ -0,0 +1,26 @@ +query Config($siteKey: String!) { + richtextConfiguration { + htmlFiltering { + richtextConfiguration(siteKey: $siteKey) { + elements + protocols + attributes { + attribute + elements + isGlobal + pattern + } + disallow { + elements + protocols + attributes { + attribute + elements + isGlobal + pattern + } + } + } + } + } +} diff --git a/tests/cypress/fixtures/filteringAPI/preview.graphql b/tests/cypress/fixtures/filteringAPI/preview.graphql new file mode 100644 index 0000000..1853d8e --- /dev/null +++ b/tests/cypress/fixtures/filteringAPI/preview.graphql @@ -0,0 +1,17 @@ +mutation PreviewFiltering($text: String!, $siteKey: String!) { + richtextConfiguration { + htmlFiltering { + testFiltering( + siteKey: $siteKey + html: $text + ) { + html + removedElements + removedAttributes { + element + attributes + } + } + } + } +}