TeslaGov / ngx-http-auth-jwt-module

Secure your NGINX locations with JWT
MIT License
309 stars 118 forks source link

Modify 'in' headers instead of 'out' headers #64

Closed hollosigergely closed 1 year ago

hollosigergely commented 2 years ago

The original code pushes the extracted claims into the HTTP 'out' headers, however, the original use case is to pass these claims as a header by reverse proxying. These headers are contained in the 'in' headers. This commit repairs the issue.

The code as in master returns the extracted claims to the calling party not the proxied application.

JoshMcCullough commented 2 years ago

Thanks, let's let @fitzyjoe review this one.

JoshMcCullough commented 1 year ago

@hollosigergely what do you mean by "the original code" / "the original use case"? I'd like to understand, for clarity.

Your PR does make sense, it would be good to pass the user id and email on to the proxy target. But there could also be use cases to pass those values back to the caller, in the response. And if we made this change now it would be a breaking change, so we should add a new flag to indicate where to write headers. Two new flags, actually...

This way, we can support writing to either, both, or neither sets of headers.

JoshMcCullough commented 1 year ago

FYI, this is fixed in a separate PR I just pushed a bit ago. Instead, you can now specify which claims to extract and where to put them (request or response headers). PR: https://github.com/TeslaGov/ngx-http-auth-jwt-module/pull/87

The change related to this issue is that you will be able to specify one-many new directives auth_jwt_extract_request_claims myClaim and auth_jwt_extract_response_claims myClaim. Then, the myClaim will be extracted from the JWT and placed on either the request or response headers. This also makes the claim(s) available as variables e.g. $http_myclaim or $sent_http_myclaim.

So I'll go ahead and close this since your PR is a breaking change for v1 and v2 will include these new features (merging soon, just waiting on PRs).

JoshMcCullough commented 1 year ago

@hollosigergely this is fixed and available in v2.0.0.