From 6a5e00bb4d714873c6196313472f8ef9770a4169 Mon Sep 17 00:00:00 2001 From: Fabrice Lecomte Date: Sat, 10 Apr 2021 23:45:16 +0200 Subject: [PATCH] Add validation on Constitution routes --- build.gradle.kts | 6 ++ .../common/request/PaginatedRequest.kt | 9 +-- .../constitution/routes/CreateConstitution.kt | 20 ++++++- .../constitution/routes/FindConstitutions.kt | 24 +++++++- src/main/resources/openapi.yaml | 16 ++++- .../constitution/find_constitutions.sql | 4 +- .../kotlin/integration/Constitution routes.kt | 58 +++++++++++++++++-- 7 files changed, 120 insertions(+), 17 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 2f95015..bfddad0 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -338,6 +338,12 @@ tasks.register("testComments", Test::class) { includeTags("comment") } } +tasks.register("testConstitutions", Test::class) { + group = "tests" + useJUnitPlatform { + includeTags("constitution") + } +} dependencyCheck { formats = listOf(ReportGenerator.Format.HTML, ReportGenerator.Format.XML) diff --git a/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt b/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt index 063c961..26a307a 100644 --- a/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt +++ b/src/main/kotlin/fr/dcproject/common/request/PaginatedRequest.kt @@ -6,9 +6,6 @@ interface PaginatedRequestI { } open class PaginatedRequest( - page: Int = 1, - limit: Int = 50 -) : PaginatedRequestI { - override val page: Int = if (page < 1) 1 else page - override val limit: Int = if (limit > 50) 50 else if (limit < 1) 1 else limit -} + override val page: Int = 1, + override val limit: Int = 50 +) : PaginatedRequestI diff --git a/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt b/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt index dfc69d0..cb4e11f 100644 --- a/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt +++ b/src/main/kotlin/fr/dcproject/component/constitution/routes/CreateConstitution.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.constitution.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.common.utils.receiveOrBadRequest @@ -15,6 +16,9 @@ import fr.dcproject.component.constitution.database.ConstitutionForUpdate.TitleF import fr.dcproject.component.constitution.database.ConstitutionRepository import fr.dcproject.component.constitution.routes.CreateConstitution.PostConstitutionRequest.Input import fr.dcproject.component.constitution.routes.CreateConstitution.PostConstitutionRequest.Input.Title +import io.konform.validation.Validation +import io.konform.validation.jsonschema.maxLength +import io.konform.validation.jsonschema.minLength import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -36,7 +40,6 @@ object CreateConstitution { val draft: Boolean = false, val versionId: UUID = UUID.randomUUID() ) { - class Title( val id: UUID = UUID.randomUUID(), val name: String, @@ -44,10 +47,25 @@ object CreateConstitution { ) { class ArticleRef(val id: UUID) } + + fun validate() = Validation { + Input::title { + minLength(10) + maxLength(80) + } + Input::titles onEach { + Title::name { + minLength(10) + maxLength(80) + } + } + }.validate(this) } } private fun getNewConstitution(input: Input, citizen: Citizen) = input.run { + validate().badRequestIfNotValid() + ConstitutionForUpdate>( id = UUID.randomUUID(), title = title, diff --git a/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt b/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt index f7184f2..c570753 100644 --- a/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt +++ b/src/main/kotlin/fr/dcproject/component/constitution/routes/FindConstitutions.kt @@ -1,5 +1,6 @@ package fr.dcproject.component.constitution.routes +import fr.dcproject.application.http.badRequestIfNotValid import fr.dcproject.common.response.toOutput import fr.dcproject.common.security.assert import fr.dcproject.component.auth.citizenOrNull @@ -8,6 +9,10 @@ import fr.dcproject.component.constitution.database.ConstitutionRepository import fr.dcproject.routes.PaginatedRequest import fr.dcproject.routes.PaginatedRequestI import fr.postgresjson.repository.RepositoryI +import io.konform.validation.Validation +import io.konform.validation.jsonschema.enum +import io.konform.validation.jsonschema.maximum +import io.konform.validation.jsonschema.minimum import io.ktor.application.call import io.ktor.http.HttpStatusCode import io.ktor.locations.KtorExperimentalLocationsAPI @@ -27,10 +32,27 @@ object FindConstitutions { val sort: String? = null, val direction: RepositoryI.Direction? = null, val search: String? = null - ) : PaginatedRequestI by PaginatedRequest(page, limit) + ) : PaginatedRequestI by PaginatedRequest(page, limit) { + fun validate() = Validation { + FindConstitutionsRequest::page { + minimum(1) + } + FindConstitutionsRequest::limit { + minimum(1) + maximum(50) + } + FindConstitutionsRequest::sort ifPresent { + enum( + "title", + "createdAt", + ) + } + }.validate(this) + } fun Route.findConstitutions(repo: ConstitutionRepository, ac: ConstitutionAccessControl) { get { + it.validate().badRequestIfNotValid() val constitutions = repo.find(it.page, it.limit, it.sort, it.direction, it.search) ac.assert { canView(constitutions.result, citizenOrNull) } call.respond( diff --git a/src/main/resources/openapi.yaml b/src/main/resources/openapi.yaml index ced4124..0dbd143 100644 --- a/src/main/resources/openapi.yaml +++ b/src/main/resources/openapi.yaml @@ -851,6 +851,12 @@ paths: type: array items: $ref: '#/components/schemas/ConstitutionListingResponse' + 400: + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' post: security: - JWTAuth: [ ] @@ -875,7 +881,11 @@ paths: 401: $ref: '#/components/responses/401' 400: - $ref: '#/components/responses/400' + description: BadReqest + content: + application/json: + schema: + $ref: '#/components/schemas/400' /constitutions/{constitution}: parameters: - $ref: '#/components/parameters/constitution' @@ -2019,6 +2029,8 @@ components: $ref: '#/components/schemas/UUID' title: type: string + minLength: 10 + maxLength: 80 example: Constitution for the liberty titles: @@ -2034,6 +2046,8 @@ components: $ref: '#/components/schemas/UUID' name: type: string + minLength: 10 + maxLength: 80 example: The liberties articles: diff --git a/src/main/resources/sql/functions/constitution/find_constitutions.sql b/src/main/resources/sql/functions/constitution/find_constitutions.sql index 1dae8c0..9f2be1c 100644 --- a/src/main/resources/sql/functions/constitution/find_constitutions.sql +++ b/src/main/resources/sql/functions/constitution/find_constitutions.sql @@ -22,14 +22,14 @@ begin case direction when 'asc' then case sort when 'title' then c.title - when 'created_at' then c.created_at::text + when 'createdAt' then c.created_at::text else null end end, case direction when 'desc' then case sort when 'title' then c.title - when 'created_at' then c.created_at::text + when 'createdAt' then c.created_at::text end end desc, diff --git a/src/test/kotlin/integration/Constitution routes.kt b/src/test/kotlin/integration/Constitution routes.kt index 75ce11d..271ca52 100644 --- a/src/test/kotlin/integration/Constitution routes.kt +++ b/src/test/kotlin/integration/Constitution routes.kt @@ -1,6 +1,8 @@ package integration -import integration.steps.`when`.Validate +import integration.steps.`when`.Validate.ALL +import integration.steps.`when`.Validate.REQUEST_BODY +import integration.steps.`when`.Validate.REQUEST_PARAM import integration.steps.`when`.`When I send a GET request` import integration.steps.`when`.`When I send a POST request` import integration.steps.`when`.`with body` @@ -9,6 +11,7 @@ import integration.steps.given.`Given I have constitution` import integration.steps.given.`Given I have constitutions` import integration.steps.given.`authenticated as` import integration.steps.then.`And have property` +import integration.steps.then.`And the response should contain` import integration.steps.then.`And the response should not be null` import integration.steps.then.`Then the response should be` import integration.steps.then.`which contains` @@ -28,12 +31,25 @@ class `Constitution routes` : BaseTest() { fun `I can get constitution list`() { withIntegrationApplication { `Given I have constitutions`(3) - `When I send a GET request`("/constitutions") `Then the response should be` OK and { + `When I send a GET request`("/constitutions?page=1&limit=10&sort=title&direction=desc") `Then the response should be` OK and { `And the response should not be null`() } } } + @Test + @Tag("BadRequest") + fun `I cannot get constitution list with wrong request`() { + withIntegrationApplication { + `Given I have constitutions`(3) + `When I send a GET request`("/constitutions?page=1&limit=5000&sort=title&direction=desc", ALL - REQUEST_PARAM) `Then the response should be` BadRequest and { + `And the response should not be null`() + `And the response should contain`("$.invalidParams[0].name", ".limit") + `And the response should contain`("$.invalidParams[0].reason", "must be at most '50'") + } + } + } + @Test fun `I can get constitution by ID`() { withIntegrationApplication { @@ -70,11 +86,11 @@ class `Constitution routes` : BaseTest() { """ { "versionId":"15814bb6-8d90-4c6a-a456-c3939a8ec75e", - "title":"Hello world!", + "title":"Cras sit amet sapien mattis nulla rutrum blandit.", "anonymous":true, "titles":[ { - "name":"plop" + "name":"Cras sit amet sapien mattis nulla rutrum blandit." } ] } @@ -83,7 +99,7 @@ class `Constitution routes` : BaseTest() { } `Then the response should be` Created and { `And the response should not be null`() `And have property`("$.versionId") `which contains` "15814bb6-8d90-4c6a-a456-c3939a8ec75e" - `And have property`("$.title") `which contains` "Hello world!" + `And have property`("$.title") `which contains` "Cras sit amet sapien mattis nulla rutrum blandit." } } } @@ -93,7 +109,7 @@ class `Constitution routes` : BaseTest() { fun `I cannot create an constitution if bad request`() { withIntegrationApplication { `Given I have citizen`("Henri", "Poincaré") - `When I send a POST request`("/constitutions", Validate.ALL - Validate.REQUEST_BODY) { + `When I send a POST request`("/constitutions", ALL - REQUEST_BODY) { `authenticated as`("Henri", "Poincaré") `with body`( """ @@ -113,4 +129,34 @@ class `Constitution routes` : BaseTest() { } `Then the response should be` BadRequest } } + + @Test + @Tag("BadRequest") + fun `I cannot create an constitution if request is not valid`() { + withIntegrationApplication { + `Given I have citizen`("Henri", "Poincaré") + `When I send a POST request`("/constitutions", ALL - REQUEST_BODY) { + `authenticated as`("Henri", "Poincaré") + `with body`( + """ + { + "versionId":"15814bb6-8d90-4c6a-a456-c3939a8ec75e", + "title":"too small", + "anonymous":true, + "titles":[ + { + "name":"too small" + } + ] + } + """ + ) + } `Then the response should be` BadRequest and { + `And the response should contain`("$.invalidParams[0].name", ".title") + `And the response should contain`("$.invalidParams[0].reason", "must have at least 10 characters") + `And the response should contain`("$.invalidParams[1].name", ".titles[0].name") + `And the response should contain`("$.invalidParams[1].reason", "must have at least 10 characters") + } + } + } }