From cf2881c890d243f82ce54f8d19a2f056c3556bff Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Tue, 17 Mar 2020 15:48:59 +0100 Subject: [PATCH] #42 Improve VoteVoter --- .../dcproject/security/voter/ArticleVoter.kt | 36 ++++++++++----- .../dcproject/security/voter/CommentVoter.kt | 2 +- .../security/voter/ConstitutionVoter.kt | 10 +--- .../security/voter/CommentVoterTest.kt | 39 +++++++++++++++- .../dcproject/security/voter/VoteVoterTest.kt | 46 ++++++++++++++++++- 5 files changed, 108 insertions(+), 25 deletions(-) diff --git a/src/main/kotlin/fr/dcproject/security/voter/ArticleVoter.kt b/src/main/kotlin/fr/dcproject/security/voter/ArticleVoter.kt index fe36380..c16b631 100644 --- a/src/main/kotlin/fr/dcproject/security/voter/ArticleVoter.kt +++ b/src/main/kotlin/fr/dcproject/security/voter/ArticleVoter.kt @@ -2,7 +2,6 @@ package fr.dcproject.security.voter import fr.dcproject.entity.ArticleAuthI import fr.dcproject.entity.ArticleI -import fr.dcproject.entity.ArticleSimpleI import fr.dcproject.entity.UserI import io.ktor.application.ApplicationCall import fr.dcproject.entity.Comment as CommentEntity @@ -27,7 +26,7 @@ class ArticleVoter : Voter { if (action == Action.VIEW) return view(subject, user) if (action == Action.DELETE) return delete(subject, user) if (action == Action.UPDATE) return update(subject, user) - if (action is CommentVoter.Action) return voteForComment(action) + if (action is CommentVoter.Action) return voteForComment(action, subject) if (action is VoteVoter.Action) return voteForVote(action, subject) if (action is Action) return Vote.DENIED @@ -67,23 +66,36 @@ class ArticleVoter : Voter { private fun voteForVote(action: VoteVoter.Action, subject: Any?): Vote { if (action == VoteVoter.Action.CREATE && subject is VoteEntity<*>) { val target = subject.target - if (target !is ArticleSimpleI) { - return Vote.ABSTAIN - } - if (target.isDeleted()) { + if (target is ArticleAuthI<*>) { + if (target.isDeleted()) { + return Vote.DENIED + } + } else if (target is ArticleI) { return Vote.DENIED } } return Vote.ABSTAIN } - private fun voteForComment(action: CommentVoter.Action): Vote { - if (action == CommentVoter.Action.CREATE) { - return Vote.GRANTED - } + private fun voteForComment(action: CommentVoter.Action, subject: Any?): Vote { + if (subject is CommentEntity<*>) { + val target = subject.target + if (target is ArticleAuthI<*>) { + if (target.isDeleted()) { + return Vote.DENIED + } + } else if (target is ArticleI) { + return Vote.DENIED + } + if (action == CommentVoter.Action.CREATE) { + return Vote.GRANTED + } - if (action == CommentVoter.Action.VIEW) { - return Vote.GRANTED + if (action == CommentVoter.Action.VIEW) { + return Vote.GRANTED + } + } else { + return Vote.DENIED } return Vote.ABSTAIN diff --git a/src/main/kotlin/fr/dcproject/security/voter/CommentVoter.kt b/src/main/kotlin/fr/dcproject/security/voter/CommentVoter.kt index af82b5d..a6aed79 100644 --- a/src/main/kotlin/fr/dcproject/security/voter/CommentVoter.kt +++ b/src/main/kotlin/fr/dcproject/security/voter/CommentVoter.kt @@ -19,7 +19,7 @@ class CommentVoter : Voter { override fun vote(action: ActionI, call: ApplicationCall, subject: Any?): Vote { val user = call.user - if (subject !is Comment<*> ) { + if (subject !is Comment<*>) { return Vote.DENIED } diff --git a/src/main/kotlin/fr/dcproject/security/voter/ConstitutionVoter.kt b/src/main/kotlin/fr/dcproject/security/voter/ConstitutionVoter.kt index a73a5bd..f114973 100644 --- a/src/main/kotlin/fr/dcproject/security/voter/ConstitutionVoter.kt +++ b/src/main/kotlin/fr/dcproject/security/voter/ConstitutionVoter.kt @@ -16,7 +16,7 @@ class ConstitutionVoter : Voter { override fun supports(action: ActionI, call: ApplicationCall, subject: Any?): Boolean { return (action is Action || action is CommentVoter.Action || action is VoteVoter.Action) - .and(subject is List<*> || subject is ConstitutionSimple<*, *>? || subject is VoteEntity<*> || subject is Comment<*>) + .and(subject is ConstitutionSimple<*, *>? || subject is VoteEntity<*> || subject is Comment<*>) } override fun vote(action: ActionI, call: ApplicationCall, subject: Any?): Vote { @@ -30,14 +30,6 @@ class ConstitutionVoter : Voter { return if (subject.isDeleted()) Vote.DENIED else Vote.GRANTED } - if (subject is List<*>) { - subject.forEach { - if (it !is ConstitutionSimple<*, *> || it.isDeleted()) { - return Vote.DENIED - } - } - return Vote.GRANTED - } return Vote.DENIED } diff --git a/src/test/kotlin/fr/dcproject/security/voter/CommentVoterTest.kt b/src/test/kotlin/fr/dcproject/security/voter/CommentVoterTest.kt index bb90a3b..a8e38ed 100644 --- a/src/test/kotlin/fr/dcproject/security/voter/CommentVoterTest.kt +++ b/src/test/kotlin/fr/dcproject/security/voter/CommentVoterTest.kt @@ -48,6 +48,23 @@ internal class CommentVoterTest { target = article1 ) + private val commentTargetDeleted = Comment( + content = "Hello", + createdBy = tesla, + target = Article( + content = "Hi", + createdBy = einstein, + description = "blablabla", + title = "Super article" + ).apply { deletedAt = DateTime.now() } + ) + + private val commentTargetNoUser = Comment( + content = "Hello", + createdBy = tesla, + target = ArticleRef() + ) + init { mockkStatic("fr.dcproject.security.voter.VoterKt") } @@ -65,7 +82,7 @@ internal class CommentVoterTest { } @Test - fun `can be view the comment`() = listOf(CommentVoter()).run { + fun `can be view the comment`() = listOf(CommentVoter(), ArticleVoter()).run { mockk { every { user } returns tesla.user }.let { @@ -110,7 +127,7 @@ internal class CommentVoterTest { } @Test - fun `can be create a comment`() = listOf(CommentVoter()).run { + fun `can be create a comment`() = listOf(CommentVoter(), ArticleVoter()).run { mockk { every { user } returns tesla.user }.let { @@ -118,6 +135,24 @@ internal class CommentVoterTest { } } + @Test + fun `can not be create a comment if target is deleted`() = listOf(CommentVoter(), ArticleVoter()).run { + mockk { + every { user } returns tesla.user + }.let { + can(CommentVoter.Action.CREATE, it, commentTargetDeleted) `should be` false + } + } + + @Test + fun `can not be create a comment if target has no user`() = listOf(CommentVoter(), ArticleVoter()).run { + mockk { + every { user } returns tesla.user + }.let { + can(CommentVoter.Action.CREATE, it, commentTargetNoUser) `should be` false + } + } + @Test fun `can not be create a comment with other creator`() = listOf(CommentVoter()).run { mockk { diff --git a/src/test/kotlin/fr/dcproject/security/voter/VoteVoterTest.kt b/src/test/kotlin/fr/dcproject/security/voter/VoteVoterTest.kt index cd5f942..4da8b13 100644 --- a/src/test/kotlin/fr/dcproject/security/voter/VoteVoterTest.kt +++ b/src/test/kotlin/fr/dcproject/security/voter/VoteVoterTest.kt @@ -52,6 +52,23 @@ internal class VoteVoterTest { note = 1 ) + private val voteOnDeleted = VoteEntity( + createdBy = tesla, + target = Article( + content = "Hi", + createdBy = einstein, + description = "blablabla", + title = "Super article" + ).apply { deletedAt = DateTime.now() }, + note = 1 + ) + + private val voteWithoutUser = VoteEntity( + createdBy = tesla, + target = ArticleRef(), + note = 1 + ) + init { mockkStatic("fr.dcproject.security.voter.VoterKt") } @@ -105,7 +122,7 @@ internal class VoteVoterTest { } @Test - fun `can be vote an article`() = listOf(VoteVoter()).run { + fun `can be vote an article`() = listOf(VoteVoter(), ArticleVoter()).run { mockk { every { user } returns tesla.user }.let { @@ -121,4 +138,31 @@ internal class VoteVoterTest { can(VoteVoter.Action.CREATE, it, vote1) `should be` false } } + + @Test + fun `can not be vote an article if article is deleted`() = listOf(VoteVoter(), ArticleVoter()).run { + mockk { + every { user } returns tesla.user + }.let { + can(VoteVoter.Action.CREATE, it, voteOnDeleted) `should be` false + } + } + + @Test + fun `can not be vote an article if article have no user`() = listOf(VoteVoter(), ArticleVoter()).run { + mockk { + every { user } returns tesla.user + }.let { + can(VoteVoter.Action.CREATE, it, voteWithoutUser) `should be` false + } + } + + @Test + fun `can not be comment an article if article is deleted`() = listOf(VoteVoter(), ArticleVoter()).run { + mockk { + every { user } returns tesla.user + }.let { + can(CommentVoter.Action.CREATE, it, voteOnDeleted) `should be` false + } + } } \ No newline at end of file