fix: allow configuration of AllowWildcard
#880
Conversation
YassineElbouchaibi
changed the title
AllowWildcardAllowWildcard
| @@ -519,6 +519,11 @@ | |||
| "default": true, | |||
| "description": "The allowed credentials. The default value is to allow credentials. This allows the browser to send cookies and authentication headers." | |||
| }, | |||
| "allow_wildcard": { | |||
There was a problem hiding this comment.
@YassineElbouchaibi You also need to pass it to core.WithTLSConfig in instance.go otherwise the setting is not applied. How did you test it?
There was a problem hiding this comment.
Ah yes sorry missed that... I have added it to core.WithCors now. I did not test unfortunately as I have made these changes directly in the web and I don't have a working local setup. I was hoping the pipeline tests would pickup anything wrong. Let me know if you think something else is missing.
There was a problem hiding this comment.
I have approved the run. After the build, you can test the image with
docker pull ghcr.io/wundergraph/cosmo/router:pr-880
Would that work for you?
There was a problem hiding this comment.
Yes I definitely can test that, thanks!
There was a problem hiding this comment.
Ideally, you can add an integration test to https://github.com/wundergraph/cosmo/tree/main/router-tests but since we did not have any tests for CORS, I can help you with that (if needed). Let me know what you think.
There was a problem hiding this comment.
My bad. Images from external contributors aren't pushed due to insufficient permissions. You can build the image locally with cd router && docker build -t router:latest .
There was a problem hiding this comment.
Sorry for the delay, my local setup was not able to build the image locally because of the following:
so I had to go through various hoops to get it working..
But I can now confirm wild card origins work and stop working when the setting is changed to false.
Regarding integration tests, It will be hard for me to find time for it.. I'm not very familiar with golang let alone testing in golang.. but if you give me enough time I can look into it.
StarpTech
changed the title
AllowWildcardAllowWildcard
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
This MR fixes #879
Motivation and Context
Wildcard added to
cors.allow_originswon't work without this MR asAllowWildcardis alwaysfalse.Let me know if my PR is missing stuff!