Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client token header removal #11

Open
c0c0n3 opened this issue Dec 18, 2019 · 2 comments
Open

Client token header removal #11

c0c0n3 opened this issue Dec 18, 2019 · 2 comments
Labels
bug Something isn't working

Comments

@c0c0n3
Copy link
Member

c0c0n3 commented Dec 18, 2019

We'd like to be able to ditch the request IDS-DTH header with the client token before passing on the request to Orion. It looks like that isn't the easiest thing to do with Istio if our adapter should be able to access the header's value---which it should since we've got to validate the token!

As things stand now, if we remove the header through Istio configuration then the header value (token) the adapter gets to see is an empty string. Hence token validation would always fail (even for valid tokens!) if we removed the IDS-DTH header from the request.

Since we've got to validate tokens, at the moment we are not removing the header from the client request and that header will also be in the request the Envoy forwards to Orion. Is there any way to have both token validation and header removal?

@c0c0n3 c0c0n3 added the bug Something isn't working label Dec 18, 2019
@c0c0n3 c0c0n3 added this to the End February Release milestone Dec 18, 2019
@c0c0n3
Copy link
Member Author

c0c0n3 commented Dec 19, 2019

Here's a bit more detail about what's happening.

The instance config below tells Istio to extract the client token (the value of the "ids-dth" HTTP header of the incoming request) and put it into the client_token parameter to be sent to our adapter.

...
kind: instance
...
spec:
    template: oriondata
    params:
        client_token: request.headers["ids-dth"] 
...

If you hit your local Minikube cluster (see README.md for our setup) with a token in the request:

$ curl -v "$BASE_URL"/headers -H ids-dth:my.fat.jwt

The adapter gets passed the "ids-dth" header value. Here's a snippet from the adapter logs:

... info   auth request
    {&InstanceMsg{ClientToken:my.fat.jwt,Name:icheck.instance.istio-system,}
    ...

Now you could configure our Istio virtual service to drop the request header before sending the request on to the target service:

...
kind: VirtualService
...
spec:
...
    http:
    - route:
        - destination:
        ...
          headers:
              request:
                  remove:
                  - ids-dth

But with this config, if you hit the cluster again with the same request

$ curl -v "$BASE_URL"/headers -H ids-dth:my.fat.jwt

the adapter gets an empty string as seen from the logs:

... info   auth request
    {&InstanceMsg{ClientToken:,Name:icheck.instance.istio-system,}
    ...

It looks like the header gets chopped off before the request makes it to the target service's Envoy sidecar which then can't obviously add that header to the request.headers attribute map and eventually our adapter gets nada, zilch, nuffin. (This is just a theory, things may work differently under the bonnet!)

Too bad, I thought, we can still do this at the rule level---i.e. in the adapter's rule config that ties the handler to the instance---or can we? With this config

...
kind: rule
...
spec:
...
    requestHeaderOperations:
    - name: ids-dth
      operation: REMOVE
...

and the remove header directive removed (pun intended!) from the virtual service config, the adapter still gets an empty string!

@c0c0n3
Copy link
Member Author

c0c0n3 commented Jan 24, 2020

Note to self. I've just bumped into this Istio wiki entry where they remove the header using a requestHeaderOperations block in the adapter's rule resource:

https://github.com/istio/istio/wiki/Route-directive-adapter-development-guide#step-5-try-request-header-operations

The wiki entry was last edited on 21 Nov 2018 so at sometime in the past the rule approach outlined in my earlier comment must've worked. The plot thickens...

@gboege gboege moved this from Backlog to In Dev in Adapter prototype Feb 18, 2020
@gboege gboege moved this from In Dev to Ready for Dev in Adapter prototype Feb 18, 2020
@c0c0n3 c0c0n3 added this to Ready for Dev in Beta release Mar 16, 2020
@c0c0n3 c0c0n3 removed this from Ready for Dev in Adapter prototype Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Beta release
Ready for Dev
Development

No branches or pull requests

1 participant