From 55cd97078a9b3f07d34f644f923a3bfe315521a4 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sun, 17 Jan 2021 23:46:51 +0100 Subject: [PATCH] Refactoring of FollowVoter --- src/main/kotlin/application/Application.kt | 5 +- src/main/kotlin/application/KoinModule.kt | 2 + src/main/kotlin/routes/FollowArticle.kt | 16 +-- src/main/kotlin/routes/FollowConstitution.kt | 16 +-- src/main/kotlin/voter/FollowVoter.kt | 46 +++----- src/test/kotlin/unit/voter/FollowVoterTest.kt | 105 ++++++------------ 6 files changed, 67 insertions(+), 123 deletions(-) diff --git a/src/main/kotlin/application/Application.kt b/src/main/kotlin/application/Application.kt index 12b8303..d6d3713 100644 --- a/src/main/kotlin/application/Application.kt +++ b/src/main/kotlin/application/Application.kt @@ -96,7 +96,6 @@ fun Application.module(env: Env = PROD) { install(AuthorizationVoter) { voters = listOf( - FollowVoter(), OpinionVoter(), OpinionChoiceVoter() ) @@ -207,8 +206,8 @@ fun Application.module(env: Env = PROD) { updateMemberOfWorkgroup(get(), get()) /* TODO */ constitution(get(), get()) - followArticle(get()) - followConstitution(get()) + followArticle(get(), get()) + followConstitution(get(), get()) commentConstitution(get(), get()) voteArticle(get(), get(), get(), get()) voteConstitution(get(), get()) diff --git a/src/main/kotlin/application/KoinModule.kt b/src/main/kotlin/application/KoinModule.kt index ff9c8dc..d9cb0b1 100644 --- a/src/main/kotlin/application/KoinModule.kt +++ b/src/main/kotlin/application/KoinModule.kt @@ -24,6 +24,7 @@ import fr.dcproject.messages.Mailer import fr.dcproject.messages.NotificationEmailSender import fr.dcproject.repository.CommentConstitutionRepository import fr.dcproject.security.voter.ConstitutionVoter +import fr.dcproject.security.voter.FollowVoter import fr.dcproject.security.voter.VoteVoter import fr.postgresjson.connexion.Connection import fr.postgresjson.connexion.Requester @@ -127,6 +128,7 @@ val KoinModule = module { single { WorkgroupVoter() } single { ConstitutionVoter() } single { VoteVoter() } + single { FollowVoter() } // Elasticsearch Client single { diff --git a/src/main/kotlin/routes/FollowArticle.kt b/src/main/kotlin/routes/FollowArticle.kt index fc6bcad..465bee5 100644 --- a/src/main/kotlin/routes/FollowArticle.kt +++ b/src/main/kotlin/routes/FollowArticle.kt @@ -2,11 +2,11 @@ package fr.dcproject.routes import fr.dcproject.component.article.ArticleRef import fr.dcproject.component.auth.citizen +import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.citizen.Citizen import fr.dcproject.entity.FollowForUpdate -import fr.dcproject.security.voter.FollowVoter.Action.* -import fr.ktorVoter.assertCan -import fr.ktorVoter.assertCanAll +import fr.dcproject.security.voter.FollowVoter +import fr.dcproject.voter.assert import io.ktor.application.* import io.ktor.http.* import io.ktor.locations.* @@ -24,24 +24,24 @@ object FollowArticlePaths { } @KtorExperimentalLocationsAPI -fun Route.followArticle(repo: FollowArticleRepository) { +fun Route.followArticle(repo: FollowArticleRepository, voter: FollowVoter) { post { val follow = FollowForUpdate(target = it.article, createdBy = this.citizen) - assertCan(CREATE, follow) + voter.assert { canCreate(follow, citizenOrNull) } repo.follow(follow) call.respond(HttpStatusCode.Created) } delete { val follow = FollowForUpdate(target = it.article, createdBy = this.citizen) - assertCan(DELETE, follow) + voter.assert { canDelete(follow, citizenOrNull) } repo.unfollow(follow) call.respond(HttpStatusCode.NoContent) } get { repo.findFollow(citizen, it.article)?.let { follow -> - assertCan(VIEW, follow) + voter.assert { canView(follow, citizenOrNull) } call.respond(follow) } ?: call.respond(HttpStatusCode.NoContent) } @@ -49,7 +49,7 @@ fun Route.followArticle(repo: FollowArticleRepository) { get { val follows = repo.findByCitizen(it.citizen) if (follows.result.isNotEmpty()) { - assertCanAll(VIEW, follows.result) + voter.assert { canView(follows.result, citizenOrNull) } } call.respond(follows) } diff --git a/src/main/kotlin/routes/FollowConstitution.kt b/src/main/kotlin/routes/FollowConstitution.kt index e1c1d2d..c0be813 100644 --- a/src/main/kotlin/routes/FollowConstitution.kt +++ b/src/main/kotlin/routes/FollowConstitution.kt @@ -1,12 +1,12 @@ package fr.dcproject.routes import fr.dcproject.component.auth.citizen +import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.citizen.CitizenRef import fr.dcproject.entity.ConstitutionRef import fr.dcproject.entity.FollowForUpdate -import fr.dcproject.security.voter.FollowVoter.Action.* -import fr.ktorVoter.assertCan -import fr.ktorVoter.assertCanAll +import fr.dcproject.security.voter.FollowVoter +import fr.dcproject.voter.assert import io.ktor.application.* import io.ktor.http.* import io.ktor.locations.* @@ -24,31 +24,31 @@ object FollowConstitutionPaths { } @KtorExperimentalLocationsAPI -fun Route.followConstitution(repo: FollowConstitutionRepository) { +fun Route.followConstitution(repo: FollowConstitutionRepository, voter: FollowVoter) { post { val follow = FollowForUpdate(target = it.constitution, createdBy = this.citizen) - assertCan(CREATE, follow) + voter.assert { canCreate(follow, citizenOrNull) } repo.follow(follow) call.respond(HttpStatusCode.Created) } delete { val follow = FollowForUpdate(target = it.constitution, createdBy = this.citizen) - assertCan(DELETE, follow) + voter.assert { canDelete(follow, citizenOrNull) } repo.unfollow(follow) call.respond(HttpStatusCode.NoContent) } get { repo.findFollow(citizen, it.constitution)?.let { follow -> - assertCan(VIEW, follow) + voter.assert { canView(follow, citizenOrNull) } call.respond(follow) } ?: call.respond(HttpStatusCode.NotFound) } get { val follows = repo.findByCitizen(it.citizen) - assertCanAll(VIEW, follows.result) + voter.assert { canView(follows.result, citizenOrNull) } call.respond(follows) } } diff --git a/src/main/kotlin/voter/FollowVoter.kt b/src/main/kotlin/voter/FollowVoter.kt index 427a576..e6598e0 100644 --- a/src/main/kotlin/voter/FollowVoter.kt +++ b/src/main/kotlin/voter/FollowVoter.kt @@ -1,46 +1,26 @@ package fr.dcproject.security.voter -import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.citizen.CitizenI import fr.dcproject.entity.FollowI -import fr.dcproject.voter.NoSubjectDefinedException -import fr.ktorVoter.* -import io.ktor.application.* +import fr.dcproject.voter.Voter +import fr.dcproject.voter.VoterResponse import fr.dcproject.entity.Follow as FollowEntity -class FollowVoter : Voter { - enum class Action : ActionI { - CREATE, - DELETE, - VIEW +class FollowVoter : Voter() { + fun canCreate(subject: FollowI, citizen: CitizenI?): VoterResponse { + return if (citizen == null) denied("You must be connected to follow", "follow.create.notConnected") + else granted() } - override fun invoke(action: Any, context: ApplicationCall, subject: Any?): VoterResponseI { - if (action !is Action) return abstain() - if (subject !is FollowI) throw NoSubjectDefinedException(action) - - val citizen = context.citizenOrNull - if (action == Action.CREATE) { - return if (citizen == null) denied("You must be connected to follow", "follow.create.notConnected") - else granted() - } - - if (action == Action.DELETE) { - return if (citizen == null) denied("You must be connected to unfollow", "follow.delete.notConnected") - else granted() - } - - if (action == Action.VIEW) { - if (subject is FollowEntity<*>) { - return voteView(citizen, subject) - } - throw NoSubjectDefinedException(action) - } - - return abstain() + fun canDelete(subject: FollowI, citizen: CitizenI?): VoterResponse { + return if (citizen == null) denied("You must be connected to unfollow", "follow.delete.notConnected") + else granted() } - private fun voteView(citizen: CitizenI?, subject: FollowEntity<*>): VoterResponseI { + fun > canView(subjects: List, citizen: CitizenI?): VoterResponse = + canAll(subjects) { canView(it, citizen) } + + fun canView(subject: FollowEntity<*>, citizen: CitizenI?): VoterResponse { return if ((citizen != null && subject.createdBy.id == citizen.id) || !subject.createdBy.followAnonymous) granted() else denied("You cannot view an anonymous follow", "follow.view.anonymous") } diff --git a/src/test/kotlin/unit/voter/FollowVoterTest.kt b/src/test/kotlin/unit/voter/FollowVoterTest.kt index cd6709e..4d67015 100644 --- a/src/test/kotlin/unit/voter/FollowVoterTest.kt +++ b/src/test/kotlin/unit/voter/FollowVoterTest.kt @@ -3,28 +3,21 @@ package unit.voter import fr.dcproject.component.article.ArticleForView import fr.dcproject.component.auth.User import fr.dcproject.component.auth.UserI -import fr.dcproject.component.auth.citizenOrNull import fr.dcproject.component.citizen.Citizen import fr.dcproject.component.citizen.CitizenBasic import fr.dcproject.component.citizen.CitizenCart import fr.dcproject.component.citizen.CitizenI import fr.dcproject.entity.Follow import fr.dcproject.security.voter.FollowVoter -import fr.dcproject.voter.NoSubjectDefinedException -import fr.ktorVoter.ActionI -import fr.ktorVoter.Vote -import fr.ktorVoter.can -import fr.ktorVoter.canAll +import fr.dcproject.voter.Vote.DENIED +import fr.dcproject.voter.Vote.GRANTED import io.ktor.application.* -import io.mockk.every -import io.mockk.mockk import io.mockk.mockkStatic import org.amshove.kluent.`should be` import org.joda.time.DateTime import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestInstance -import org.junit.jupiter.api.assertThrows import org.junit.jupiter.api.parallel.Execution import org.junit.jupiter.api.parallel.ExecutionMode.CONCURRENT import java.util.* @@ -109,88 +102,58 @@ internal class FollowVoterTest { } @Test - fun `support follow`(): Unit = FollowVoter().run { - val p = object : ActionI {} - mockk { - every { citizenOrNull } returns tesla2 - }.let { - this(FollowVoter.Action.VIEW, it, follow1).vote `should be` Vote.GRANTED - assertThrows { - this(FollowVoter.Action.VIEW, it, article1) - } - this(p, it, follow1).vote `should be` Vote.ABSTAIN - } + fun `can be view the follow`() { + FollowVoter() + .canView(follow1, tesla2) + .vote `should be` GRANTED } @Test - fun `can be view the follow`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns tesla2 - }.let { - can(FollowVoter.Action.VIEW, it, follow1) `should be` true - } + fun `can be view the follow list`() { + FollowVoter() + .canView(listOf(follow1), tesla2) + .vote `should be` GRANTED } @Test - fun `can be view the follow list`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns tesla2 - }.let { - canAll(FollowVoter.Action.VIEW, it, listOf(follow1)) `should be` true - } + fun `can be view your anonymous follow`() { + FollowVoter() + .canView(followAnon, einstein3) + .vote `should be` GRANTED } @Test - fun `can be view your anonymous follow`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns einstein3 - }.let { - can(FollowVoter.Action.VIEW, it, followAnon) `should be` true - } + fun `can not be view the anonymous follow of other`() { + FollowVoter() + .canView(followAnon, tesla2) + .vote `should be` DENIED } @Test - fun `can not be view the anonymous follow of other`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns tesla2 - }.let { - can(FollowVoter.Action.VIEW, it, followAnon) `should be` false - } + fun `can be follow article`() { + FollowVoter() + .canCreate(follow1, tesla2) + .vote `should be` GRANTED } @Test - fun `can be follow article`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns tesla2 - }.let { - can(FollowVoter.Action.CREATE, it, follow1) `should be` true - } + fun `can not be follow article if not connected`() { + FollowVoter() + .canCreate(follow1, null) + .vote `should be` DENIED } @Test - fun `can not be follow article if not connected`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns null - }.let { - can(FollowVoter.Action.CREATE, it, follow1) `should be` false - } + fun `can be unfollow article`() { + FollowVoter() + .canDelete(follow1, tesla2) + .vote `should be` GRANTED } @Test - fun `can be unfollow article`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns tesla2 - }.let { - can(FollowVoter.Action.DELETE, it, follow1) `should be` true - } - } - - @Test - fun `can not be unfollow article if not connected`(): Unit = listOf(FollowVoter()).run { - mockk { - every { citizenOrNull } returns null - }.let { - can(FollowVoter.Action.DELETE, it, follow1) `should be` false - } + fun `can not be unfollow article if not connected`() { + FollowVoter() + .canDelete(follow1, null) + .vote `should be` DENIED } } \ No newline at end of file