-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat: add config to specify CORS origins #2986
Conversation
c30e87f
to
f3c603a
Compare
20b6af8
to
3fa6ab5
Compare
With config's type as Could this be a problem? If { Wai.corsOrigins = Just ([], True) and { Wai.corsOrigins = Nothing both return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both return Access-Control-Allow-Origin: * then it should be fine I think. WDYT?
Yes, I agree. Not setting anything or leaving it empty means "*" (any origin allowed). This also needs to be documented to avoid any confusion.
Edit: Ah, I just noticed that cors-allowed-origins = ""
translates to Just ([""], True)
, not Just ([], True)
, so any preflight and CORS will be invalidated. I guess it makes sense, other configurations behave the same way when they are present but empty (e.g. db-extra-search-path
). Again, needs to be documented. No need to change anything, you get that result due to the problem with the OPTIONS
implementation that I mention below.
LGTM! Good job! The only thing that is missing is to tidy up the OPTIONS requests to better use the middleware. But I believe that is a bigger problem with the implementation of OPTIONS itself, so it's outside of the scope of this PR.
3fa6ab5
to
7ec7acf
Compare
Co-authored-by: Steve Chavez <[email protected]>
@taimoorzaeem Awesome work! 🚀 You can document it on https://postgrest.org/en/stable/references/api/cors.html |
This should fix #2441.