search

jetstack / kube-oidc-proxy

Reverse proxy to authenticate to managed Kubernetes API servers via OIDC.
https://jetstack.io
Apache License 2.0
477 stars 91 forks source link

Extra User Info Impersonation Headers #128

Closed JoshVanL closed 4 years ago

JoshVanL commented 4 years ago

This PR adds the flags:

--extra-user-headers which takes a list of key value pairs, separated by a comma, where keys may have multiple values. These are then appended to the impersonation extra user info, which are passed as headers to proxied requests.

--extra-user-header-client-ip which is will enable the proxy to append the source client address that made the request as an extra impersonation user info header.

This PR also included e2e tests to encapsulate these flags by using a "fake apiserver". This is just a simple echo server that will respond with the identical body and headers so that we can test what is getting proxied.

I am quite happy with how this is working generally however am not certain with some of the naming of flags and would like some feedback.

/assign @simonswine /cc @amit-handda

fixes #125

jetstack-bot commented 4 years ago

@JoshVanL: GitHub didn't allow me to request PR reviews from the following users: amit-handda.

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

In response to [this](https://github.com/jetstack/kube-oidc-proxy/pull/128): >This PR adds the flags: > >`--extra-user-headers` which takes a list of key value pairs, separated by a comma, where keys may have multiple values. These are then appended to the impersonation extra user info, which are passed as headers to proxied requests. > >`--extra-user-header-client-ip` which is will enable the proxy to append the source client address that made the request as an extra impersonation user info header. > >This PR also included e2e tests to encapsulate these flags by using a "fake apiserver". This is just a simple echo server that will respond with the identical body and headers so that we can test what is getting proxied. > >I am quite happy with how this is working generally however am not certain with some of the naming of flags and would like some feedback. > >/assign @simonswine >/cc @amit-handda > > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
jetstack-bot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoshVanL

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/jetstack/kube-oidc-proxy/blob/master/OWNERS)~~ [JoshVanL] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
amit-handda commented 4 years ago

Thank you very much @JoshVanL

amit-handda commented 4 years ago

LGTM

simonswine commented 4 years ago

/unassign /assign @JoshVanL

JoshVanL commented 4 years ago

Thanks for the review @simonswine, I have updated those and will create an issue now.

/unassign /assign @simonswine

simonswine commented 4 years ago

Thanks @JoshVanL

/lgtm

amit-handda commented 4 years ago

@JoshVanL thanks for the work into this. I just reviewed the extra-user-headers code more thoroughly. I dont have a particular need for following scenario, currently. just wanted to raise it, for closure.

I thought through --extra-user-headers param we could specify req-header=extra-impersonation-header and oidc-proxy will map the incoming request header req-header:value1 to Impersonate-Extra-extra-impersonation-header: value1. does it seem useful ? Thanks again.

JoshVanL commented 4 years ago

No problem @amit-handda.

What would the use case for this be? I guess to dynamically add user information based on each request? All the headers get passed through as is so I would like to see a compelling use case for it first.