Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add support for http4s AuthedRoutes middleware #374

Closed
wants to merge 1 commit into from
Closed

feat: add support for http4s AuthedRoutes middleware #374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion build.sbt
View file
Open in desktop
Expand Up @@ -14,7 +14,7 @@ val akkaVersion = "10.0.14"
val catsVersion = "1.6.0"
val catsEffectVersion = "1.0.0"
val circeVersion = "0.10.1"
val http4sVersion = "0.20.0"
val http4sVersion = "0.20.10"
val scalatestVersion = "3.0.8"
val javaparserVersion = "3.7.1"
val endpointsVersion = "0.8.0"
Expand Down Expand Up @@ -70,6 +70,7 @@ val exampleCases: List[ExampleCase] = List(
ExampleCase(sampleResource("redaction.yaml"), "redaction"),
ExampleCase(sampleResource("server1.yaml"), "tracer").args("--tracing"),
ExampleCase(sampleResource("server2.yaml"), "tracer").args("--tracing"),
ExampleCase(sampleResource("server3.yaml"), "tracer").args("--tracing", "--http4sAuthedRoutes"),
ExampleCase(sampleResource("pathological-parameters.yaml"), "pathological")
)

Expand Down
4 changes: 2 additions & 2 deletions modules/codegen/src/main/scala/com/twilio/guardrail/Context.scala
View file
Open in desktop
@@ -1,7 +1,7 @@
package com.twilio.guardrail

case class Context(framework: Option[String], tracing: Boolean)
case class Context(framework: Option[String], tracing: Boolean, http4sAuthedRoutes: Boolean)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR represents a significant time investment, I appreciate what you've done so far. One of the design principles that may not have been clear is that the core of guardrail does not actually know anything about implementation details of any libraries; as a result, having a top-level context boolean that only applies to a single framework is undesirable.

An alternate strategy could be what we do in the dropwizard generator, where we build SecurityRequirements both globally (set top-level in the OpenAPI specification) as well as locally (configured for an individual operation).

You could then use the presence or absence of SecurityRequirements to enable or disable this functionality.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other consideration here is that as this is a global boolean, there's no convenient way to combine authed and unauthed paths in the same configuration. By using this security context configuration, it could be turned on and off by route.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expected some feedback here. As you mention, it didn't fit so well with the rest of the design. The intention here was to not change the behavior of the existing solution if you didn't want to opt-in to it. I'll do another pass on it and see what I can find a better way to solve it.


