nginx: annotations: authrequest: support "signin url" #190

colemickens · 2017-01-30T21:03:04Z

I finally got around to trying to put together an OAuth2/OIDC reverse proxy using nginx-ingress-controller and CoreOS's fork of bitly's oauth2_proxy. Initial support for thi scenario was added as a result of this issue: kubernetes-retired/contrib#1492

I had to do a couple things to get it to work:

Build nginx-ingress-controller myself since it has not been published in quite a long time.
Downgrade to a version of nginx-slim that is actually published (kind of weird)...
Add a new annotation for auth-signin that tells nginx where to send 401s to.
Add a couple extra X-* headers for things being reverse-proxied that expect them.

As a bonus:

Parameterize DOCKER in the Makefile similar to PREFIX and RELEASE for those of us building and pushing to non-GCR repositories.
Pulled out the URL parsing logic from the authrequest plugin to the parser since it seems to already hold util-type helpers.

cc: @aledbf

k8s-reviewable · 2017-01-30T21:04:09Z

This change is

coveralls · 2017-01-30T21:31:29Z

Coverage decreased (-0.3%) to 43.92% when pulling a4429c7 on colemickens:colemickens-signin-url into 910b706 on kubernetes:master.

bprashanth · 2017-01-31T02:51:55Z

Please add an example of how to use this here: https://github.com/kubernetes/ingress/tree/master/examples (even if it's not part of a released image yet, please say that in the example and we can update it once 0.9.0 is out)

pieterlange · 2017-01-31T11:52:21Z

controllers/nginx/rootfs/Dockerfile

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.

-FROM gcr.io/google_containers/nginx-slim:0.13
+FROM gcr.io/google_containers/nginx-slim:0.12


nginx-slim:0.13 is published now.

Ack. I've switched back to 0.13 now.

colemickens · 2017-01-31T18:31:11Z

Where am I meant to add an example? It seems like all of the nginx annotation examples got left over and forgotten in contrib/.

I can just move over the authrequest example and add a single line to show this attribute, is that what is desired? I would've assumed y'all would've moved the examples over already.

colemickens · 2017-01-31T23:14:46Z

Rebased. Still seeking clarification about where/how to add the example.

bprashanth · 2017-01-31T23:17:55Z

I can just move over the authrequest example and add a single line to show this attribute, is that what is desired? I would've assumed y'all would've moved the examples over already.

