ProxyFixMiddleware doesn't fix Starlette/FastAPI

[MajorDallas]

I wrapped my FastAPI app according to the docs:

fast_app = FastAPI(...)
# add routers, mounts, other middlewares...
# The final middleware is ProxyFixMiddleware
app = ProxyFixMiddleware(fast_app, mode='legacy')

# app is targeted by the hypercorn cli or started via trio when the module is executed

(The docs should probably make it clear it must be done this way: I lost almost two hours wondering why it wasn't doing anything with FastAPI.add_middleware before coming across #179)

I have a view function:

@router.get(
    "/signin",
    status_code=HTTPStatus.TEMPORARY_REDIRECT,
)
async def sign_in(req: Request):
    logger.debug(req.scope)
    async with AsyncOAuth2Client(
        b.config.OAUTH["client_id"],
        redirect_uri=str(req.url_for(authorize.__name__)),
        scope=["openid", "email", "profile"],
        **b.config.OAUTH["httpxkw"],
    ) as client:
        uri, state = client.create_authorization_url(
            str(b.config.OAUTH["authorization_endpoint"])
        )
    return RedirectResponse(uri)

My NGINX location block has these settings:

    proxy_pass http://unix:/run/hypercorn.sock;
    proxy_set_header Host $host;
    proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
    proxy_set_header X-Forwarded-Proto $scheme;
    proxy_http_version 1.1;

The problem started with req.url_for: it keeps rendering a URL with http:// and breaking OAuth.

Hypercorn correctly logs that X-Forwarded-Proto is https. However, when I log req.scope within the app, pretty much everything I expected the middleware to fix is set as if the middleware weren't present at all:

{
  'type': 'http',
  'http_version': '1.1', 
  'asgi': {'spec_version': '2.1', 'version': '3.0'},
  'method': 'GET',
  'scheme': 'http',  # WRONG
  ...,
  'client': None,  # WRONG -- should equal X-Forwarded-For
  'server': None,
  'headers': [
    (b'host', b'<server_name in nginx and hypercorn.toml>),  # Correct
    (b'x-forwarded-proto', b'https'),
    (b'x-forwarded-for', b'<my ip>'), ...
  ],
  ...
}

So Hypercorn itself clearly sees the proxy headers and is consuming them itself more or less correctly, but it's not adding that information to the Scope in a way that Starlette can see it.

I hope this is just my error, because that would be easy to fix. Maybe I need to wrap the Router instead of the FastAPI? Or maybe it should be the first instead of the last middleware?

[MajorDallas]

After a little more blind testing, it turns out the problem was trusted_hops.

The docs say

The trusted_hops argument should be set to the number of proxies that are chained in front of Hypercorn. You should set this to how many proxies are setting the headers so the middleware knows what to trust.

Which sounds like a suggestion. My dev and prod environments have a different number of proxies, so for convenience I set it the higher number thinking the case where fewer hops occur wouldn't cause any issues and ignored the suggestion.

Turns out this is a requirement.

The good news is I can refine the issue from a somewhat vague possible bug to a specific request:

Please update the "Fixing proxy headers" page to emphasize the two following points:

• The middleware object MUST be applied as demonstrated. Passing it to FlastAPI.add_middleware does not work (and I might presume it also won't work when passed to Starlette(middleware=...), though I have not tested this).
• The trusted_hops option MUST exactly equal the actual number of proxies in the environment.

The latter condition fails silently, manifesting as eg. starlette.Request.url_for rendering an http:// scheme instead of https:// or scope["client"] being set to None.

[nightcityblade]

Hi, I'd like to work on this. I'll submit a PR shortly.