From dd174a632744f1d6f6c5c8e5773ae0a6ed31d102 Mon Sep 17 00:00:00 2001 From: Hubert Van De Walle Date: Tue, 3 Nov 2020 18:13:45 +0100 Subject: [PATCH] Log discarded html tags + small refactor --- .../simplenotes/app/api/ApiNoteController.kt | 4 +- .../app/controllers/NoteController.kt | 4 +- .../be/simplenotes/app/routes/ApiRoutes.kt | 2 +- .../domain/security/HtmlSanitizer.kt | 22 +++++- .../domain/usecases/NoteService.kt | 23 ++++--- .../usecases/markdown/FlexmarkFactory.kt | 34 +++++++++ .../usecases/markdown/MarkdownConverter.kt | 69 ------------------- .../markdown/MarkdownConverterImpl.kt | 63 +++++++++++++++++ 8 files changed, 137 insertions(+), 84 deletions(-) create mode 100644 simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/FlexmarkFactory.kt create mode 100644 simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverterImpl.kt diff --git a/simplenotes-app/src/main/kotlin/be/simplenotes/app/api/ApiNoteController.kt b/simplenotes-app/src/main/kotlin/be/simplenotes/app/api/ApiNoteController.kt index 7ba07b0..4c226bc 100644 --- a/simplenotes-app/src/main/kotlin/be/simplenotes/app/api/ApiNoteController.kt +++ b/simplenotes-app/src/main/kotlin/be/simplenotes/app/api/ApiNoteController.kt @@ -27,7 +27,7 @@ class ApiNoteController( fun createNote(request: Request, loggedInUser: LoggedInUser): Response { val content = noteContentLens(request) - return noteService.create(loggedInUser.userId, content).fold( + return noteService.create(loggedInUser, content).fold( { Response(BAD_REQUEST) }, { uuidContentLens(UuidContent(it.uuid), Response(OK)) } ) @@ -45,7 +45,7 @@ class ApiNoteController( fun update(request: Request, loggedInUser: LoggedInUser): Response { val content = noteContentLens(request) - return noteService.update(loggedInUser.userId, uuidLens(request), content).fold( + return noteService.update(loggedInUser, uuidLens(request), content).fold( { Response(BAD_REQUEST) }, diff --git a/simplenotes-app/src/main/kotlin/be/simplenotes/app/controllers/NoteController.kt b/simplenotes-app/src/main/kotlin/be/simplenotes/app/controllers/NoteController.kt index 15d5b3b..09a7d0b 100644 --- a/simplenotes-app/src/main/kotlin/be/simplenotes/app/controllers/NoteController.kt +++ b/simplenotes-app/src/main/kotlin/be/simplenotes/app/controllers/NoteController.kt @@ -32,7 +32,7 @@ class NoteController( val markdownForm = request.form("markdown") ?: "" - return noteService.create(loggedInUser.userId, markdownForm).fold( + return noteService.create(loggedInUser, markdownForm).fold( { val html = when (it) { MissingMeta -> view.noteEditor( @@ -112,7 +112,7 @@ class NoteController( val markdownForm = request.form("markdown") ?: "" - return noteService.update(loggedInUser.userId, note.uuid, markdownForm).fold( + return noteService.update(loggedInUser, note.uuid, markdownForm).fold( { val html = when (it) { MissingMeta -> view.noteEditor( diff --git a/simplenotes-app/src/main/kotlin/be/simplenotes/app/routes/ApiRoutes.kt b/simplenotes-app/src/main/kotlin/be/simplenotes/app/routes/ApiRoutes.kt index 5485b67..e45a88e 100644 --- a/simplenotes-app/src/main/kotlin/be/simplenotes/app/routes/ApiRoutes.kt +++ b/simplenotes-app/src/main/kotlin/be/simplenotes/app/routes/ApiRoutes.kt @@ -44,7 +44,7 @@ class ApiRoutes( "/" bind POST to transaction.then(::createNote), "/search" bind POST to ::search, "/{uuid}" bind GET to ::note, - "/{uuid}" bind PUT to transaction.then(::note), + "/{uuid}" bind PUT to transaction.then(::update), ) ).withBasePath("/notes") } diff --git a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/security/HtmlSanitizer.kt b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/security/HtmlSanitizer.kt index 4e1b87d..7c39244 100644 --- a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/security/HtmlSanitizer.kt +++ b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/security/HtmlSanitizer.kt @@ -1,8 +1,13 @@ package be.simplenotes.domain.security +import be.simplenotes.types.LoggedInUser +import org.owasp.html.HtmlChangeListener import org.owasp.html.HtmlPolicyBuilder +import org.slf4j.LoggerFactory +import javax.inject.Singleton -internal object HtmlSanitizer { +@Singleton +class HtmlSanitizer { private val htmlPolicy = HtmlPolicyBuilder() .allowElements("a") .allowCommonBlockElements() @@ -16,5 +21,18 @@ internal object HtmlSanitizer { .requireRelNofollowOnLinks() .toFactory()!! - fun sanitize(unsafeHtml: String) = htmlPolicy.sanitize(unsafeHtml)!! + private val logger = LoggerFactory.getLogger(javaClass) + + private val htmlChangeListener = object : HtmlChangeListener { + override fun discardedTag(context: LoggedInUser?, elementName: String) { + logger.warn("Discarded tag $elementName for user $context") + } + + override fun discardedAttributes(context: LoggedInUser?, tagName: String, vararg attributeNames: String) { + logger.warn("Discarded attributes ${attributeNames.contentToString()} on tag $tagName for user $context") + } + } + + fun sanitize(userId: LoggedInUser, unsafeHtml: String) = + htmlPolicy.sanitize(unsafeHtml, htmlChangeListener, userId)!! } diff --git a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/NoteService.kt b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/NoteService.kt index ce3c9d8..c70fa92 100644 --- a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/NoteService.kt +++ b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/NoteService.kt @@ -8,10 +8,13 @@ import be.simplenotes.persistance.repositories.NoteRepository import be.simplenotes.persistance.repositories.UserRepository import be.simplenotes.search.NoteSearcher import be.simplenotes.search.SearchTerms +import be.simplenotes.types.LoggedInUser import be.simplenotes.types.Note import be.simplenotes.types.PersistedNote import be.simplenotes.types.PersistedNoteMetadata import java.util.* +import javax.annotation.PostConstruct +import javax.annotation.PreDestroy import javax.inject.Singleton @Singleton @@ -20,25 +23,26 @@ class NoteService( private val noteRepository: NoteRepository, private val userRepository: UserRepository, private val searcher: NoteSearcher, + private val htmlSanitizer: HtmlSanitizer, ) { - fun create(userId: Int, markdownText: String) = either.eager { + fun create(user: LoggedInUser, markdownText: String) = either.eager { val persistedNote = !markdownConverter.renderDocument(markdownText) - .map { it.copy(html = HtmlSanitizer.sanitize(it.html)) } + .map { it.copy(html = htmlSanitizer.sanitize(user, it.html)) } .map { Note(it.metadata, markdown = markdownText, html = it.html) } - .map { noteRepository.create(userId, it) } + .map { noteRepository.create(user.userId, it) } - searcher.indexNote(userId, persistedNote) + searcher.indexNote(user.userId, persistedNote) persistedNote } - fun update(userId: Int, uuid: UUID, markdownText: String) = either.eager { + fun update(user: LoggedInUser, uuid: UUID, markdownText: String) = either.eager { val persistedNote = !markdownConverter.renderDocument(markdownText) - .map { it.copy(html = HtmlSanitizer.sanitize(it.html)) } + .map { it.copy(html = htmlSanitizer.sanitize(user, it.html)) } .map { Note(it.metadata, markdown = markdownText, html = it.html) } - .map { noteRepository.update(userId, uuid, it) } + .map { noteRepository.update(user.userId, uuid, it) } - persistedNote?.let { searcher.updateIndex(userId, it) } + persistedNote?.let { searcher.updateIndex(user.userId, it) } persistedNote } @@ -78,7 +82,9 @@ class NoteService( fun countDeleted(userId: Int) = noteRepository.count(userId, deleted = true) + @PostConstruct fun indexAll() { + dropAllIndexes() val userIds = userRepository.findAll() userIds.forEach { id -> val notes = noteRepository.findAllDetails(id) @@ -88,6 +94,7 @@ class NoteService( fun search(userId: Int, searchTerms: SearchTerms) = searcher.search(userId, searchTerms) + @PreDestroy fun dropAllIndexes() = searcher.dropAll() fun makePublic(userId: Int, uuid: UUID) = noteRepository.makePublic(userId, uuid) diff --git a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/FlexmarkFactory.kt b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/FlexmarkFactory.kt new file mode 100644 index 0000000..b56063f --- /dev/null +++ b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/FlexmarkFactory.kt @@ -0,0 +1,34 @@ +package be.simplenotes.domain.usecases.markdown + +import com.vladsch.flexmark.ext.gfm.tasklist.TaskListExtension +import com.vladsch.flexmark.html.HtmlRenderer +import com.vladsch.flexmark.parser.Parser +import com.vladsch.flexmark.util.data.MutableDataSet +import io.micronaut.context.annotation.Factory +import javax.inject.Singleton + +@Factory +class FlexmarkFactory { + + @Singleton + fun parser(options: MutableDataSet) = Parser.builder(options).build() + + @Singleton + fun htmlRenderer(options: MutableDataSet) = HtmlRenderer.builder(options).build() + + @Singleton + fun options() = MutableDataSet().apply { + set(Parser.EXTENSIONS, listOf(TaskListExtension.create())) + set(TaskListExtension.TIGHT_ITEM_CLASS, "") + set(TaskListExtension.LOOSE_ITEM_CLASS, "") + set( + TaskListExtension.ITEM_DONE_MARKER, + """ """ + ) + set( + TaskListExtension.ITEM_NOT_DONE_MARKER, + """ """ + ) + set(HtmlRenderer.SOFT_BREAK, "
") + } +} diff --git a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverter.kt b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverter.kt index 833e4dc..c6e45fa 100644 --- a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverter.kt +++ b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverter.kt @@ -1,20 +1,8 @@ package be.simplenotes.domain.usecases.markdown import arrow.core.Either -import arrow.core.computations.either -import arrow.core.left -import arrow.core.right -import be.simplenotes.domain.validation.NoteValidations import be.simplenotes.types.NoteMetadata -import com.vladsch.flexmark.ext.gfm.tasklist.TaskListExtension -import com.vladsch.flexmark.html.HtmlRenderer -import com.vladsch.flexmark.parser.Parser -import com.vladsch.flexmark.util.data.MutableDataSet import io.konform.validation.ValidationErrors -import org.yaml.snakeyaml.Yaml -import org.yaml.snakeyaml.parser.ParserException -import org.yaml.snakeyaml.scanner.ScannerException -import javax.inject.Singleton sealed class MarkdownParsingError object MissingMeta : MarkdownParsingError() @@ -23,63 +11,6 @@ class ValidationError(val validationErrors: ValidationErrors) : MarkdownParsingE data class Document(val metadata: NoteMetadata, val html: String) -typealias MetaMdPair = Pair - interface MarkdownConverter { fun renderDocument(input: String): Either } - -@Singleton -internal class MarkdownConverterImpl : MarkdownConverter { - private val yamlBoundPattern = "-{3}".toRegex() - private fun splitMetaFromDocument(input: String): Either { - val split = input.split(yamlBoundPattern, 3) - if (split.size < 3) return MissingMeta.left() - return (split[1].trim() to split[2].trim()).right() - } - - private val yaml = Yaml() - private fun parseMeta(input: String): Either { - val load: Map = try { - yaml.load(input) - } catch (e: ParserException) { - return InvalidMeta.left() - } catch (e: ScannerException) { - return InvalidMeta.left() - } - - val title = when (val titleNode = load["title"]) { - is String, is Number -> titleNode.toString() - else -> return InvalidMeta.left() - } - - val tagsNode = load["tags"] - val tags = if (tagsNode !is List<*>) - emptyList() - else - tagsNode.map { it.toString() } - - return NoteMetadata(title, tags).right() - } - - private val parser: Parser - private val renderer: HtmlRenderer - - init { - val options = MutableDataSet() - options.set(Parser.EXTENSIONS, listOf(TaskListExtension.create())) - options.set(HtmlRenderer.SOFT_BREAK, "
") - parser = Parser.builder(options).build() - renderer = HtmlRenderer.builder(options).build() - } - - private fun renderMarkdown(markdown: String) = parser.parse(markdown).run(renderer::render) - - override fun renderDocument(input: String) = either.eager { - val (meta, md) = !splitMetaFromDocument(input) - val parsedMeta = !parseMeta(meta) - !Either.fromNullable(NoteValidations.validateMetadata(parsedMeta)).swap() - val html = renderMarkdown(md) - Document(parsedMeta, html) - } -} diff --git a/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverterImpl.kt b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverterImpl.kt new file mode 100644 index 0000000..6fcd9e6 --- /dev/null +++ b/simplenotes-domain/src/main/kotlin/be/simplenotes/domain/usecases/markdown/MarkdownConverterImpl.kt @@ -0,0 +1,63 @@ +package be.simplenotes.domain.usecases.markdown + +import arrow.core.Either +import arrow.core.computations.either +import arrow.core.left +import arrow.core.right +import be.simplenotes.domain.validation.NoteValidations +import be.simplenotes.types.NoteMetadata +import com.vladsch.flexmark.html.HtmlRenderer +import com.vladsch.flexmark.parser.Parser +import org.yaml.snakeyaml.Yaml +import org.yaml.snakeyaml.parser.ParserException +import org.yaml.snakeyaml.scanner.ScannerException +import javax.inject.Singleton + +private typealias MetaMdPair = Pair + +@Singleton +internal class MarkdownConverterImpl( + private val parser: Parser, + private val renderer: HtmlRenderer, +) : MarkdownConverter { + private val yamlBoundPattern = "-{3}".toRegex() + private fun splitMetaFromDocument(input: String): Either { + val split = input.split(yamlBoundPattern, 3) + if (split.size < 3) return MissingMeta.left() + return (split[1].trim() to split[2].trim()).right() + } + + private val yaml = Yaml() + private fun parseMeta(input: String): Either { + val load: Map = try { + yaml.load(input) + } catch (e: ParserException) { + return InvalidMeta.left() + } catch (e: ScannerException) { + return InvalidMeta.left() + } + + val title = when (val titleNode = load["title"]) { + is String, is Number -> titleNode.toString() + else -> return InvalidMeta.left() + } + + val tagsNode = load["tags"] + val tags = if (tagsNode !is List<*>) + emptyList() + else + tagsNode.map { it.toString() } + + return NoteMetadata(title, tags).right() + } + + private fun renderMarkdown(markdown: String) = parser.parse(markdown).run(renderer::render) + + override fun renderDocument(input: String) = either.eager { + val (meta, md) = !splitMetaFromDocument(input) + val parsedMeta = !parseMeta(meta) + !Either.fromNullable(NoteValidations.validateMetadata(parsedMeta)).swap() + val html = renderMarkdown(md) + Document(parsedMeta, html) + } +}