object Context {
val empty = Context(None, tracing = false)
val empty = Context(None, tracing = false, http4sAuthedRoutes = false)
}
18 changes: 16 additions & 2 deletions modules/codegen/src/main/scala/com/twilio/guardrail/ServerGenerator.scala
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ case class Server[L <: LA](pkg: List[String], extraImports: List[L#Import], hand
case class TracingField[L <: LA](param: ScalaParameter[L], term: L#Term)
case class RenderedRoutes[L <: LA](
routes: List[L#Term],
secureRoutes: List[L#Term],
classAnnotations: List[L#Annotation],
methodSigs: List[L#MethodDeclaration],
supportDefinitions: List[L#Definition],
Expand Down Expand Up @@ -60,14 +61,27 @@ object ServerGenerator {
} yield (responseDefinitions, (operationId, tracingField, route, parameters, responses))
}
(responseDefinitions, serverOperations) = responseServerPair.unzip
renderedRoutes <- generateRoutes(context.tracing, resourceName, basePath, serverOperations, protocolElems, securitySchemes)
handlerSrc <- renderHandler(handlerName, renderedRoutes.methodSigs, renderedRoutes.handlerDefinitions, responseDefinitions.flatten)
renderedRoutes <- generateRoutes(context.tracing,
resourceName,
basePath,
serverOperations,
protocolElems,
securitySchemes,
context.http4sAuthedRoutes)
handlerSrc <- renderHandler(
handlerName,
renderedRoutes.methodSigs,
renderedRoutes.handlerDefinitions,
responseDefinitions.flatten,
renderedRoutes.secureRoutes.nonEmpty && context.http4sAuthedRoutes
)
extraRouteParams <- getExtraRouteParams(context.tracing)
classSrc <- renderClass(
resourceName,
handlerName,
renderedRoutes.classAnnotations,
renderedRoutes.routes,
renderedRoutes.secureRoutes,
extraRouteParams,
responseDefinitions.flatten,
renderedRoutes.supportDefinitions
Expand Down
2 changes: 2 additions & 0 deletions modules/codegen/src/main/scala/com/twilio/guardrail/core/CoreTermInterp.scala
View file
Open in desktop
Expand Up @@ -90,6 +90,8 @@ case class CoreTermInterp[L <: LA](defaultFramework: String,
Continue((sofar.copy(dtoPackage = value.trim.split('.').to[List]) :: already, xs))
case (sofar :: already, "--import" :: value :: xs) =>
Continue((sofar.copy(imports = sofar.imports :+ value) :: already, xs))
case (sofar :: already, "--http4sAuthedRoutes" :: xs) =>
Continue((sofar.copy(context = sofar.context.copy(http4sAuthedRoutes = true)) :: already, xs))
case (_, unknown) =>
debug("Unknown argument") >> Bail(UnknownArguments(unknown))
}
Expand Down
9 changes: 5 additions & 4 deletions modules/codegen/src/main/scala/com/twilio/guardrail/generators/AkkaHttpServerGenerator.scala
View file
Open in desktop
Expand Up @@ -114,7 +114,7 @@ object AkkaHttpServerGenerator {
} else Target.pure(None)
} yield res

case GenerateRoutes(tracing, resourceName, basePath, routes, protocolElems, securitySchemes) =>
case GenerateRoutes(tracing, resourceName, basePath, routes, protocolElems, securitySchemes, authedRoutes) =>
for {
renderedRoutes <- routes.traverse {
case (operationId, tracingFields, sr @ RouteMeta(path, method, operation, securityRequirements), parameters, responses) =>
Expand All @@ -129,13 +129,14 @@ object AkkaHttpServerGenerator {
RenderedRoutes[ScalaLanguage](
List(combinedRouteTerms),
List.empty,
List.empty,
methodSigs,
renderedRoutes.flatMap(_.supportDefinitions),
renderedRoutes.flatMap(_.handlerDefinitions)
)
}

case RenderHandler(handlerName, methodSigs, handlerDefinitions, responseDefinitions) =>
case RenderHandler(handlerName, methodSigs, handlerDefinitions, responseDefinitions, securityRequirements) =>
for {
_ <- Target.log.debug(s"renderHandler(${handlerName}, ${methodSigs}")
} yield q"""
Expand All @@ -152,15 +153,15 @@ object AkkaHttpServerGenerator {
} else Target.pure(List.empty)
} yield res

case RenderClass(resourceName, handlerName, _, combinedRouteTerms, extraRouteParams, responseDefinitions, supportDefinitions) =>
case RenderClass(resourceName, handlerName, _, routeTerms, secureRouteTerms, extraRouteParams, responseDefinitions, supportDefinitions) =>
for {
_ <- Target.log.debug(s"renderClass(${resourceName}, ${handlerName}, <combinedRouteTerms>, ${extraRouteParams})")
routesParams = List(param"handler: ${Type.Name(handlerName)}") ++ extraRouteParams
} yield List(q"""
object ${Term.Name(resourceName)} {
..${supportDefinitions};
def routes(..${routesParams})(implicit mat: akka.stream.Materializer): Route = {
..${combinedRouteTerms}
..${routeTerms ++ secureRouteTerms}
}

..${responseDefinitions}
Expand Down
16 changes: 8 additions & 8 deletions ...les/codegen/src/main/scala/com/twilio/guardrail/generators/EndpointsServerGenerator.scala
View file
Open in desktop
Expand Up @@ -8,14 +8,14 @@ import com.twilio.guardrail.protocol.terms.server._
object EndpointsServerGenerator {
object ServerTermInterp extends FunctionK[ServerTerm[ScalaLanguage, ?], Target] {
def apply[T](term: ServerTerm[ScalaLanguage, T]): Target[T] = term match {
case GenerateResponseDefinitions(operationId, responses, protocolElems) => ???
case BuildTracingFields(operation, resourceName, tracing) => ???
case GenerateRoutes(tracing, resourceName, basePath, routes, protocolElems, securitySchemes) => ???
case RenderHandler(handlerName, methodSigs, handlerDefinitions, responseDefinitions) => ???
case GetExtraRouteParams(tracing) => ???
case GenerateSupportDefinitions(tracing, securitySchemes) => ???
case RenderClass(resourceName, handlerName, annotations, combinedRouteTerms, extraRouteParams, responseDefinitions, supportDefinitions) => ???
case GetExtraImports(tracing) => ???
case GenerateResponseDefinitions(operationId, responses, protocolElems) => ???
case BuildTracingFields(operation, resourceName, tracing) => ???
case GenerateRoutes(tracing, resourceName, basePath, routes, protocolElems, securitySchemes, authedRoutes) => ???
case RenderHandler(handlerName, methodSigs, handlerDefinitions, responseDefinitions, securityRequirements) => ???
case GetExtraRouteParams(tracing) => ???
case GenerateSupportDefinitions(tracing, securitySchemes) => ???
case RenderClass(resourceName, handlerName, annotations, routeTerms, secureRouteTerms, extraRouteParams, responseDefinitions, supportDefinitions) => ???
case GetExtraImports(tracing) => ???
}
}
}