Add CORSMiddleware to shortfin servers

[stbaione]

We need to add the CORSMiddleware to our server in order to receive requests from shark-ui, otherwise they get denied.

Here's a quick article on CORS.

@bjacobgordon and I would like to hear your thoughts on if we need further restrictions or not. Specifically, if the origins need to be restricted.

What the origin settings does, is disallow requests from clients, unless those clients either belong to some specific ip address or ip subnet, or belong to some specified domain. So, for example, if we had a top-secret govt project, we'd say, only these 4 ip addresses are allowed to send requests to our server.

Normally, in a commercial deployment scenario, this is a consideration. For us, I don't think it is. Here's why:

1.) We're open source. So, our model is to have people spin up this API anywhere and send requests to it from anywhere. So, we don't have a set of origins we would want to set it to. Otherwise, we would cut off our users. Maybe if we had a prod deployment, we would want to set it to only allow requests from some load balancer, but we don't have that right now.
2.) We're running open source software, serving open source models, so I don't know if we have security considerations like that.
3.) It's easy to spoof the origin header, so this isn't really a strong security feature anyways. We'd rely on infra components, like private net or load balancers, IF this was a commercial deployment.
4.) We're not yet at the point where we would really need to worry about this, and doing more advanced features would require time we don't have right now.

All of this to say, I think it's fine like this, but would like to hear other opinions. I would be open to making the settings configurable, where we default to wildcard, but users are able to specify specific security settings. That means that if someone wanted to deploy in production with stronger settings, they can. If we do this, I would like it to be done the same way on both servers, so would like to know if we'd like to do:

a.) Env var (something like "SHORTFIN_ALLOW_ORIGINS=a,b"
b.) Discuss an idea for a json based config that both servers could share for settings like this.
c.) Something else

[renxida]

Shortfin is not going to be anybody's first line of defense. Fine to disable origins checking.

[monorimet]

I would start with no restrictions unless it is expected to require a significant rework to add them in later. When we have an explicit need to introduce restrictions for some deployment scenario, we will be able to tailor our cors requirements in consideration of said scenario instead of trying to solve the world now.

Or as Xida said, leave it to deployers to manage connection security.

[stbaione]

I would start with no restrictions unless it is expected to require a significant rework to add them in later. When we have an explicit need to introduce restrictions for some deployment scenario, we will be able to tailor our cors requirements in consideration of said scenario instead of trying to solve the world now.

I think this is the way to go. It'll be very easy in the future. As simple as read config line, insert line into argument

[bjacobgordon]

Given the current context, seems like a wildcard origin is appropriate! It seems to be serving up the sd image data just fine in my testing.