REP-7789 Added Rax Roles extension to OpenApi #2070
Conversation
| @@ -172,6 +173,7 @@ object OpenApiValidatorFilter { | |||
| "validation.request.accept.invalid" -> HttpServletResponse.SC_BAD_REQUEST, | |||
| "validation.request.accept.notAllowed" -> HttpServletResponse.SC_NOT_ACCEPTABLE, | |||
| "validation.schema.invalidJson" -> HttpServletResponse.SC_BAD_REQUEST, | |||
| RaxRolesValidator.RoleValidationMessageKey-> HttpServletResponse.SC_UNAUTHORIZED, | |||
There was a problem hiding this comment.
Missing a space before the ->.
There was a problem hiding this comment.
After thinking about it a little more, I think we are supposed to add a WWW-Authenticate header to the response when we set the status to 401. That would probably mean adding another case to the validation report match:
case Some(ValidationFailure(HttpServletResponse.SC_UNAUTHORIZED, message)) =>
logger.debug("blahblahblah")
httpResponse.addHeader("WWW-Authenticate", "blah blah")
httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, message)
| } | ||
|
|
||
| object RaxRolesValidator { | ||
| val RoleValidationMessageKey = "rax.roles" |
There was a problem hiding this comment.
By convention, we normally use final val to signify constants, right?
|
|
||
| class RaxRolesValidator extends CustomRequestValidator { | ||
| override def validate(request: Request, apiOperation: ApiOperation): ValidationReport = { | ||
| apiOperation.getOperation.getExtensions.asScala.get("x-rax-roles") |
There was a problem hiding this comment.
It might make sense to extract the x-rax-roles string into a constant.
| override def validate(request: Request, apiOperation: ApiOperation): ValidationReport = { | ||
| apiOperation.getOperation.getExtensions.asScala.get("x-rax-roles") | ||
| .map(_.asInstanceOf[java.util.ArrayList[String]]) | ||
| .find(configuredRoles => !request.getHeaderValues(ROLES).asScala.exists(configuredRoles.contains(_))) |
There was a problem hiding this comment.
configuredRoles.contains(_) can be simplified to configuredRoles.contains (dropping the parameter list with wildcard).
| class RaxRolesValidator extends CustomRequestValidator { | ||
| override def validate(request: Request, apiOperation: ApiOperation): ValidationReport = { | ||
| apiOperation.getOperation.getExtensions.asScala.get("x-rax-roles") | ||
| .map(_.asInstanceOf[java.util.ArrayList[String]]) |
There was a problem hiding this comment.
It is safe to cast to an ArrayList? Would casting to a List be safer? Is throwing an exception here acceptable if the user provided something other than a list as the value to our extension (in configuration)? Ideally, I guess, we would validate the document itself on configuration update, but that is a new challenge.
| } | ||
|
|
||
| object OpenApiValidatorFilterSimulation { | ||
| val HeaderMockRespStatus = "Mock-Origin-Res-Status" | ||
| val HeaderRole = "X-Role" |
| @@ -55,26 +55,38 @@ class OpenApiValidatorFilterSimulation extends AbstractReposeSimulation { | |||
| ) | |||
| } | |||
|
|
|||
| var feederRole: Array[Map[String, Any]] = | |||
There was a problem hiding this comment.
No idea why these are vars. I think I copied this pattern when I wrote this simulation.
Also, a feeder just has to be of type Iterator[Map[String, T]]:
https://gatling.io/docs/current/session/feeder
So really, if you make ScenariosRole a map and want to rely on implicits, this could just be:
val feederRole = Array(ScenariosRole)
Or (I think) you could drop the reliance on an implicit conversion and do:
val feederRole = Iterator.continually(ScenariosRole)
| val StatusCodeOk = 200 | ||
| val StatusCodeNoContent = 204 | ||
| val StatusCodeUnauthorized =401 |
There was a problem hiding this comment.
Missing a space between = and 401.
| private val ScenariosRole = Array[(String, Int)]( | ||
| ("foo", StatusCodeNoContent), | ||
| ("bar", StatusCodeNoContent), | ||
| ("baz", StatusCodeUnauthorized)) |
There was a problem hiding this comment.
I know this was done this way for consistency, but honestly, this should probably just be a map.
| @@ -126,7 +127,23 @@ This filter provides an RBAC mechanism for API's that don't conveniently fit int | |||
|
|
|||
| Please refer to the <<../filters/regex-rbac.adoc#,RegEx RBAC filter>> documentation for more information about the available configuration options. | |||
|
|
|||
| ==== 3. API Validator filter | |||
| ==== 3. OpenAPI Validator filter | |||
| Role based access control is provided by a custom extension to the OpenApi specification. | |||
No description provided.