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

CORS Headers missing when using auth-url and auth-signin with Oauth2-Proxy #8786

Closed
sameer-m-dev opened this issue Jul 5, 2022 · 29 comments · Fixed by #8814 or #9251
Closed

CORS Headers missing when using auth-url and auth-signin with Oauth2-Proxy #8786

sameer-m-dev opened this issue Jul 5, 2022 · 29 comments · Fixed by #8814 or #9251
Assignees
Labels
area/stabilization Work for increasing stabilization of the ingress-nginx codebase help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@sameer-m-dev
Copy link

What happened:
I have added the following annotations in my Ingress definition

nginx.ingress.kubernetes.io/auth-url: "https://api-auth.xyz.com/oauth2/auth"
nginx.ingress.kubernetes.io/auth-signin: "https://api-auth.xyz.com/oauth2/start?rd=$scheme://$best_http_host$request_uri"

with the host configured as api.xyz.com and the OIDC issuers being keycloak

On doing a curl to https://api.xyz.com/api/v1/onboard/getAssociatedOrganization here is the response I have gotten from NGINX which does not include any kind of CORS-specific headers

* Using HTTP2, server supports multiplexing
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x7fc205013600)
> GET /api/v1/onboard/getAssociatedOrganization HTTP/2
> Host: api.xyz.com
> user-agent: curl/7.79.1
> accept: */*
>
* Connection state changed (MAX_CONCURRENT_STREAMS == 128)!
< HTTP/2 302
< date: Tue, 05 Jul 2022 21:16:50 GMT
< content-type: text/html
< content-length: 138
< location: https://api-auth.xyz.com/oauth2/start?rd=http://api.xyz.com/api/v1/onboard/getAssociatedOrganization
<
<html>
<head><title>302 Found</title></head>
<body>
<center><h1>302 Found</h1></center>
<hr><center>nginx</center>
</body>
</html>

This is causing issues in the browser since on getting the 302 from NGINX due to the auth-signin annotation I start getting CORS errors

Access to XMLHttpRequest at 'https://api.xyz.com/api/v1/onboard/getAssociatedOrganization' from origin 'https://app.xyz.com' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

I have tried the following annotation to enable CORS headers in the 302 response but to no avail

nginx.ingress.kubernetes.io/cors-allow-credentials: "false"
nginx.ingress.kubernetes.io/cors-allow-methods: PUT, GET, POST, OPTIONS
nginx.ingress.kubernetes.io/enable-cors: "true"

What you expected to happen:

  • I expect the 302 request being generated by NGINX for auth-signin to have CORS headers present in them so that get not get blocked by the browser. Any help/suggestions would be appreciated

NGINX Ingress controller version (exec into the pod and run nginx-ingress-controller --version.):

Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.2", GitCommit:"f66044f4361b9f1f96f0053dd46cb7dce5e990a8", GitTreeState:"clean", BuildDate:"2022-06-15T14:14:10Z", GoVersion:"go1.18.3", Compiler:"gc", Platform:"darwin/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"22+", GitVersion:"v1.22.9-eks-a64ea69", GitCommit:"540410f9a2e24b7a2a870ebfacb3212744b5f878", GitTreeState:"clean", BuildDate:"2022-05-12T19:15:31Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}
@sameer-m-dev sameer-m-dev added the kind/bug Categorizes issue or PR as related to a bug. label Jul 5, 2022
@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Jul 5, 2022
@longwuyuan
Copy link
Contributor

/remove-kind bug

  • Please post the information asked in the issue template
  • Please post all other related information like
    • kubectl describe ing -A -o wide
    • kubectl get svc -A
    • kubectl -n logs
    • Your entire and exact curl command and its response with -v to access the application for which auth is configured
    • Any other related information
  • Once this information is posted, please re-open the issue
  • If you can post a step-by-step instruction set to reproduce the problem in a kind or minikube cluster, it will help. If you do that, please help out and write the instructions with enough detail for someone to copy/paste from your instructions, so that they don't have to think/plan and cause a deviation from your use-case

/close

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. and removed kind/bug Categorizes issue or PR as related to a bug. labels Jul 6, 2022
@k8s-ci-robot
Copy link
Contributor

@longwuyuan: Closing this issue.

In response to this:

/remove-kind bug

  • Please post the information asked in the issue template
  • Please post all other related information like
  • kubectl describe ing -A -o wide
  • kubectl get svc -A
  • kubectl -n logs
  • Your entire and exact curl command and its response with -v to access the application for which auth is configured
  • Any other related information
  • Once this information is posted, please re-open the issue
  • If you can post a step-by-step instruction set to reproduce the problem in a kind or minikube cluster, it will help. If you do that, please help out and write the instructions with enough detail for someone to copy/paste from your instructions, so that they don't have to think/plan and cause a deviation from your use-case

/close

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.

@schmidtk
Copy link

schmidtk commented Jul 7, 2022

@longwuyuan

I can confirm the reported behavior. The information provided below should reproduce the issue. We've seen it on multiple versions of the ingress controller, currently with 1.2.0.

Our nginx ingress controller has the following configuration, with URL's updated for privacy reasons:

nginx.ingress.kubernetes.io/auth-signin: 'https://api.xyz.com/oauth2/start'
nginx.ingress.kubernetes.io/auth-url: 'http://api.xyz.com/oauth2/auth'
nginx.ingress.kubernetes.io/cors-allow-credentials: 'true'
nginx.ingress.kubernetes.io/cors-allow-origin: 'https://allowed.origin.com'
nginx.ingress.kubernetes.io/enable-cors: 'true'

With the above configuration, we're making a request to our service with a curl command like:

curl -v 'https://api.xyz.com/our-service' -H 'origin: https://allowed.origin.com'

Which produces a response like:

< HTTP/2 302
< date: Thu, 07 Jul 2022 12:54:22 GMT
< content-type: text/html
< content-length: 138
< location: https://api.xyz.com/oauth2/start?rd=https://api.xyz.com%2Four-service

Note that CORS headers are missing from this response, which causes browsers to block it. If we remove auth-signin and auth-url from the above configuration and make the same curl request, the response looks like this:

< HTTP/2 200
< date: Thu, 07 Jul 2022 12:53:57 GMT
< content-type: application/json; charset=utf-8
< content-length: 96
< x-powered-by: Express
< access-control-allow-origin: *
< etag: W/"<omitted>"
< vary: Accept-Encoding
< strict-transport-security: max-age=15724800; includeSubDomains

CORS headers are present and the response does not get blocked by the browser.

NGINX Ingress controller version: 1.2.0
Specifically, we're using the rancher/nginx-ingress-controller:nginx-1.2.0-hardened6 image.

Kubernetes version:

serverVersion:
  buildDate: "2022-04-28T19:11:38Z"
  compiler: gc
  gitCommit: 6df4433e288edc9c40c2e344eb336f63fad45cd2
  gitTreeState: clean
  gitVersion: v1.22.9+rke2r2
  goVersion: go1.16.15b7
  major: "1"
  minor: "22"
  platform: linux/amd64

Browsers used:

  • Chrome 103.0.5060.114 on macOS and Windows
  • Firefox 102.0.1 on macOS and Windows

@schmidtk
Copy link

schmidtk commented Jul 7, 2022

/reopen

@k8s-ci-robot
Copy link
Contributor

@schmidtk: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

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.

@schmidtk
Copy link

schmidtk commented Jul 7, 2022

Sorry, robot. Worth a shot.

@longwuyuan
Copy link
Contributor

/reopen

@k8s-ci-robot
Copy link
Contributor

@longwuyuan: Reopened this issue.

In response to this:

/reopen

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.

@k8s-ci-robot k8s-ci-robot reopened this Jul 7, 2022
@longwuyuan
Copy link
Contributor

@schmidtk I recall a PR where we narrowed the scope of what a valid origin ought to be.
@theunrealgeek , if you get a chance, please comment on this.

If you can search and find, it will be great but I hope I will I will search for that PR and check if its related to you using a random allowed-origin of your choice.

@schmidtk
Copy link

schmidtk commented Jul 7, 2022

Thank you, I can look around and see if there are other issues that might be related. I don't think the cors-allow-origin configuration is related though. If I remove that and enable CORS for all origins, I still see the same behavior where CORS headers are omitted when redirecting to the auth-signin URL. It seems when the ingress controller returns a 302 HTTP response to redirect to auth, the CORS config is ignored and those headers are not provided in the response.

@longwuyuan
Copy link
Contributor

Oh, that is important insight.

Now need to get some dev time on it. Its hard as we are on freeze for features but this seems like a bug. Will update.

/area stabilization
/kind bug
/triage accepted

@k8s-ci-robot k8s-ci-robot added area/stabilization Work for increasing stabilization of the ingress-nginx codebase kind/bug Categorizes issue or PR as related to a bug. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 7, 2022
@longwuyuan
Copy link
Contributor

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority labels Jul 8, 2022
@harry1064
Copy link
Contributor

Hi @longwuyuan
Checked the generated nginx.conf and found the location block generated for redirect of auth-sign URL

location @8638240922c3aa074994ed6194c30efb21bab86e {
                        internal;

                        add_header Set-Cookie $auth_cookie;

                        return 302 https://api-auth.xyz.com/oauth2/start?rd=$scheme://$best_http_host$request_uri;
                }

Inside this block even the CORS header set in parent request are not retained. Should we also add template which generate code to add headers related to CORS also inside the above block generation in nginx.tmpl?

@longwuyuan
Copy link
Contributor

@harry1064 , I don't know yet.

You have the option to ;

@harry1064
Copy link
Contributor

/assign

@longwuyuan
Copy link
Contributor

@harry1064 , before you conclude that a new template is enough, kindly help out and test for security by using random origin. There was a PR merged to not allow random origins. In fact it checked for more. Can't recall the PR number now but regardless, it will be important to figure out if generating headers allows for random origin or not.

@harry1064
Copy link
Contributor

@longwuyuan Yes sure. I will try find the PR and check it.

@harry1064
Copy link
Contributor

harry1064 commented Jul 12, 2022

@longwuyuan Is this the PR you were talking about?

@longwuyuan
Copy link
Contributor

longwuyuan commented Jul 12, 2022 via email

@harry1064
Copy link
Contributor

Hi @longwuyuan

Point
being some work was done to secure CORS so need to check if that is causing
missing headers.

I have checked the implementation. It is not causing any issue. It is working as it should be.
As you can see in following lines
https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/template/nginx.tmpl#L1172-L1173
we need to add header again add_header Set-Cookie $auth_cookie; even though it was added here in generated nginx.conf

Screenshot 2022-07-12 at 8 03 14 PM
this was done in this PR https://github.com/kubernetes/ingress-nginx/pull/5067/files

So it is clear that, when error 401 case happens, and user taken to internal location block, headers are not retained.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 10, 2022
@chuegel
Copy link

chuegel commented Oct 14, 2022

We experiencing the same issue with image nginx-1.2.1-rancher1
Instead of Keycloak we are using oauth2-proxy for this particular ingress

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    cert-manager.io/cluster-issuer: letsencrypt-prod
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/auth-response-headers: x-auth-request-user, x-auth-request-email,
      x-auth-request-access-token
    nginx.ingress.kubernetes.io/auth-signin: https://oauth2.example.com.com/oauth2/start?rd=$scheme://$best_http_host$request_uri
    nginx.ingress.kubernetes.io/auth-url: https://oauth2.example.com/oauth2/auth
    nginx.ingress.kubernetes.io/cors-allow-origin: http://localhost:9898, https://podinfo.example.com
    nginx.ingress.kubernetes.io/cors-expose-headers: '*'
    nginx.ingress.kubernetes.io/enable-cors: "true"
    nginx.ingress.kubernetes.io/proxy-buffer-size: 16k
    nginx.ingress.kubernetes.io/service-upstream: "true"

--- snip ---

image

@0x0dr1y
Copy link

0x0dr1y commented Oct 21, 2022

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 21, 2022
@johanneswuerbach
Copy link
Contributor

johanneswuerbach commented Nov 3, 2022

We ran into a similar issue and switching the order in the template is resolving the issue for us #9251

Maybe that is a more complete fix compared to #8814 as it fixes missing headers for 401/403 and redirects?

@longwuyuan
Copy link
Contributor

/help

@k8s-ci-robot
Copy link
Contributor

@longwuyuan:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

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.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Nov 18, 2022
@0x0dr1y
Copy link

0x0dr1y commented Dec 7, 2022

@johanneswuerbach Your PR just fixed the missing CORS headers for failed auth requests, right? So this issue still persists and should be open

@johanneswuerbach
Copy link
Contributor

Yes, I didn't check whether this is also enough to pass the tests in :( #8814

@0x0dr1y
Copy link

0x0dr1y commented Dec 7, 2022

Yes, I didn't check whether this is also enough to pass the tests in :( #8814

Will take care of it later!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/stabilization Work for increasing stabilization of the ingress-nginx codebase help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
9 participants