KEYCLOAK-10864 Pass URI path upstream verbatim #535
Conversation
| newFakeProxy(cfg).RunTests(t, requests) | ||
| } | ||
|
|
||
| func TestPreserveURLEncoding(t *testing.T) { |
There was a problem hiding this comment.
Function 'TestPreserveURLEncoding' is too long (91 > 60) (from funlen)
There was a problem hiding this comment.
8 other functions in middleware_test.go are also > 60 lines. It's because the test cases are long. I suggest living with it and keeping the tests together rather than splitting them up.
Nuru
force-pushed
the
proxy-pass-through
branch
from
0261092 to
6b212e3
Compare
|
@abstractj Please consider this PR as an improvement over #483 (details on the dev list) |
|
If this PR is using the original path when invoking upstream, while using the normalized path when applying permission checks, then I'm in favour of this approach. |
@stianst That is it exactly. |
|
+1 On this approach from me - haven't looked at the code though |
4 participants
No documentation needed, as this is a bug fix.
Testing
Very importantly, it passes all the existing unit tests, plus new ones I wrote with URLs associated with open bugs. I also corrected the test rig to properly test the URLs that are being passed upstream rather than the URLs sent to Gatekeeper for proper values.
Of course, the question is, does it also fix the Jenkins problem, and that it does.
If you use the test setup at https://github.com/primeroz/keycloak-10864-test-jenkins (not written by me) you will see the error message:
Then, in the docker-compose.yml, replace the proxy image "quay.io/keycloak/keycloak-gatekeeper:8.0.1" with "nuru/key-gate:KEYCLOAK-10864" (which contains the
keycloak-gatekeeperbinary from this PR) and then docker down, docker up, and you will see the error message is gone.See discussion on Keycloak Dev but please add comments here.