How to handle CORS

[carllerche]

[shepmaster]

For the purposes of the playground, I need this specific configuration:

        chain.link_around(CorsMiddleware {
            // A null origin occurs when you make a request from a
            // page hosted on a filesystem, such as when you read the
            // Rust book locally
            allowed_origins: AllowedOrigins::Any { allow_null: true },
            allowed_headers: vec![UniCase("Content-Type".to_owned())],
            allowed_methods: vec![Get, Post],
            exposed_headers: vec![],
            allow_credentials: false,
            max_age_seconds: ONE_HOUR_IN_SECONDS,
            prefer_wildcard: true,
        });

[manifest]

I have some notes on the current implementation that could be worth a discussion :-)

• It seems to me that responses on preflight requests are resource specific, so they should be managed in an application logic as any other methods on resource:
  • Different methods might be allowed for different resources. For instance, allowing PUT method only make sense for resources that could be modified.
  • Different headers might be allowed for different methods. For instance, range header is only make sense for GET requests. There is access-control-request-method to understand intention of a user agent.
• null is a valid origin according to the specification. It should be possible to use something like allowed_origins = ["https://example.org", "null"]. In current implementation usage of null is limited by AllowedOrigins::Any { allow_null: true }.
• Probably it might make sense to deserialize origin into some struct representing a triple: scheme, host, port before comparing allowed_origins with value of origin header, because strings: "https://example.org" and "https://example.org:443" represent the same origin.
• Do we really need prefer_wildcard: true? Are there cases where using the exact value of origin header in access-control-allow-origin header wouldn't be enough?

[manifest]

I've also got the error trying to use into_config method of CorsBuilder in my app.

error[E0446]: private type `middleware::cors::config::Config` in public interface
   --> /Users/manifest/projects/github/tower-web/src/middleware/cors/builder.rs:90:5
    |
90  | /     pub fn into_config(self) -> Config {
91  | |         let Self {
92  | |             allow_credentials,
93  | |             allowed_headers,
...   |
129 | |         }
130 | |     }
    | |_____^ can't leak private type

[shepmaster]

As general background, we mostly followed corsware's lead on this because that best matched with my needs for the playground. It's certainly possible there are better ways!

responses on preflight requests are resource specific

That's entirely possible.

For the playground, I'd like to be able to blanket-apply the CORS rules to maintain parity, but dialing it back to prevent CORS requests for non-public APIs would actually be nice.

@carllerche, note that this is in the realm of "middleware vs. higher-order service vs. warp filter" which we were talking about elsewhere, so you might have thoughts.

null is a valid origin according to the specification. It should be possible

Is this not already supported via (untested) AllowedOrigins::from_iter(vec![HeaderValue::from_static("null")])?

because strings: "https://example.org" and "https://example.org:443" represent the same origin.

Are you referring to

6. If there is no port component of the URI:

   1. Let uri-port be the default port for the protocol given by uri-scheme.

Unfortunately, it conflicts with the CORS spec as written:

If the value of the Origin header is not a case-sensitive match for any of the values in list of origins

From a user POV, I think I'd expect your reading; is there any way to see what other implementations do? It's a tiny bit sad because it will add more failure cases due to parsing.

Do we really need prefer_wildcard: true

I am not aware of a strong need. It has the "benefit" of being a fixed amount of data to send, but perhaps removing features until a proven need arises is the right thing to do.

error trying to use into_config

That function probably should have been pub(crate) as it was mostly exposed for testing purposes. While you are trying to work with things, I'd say to make what you need public and then we can see what "should" be public.

[manifest]

Is this not already supported via (untested) AllowedOrigins::from_iter(vec![HeaderValue::from_static("null")])?

Probably it is. I've just wanted to note that using AllowedOrigins::Any { allow_null: true } is neither obvious, nor flexible.

Unfortunately, it conflicts with the CORS spec as written

That's true. It looks useful only from a user POV. Probably, it will be better to wait and see how people would use it.

I am not aware of a strong need. It has the "benefit" of being a fixed amount of data to send, but perhaps removing features until a proven need arises is the right thing to do.

Yes, I think the same. It looks like it doesn't add any value, but makes source code a bit noisy and require more tests.

While you are trying to work with things, I'd say to make what you need public and then we can see what "should" be public.

I see. I just want to try this on a real application, but can't without making a fork of the project :-)

[shepmaster]

try this on a real application

How would your application make use of the Config?

[manifest]

I see it in this way

#[derive(Clone, Debug)]
struct Resource;

#[derive(Response)]
struct ReadResponse {
    message: &'static str,
}

#[derive(Response)]
#[web(status = "200")]
#[web(header(name = "access-control-allow-methods", value = "GET"))]
#[web(header(name = "access-control-allow-headers", value = "authorization, cache-control, if-match, if-modified-since, if-none-match, if-unmodified-since, range"))]
#[web(header(name = "access-control-expose-headers", value = "content-length, content-type"))]
#[web(header(name = "access-control-allow-credentials", value = "true"))]
struct ResponsePreflight;

