Add XFF header for external-auth requests

[Miouge1]

What this PR does / why we need it:

The source IP of the request can be useful for the external-auth to make a decision.

Special notes for your reviewer:

fixes #2938

[antoineco]

Thanks!

We should consider whether we really want to rewrite the header or rather append to it with $proxy_add_x_forwarded_for:
http://nginx.org/en/docs/http/ngx_http_proxy_module.html#variables

Any opinion in favour or against it?

[Miouge1]

I am not well versed with $proxy_add_x_forwarded_for, what would be appended to what?

On the server side of the external-auth, I would expect to see $remote_addr with the value of the ingress controller making the request and $http_x_forwarded_for the IP of the client making the original request.

[antoineco]

Most web proxies append the IP address of the previous hop separated by a comma to the X-Forwarded-For header instead of replacing its value, as a friendly convention (this principle was also carried out to the more recent and standardized Forwarded header).

The rationale is that you can have a chain of web proxies and each of them can determine from the header:

• the source IP of the request (client IP)
• whether the last hop was in a trusted network

I said "friendly" because of course any proxy in the chain can deliberately spoof the header (like you did 🙂).

I found that page as the first Google result, but I know there are many others.

[ElvinEfendi]

We should consider whether we really want to rewrite the header or rather append to it with $proxy_add_x_forwarded_for:

maybe we should follow what we have for main location block? i.e

            {{ if $all.Cfg.ComputeFullForwardedFor }}
            {{ $proxySetHeader }} X-Forwarded-For        $full_x_forwarded_for;
            {{ else }}
            {{ $proxySetHeader }} X-Forwarded-For        $the_real_ip;
            {{ end }}

here $full_x_forwarded_for will have the whole proxy chain included (see how it's being computed in nginx.tmpl)

[antoineco]

Even better, let's be consistent.

[antoineco]

@Miouge1 does the solution suggested by @ElvinEfendi sound good to you?

[aledbf]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, Miouge1

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 [aledbf]

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

[aledbf]

This PR also fixes #2938