Yes, please create a directory in examples (https://github.com/kubernetes/ingress/tree/master/examples) called auth, and write a README file describing how to use it, kind of like how we describe tls-termination (https://github.com/kubernetes/ingress/tree/master/examples/tls-termination)

bprashanth · 2017-01-31T23:18:44Z

I already added a section for auth in the example overview, just needs filling in: https://github.com/kubernetes/ingress/tree/master/examples#auth

coveralls · 2017-01-31T23:32:25Z

Coverage decreased (-0.3%) to 43.92% when pulling b9d0878 on colemickens:colemickens-signin-url into b9d272a on kubernetes:master.

aledbf · 2017-01-31T23:33:38Z

controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl

@@ -235,6 +235,8 @@ http {
            {{ end }}
            {{ if not (empty $location.ExternalAuth.Method) }}           
            proxy_method {{ $location.ExternalAuth.Method }};
+            proxy_set_header X-Original-URI $request_uri;
+            proxy_set_header X-Scheme $scheme;


Please change $scheme to $pass_access_scheme

aledbf · 2017-01-31T23:33:48Z

controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl

@@ -301,6 +307,8 @@ http {
            proxy_set_header X-Forwarded-Host       $host;
            proxy_set_header X-Forwarded-Port       $pass_port;
            proxy_set_header X-Forwarded-Proto      $pass_access_scheme;
+            proxy_set_header X-Original-URI $request_uri;
+            proxy_set_header X-Scheme $scheme;


Same here. Please change $scheme to $pass_access_scheme

colemickens · 2017-02-01T04:59:15Z

Updated:

fixed the headers, per @aledbf.
added an example, per @bprashanth

coveralls · 2017-02-01T05:15:09Z

Coverage decreased (-0.3%) to 43.92% when pulling 84003c4a381f1aa9cf410122ae78b3db3cb64ded on colemickens:colemickens-signin-url into b9d272a on kubernetes:master.

colemickens · 2017-02-01T06:03:42Z

Can I relax the URL check? I want to make the auth_request a relative url, but I can't because of how the auth_request is handled -> proxy_pass. proxy_pass seems to require an absolute URL.

I want to make the annotation values $scheme$host:$serverport/oauth2/{callback,signin}, but I can't because of the URL checks.

coveralls · 2017-02-01T06:43:48Z

Coverage decreased (-0.3%) to 43.92% when pulling 7240359066e70a75c7436689deedeb41d41b3129 on colemickens:colemickens-signin-url into b9d272a on kubernetes:master.

coveralls · 2017-02-01T06:44:11Z

Coverage decreased (-0.3%) to 43.92% when pulling 7240359066e70a75c7436689deedeb41d41b3129 on colemickens:colemickens-signin-url into b9d272a on kubernetes:master.

coveralls · 2017-02-01T06:44:55Z

Coverage decreased (-0.3%) to 43.92% when pulling 7240359066e70a75c7436689deedeb41d41b3129 on colemickens:colemickens-signin-url into b9d272a on kubernetes:master.

coveralls · 2017-02-01T06:55:45Z

Coverage decreased (-0.3%) to 43.92% when pulling 7240359066e70a75c7436689deedeb41d41b3129 on colemickens:colemickens-signin-url into b9d272a on kubernetes:master.

coveralls · 2017-02-01T09:22:35Z

Coverage decreased (-0.3%) to 43.92% when pulling f0709cb54bb5a07cc9cb0be5687a203f6460d47c on colemickens:colemickens-signin-url into b9d272a on kubernetes:master.

colemickens · 2017-02-18T02:15:47Z

Really hoping to get back to this tomorrow. For when I do...

Is the oauth2_proxy overkill for an example here? I'm not sure how to demonstrate the annotation otherwise to be frank, and I think it might be a popular configuration as people want to get to the kubernetes-dashboard more easily.

I plan to add a simple chunk in one of the READMEs for deploying nginx-ingress-controller which could be linked to from the oauth2_proxy example I've written here. That would effectively make it a nice one-stop shop for setting up nginx-ingress-controller and oauth2_proxy and protecting kubernetes-dashboard. Might be out of scope for this repo though, so I wanted to ask.

aledbf · 2017-02-18T12:31:09Z

Is the oauth2_proxy overkill for an example here?

I think is a good example showing how to integrate an oauth2 app

yawboateng · 2017-02-20T22:17:00Z

Thanks for this PR, works very well with coreOS's dex.

question:
what is the right way to configure this to work with multiple host names?

I have 1 Ingress object that handles authentication - points to an oauth2 client that sits in front of dex at foo.bar.com/auth/oauth2 ...and I have another ingress object pointing to some application foo.bar.com/home
authentication and redirection works fine with the above.

But I keep getting 499 error codes when i try to add other ingress objects at like bar.foo.com or echo.bar.com that uses the annotation of the auth object at foo.bar.com/auth/oauth2 to check for authentication status.

do I need to deploy a separate auth ingress object for each host name i want to authenticate?

aledbf · 2017-02-24T23:58:52Z

@colemickens ping

colemickens · 2017-03-01T09:19:29Z

I think it's nearly ready to go... except that 0.9.0-beta.2 seems to have a regression. Even the auth_request that was added months ago doesn't seem to be working. And then when I was on master, I couldn't even get external connections to work. Since 0.9.0-beta.2 auth_request seems broken, it seems there's a regression unrelated to this PR. I don't have BW to dig in a lot more.

Other than that, this is ready to go. The README doesn't go into extensive detail on each individual step, but the shell script is short and it's easy enough to figure out what it's doing.

I tested with the helm path suggested in the README and then editted the nginx-ingress deployment to fix the flags and then pointed at an image I'd built a couple weeks ago... due to the aforementioned issues. I used the config examples that are in there, for Azure. But I've also used it with GitHub and anything in oauth2_proxy should work presumably.

colemickens · 2017-03-01T09:23:22Z

I can say it was between these commits at least, based on the Dockerfile in a working container I have on Docker hub from a couple weeks ago. 08eda50#diff-ce0eea6892ca5cae9d5b726f52b659ba and 2d0971d#diff-ce0eea6892ca5cae9d5b726f52b659ba

aledbf · 2017-03-09T14:22:08Z

Closing. Replaced by #410

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 30, 2017

pieterlange reviewed Jan 31, 2017

View reviewed changes

colemickens force-pushed the colemickens-signin-url branch 2 times, most recently from 1943f3f to b9d0878 Compare January 31, 2017 23:14

aledbf reviewed Jan 31, 2017

View reviewed changes

colemickens force-pushed the colemickens-signin-url branch 4 times, most recently from ca1a4c9 to 7240359 Compare February 1, 2017 06:38

colemickens force-pushed the colemickens-signin-url branch from 7240359 to f0709cb Compare February 1, 2017 09:05

colemickens added 4 commits February 28, 2017 23:10

relax url constraint for auth request

ba8aea4

add signin url req support

2b05c90

add external auth example

8e72bdc

makefile can push to non gcr

245d7f8

colemickens force-pushed the colemickens-signin-url branch from f0709cb to 245d7f8 Compare March 1, 2017 09:16

aledbf mentioned this pull request Mar 9, 2017

Add support for "signin url" #410

Merged

aledbf closed this Mar 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nginx: annotations: authrequest: support "signin url" #190

nginx: annotations: authrequest: support "signin url" #190

colemickens commented Jan 30, 2017

k8s-reviewable commented Jan 30, 2017

coveralls commented Jan 30, 2017

bprashanth commented Jan 31, 2017

pieterlange Jan 31, 2017

colemickens Jan 31, 2017

colemickens commented Jan 31, 2017

colemickens commented Jan 31, 2017

bprashanth commented Jan 31, 2017

bprashanth commented Jan 31, 2017

coveralls commented Jan 31, 2017

aledbf Jan 31, 2017

aledbf Jan 31, 2017

colemickens commented Feb 1, 2017

coveralls commented Feb 1, 2017

colemickens commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

colemickens commented Feb 18, 2017

aledbf commented Feb 18, 2017

yawboateng commented Feb 20, 2017

aledbf commented Feb 24, 2017

colemickens commented Mar 1, 2017

colemickens commented Mar 1, 2017

aledbf commented Mar 9, 2017

nginx: annotations: authrequest: support "signin url" #190

nginx: annotations: authrequest: support "signin url" #190

Conversation

colemickens commented Jan 30, 2017

k8s-reviewable commented Jan 30, 2017

coveralls commented Jan 30, 2017

bprashanth commented Jan 31, 2017

pieterlange Jan 31, 2017

Choose a reason for hiding this comment

colemickens Jan 31, 2017

Choose a reason for hiding this comment

colemickens commented Jan 31, 2017

colemickens commented Jan 31, 2017

bprashanth commented Jan 31, 2017

bprashanth commented Jan 31, 2017

coveralls commented Jan 31, 2017

aledbf Jan 31, 2017

Choose a reason for hiding this comment

aledbf Jan 31, 2017

Choose a reason for hiding this comment

colemickens commented Feb 1, 2017

coveralls commented Feb 1, 2017

colemickens commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

coveralls commented Feb 1, 2017

colemickens commented Feb 18, 2017

aledbf commented Feb 18, 2017

yawboateng commented Feb 20, 2017

aledbf commented Feb 24, 2017

colemickens commented Mar 1, 2017

colemickens commented Mar 1, 2017

aledbf commented Mar 9, 2017