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

NOJIRA fix body consumption in http4s #160

Merged
merged 3 commits into from
Jul 9, 2024
Merged

NOJIRA fix body consumption in http4s #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1911603
NOJIRA update sbt and scala versions
fserra-mdsol Jul 8, 2024
6f6b264
NOJIRA put explicit return types to implicit loggers
fserra-mdsol Jul 8, 2024
bf42749
NOJIRA make request body strict,
fserra-mdsol Jul 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions ...es/mauth-authenticator-http4s/src/main/scala/com/mdsol/mauth/http4s/MAuthMiddleware.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,32 +78,32 @@ object MAuthMiddleware {
else
extractAll(V2) orElse extractAll(V1)

fk(request.as[Array[Byte]].flatMap { byteArray =>
authHeaderTimeHeader.flatMap { authCtx: MAuthContext =>
val mAuthRequest: MAuthRequest = new MAuthRequest(
authCtx.authHeader,
byteArray,
request.method.name,
authCtx.timeHeader.toString,
request.uri.path.renderString,
request.uri.query.renderString
)

// this mimics MAuthDirectives in the akka package - really needed?
val req = if (!authenticator.isV2OnlyAuthenticate) {
mAuthRequest.setXmwsSignature(getHeaderValOrEmpty(V1.authHeaderName)) // dreadful mutating type
mAuthRequest.setXmwsTime(getHeaderValOrEmpty(V1.timeHeaderName))
mAuthRequest
} else mAuthRequest

authenticator.authenticate(req)(requestValidationTimeout).map(res => (res, authCtx))
fk(for {
strictBody <- request.toStrict(none)
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, toStrict enforces a request size limit doesn't it? Could pass that in as an option (potentially with a default to keep it backwards compatible?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, it doesn't enforce any limit - it would be recommended to enforce one, because toStrict only consumes the whole EntityBody and replaces any multipart header with a content-length (as a testament to its function), but there is no fixed size limit. In practice, this means that your application might blow up if the request is too large to be held in memory while it gets deserialised. I decided to not put any 'feature flag' on this, because I gauged that for our use cases there is no such risk - but if you know that it's not the case otherwise, I can add it.

byteArray <- strictBody.as[Array[Byte]]
authCtx <- authHeaderTimeHeader
mAuthRequest = new MAuthRequest(
authCtx.authHeader,
byteArray,
request.method.name,
authCtx.timeHeader.toString,
request.uri.path.renderString,
request.uri.query.renderString
)
req = if (!authenticator.isV2OnlyAuthenticate) {
mAuthRequest.setXmwsSignature(getHeaderValOrEmpty(V1.authHeaderName)) // dreadful mutating type
mAuthRequest.setXmwsTime(getHeaderValOrEmpty(V1.timeHeaderName))
mAuthRequest
} else mAuthRequest
res <- authenticator.authenticate(req)(requestValidationTimeout).map(res => (res, authCtx))
} yield res)
.flatMap { case (b, ctx) =>
if (b) http(AuthedRequest(ctx, request))
else logAndReturnDefaultUnauthorizedReq(s"Rejecting request as authentication failed")
}
.recoverWith { case MdsolAuthMissingHeaderRejection(hn) =>
logAndReturnDefaultUnauthorizedReq(s"Rejecting request as header $hn missing")
}
}).flatMap { case (b, ctx) =>
if (b) http(AuthedRequest(ctx, request))
else logAndReturnDefaultUnauthorizedReq(s"Rejecting request as authentication failed")
}.recoverWith { case MdsolAuthMissingHeaderRejection(hn) =>
logAndReturnDefaultUnauthorizedReq(s"Rejecting request as header $hn missing")
}
}

def httpRoutes[F[_]: Async](requestValidationTimeout: Duration, authenticator: Authenticator[F])(
Expand Down
3 changes: 2 additions & 1 deletion ...uth-authenticator-http4s/src/test/scala/com/mdsol/mauth/http4s/MAuthMiddlewareSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import org.http4s._
import org.http4s.{HttpRoutes, Request, Response}
import org.http4s.syntax.literals._
import org.http4s.Method._
import org.typelevel.log4cats._

import java.security.{PublicKey, Security}
import java.util.UUID
Expand All @@ -22,7 +23,7 @@ import org.typelevel.log4cats.noop.NoOpLogger

class MAuthMiddlewareSuite extends CatsEffectSuite {

implicit val logger = NoOpLogger[IO]
implicit val logger: Logger[IO] = NoOpLogger[IO]
private val route: HttpRoutes[IO] =
HttpRoutes.of {
case req if req.uri.path === path"/" =>
Expand Down
3 changes: 2 additions & 1 deletion ...henticator-http4s/src/test/scala/com/mdsol/mauth/http4s/MauthPublicKeyProviderSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ import scalacache.{Cache, Entry}

import java.security.PublicKey
import cats.implicits._
import org.typelevel.log4cats.Logger

import java.util.UUID

class MauthPublicKeyProviderSuite extends CatsEffectSuite {

implicit val logger = NoOpLogger[IO]
implicit val logger: Logger[IO] = NoOpLogger[IO]
private val MAUTH_PORT = PortFinder.findFreePort()
private val MAUTH_BASE_URL = s"http://localhost:$MAUTH_PORT"
private val MAUTH_URL_PATH = "/mauth/v1"
Expand Down
2 changes: 1 addition & 1 deletion project/BuildSettings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import sbt._
object BuildSettings {
val env: util.Map[String, String] = System.getenv()
val scala212 = "2.12.17"
val scala213 = "2.13.10"
val scala213 = "2.13.14"

lazy val basicSettings = Seq(
homepage := Some(new URL("https://github.com/mdsol/mauth-jvm-clients")),
Expand Down
2 changes: 1 addition & 1 deletion project/build.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
# suppress inspection "UnusedProperty"
sbt.version = 1.8.3
sbt.version =1.10.0