feat(common): return an InvalidRequest error when a parameter is in…

…valid - 6.3.20 (#1271) * wip: refacto params * wip: refacto params * wip: refacto params * wip: refacto params compiling * refacto: move param parser into the class they are building * feat: first working AllowedParameters annotation * feat: first working AllowedParameters annotation * feat: rename QueryParameter add allowed parameters to entityhandler * feat: add AllowedParameters to all specification endpoint + shortcut QP * feat: first PR comments * feat: first PR comments * feat: first PR comments * feat: move to queryparameter * feat: rename parameter params into queryParams * chore: misc fixes and renaming * feat: PR comments * feat: harmonize reception of queryParameters * feat: add tests * fix: error message formatting * Apply suggestions from code review Co-authored-by: Benoit Orihuela <[email protected]> * fix: error message formatting * fix: allowed parameters on entityAccessControl endpoints * Update search-service/src/main/kotlin/com/egm/stellio/search/csr/web/ContextSourceRegistrationHandler.kt Co-authored-by: Ranim Naimi <[email protected]> * fix: missing validated --------- Co-authored-by: Benoit Orihuela <[email protected]> Co-authored-by: Ranim Naimi <[email protected]>
stellio-hub · Dec 6, 2024 · 5468f6d · 5468f6d
1 parent 2aea98f
commit 5468f6d
Show file tree

Hide file tree

Showing 57 changed files with 904 additions and 566 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -58,6 +58,7 @@ subprojects {
         implementation("org.springframework.boot:spring-boot-starter-webflux")
         implementation("org.springframework.boot:spring-boot-starter-oauth2-resource-server")
         implementation("org.springframework.boot:spring-boot-starter-security")
+        implementation("org.springframework.boot:spring-boot-starter-validation")
         // it provides support for JWT decoding and verification
         implementation("org.springframework.security:spring-security-oauth2-jose")
 

diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml
@@ -733,7 +733,7 @@ style:
     active: true
   UnusedParameter:
     active: true
-    allowedNames: 'ignored|expected'
+    allowedNames: 'ignored|expected|queryParams'
   UnusedPrivateClass:
     active: true
   UnusedPrivateMember:

diff --git a/search-service/config/detekt/baseline.xml b/search-service/config/detekt/baseline.xml
@@ -9,7 +9,7 @@
     <ID>Filename:V0_29__JsonLd_migration.kt$db.migration.V0_29__JsonLd_migration.kt</ID>
     <ID>LongMethod:AttributeInstanceService.kt$AttributeInstanceService$@Transactional suspend fun create(attributeInstance: AttributeInstance): Either&lt;APIException, Unit&gt;</ID>
     <ID>LongMethod:EnabledAuthorizationServiceTests.kt$EnabledAuthorizationServiceTests$@Test fun `it should return serialized access control entities with other rigths if user is owner`()</ID>
-    <ID>LongMethod:EntityAccessControlHandler.kt$EntityAccessControlHandler$@PostMapping("/{subjectId}/attrs", consumes = [MediaType.APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE]) suspend fun addRightsOnEntities( @RequestHeader httpHeaders: HttpHeaders, @PathVariable subjectId: String, @RequestBody requestBody: Mono&lt;String&gt; ): ResponseEntity&lt;*&gt;</ID>
+    <ID>LongMethod:EntityAccessControlHandler.kt$EntityAccessControlHandler$@PostMapping("/{subjectId}/attrs", consumes = [MediaType.APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE]) suspend fun addRightsOnEntities( @RequestHeader httpHeaders: HttpHeaders, @PathVariable subjectId: String, @RequestBody requestBody: Mono&lt;String&gt;, @AllowedParameters @RequestParam queryParams: MultiValueMap&lt;String, String&gt; ): ResponseEntity&lt;*&gt;</ID>
     <ID>LongMethod:LinkedEntityServiceTests.kt$LinkedEntityServiceTests$@Test fun `it should inline entities up to the asked 2nd level`()</ID>
     <ID>LongMethod:PatchAttributeTests.kt$PatchAttributeTests.Companion$@JvmStatic fun mergePatchProvider(): Stream&lt;Arguments&gt;</ID>
     <ID>LongMethod:PatchAttributeTests.kt$PatchAttributeTests.Companion$@JvmStatic fun partialUpdatePatchProvider(): Stream&lt;Arguments&gt;</ID>
@@ -27,6 +27,7 @@
     <ID>LongParameterList:EntityAttributeService.kt$EntityAttributeService$( entityUri: URI, ngsiLdAttributes: List&lt;NgsiLdAttribute&gt;, expandedAttributes: ExpandedAttributes, createdAt: ZonedDateTime, observedAt: ZonedDateTime?, sub: Sub? )</ID>
     <ID>LongParameterList:EntityAttributeService.kt$EntityAttributeService$( entityUri: URI, ngsiLdAttributes: List&lt;NgsiLdAttribute&gt;, expandedAttributes: ExpandedAttributes, disallowOverwrite: Boolean, createdAt: ZonedDateTime, sub: Sub? )</ID>
     <ID>LongParameterList:EntityEventService.kt$EntityEventService$( updatedDetails: UpdatedDetails, sub: String?, tenantName: String, entityId: URI, entityTypesAndPayload: Pair&lt;List&lt;ExpandedTerm&gt;, String&gt;, serializedAttribute: Pair&lt;ExpandedTerm, String&gt;, overwrite: Boolean )</ID>
+    <ID>LongParameterList:TemporalEntityHandler.kt$TemporalEntityHandler$( @RequestHeader httpHeaders: HttpHeaders, @PathVariable entityId: URI, @PathVariable attrId: String, @PathVariable instanceId: URI, @RequestBody requestBody: Mono&lt;String&gt;, @AllowedParameters(notImplemented = [QP.LOCAL, QP.VIA]) @RequestParam queryParams: MultiValueMap&lt;String, String&gt; )</ID>
     <ID>LongParameterList:V0_29__JsonLd_migration.kt$V0_29__JsonLd_migration$( entityId: URI, attributeName: ExpandedTerm, datasetId: URI?, attributePayload: ExpandedAttributeInstance, ngsiLdAttributeInstance: NgsiLdAttributeInstance, defaultCreatedAt: ZonedDateTime )</ID>
     <ID>NestedBlockDepth:V0_29__JsonLd_migration.kt$V0_29__JsonLd_migration$override fun migrate(context: Context)</ID>
     <ID>SwallowedException:TemporalQueryUtils.kt$e: IllegalArgumentException</ID>

diff --git a/...src/main/kotlin/com/egm/stellio/search/authorization/service/EntityAccessRightsService.kt b/...src/main/kotlin/com/egm/stellio/search/authorization/service/EntityAccessRightsService.kt
@@ -23,8 +23,8 @@ import com.egm.stellio.shared.model.APIException
 import com.egm.stellio.shared.model.AccessDeniedException
 import com.egm.stellio.shared.model.EntityTypeSelection
 import com.egm.stellio.shared.model.NgsiLdAttribute
-import com.egm.stellio.shared.model.PaginationQuery
 import com.egm.stellio.shared.model.ResourceNotFoundException
+import com.egm.stellio.shared.queryparameter.PaginationQuery
 import com.egm.stellio.shared.util.AccessRight
 import com.egm.stellio.shared.util.AccessRight.CAN_ADMIN
 import com.egm.stellio.shared.util.AccessRight.CAN_READ

diff --git a/...ce/src/main/kotlin/com/egm/stellio/search/authorization/web/EntityAccessControlHandler.kt b/...ce/src/main/kotlin/com/egm/stellio/search/authorization/web/EntityAccessControlHandler.kt
@@ -1,5 +1,6 @@
 package com.egm.stellio.search.authorization.web
 
+import arrow.core.computations.ResultEffect.bind
 import arrow.core.left
 import arrow.core.raise.either
 import com.egm.stellio.search.authorization.service.AuthorizationService
@@ -12,10 +13,13 @@ import com.egm.stellio.search.entity.util.composeEntitiesQueryFromGet
 import com.egm.stellio.shared.config.ApplicationProperties
 import com.egm.stellio.shared.model.AccessDeniedException
 import com.egm.stellio.shared.model.BadRequestDataException
+import com.egm.stellio.shared.model.NgsiLdDataRepresentation.Companion.parseRepresentations
 import com.egm.stellio.shared.model.NgsiLdRelationship
 import com.egm.stellio.shared.model.toFinalRepresentation
 import com.egm.stellio.shared.model.toNgsiLdAttribute
 import com.egm.stellio.shared.model.toNgsiLdAttributes
+import com.egm.stellio.shared.queryparameter.AllowedParameters
+import com.egm.stellio.shared.queryparameter.QP
 import com.egm.stellio.shared.util.AccessRight
 import com.egm.stellio.shared.util.AuthContextModel.ALL_ASSIGNABLE_IAM_RIGHTS
 import com.egm.stellio.shared.util.AuthContextModel.ALL_IAM_RIGHTS
@@ -34,7 +38,6 @@ import com.egm.stellio.shared.util.checkAndGetContext
 import com.egm.stellio.shared.util.getApplicableMediaType
 import com.egm.stellio.shared.util.getAuthzContextFromLinkHeaderOrDefault
 import com.egm.stellio.shared.util.getSubFromSecurityContext
-import com.egm.stellio.shared.util.parseRepresentations
 import com.egm.stellio.shared.util.replaceDefaultContextToAuthzContext
 import com.egm.stellio.shared.web.BaseHandler
 import kotlinx.coroutines.reactive.awaitFirst
@@ -43,6 +46,7 @@ import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
 import org.springframework.http.ResponseEntity
 import org.springframework.util.MultiValueMap
+import org.springframework.validation.annotation.Validated
 import org.springframework.web.bind.annotation.DeleteMapping
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PathVariable
@@ -55,6 +59,7 @@ import org.springframework.web.bind.annotation.RestController
 import reactor.core.publisher.Mono
 import java.net.URI
 
+@Validated
 @RestController
 @RequestMapping("/ngsi-ld/v1/entityAccessControl")
 class EntityAccessControlHandler(
@@ -66,7 +71,8 @@ class EntityAccessControlHandler(
     @GetMapping("/entities", produces = [MediaType.APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE])
     suspend fun getAuthorizedEntities(
         @RequestHeader httpHeaders: HttpHeaders,
-        @RequestParam params: MultiValueMap<String, String>
+        @AllowedParameters(implemented = [QP.ID, QP.TYPE, QP.ATTRS, QP.COUNT, QP.OFFSET, QP.LIMIT])
+        @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
 
@@ -75,7 +81,7 @@ class EntityAccessControlHandler(
 
         val entitiesQuery = composeEntitiesQueryFromGet(
             applicationProperties.pagination,
-            params,
+            queryParams,
             contexts
         ).bind()
 
@@ -96,13 +102,13 @@ class EntityAccessControlHandler(
 
         val compactedEntities = compactEntities(entities, contexts)
 
-        val ngsiLdDataRepresentation = parseRepresentations(params, mediaType)
+        val ngsiLdDataRepresentation = parseRepresentations(queryParams, mediaType)
         buildQueryResponse(
             compactedEntities.toFinalRepresentation(ngsiLdDataRepresentation),
             count,
             "/ngsi-ld/v1/entityAccessControl/entities",
             entitiesQuery.paginationQuery,
-            params,
+            queryParams,
             mediaType,
             contexts
         )
@@ -114,6 +120,7 @@ class EntityAccessControlHandler(
     @GetMapping("/groups", produces = [MediaType.APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE])
     suspend fun getGroupsMemberships(
         @RequestHeader httpHeaders: HttpHeaders,
+        @AllowedParameters(implemented = [QP.COUNT, QP.OFFSET, QP.LIMIT])
         @RequestParam params: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
@@ -158,6 +165,7 @@ class EntityAccessControlHandler(
     @GetMapping("/users", produces = [MediaType.APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE])
     suspend fun getUsers(
         @RequestHeader httpHeaders: HttpHeaders,
+        @AllowedParameters(implemented = [QP.COUNT, QP.OFFSET, QP.LIMIT])
         @RequestParam params: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
@@ -204,7 +212,9 @@ class EntityAccessControlHandler(
     suspend fun addRightsOnEntities(
         @RequestHeader httpHeaders: HttpHeaders,
         @PathVariable subjectId: String,
-        @RequestBody requestBody: Mono<String>
+        @RequestBody requestBody: Mono<String>,
+        @AllowedParameters
+        @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
 
@@ -282,7 +292,9 @@ class EntityAccessControlHandler(
     @DeleteMapping("/{subjectId}/attrs/{entityId}")
     suspend fun removeRightsOnEntity(
         @PathVariable subjectId: String,
-        @PathVariable entityId: URI
+        @PathVariable entityId: URI,
+        @AllowedParameters
+        @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
 
@@ -305,7 +317,9 @@ class EntityAccessControlHandler(
     suspend fun updateSpecificAccessPolicy(
         @RequestHeader httpHeaders: HttpHeaders,
         @PathVariable entityId: URI,
-        @RequestBody requestBody: Mono<String>
+        @RequestBody requestBody: Mono<String>,
+        @AllowedParameters
+        @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
 
@@ -331,7 +345,9 @@ class EntityAccessControlHandler(
 
     @DeleteMapping("/{entityId}/attrs/specificAccessPolicy")
     suspend fun deleteSpecificAccessPolicy(
-        @PathVariable entityId: URI
+        @PathVariable entityId: URI,
+        @AllowedParameters
+        @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
 

diff --git a/search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceCaller.kt b/search-service/src/main/kotlin/com/egm/stellio/search/csr/service/ContextSourceCaller.kt
@@ -10,9 +10,7 @@ import com.egm.stellio.search.csr.model.MiscellaneousWarning
 import com.egm.stellio.search.csr.model.NGSILDWarning
 import com.egm.stellio.search.csr.model.RevalidationFailedWarning
 import com.egm.stellio.shared.model.CompactedEntity
-import com.egm.stellio.shared.util.QUERY_PARAM_GEOMETRY_PROPERTY
-import com.egm.stellio.shared.util.QUERY_PARAM_LANG
-import com.egm.stellio.shared.util.QUERY_PARAM_OPTIONS
+import com.egm.stellio.shared.queryparameter.QueryParameter
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 import org.springframework.core.codec.DecodingException
@@ -38,9 +36,9 @@ object ContextSourceCaller {
         val uri = URI("${csr.endpoint}$path")
 
         val queryParams = CollectionUtils.toMultiValueMap(params.toMutableMap())
-        queryParams.remove(QUERY_PARAM_GEOMETRY_PROPERTY)
-        queryParams.remove(QUERY_PARAM_OPTIONS) // only normalized request
-        queryParams.remove(QUERY_PARAM_LANG)
+        queryParams.remove(QueryParameter.GEOMETRY_PROPERTY.key)
+        queryParams.remove(QueryParameter.OPTIONS.key) // only normalized request
+        queryParams.remove(QueryParameter.LANG.key)
 
         val request = WebClient.create()
             .method(HttpMethod.GET)

diff --git a/...ervice/src/main/kotlin/com/egm/stellio/search/csr/web/ContextSourceRegistrationHandler.kt b/...ervice/src/main/kotlin/com/egm/stellio/search/csr/web/ContextSourceRegistrationHandler.kt
@@ -13,17 +13,19 @@ import com.egm.stellio.search.csr.service.ContextSourceRegistrationService
 import com.egm.stellio.shared.config.ApplicationProperties
 import com.egm.stellio.shared.model.APIException
 import com.egm.stellio.shared.model.AccessDeniedException
+import com.egm.stellio.shared.queryparameter.AllowedParameters
+import com.egm.stellio.shared.queryparameter.OptionsValue
+import com.egm.stellio.shared.queryparameter.PaginationQuery.Companion.parsePaginationParameters
+import com.egm.stellio.shared.queryparameter.QP
+import com.egm.stellio.shared.queryparameter.QueryParameter
 import com.egm.stellio.shared.util.JSON_LD_CONTENT_TYPE
 import com.egm.stellio.shared.util.JsonUtils.deserializeAsMap
-import com.egm.stellio.shared.util.QUERY_PARAM_OPTIONS
-import com.egm.stellio.shared.util.QUERY_PARAM_OPTIONS_SYSATTRS_VALUE
 import com.egm.stellio.shared.util.Sub
 import com.egm.stellio.shared.util.buildQueryResponse
 import com.egm.stellio.shared.util.checkAndGetContext
 import com.egm.stellio.shared.util.getApplicableMediaType
 import com.egm.stellio.shared.util.getContextFromLinkHeaderOrDefault
 import com.egm.stellio.shared.util.getSubFromSecurityContext
-import com.egm.stellio.shared.util.parsePaginationParameters
 import com.egm.stellio.shared.util.prepareGetSuccessResponseHeaders
 import com.egm.stellio.shared.web.BaseHandler
 import kotlinx.coroutines.reactive.awaitFirst
@@ -32,6 +34,7 @@ import org.springframework.http.HttpStatus
 import org.springframework.http.MediaType
 import org.springframework.http.ResponseEntity
 import org.springframework.util.MultiValueMap
+import org.springframework.validation.annotation.Validated
 import org.springframework.web.bind.annotation.DeleteMapping
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PathVariable
@@ -46,6 +49,7 @@ import java.net.URI
 
 @RestController
 @RequestMapping("/ngsi-ld/v1/csourceRegistrations")
+@Validated
 class ContextSourceRegistrationHandler(
     private val applicationProperties: ApplicationProperties,
     private val contextSourceRegistrationService: ContextSourceRegistrationService
@@ -81,16 +85,25 @@ class ContextSourceRegistrationHandler(
     @GetMapping(produces = [MediaType.APPLICATION_JSON_VALUE, JSON_LD_CONTENT_TYPE])
     suspend fun get(
         @RequestHeader httpHeaders: HttpHeaders,
-        @RequestParam params: MultiValueMap<String, String>
+        @AllowedParameters(
+            implemented = [QP.OPTIONS, QP.COUNT, QP.OFFSET, QP.LIMIT],
+            notImplemented = [
+                QP.ID, QP.TYPE, QP.ID_PATTERN, QP.ATTRS, QP.Q, QP.CSF,
+                QP.GEOMETRY, QP.GEOREL, QP.COORDINATES, QP.GEOPROPERTY,
+                QP.TIMEPROPERTY, QP.TIMEREL, QP.TIMEAT, QP.ENDTIMEAT,
+                QP.GEOMETRY_PROPERTY, QP.LANG, QP.SCOPEQ,
+            ]
+        )
+        @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
         val contexts = getContextFromLinkHeaderOrDefault(httpHeaders, applicationProperties.contexts.core).bind()
         val mediaType = getApplicableMediaType(httpHeaders).bind()
         val sub = getSubFromSecurityContext()
 
-        val includeSysAttrs = params.getOrDefault(QUERY_PARAM_OPTIONS, emptyList())
-            .contains(QUERY_PARAM_OPTIONS_SYSATTRS_VALUE)
+        val includeSysAttrs = queryParams.getOrDefault(QueryParameter.OPTIONS.key, emptyList())
+            .contains(OptionsValue.SYS_ATTRS.value)
         val paginationQuery = parsePaginationParameters(
-            params,
+            queryParams,
             applicationProperties.pagination.limitDefault,
             applicationProperties.pagination.limitMax
         ).bind()
@@ -107,7 +120,7 @@ class ContextSourceRegistrationHandler(
             contextSourceRegistrationsCount,
             "/ngsi-ld/v1/csourceRegistrations",
             paginationQuery,
-            params,
+            queryParams,
             mediaType,
             contexts
         )
@@ -123,9 +136,11 @@ class ContextSourceRegistrationHandler(
     suspend fun getByURI(
         @RequestHeader httpHeaders: HttpHeaders,
         @PathVariable contextSourceRegistrationId: URI,
-        @RequestParam options: String?
+        @AllowedParameters(implemented = [QP.OPTIONS])
+        @RequestParam queryParams: MultiValueMap<String, String>
     ): ResponseEntity<*> = either {
-        val includeSysAttrs = options == QUERY_PARAM_OPTIONS_SYSATTRS_VALUE
+        val options = queryParams.getFirst(QP.OPTIONS.key)
+        val includeSysAttrs = options?.contains(OptionsValue.SYS_ATTRS.value) ?: false
         val contexts = getContextFromLinkHeaderOrDefault(httpHeaders, applicationProperties.contexts.core).bind()
         val mediaType = getApplicableMediaType(httpHeaders).bind()
 
@@ -144,7 +159,11 @@ class ContextSourceRegistrationHandler(
      * Implements 6.9.3.3 - Delete ContextSourceRegistration
      */
     @DeleteMapping("/{contextSourceRegistrationId}")
-    suspend fun delete(@PathVariable contextSourceRegistrationId: URI): ResponseEntity<*> = either {
+    suspend fun delete(
+        @PathVariable contextSourceRegistrationId: URI,
+        @AllowedParameters // no query parameter is defined in the specification
+        @RequestParam queryParams: MultiValueMap<String, String>
+    ): ResponseEntity<*> = either {
         val sub = getSubFromSecurityContext()
         checkIsAllowed(contextSourceRegistrationId, sub).bind()