Making `http_ssl_module` optional

[mroach]

Currently, nginx has to be compiled --with-http_ssl_module in order for this module to be compiled. There's just one line in the module that depends on this module being available:

ngx-http-auth-jwt-module/src/ngx_http_auth_jwt_module.c

Line 498 in 05a3798

const char *scheme = (r->connection->ssl) ? "https" : "http";

I took a whack at this locally, and this works:

#ifdef NGX_HTTP_SSL
      const char *scheme = (r->connection->ssl) ? "https" : "http";
#else
      const char *scheme = "http";
#endif

Would this be an acceptable change to make to the module? I'm happy to open a PR if so.
My C skills are nearly nil and I've never developed an nginx module, so I wanted to check first.

[JoshMcCullough]

Yes, sounds good -- can you submit a PR with that change?

[mroach]

I ran into an issue during my testing I wanted to bring up before delivering a change that could lead to headaches for other users using this behind an SSL-terminating load balancer.

In my test config, I have this:

location /secured/token/redirect {
  auth_jwt_location HEADER=Authorization;
  auth_jwt_redirect on;
  auth_jwt_loginurl /login;
}

Next, in my test, I set both the host and x-forwarded-proto headers when making a request, just like an SSL-terminating load balancer would:

curl http://localhost:8123/secured/token/redirect \
  --header "host: api.secureapp.test:8123" \
  --header "x-forwarded-proto: https" 

The issues are in the response Location header:

< HTTP/1.1 302 Moved Temporarily
< Server: nginx
< Location: http://api.secureapp.test:8080/login?return_url=https://api.secureapp.test/secured/token/redirect%3Ftest=path%2Bsecured%2Bby%2Btoken%252c%2Bredirect%2Bon%2Bfailure%2Bwithout%2Btoken

There are two issues here:

1. The scheme is http instead of https.
2. The port is wrong. No matter what I set in the host header, it's always using the listen port.

Next, I tried a more manual approach here by setting my config to use: auth_jwt_loginurl https://$host:$port/login.
This doesn't work. The response is a literal Location: https://$host:$port/login?redirect_url=...
That seems like a separate issue to me where config values aren't supporting variable substitution. I have no idea how that's meant to work with nginx modules.

Looking at the code for this module, it doesn't appear to me that it's in control of the the scheme, host, and port part of the Location response header. Looking around the code of nginx itself, I think it's caused by this output header filtering: https://github.com/nginx/nginx/blob/a13ed7f5ed5bebdc0b9217ffafb75ab69f835a84/src/http/ngx_http_header_filter_module.c#L327-L354

This led me to discover a workaround: use absolute_redirect off; in the nginx configuration

My suggested patch as-is allows the module to be compiled without SSL and work great behind a load balancer as long as absolute redirects aren't used since the redirect values will be wrong. Having to switch-off absolute redirects as a workaround feels like a decently large caveat and that there's room for improvement here.

For my particular use case, I've already worked-around all of this with a sed one-liner to replace that one r->connection-ssl line causing my issue and I don't use redirects at all, so I don't need these changes.

Do you think it's worth including this patch even though it comes with some downsides and caveats?

[JoshMcCullough]

The port issue is fixed in 2.3.1.

You're correct that the module doesn't currently support variable substitution auth_jwt_login_url and other directives. I think we have a ticket for that somewhere.

We do build the redirect URL which appears in the Location response header.

Would you mind testing this again w/the latest code and seeing if it's resolved with your NGX_HTTP_SSL fix?