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

Add CORS support #36

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Add CORS support #36

merged 2 commits into from
Nov 6, 2023

Conversation

cben
Copy link
Member

@cben cben commented Nov 5, 2023

Closes #29. I hope.

Testing with this UI patch:

--- src/app/where/page.tsx
+++ src/app/where/page.tsx
@@ -43,7 +43,7 @@ export default function Where() {
   useEffect(() => {
     const fetchData = async () => {
       try {
-        const response = await fetch(`${process.env.API_URL}/schedule`)
+        const response = await fetch(`https://localhost:8443/schedule`)
         const fetchLocation = await response.json()

(setting API_URL seems not working right now)

Result

before after
browser CORS errors browser loads schedule
curl no CORS curl CORS headers

test commands used, for "simple" vs. preflight modes respectively:

curl -i --insecure -X GET -H 'Origin: foo.bar' https://localhost:8443/users
curl -i --insecure -X OPTIONS -H 'Origin: foo.bar' -H 'Access-Control-Request-Method: POST' -H 'Access-Control-Request-Headers: Content-Type' https://localhost:8443/users

  • Also documented some issues with the self-signed cert (out of scope here).

cben referenced this pull request in il-blood-donation-info/dracula-web-frontend Nov 5, 2023
@@ -20,6 +20,8 @@ COPY --from=build /app/blood-info blood-info
COPY --from=build /app/tls-self-signed-cert tls-self-signed-cert

# FIXME: This is a hack to get the self-signed certificate to work. There must be a better way.
# FIXME: This makes the secret `key.pem` also part of the image; uploading the image anywhere would leak it!
# FIXME: any prior `cert.pem` & `key.pem` you had in current dir are *also* leaked in the COPY layer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #37 to track this

@masayag
Copy link
Contributor

masayag commented Nov 6, 2023

Thank you, Beni!

@masayag masayag merged commit 7b419d1 into il-blood-donation-info:main Nov 6, 2023
1 check passed
@cben
Copy link
Member Author

cben commented Nov 6, 2023

Forgot to mention on choice of packages: rs/cors package is popular; chi router had forked it into their own go-chi/cors with major differences listed here: go-chi/cors#21. Looks like the more important stuff made it back into rs/cors.

Unlikely to matter for us, we're good with default CORS config and rs/cors looked like simpler bet with official support for many routers. Anyway would be easy to switch in future if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BACKEND][Feature] return CORS header
2 participants