@liggitt wasn't this there for a reason?

rephrase - I thought there are use cases where callers are using
Authorization.

On Wed, Oct 5, 2016 at 2:06 AM, k8s-ci-robot notifications@github.com
wrote:

Jenkins verification failed
https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/34076/kubernetes-pull-verify-all/16888/
for commit 75b8465
75b8465.
Full PR test history http://pr-test.k8s.io/34076.

The magic incantation to run this job again is @k8s-bot verify test this.
Please help us cut down flakes by linking to an open flake issue
https://github.com/kubernetes/kubernetes/issues?q=is:issue+label:kind/flake+is:open
when you hit one in your PR.

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#34076 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p7aYFL7xTf9uPJuLrrLvsK-NJs4gks5qwz5cgaJpZM4KOcBS
.

I think the proxy target should not use authorization token meant to be for API server.

I think the proxy target should not use authorization token meant to be for API server.

agree, but we proxy in several places, and I'm not sure this is the right place to remove the header

It's an open question (to me) as to whether there are legitimate use cases for having user creds passed through the proxy to the k8s service -- e.g. the k8s UI/dashboard?

It also seems like an open question as to whether there's a security issue in blithely always forwarding these tokens along, which could result in unexpected disclosure of user creds.

If it's better security, but a nominally-breaking change, need to work through those impacts?

Also @philips fyi in case there's a CoreOS perspective here. Any other vendors who we should also CC?

@liggitt What other places are you talking about? As far as I can tell, this is the entry point for all proxy calls.

@erictune has plans for an impersonation mechanism. I do not think it is reasonable for people to use this in the short term--hopefully no one is. Perhaps we should test the UIs to make sure.

I've tried UI with this change (getting lists of pods, deployments, etc., creating apps, pods, ...). It looks fine. If there is an automated testing or a process to test the whole UI, I am not aware of it, but the UI looks fine to me.

@liggitt What other places are you talking about? As far as I can tell, this is the entry point for all proxy calls.

there are also proxy calls via nodes/proxy, pods/proxy, and services/proxy subresources, handled by https://github.com/kubernetes/kubernetes/blob/master/pkg/registry/generic/rest/proxy.go#L72

more broadly, I wonder if the authentication layer should consume the Authorization header (we do that in OpenShift as seen here)

@liggitt Removing authorization header in authentication layer is a good idea, but I think an Authenticator is not a place for it. First we do not expect an authenticator to mutate the request, and second we are chaining authenticators together and if a valid token is passed in header but another authenticator succeeded before we consume it, we will still send a valid token to the proxy target. I've removed it at the end of authentication cycle (if succeeded). Please take another look.

LGTM

cc @kubernetes/sig-auth

Jenkins verification failed for commit 5d3a8c7. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

This is in the v1.4 milestone do you need it for 1.4.1?

Automatic merge from submit-queue

@jessfraz Yes, we would like it cherrypicked.

@mbohlool can you make the cherrypick PR? There is a script under hack/ that will do that for you. Thanks

Sure, will do.

Done #34280 and #34281

Thanks!

On Thu, Oct 6, 2016 at 4:46 PM, mbohlool notifications@github.com wrote:

Done #34280 #34280 and
#34281 #34281

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34076 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABYNbIB2gJCanrz4OvM-5lG7x4q0Aq6Xks5qxYhNgaJpZM4KOcBS
.

Commit found in the "release-1.4" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.