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

feat: Add flag to control CORS Origin header #942

Closed
wants to merge 4 commits into from

Conversation

DeepDiver1975
Copy link
Contributor

rebased version of #504

should also fulfill now the requested change to only apply the CORS header if the origin header is set on request.

@DeepDiver1975
Copy link
Contributor Author

@Acconut anything missing to merged this? THX a lot

@Acconut
Copy link
Member

Acconut commented Jun 12, 2023

Thank you for taking on this PR! One thing that was missing from the original PR is that there was no check on the server to see if the origin from the request actually matched the allowed origin as configured using the command line flag. According to #504 (comment), this should be the correct behavior of a server. Could you implement this here as well? If so, I think we could move towards merging this.

@DeepDiver1975
Copy link
Contributor Author

I'll take care of this. ETA: gimme a day or two ;-)

@Acconut
Copy link
Member

Acconut commented Jun 12, 2023

Great! Thank you very much already!

@DeepDiver1975

This comment was marked as outdated.

@DeepDiver1975
Copy link
Contributor Author

@Acconut now .... af88b88

@DeepDiver1975
Copy link
Contributor Author

@Acconut mind approving the workflow? Would like to know if all test pass ..... THX

@DeepDiver1975
Copy link
Contributor Author

@Acconut 👍 or 👎 😀 - THX

@DeepDiver1975
Copy link
Contributor Author

Looks like we are having conflicts again .... rebasing .... 🤷

@DeepDiver1975
Copy link
Contributor Author

@Acconut please approve ci - thx

@DeepDiver1975
Copy link
Contributor Author

👋

@Acconut
Copy link
Member

Acconut commented Aug 29, 2023

I don't see any option to allow the CI to run for you as a first-time contributor, even though it should be there as explained in https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks. This might be a bug with GitHub. Could you try creating a new PR and seeing if that helps?

@DeepDiver1975
Copy link
Contributor Author

#987

@DeepDiver1975
Copy link
Contributor Author

@Acconut see above - THX

@Acconut
Copy link
Member

Acconut commented Aug 29, 2023

That works. I was able to approve the CI run for you in #987. Let's continue in there :)

@Acconut Acconut closed this Aug 29, 2023
@DeepDiver1975 DeepDiver1975 deleted the cors-2 branch August 29, 2023 15:19
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.

2 participants