server: Protect against cross-origin websocket requests

added 3 commits

• 73354969...df982193 - 2 commits from branch main
• 43c76ef0 - server: Protect against cross-origin websocket requests

Compare with previous version

LGTM overall ! If you think the hasAcceptableOriginHeader method will be reused and that other checks might be added (like rate limiting), you could refactor it as a middleware for better modularity and clarity.

def originValidationMiddleware(
    handler: (String, String, cask.Request) => cask.WebsocketResult
): (String, String, cask.Request) => cask.WebsocketResult = {
  (instanceId, userId, request) =>
    // Check if the request has a valid Origin header
    val isSourceValid = request.headers.get("host").flatMap(_.headOption).exists { host =>
      request.headers.get("origin").flatMap(_.headOption).exists { origin =>
        origin == s"http://$host" || origin == s"https://$host"
      }
    }

    if isSourceValid then
      handler(instanceId, userId, request) // Delegate to the main function
    else
      cask.Response("Invalid or missing 'Origin' header", 403) 
}
/* You can use this function to apply the checks, and it can be composed with more 
middleware as needed */
val validatedWebsocketHandler = originValidationMiddleware(websocketHandler)

Middleware helps keep the core handler clean and make the focus only on the main WebSocket logic :)

I like it! Can you turn it into a decorator with https://com-lihaoyi.github.io/cask/#extending-endpoints-with-decorators and push a commit on top of this MR?

I've done it ! If you could give me access rights to the repository so that i can push ! Thanks

Done!

mentioned in merge request !36 (closed)

added 5 commits

• 4eb86cd2 - feat: Add ScalaCheck
• 5c79ff60 - feat: OriginValidation Decorator Class
• 33db8a8a - tests: OriginValidation Test Class
• 34a6a103 - feat: Add Origin Validation Decorator to Websocket
• 383856de - docs: Add Documentation for OriginValidation Decorator

Compare with previous version

I'd avoid bringing in scalacheck just to generate a random string.

changed this line in version 6 of the diff

Can you take a look at https://docs3.scala-lang.org/toolkit/web-server-cookies-and-decorators.html#using-decorators ? I'd like to get things to be a bit shorter than what we ended up with, and perhaps a RawDecorator is the way to do that? Ideally we'd keep the decorator just next to the endpoint declaration, to make the code easily readable.

added 1 commit

• 7eb9c770 - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

Why a Generic class and not a RawDecorator ?

RawDecorator is a trait that extends Decorator[Response.Raw, Response.Raw, Any] which doesn't exactly match Decorator[_, cask.endpoints.WebsocketResult, _] that is a strict requirement for the Decorator (It doesn't compile).
So the only way i found was to make a generic class. I think for most of the HTTP communications a RawDecorator will work. But for the specific websocket function it's a different return type !

DevOps Side

Now from a Devops perspective imagine we make it shorter and inside the class WebServerRoutes class it would make the decorator specific to the websocket function which will make it less maintainable. Personally I think centralizing all the security related functions in a single object/folder, would be a good starting point to ensure :

• Separation of concerns
• Maintainability
• Reusability
• Single source of trust for sensitive functions !

We can also stop using cask decorators and try to find other solutions !

Let me know your preference, and I’ll refactor accordingly !

I think Cask uses implicit to lift between different response types, so I'd be inclined to write something like this:

  extension (request: cask.Request)
    def hasAcceptableOriginHeader: Boolean =
      // request.headers.get("sec-fetch-site").exists(_.contains("same-origin")) // Not supported in Safari
      request.headers.get("host").flatMap(_.headOption).exists: host =>
        request.headers.get("origin").flatMap(_.headOption).exists: origin =>
          origin == f"http://$host" || origin == f"https://$host"

  class withOriginValidation[T](implicit ofResponse: cask.model.Response[String] => T) extends cask.router.Decorator[T, T, Any]:
    override def wrapFunction(request: cask.Request, delegate: Delegate) =
      if request.hasAcceptableOriginHeader then
        delegate(Map())
      else
        cask.router.Result.Success:
          ofResponse(cask.Response(f"Invalid or missing 'Origin' header", 403))

Which I think you should be able to use as @withOriginValidation[cask.WebsocketResult]() on the def websocket endpoint, because WebSocketResult defines an implicit class Response whose constructor will match the implicit ofResponse argument?

I'd wait to create the decorators file until we have more than one of them (there's a cost to splitting things across files)

added 1 commit

• cfcd863f - server: Refactor websocket decorator implementation

Compare with previous version