Feature/support token in query string

[santerioksanen]

First of all, thanks for a great repository!

This PR adds the possibility to specify the JWT-token to be provided as a query string parameter:

• auth_jwt_location QUERY_STRING=jwt-token defines the token to be provided with the key jwt-token
• Given key is stripped from outgoing request
• Adds tests to verify parsing of query-string arguments

I've verified that the key is stripped from the outgoing request manually, but couldn't find a way to add this as an automatic test case in a reasonable amount of time.

Note that only after forking and creating the modifications, did I encounter a previous PR on the same topic: https://github.com/TeslaGov/ngx-http-auth-jwt-module/pull/103

The same concerns regarding security of course applies, and I totally agree with that tokens should not be passed in the query string. However, I see use-cases where the trade-off concerning security would be justified. Nginx plus seems to have the same conclusion:

NGINX Plus can also obtain the JWT from a query string parameter. To configure this, include the token= parameter to the [auth_jwt](https://nginx.org/en/docs/http/ngx_http_auth_jwt_module.html#auth_jwt) directive:

My use-case is to create a proxy for providing map-tiles for Leaflet from a 3rd-party tile-provider, who are behind authentication that is done by the proxy. In this case, I also want the clients to authenticate against the proxy. Providing the token in the query-string for the individual tiles greatly simplifies the application, as opposed to providing authentication-headers inside elements.

I also noticed possible bug in

static const char *HEADER_PREFIX = "HEADER=";
static const char *COOKIE_PREFIX = "COOKIE=";

as sizeof is used for these, which returns the size of the pointer instead of the data?

[santerioksanen]

I removed the ngx_pfree call from:

int free_ok = ngx_pfree(r->pool, r->args.data);
if (free_ok == NGX_OK) {

This call always failed, preventing the removal of the token from args for following processing steps. The reason for the fail of the free-call, is that the args are not separately allocated in memory, the pointer references to an offset from the uri-component, which takes care of the allocation. Furthermore, the mutated args are allocated from the request-pool, which gets destroyed upon completion of processing of the request, and as such should not cause any possibilities for memory leaks.

It doesn't however seem to remove the token without the set $args directive in conf.

[JoshMcCullough]

It doesn't however seem to remove the token without the set $args directive in conf.

Thanks, can you elaborate on this part?

[santerioksanen]

It doesn't however seem to remove the token without the set $args directive in conf.

Thanks, can you elaborate on this part?

Sure. Initially when I was testing, I was testing it with the nginx-config we are going to use in production. This involves injecting the api-key for the upstream request, so we have a following nginx.conf for that particular endpoint:

set $args $args&api-key=APIKEY;
proxy_pass https://api.somewhere.com;

With this config, the target api is called without the token, which we use for authentication against the proxy. However, if I remove the first row (don't append api-key), token remains in the query.

Unfortunately I am not that well aware of the processing stages for a request inside nginx, but I am assuming the set $args sets some kind of flag to re-calculate the arguments while forming the upstream request, while without the flag nginx could use the original uri with just host replaced. However, keep in mind that this part is pure speculation from my part, as I don't know the internals of nginx well enough.

I can look into this, if this is seen as an issue. Possible approaches could be to locate whether it would be possible to set that same flag if the token is to be removed, or whether mutating the uri string in a similar fashion would prevent the token from appearing in the upstream request.

I'll be offline until Monday 28.8, but will get back to this then.

[JoshMcCullough]

So I think what you're saying is that this approach will only work (e.g. the JWT will only be removed from the proxied URL) if you add set $args ... to the config as in your example?

set $args $args&api-key=APIKEY;
proxy_pass https://api.somewhere.com;

So that is counter-intuitive, because this config will include the JWT?

proxy_pass https://api.somewhere.com;

[santerioksanen]

Exactly! I am unsure of the reason though, but would assume that mutating the args triggers a different process for building the upstream request.

[santerioksanen]

@JoshMcCullough, just pinging on the status, is this going to move forward or do you have further changes required? We have it currently running in production, but am of course curious if the feature will be implemented upstream as well.

[JoshMcCullough]

Yes, it will get merged but it'd be good to figure out the set $args ... part.

[JoshMcCullough]

@santerioksanen IIRC there is code in this PR to remove the arg from $args, but if that is not doing what we want then we can just remove that code and add some text to the README indicating that the JWT will be included in the proxied URL. We can look into it more at a later date.