openidc_get_redirect_uri inconsistency when using forwarded host

[SmasherHell]

Hello,

I have an issue with the hostname when redirect_uri is calculated. I encountered it when using X-Forwarded-Host and X-Forwarded-Port header in incoming request.

When headers are :

{
        "accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7",
        "host": "localhost:9080",
        "connection": "keep-alive",
        "cache-control": "max-age=0",
        "upgrade-insecure-requests": "1",
        "accept-encoding": "gzip, deflate"
}

openidc_get_redirect_uri

lua-resty-openidc/lib/resty/openidc.lua

Line 278 in fabf64e

local function openidc_get_redirect_uri(opts, session)

get hostname from host headers because there is no forwarded header to be found, get_forwarded_parameter falls back to host parameter in this case.

lua-resty-openidc/lib/resty/openidc.lua

Line 232 in fabf64e

local function get_forwarded_parameter(headers, param_name)

Generated redirect_uri in this case from /callback : http://localhost:9080/callback

But in the case you are using forwarded headers such as

    {
        "x-forwarded-host": "localhost",
        "x-forwarded-port": "9080",
        "cache-control": "max-age=0",
        "host": "localhost:9080",
        "x-forwarded-proto": "http",
        "upgrade-insecure-requests": "1",
        "connection": "keep-alive",
        "accept-encoding": "gzip, deflate",
        "accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7"
    }

Only x-forwarded-host is used whithout taking the forwarded port which lead to inconsistency in the generated redirect_url

Generated redirect_uri in this case from /callback : http://localhost/callback

I would be more than happy to have your opinion, and I will be more than happy to participate if needed with a PR on this subject

[bodewig]

Well, there is no real standard for x-forwarded-host but the code (and I, which likely is no coincidence) expects it to reflect the original host header, which contains both the domain name and the port. The code doesn't know anything about x-forwarded-port (neither does MDN for example).

If you wanted to support the x-forwarded-port header than please make it very optional. Personally I wish people would use Forwarded which at least is standardized, but that is likely never going to happen. To me splitting the port out of x-forwarded-host into yet another non-standard header looks like a really bad idea.

[SmasherHell]

Make sense, I will try to discuss the case with Apisix, which is the proxy injecting those headers before trying to support x-forwarded-port directly here.