impl_web! {
    impl Resource {
        #[get("/resource")]
        #[content_type("json")]
        fn read(&self) -> Result<ReadResponse, ()> {
            Ok(Resource {
                message: "hello world",
            })
        }

        // The headers are resource specific:
        // - access-control-allow-methods
        // - access-control-allow-headers
        // - access-control-expose-headers
        // - access-control-allow-credentials
        #[options("/resource")]
        fn options(&self) -> Result<ResourcePreflight, ()> {
            Ok(ResourcePreflight{})
        }
    }
}

pub fn main() {
    env_logger::init();

    let addr = "0.0.0.0:8080".parse().expect("Invalid address");
    println!("Listening on http://{}", addr);

    // The only headers that might be configured in the middleware:
    // - access-control-allow-origin
    // - access-control-max-age
    // - vary
    let cors = CorsBuilder::new()
        .allow_origins(&["https://example.org"])
        .max_age(300)
        .build();

    ServiceBuilder::new()
        .resource(Resource)
        .middleware(CorsMiddleware::new(cors))
        .run(&addr)
        .unwrap();
}

[manifest]

We may also generate responses with respect to access-control-request-method header

fn read(&self) -> Result<ReadResponse, ()> {
    ...
}

fn create(&self) -> Result<CreateResponse, ()> {
    ...
}

#[options("/resource")]
fn options(&self) -> Result<ResourcePreflight, ()> {
    let access_control_request_method = ...
    match access_control_request_method {
        "GET" => Ok(ResourcePreflight{
            access_control_allow_header = "cache-control, if-match, if-modified-since, if-none-match, if-unmodified-since, range",
            access_control_expose_headers = "content-length, content-type",
            access_control_allow_credentials = false,
        }),
        "POST" => Ok(ResourcePreflight{
            access_control_allow_header = "authorization, content-type",
            access_control_allow_credentials = true,
        }),
        _ => unimplemented!()
    }
}

#[derive(Response)]
#[web(status = "200")]
#[web(header(name = "access-control-allow-methods", value = "GET, POST"))]
struct ResourcePreflight {
    #[web(header)]
    access_control_allow_header: String,
    #[web(header)]
    access_control_allow_header: String,
    #[web(header)]
    access_control_allow_credentials: bool
}

[manifest]

I've checked out corsware project. It seems like they deserialize origin. https://atorstling.github.io/corsware/corsware/enum.Origin.html

A struct which implements the concept 'Web Origin' as defined in https://tools.ietf.org/html/rfc6454

[shepmaster]

more failure cases due to parsing.

Perhaps worth looking into typed-headers (specifically this pull request) to leverage some of the parsing.

[shepmaster]

Or at hyperx

[manifest]

I've just got a thought that we may not need a CORS config. Attributes look like a better option. The code is cleaner, we don't need to implement OPTIONS handler, and specify allowed methods. Attributes on resource could be overridden by attributes of resource`s methods making the configuration quite flexible. Not sure if it is possible to implement.

#[derive(Clone, Debug)]
#[web(cors(allow_origin = ["https://example.org"]))]
#[web(cors(max_age = 300))]
struct Resource;

#[derive(Response)]
struct ReadResponse {
    message: &'static str,
}

impl_web! {
    impl Resource {
        #[get("/resource")]
        #[content_type("json")]
        #[web(cors(allow-headers = "authorization, cache-control, range"))]
        #[web(cors(expose-headers = "content-length, content-type"]))]
        #[web(cors(allow-credentials = true))]
        fn read(&self) -> Result<ReadResponse, ()> {
            Ok(Resource {
                message: "hello world",
            })
        }
    }
}

[shepmaster]

It feels like allowing only attributes would be highly resistant to reuse. For example, I believe I need CORS for these endpoints; note that there are endpoints across impls:

    impl SandboxFixme {
        #[post("/execute")]
        #[content_type("application/json")]
        fn execute(&self, body: ExecuteRequest) -> Result<ExecuteResponse> {}

        #[post("/evaluate.json")]
        #[content_type("application/json")]
        fn evaluate(&self, body: EvaluateRequest) -> Result<EvaluateResponse> {}
    }

    impl Meta {
        #[get("/meta/crates")]
        #[content_type("application/json")]
        fn meta_crates(&self) -> Result<MetaCratesResponse> {}
    }

With only attributes to solve the problem, I'd have to copy-paste several bits of configuration.

[manifest]

A middleware config is setting up CORS on an application level and attributes – on resource / method level probably the best option.

[qmx]

did we ever came to a consensus on how to deal with CORS?

[carllerche]

My take away of the rough consensus is:

• Attributes on resource / method level
• Middleware at the multi resource / application level.