Skip to content

[Cherry pick] "Add the support of bypassing JWT authn for CORS requests" to release 1.1#2165

Merged
duderino merged 2 commits intoistio:release-1.1from
lei-tang:cherry-pick-cors-bypass-to-release-1.1
Jun 13, 2019
Merged

[Cherry pick] "Add the support of bypassing JWT authn for CORS requests" to release 1.1#2165
duderino merged 2 commits intoistio:release-1.1from
lei-tang:cherry-pick-cors-bypass-to-release-1.1

Conversation

@lei-tang
Copy link
Contributor

What this PR does / why we need it: this PR is for the issue https://github.com/istio/proxy/issues/2160. The two PRs related to "Add the support of bypassing JWT authn for CORS requests" are in the master branch but not in the release 1.1 branch. This PR cherry-picks the two PRs from master branch to release 1.1 branch.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #2160

Special notes for your reviewer:

Release note:

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Apr 10, 2019
@lei-tang lei-tang requested a review from duderino April 10, 2019 17:17
@lei-tang
Copy link
Contributor Author

/cc @procyclinsur

@istio-testing
Copy link
Collaborator

@lei-tang: GitHub didn't allow me to request PR reviews from the following users: procyclinsur.

Note that only istio members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @procyclinsur

Instructions 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/test-infra repository.

@rustysys-dev
Copy link

Can we get someone to restart the checks? It seems like there wasn't vms available to perform them so they failed?

* Add the support of bypassing JWT authn for CORS requests

* Bail out earlier for CORS preflight requests

* Use OPTIONS constant value from Envoy

* Remove changing to lowercase
@lei-tang lei-tang force-pushed the cherry-pick-cors-bypass-to-release-1.1 branch from 8ba3ec7 to 485fd6f Compare April 12, 2019 18:05
@lei-tang
Copy link
Contributor Author

I rebased the PR to latest release 1.1 and git pushed the PR to restart the presubmit checks.

@lei-tang
Copy link
Contributor Author

The PR has passed all presubmit tests and is waiting for approvals from the reviewers.

@rustysys-dev
Copy link

@duderino @rshriram @qiwzhang Can we get this approved for the 1.1 release?

@rshriram
Copy link
Member

@lizan ?

@duderino
Copy link

@procyclinsur @lei-tang I need to push back more on 1.1.x PRs. At this point attention should really shift to master and then 1.2. So what are the implications of not merging this?

@rustysys-dev
Copy link

@procyclinsur @lei-tang I need to push back more on 1.1.x PRs. At this point attention should really shift to master and then 1.2. So what are the implications of not merging this?

@duderino Thank you for the response 😄! It makes sense to concentrate on 1.2, and from a business standpoint (for me) it may not be completely necessary to have this merged into the 1.1 branch if 1.2 will be coming within the next month, and will contain these commits?

The technical implications of not merging this commit are that Istio will continue to deny all traffic to cross-site jwt authenticated paths due to the preflight CORS request failing. CORS requests by specification do not/should not contain authentication credentials. Therefore the request is rejected before it even starts.

Also, I have been searching for documents describing policy for the Test and Release SIG, and have found nothing regarding release schedule. Does Istio currently have a fixed release schedule?

@duderino
Copy link

duderino commented Jun 2, 2019

@fpesce could you give this a security review? It is in master and probably in 1.2

@fpesce
Copy link

fpesce commented Jun 12, 2019

Not a CORS expert.
/lgtm

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fpesce, lei-tang
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: linsun

If they are not already assigned, you can assign the PR to them by writing /assign @linsun in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@duderino duderino merged commit e9dd2a0 into istio:release-1.1 Jun 13, 2019
@duderino
Copy link

/approve

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

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants