Wildcards are forbidden in CORS response headers to credentialed requests rather than with allowCredentials: True #4281
Conversation
k8s-ci-robot added do-not-merge/work-in-progress k8s-ci-robot added the cncf-cla: yes k8s-ci-robot added the do-not-merge/hold 
| Welcome @DamianSawicki! |
k8s-ci-robot added size/M | Hi @DamianSawicki. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
DamianSawicki force-pushed the rectifyCORSCredentials branch 6 times, most recently from 74f0e18 to e4200a6 Compare 
This is a fix to GEP-1767. It specifies that wildcards are not allowed in CORS response headers when the request is credentialed (rather than when the allowCredentials config field is true). The corresponding GitHub Issue is kubernetes-sigs#3861. Signed-off-by: Damian Sawicki <dsawicki@google.com>
DamianSawicki force-pushed the rectifyCORSCredentials branch from e4200a6 to 94261d2 Compare DamianSawicki changed the title 
k8s-ci-robot removed the do-not-merge/work-in-progress 
| /ok-to-test |
k8s-ci-robot added ok-to-test 
| Of course, i'll take a look today |
| Looks Good! |
The Gateway behavior on the reception of a credentialed request despite allowCredentials being false is not preciesly specified, so when considering responses to credentialed requests let us assume that allowCredentials is true. Signed-off-by: Damian Sawicki <dsawicki@google.com>
Replace "Cookie: pageAccess=2" with "Cookie: foo=bar" to avoid confusion. Signed-off-by: Damian Sawicki <dsawicki@google.com>
| // | ||
| // The `Access-Control-Allow-Origin` response header can only use `*` | ||
| // wildcard as value when the `AllowCredentials` field is false. | ||
| // Conversely, if the request `Origin` matches one of the configured |
There was a problem hiding this comment.
Are you planning to update the corresponding go types in this PR or save that for a follow up?
There was a problem hiding this comment.
Hi @robscott, thanks for review! I've just updated the Go struct, and this gave me an idea how to adjust the wording in the GEP. PTAL.
k8s-ci-robot added the approved This updates the Go struct in accordance with the GEP changes from kubernetes-sigs#4281 At the same time, I'm slightly adjusting the wording of the GEP itself.
| [APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DamianSawicki, robscott The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot added size/L This updates the Go struct in accordance with the GEP changes from kubernetes-sigs#4281 At the same time, I'm slightly adjusting the wording of the GEP itself.
DamianSawicki force-pushed the rectifyCORSCredentials branch from ed16f00 to d02cfb5 Compare This updates the Go struct in accordance with the GEP changes from kubernetes-sigs#4281 At the same time, I'm slightly adjusting the wording of the GEP itself.
DamianSawicki force-pushed the rectifyCORSCredentials branch from d02cfb5 to 209f73e Compare This updates the Go struct in accordance with the GEP changes from kubernetes-sigs#4281 At the same time, I'm slightly adjusting the wording of the GEP itself.
DamianSawicki force-pushed the rectifyCORSCredentials branch from 209f73e to a3c1762 Compare This updates the Go struct in accordance with the GEP changes from kubernetes-sigs#4281 At the same time, I'm slightly adjusting the wording of the GEP itself.
DamianSawicki force-pushed the rectifyCORSCredentials branch from a3c1762 to 4966936 Compare | /retest |
This updates the Go struct in accordance with the GEP changes from kubernetes-sigs#4281 At the same time, I'm slightly adjusting the wording of the GEP itself.
DamianSawicki force-pushed the rectifyCORSCredentials branch from 4966936 to 25eefd8 Compare k8s-ci-robot added size/M This updates the Go struct in accordance with the GEP changes from kubernetes-sigs#4281 At the same time, I'm slightly adjusting the wording of the GEP itself.
DamianSawicki force-pushed the rectifyCORSCredentials branch from 25eefd8 to 0650d7a Compare | @DamianSawicki: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
5 participants
This is a fix of #3861 in GEP-1767. It specifies that wildcards are not allowed in CORS response headers (
Access-Control-Allow-Origin,Access-Control-Allow-Methods,Access-Control-Allow-Headers, andAccess-Control-Expose-Headers) when the request is credentialed (rather than when theallowCredentialsconfig field is true).What type of PR is this?
/kind gep
What this PR does / why we need it:
It fixes an Issue (see below).
Which issue(s) this PR fixes:
Fixes #3861
Does this PR introduce a user-facing change?: