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.

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

CORS middleware not working as expected #2298

Closed
bjenkinsgit opened this issue Jul 14, 2023 · 11 comments
Closed

CORS middleware not working as expected #2298

bjenkinsgit opened this issue Jul 14, 2023 · 11 comments
Labels
💎 Bounty bug Something isn't working

Comments

@bjenkinsgit
Copy link

Describe the bug
Injecting CORS as middleware via the @@ operator does not seem to do anything.

To Reproduce
Steps to reproduce the behaviour:

  1. Create a simple ZIO HTTP app
  2. Somewhere define a CORS config instance like:

// Create CORS configuration
val config: CorsConfig =
CorsConfig(
allowedOrigin = {
case origin@Origin.Value(_, host, _) => Console.printLine(s"*** host=$host");Some(AccessControlAllowOrigin.Specific(origin))
case _ => Console.print("*** Taking NONE path in CorsConfig ***");None
},
allowedMethods = AccessControlAllowMethods(Method.PUT, Method.DELETE, Method.GET),
)

  1. Use that CorsConfig from a Request call like:

val nonZIOsApp: App[Any] =
Http.collect[Request] {
case Method.GET -> Root / "hello" => Response.text("Hello World x 2!")
} @@ cors(config)

Expected behaviour

I would expect that I would see some console output indicating that the config object is being used as middleware and that an http testing client like postman would show me the header when the endpoint is invoked.

WORKAROUND: Just use the .addHeaders() mechanism to add it per endpoint like so:

case Method.GET -> Root / "hello" => Response.text("Hello World x 2!").addHeaders(myHeaders)

...where...

val corsHeader = Header.AccessControlAllowOrigin.All
val myHeaders = Headers(corsHeader)

@bjenkinsgit bjenkinsgit added the bug Something isn't working label Jul 14, 2023
@jdegoes
Copy link
Member

jdegoes commented Jul 27, 2023

/bounty $75

@algora-pbc
Copy link

algora-pbc bot commented Jul 27, 2023

💎 $75 bounty created by ZIO
🙋 If you start working on this, comment /attempt #2298 along with your implementation plan
👉 To claim this bounty, submit a pull request that includes the text /claim #2298 somewhere in its body
📝 Before proceeding, please make sure you can receive payouts in your country
💵 Payment arrives in your account 2-5 days after the bounty is rewarded
💯 You keep 100% of the bounty award
🙏 Thank you for contributing to zio/zio-http!

👉 Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🔴 @aadarsh-nagrath Nov 23, 2023, 3:28:57 PM WIP
🔴 @sankalp142002 Nov 26, 2023, 1:08:34 PM WIP

@guersam
Copy link
Contributor

guersam commented Aug 2, 2023

The reason why there is no console output is misused zio.Console.

allowedOrigin = {
  case origin @ Origin.Value(_, host, _) => 
    Console.printLine(s"*** host=$host") // discarded ZIO program
    Some(AccessControlAllowOrigin.Specific(origin))
  case _ =>
    Console.print("*** Taking NONE path in CorsConfig ***") // discarded ZIO program
    None
},

Replacing Console.print(Line) with println will let the message printed when the CORS middleware is used.

@guersam
Copy link
Contributor

guersam commented Aug 2, 2023

Also, make sure that you're sending a CORS request when testing the CORS headers.

A CORS request is an HTTP request that includes an Origin header. It cannot be reliably identified as participating in the CORS protocol as the Origin header is also included for all requests whose method is neither GET nor HEAD.

A CORS-preflight request is a CORS request that checks to see if the CORS protocol is understood. It uses OPTIONS as method and includes these headers:

  • Access-Control-Request-Method
    Indicates which method a future CORS request to the same resource might use.

  • Access-Control-Request-Headers
    Indicates which headers a future CORS request to the same resource might use.

@aadarsh-nagrath
Copy link

aadarsh-nagrath commented Nov 23, 2023

/attempt #2298

Options

Copy link

algora-pbc bot commented Nov 26, 2023

Note

The user @aadarsh-nagrath is already attempting to complete issue #2298 and claim the bounty. We recommend checking in on @aadarsh-nagrath's progress, and potentially collaborating, before starting a new solution.

@sankalp142002
Copy link

/attempt #2298

Copy link

algora-pbc bot commented Nov 30, 2023

@aadarsh-nagrath: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

Copy link

algora-pbc bot commented Dec 3, 2023

@sankalp142002: Reminder that in 7 days the bounty will become up for grabs, so please submit a pull request before then 🙏

Copy link

algora-pbc bot commented Dec 10, 2023

The bounty is up for grabs! Everyone is welcome to /attempt #2298 🙌

@mrico
Copy link

mrico commented Mar 12, 2024

The middleware only sends the CORS headers in the response if the request contains an origin header:

https://github.com/zio/zio-http/blob/main/zio-http/shared/src/main/scala/zio/http/Middleware.scala#L121

Here is an example from a test case that works if origin is present:

testZ("/status returns CORS headers") {
    val request = Request.get(URL.decode("/status").toOption.get)
      .addHeader(Header.Origin("http", "localhost"))

    restApiServer.app
      .runZIO(request)
      .map { response =>
        assertEquals(
          response.headers.get(Header.AccessControlAllowOrigin).map(_.renderedValue),
          Some("http://localhost")
        )
      }
      .provide(deps)
  }

IMO the behaviour of the middleware is correct. CORS rules don't apply to requests without an origin header (request not from a web browser).

@jdegoes jdegoes closed this as completed Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants