Skip to content

Conversation

@taimoorzaeem
Copy link
Collaborator

This should fix #2441.

@taimoorzaeem taimoorzaeem force-pushed the config/cors branch 2 times, most recently from 20b6af8 to 3fa6ab5 Compare October 23, 2023 09:49
@taimoorzaeem
Copy link
Collaborator Author

@laurenceisla

With config's type as Maybe [Text] if the config is not specified by the user then it would evaluate to Nothing. However if the user specifies this config as cors-allowed-origins = "" it would evaluate to Just [].

Could this be a problem? If

{ Wai.corsOrigins = Just ([], True)

and

{ Wai.corsOrigins = Nothing

both return Access-Control-Allow-Origin: * then it should be fine I think. WDYT?

Copy link
Member

@laurenceisla laurenceisla left a 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.

Co-authored-by: Steve Chavez <[email protected]>
@steve-chavez
Copy link
Member

@taimoorzaeem Awesome work! 🚀

You can document it on https://postgrest.org/en/stable/references/api/cors.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Is there a way to setup CORS origins? Nginx config overwritten by postgrest

3 participants