nginx-shib / nginx-http-shibboleth

Shibboleth auth request module for nginx
https://github.com/nginx-shib/nginx-http-shibboleth/wiki
Other
209 stars 27 forks source link

Implement better merging of subrequest and main request #9

Closed davidjb closed 8 years ago

davidjb commented 8 years ago

Previously, the way this module worked was to replace the main request's outgoing headers with the subrequest's headers (r->headers_out = sr->headers_out, this line). This kind of works and does deliver the right headers to a client. However, Nginx's directives such as add_header and error_page do not function correctly. Headers-more works, but only for clearing headers; not setting them.

At present, there's some workarounds in place for 401/403 responses to enable Nginx to correctly manipulate outgoing headers and allowing the above directives to work. This implementation needs to be reworked into correctly merging the two sets of headers.

Apparently https://github.com/openresty/lua-nginx-module/ has a live example for merging subrequest headers and body into the main request.

Originally reported as #8.

davidjb commented 8 years ago

As per http://forum.nginx.org/read.php?2,238444,238453#msg-238453, the original mechanism wasn't recommended and as Maxim points out:

While this might work, I wouldn't recommend relying on it. This way headers are processed within subrequest context, and then again within main request context. This might potentially result in headers being not in sync with filter module contexts, resulting in invalid response. Safer aproach would be to copy only specific headers.

(On the other hand, I don't think anything bad would happen with standard filter modules, as most of them just ignore subrequests.)

As we've seen from #8, this is a problem and the Gzip filter is modifying the body of the outgoing request, but the response is broken because the headers aren't manipulated/delivered correctly.

So there's two options:

The latter seems to be suggested option.

davidjb commented 8 years ago

This is resolved. Code from ngx-lua has been adapted to merge the subrequest response headers into the main response. Headers that concern Content-* from the authorizer are ignored at present because no response body is returned from the subrequest as Nginx doesn't support it.

Header filters, such as gzip, can now manipulate outgoing headers correctly.