Skip to content

Commit

Permalink
Merge pull request #3420 from AtlasOfLivingAustralia/feature/issue3419
Browse files Browse the repository at this point in the history
Replaced grails markdown plugin with commonmark #3419
  • Loading branch information
chrisala authored Jan 27, 2025
2 parents 0a85acf + 08c7f69 commit 32771a5
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 31 deletions.
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ dependencies {
exclude module: "xercesImpl"
}
implementation 'org.grails.plugins:grails-cookie:2.0.3'
implementation 'org.grails.plugins:grails-markdown:3.0.0'

implementation 'org.apache.poi:ooxml-schemas:1.4'
implementation 'org.apache.poi:poi:4.1.2'
Expand All @@ -150,7 +149,8 @@ dependencies {
implementation "commons-io:commons-io:2.6"
implementation "org.seleniumhq.selenium:selenium-chrome-driver:3.14.0"
implementation "com.bertramlabs.plugins:asset-pipeline-grails:$assetPipelineVersion"
implementation group: 'com.googlecode.owasp-java-html-sanitizer', name: 'owasp-java-html-sanitizer', version: '20220608.1'
implementation "org.commonmark:commonmark:0.24.0"
implementation "com.googlecode.owasp-java-html-sanitizer:owasp-java-html-sanitizer:20240325.1"

compileOnly "io.micronaut:micronaut-inject-groovy"
console "org.grails:grails-console"
Expand Down
4 changes: 3 additions & 1 deletion grails-app/services/au/org/ala/merit/EmailService.groovy
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package au.org.ala.merit

import au.org.ala.merit.config.EmailTemplate
import au.org.ala.merit.util.MarkdownUtils
import groovy.util.logging.Slf4j


Expand All @@ -17,7 +18,8 @@ class EmailService {
def systemEmailAddress = grailsApplication.config.getProperty('fieldcapture.system.email.address')
try {
def subjectLine = settingService.getSettingText(mailSubjectTemplate, model)
def body = settingService.getSettingText(mailTemplate, model).markdownToHtml()
String bodyMarkdown = settingService.getSettingText(mailTemplate, model)
String body = MarkdownUtils.markdownToHtmlAndSanitise(bodyMarkdown)

log.info("Sending email: ${subjectLine} to: ${recipient}, from: ${sender}, cc:${ccList}, body: ${body}")
// This is to prevent spamming real users while testing.
Expand Down
27 changes: 3 additions & 24 deletions grails-app/taglib/au/org/ala/merit/FCTagLib.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,16 @@ package au.org.ala.merit

import au.org.ala.cas.util.AuthenticationCookieUtils
import au.org.ala.merit.config.ProgramConfig
import au.org.ala.merit.util.MarkdownUtils
import au.org.ala.web.AuthService
import bootstrap.Attribute
import com.naleid.grails.MarkdownService
import grails.converters.JSON
import grails.web.servlet.mvc.GrailsParameterMap
import groovy.util.logging.Slf4j
import groovy.xml.MarkupBuilder
import org.apache.commons.lang.WordUtils
import org.grails.web.json.JSONArray
import org.grails.web.json.JSONObject
import org.owasp.html.HtmlChangeListener
import org.owasp.html.HtmlPolicyBuilder
import org.owasp.html.PolicyFactory
import org.owasp.html.Sanitizers

@Slf4j
class FCTagLib {
Expand All @@ -25,12 +21,7 @@ class FCTagLib {
def commonService
def userService
def settingService
MarkdownService markdownService
AuthService authService
MetadataService metadataService

/** Allow simple formatting, links and text within p and divs by default */
def policy = (Sanitizers.FORMATTING & Sanitizers.LINKS & Sanitizers.BLOCKS) & new HtmlPolicyBuilder().allowTextIn("p", "div").toFactory()

def textField = { attrs ->
def outerClass = attrs.remove 'outerClass'
Expand Down Expand Up @@ -1170,23 +1161,11 @@ class FCTagLib {
def markdownToHtml = { Map attrs, body ->
String text = attrs.text ?: body()

out << markdownToHtmlAndSanitise(text)
out << MarkdownUtils.markdownToHtmlAndSanitise(text)
}

private String markdownToHtmlAndSanitise(String text) {
String html = markdownService.markdown(text)
internalSanitise(policy, html)
}

private static String internalSanitise(PolicyFactory policyFactory, String input, String imageId = '', String metadataName = '') {
policyFactory.sanitize(input, new HtmlChangeListener<Object>() {
void discardedTag(Object context, String elementName) {
log.warn("Dropping element $elementName in $imageId.$metadataName")
}
void discardedAttributes(Object context, String tagName, String... attributeNames) {
log.warn("Dropping attributes $attributeNames from $tagName in $imageId.$metadataName")
}
}, null)
MarkdownUtils.markdownToHtmlAndSanitise(text)
}

private static String getScoreLabels(def scoreIds, ProgramConfig config, Boolean includeService) {
Expand Down
39 changes: 39 additions & 0 deletions src/main/groovy/au/org/ala/merit/util/MarkdownUtils.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package au.org.ala.merit.util

import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
import org.commonmark.parser.Parser
import org.commonmark.renderer.html.HtmlRenderer
import org.owasp.html.HtmlChangeListener
import org.owasp.html.HtmlPolicyBuilder
import org.owasp.html.PolicyFactory
import org.owasp.html.Sanitizers

@CompileStatic
@Slf4j
class MarkdownUtils {

/** Allow simple formatting, links and text within p and divs by default */
static PolicyFactory policy = (Sanitizers.FORMATTING & Sanitizers.LINKS & Sanitizers.BLOCKS) & new HtmlPolicyBuilder().allowTextIn("p", "div").toFactory()

static String markdownToHtmlAndSanitise(String text) {
Parser parser = Parser.builder().build()
org.commonmark.node.Node document = parser.parse(text)
HtmlRenderer renderer = HtmlRenderer.builder().build()
String html = renderer.render(document)

internalSanitise(policy, html)
}

private static String internalSanitise(PolicyFactory policyFactory, String input, String imageId = '', String metadataName = '') {
policyFactory.sanitize(input, new HtmlChangeListener<Object>() {
void discardedTag(Object context, String elementName) {
log.warn("Dropping element $elementName in $imageId.$metadataName")
}
void discardedAttributes(Object context, String tagName, String... attributeNames) {
log.warn("Dropping attributes $attributeNames from $tagName in $imageId.$metadataName")
}
}, null)
}

}
6 changes: 2 additions & 4 deletions src/test/groovy/au/org/ala/merit/EmailServiceSpec.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class EmailServiceSpec extends Specification implements AutowiredTest{
List usersAndRoles = [admin1, grantManager1, editor]
EmailTemplate emailTemplate = EmailTemplate.DEFAULT_PLAN_SUBMITTED_EMAIL_TEMPLATE
String body = "body"
body.metaClass.markdownToHtml = { "Body" }
EmailParams email

when:
Expand All @@ -69,7 +68,7 @@ class EmailServiceSpec extends Specification implements AutowiredTest{
email.params.from == "[email protected]"
email.params.replyTo == "[email protected]"
email.params.subject == "Subject"
email.params.html == "Body"
email.params.html == "<p>body</p>\n"
}


Expand All @@ -88,7 +87,6 @@ class EmailServiceSpec extends Specification implements AutowiredTest{
List usersAndRoles = [admin1, admin2, editor]
EmailTemplate emailTemplate = EmailTemplate.DEFAULT_PLAN_APPROVED_EMAIL_TEMPLATE
String body = "body"
body.metaClass.markdownToHtml = { "Body" }
EmailParams email

when:
Expand All @@ -102,7 +100,7 @@ class EmailServiceSpec extends Specification implements AutowiredTest{
email.params.from == "[email protected]"
email.params.replyTo == "[email protected]"
email.params.subject == "Subject"
email.params.html == "Body"
email.params.html == "<p>body</p>\n"

}

Expand Down
50 changes: 50 additions & 0 deletions src/test/groovy/au/org/ala/merit/util/MarkdownUtilsSpec.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package au.org.ala.merit.util

import spock.lang.Specification

class MarkdownUtilsSpec extends Specification {

def "markdownToHtmlAndSanitise should convert markdown to HTML and sanitize it"() {
given:
String markdown = "# Heading\n\nThis is a [link](http://example.com)."

when:
String result = MarkdownUtils.markdownToHtmlAndSanitise(markdown)

then:
result == "<h1>Heading</h1>\n<p>This is a <a href=\"http://example.com\" rel=\"nofollow\">link</a>.</p>\n"
}

def "markdownToHtmlAndSanitise should remove disallowed tags"() {
given:
String markdown = "<script>alert('XSS');</script>"

when:
String result = MarkdownUtils.markdownToHtmlAndSanitise(markdown)

then:
result == "\n"
}

def "markdownToHtmlAndSanitise should allow simple formatting"() {
given:
String markdown = "**bold** *italic*"

when:
String result = MarkdownUtils.markdownToHtmlAndSanitise(markdown)

then:
result == "<p><strong>bold</strong> <em>italic</em></p>\n"
}

def "markdownToHtmlAndSanitise should allow text within p and div tags"() {
given:
String markdown = "<p>Paragraph</p><div>Division</div>"

when:
String result = MarkdownUtils.markdownToHtmlAndSanitise(markdown)

then:
result == "<p>Paragraph</p><div>Division</div>\n"
}
}

0 comments on commit 32771a5

Please sign in to comment.