Implement cookie forwarding for api proxy

[lo1tuma]

The cookie name on the client gets prefixed with the name of the api and a separator. This prevents overwriting a cookie with the same name from a different api host and ensures that the cookies retrieved from the client will forwarded only to the correct api host.

[spikebrehm]

This is interesting. What are your use cases?

I don't think we want to include this as a default for Rendr. Passing through all client-side cookies to the API seems like a very app-specific change that does not generalize to most use cases.

It's easy to pass a custom apiProxy middleware to rendr.createServer(). Perhaps you could release a small module like rendr-cookie-api-proxy or something?

[lo1tuma]

Our API is not stateless and requires a session id sent in a cookie.

This PR doesn’t forwards all client-side cookies to all api hosts. If we get a set-cookie header in the response of an api host we will save this header in a new cookie to the client. Only these persisted set-cookie headers will then be sent to the original api host.

cc: @c089

[spikebrehm]

Ah, I see, thanks for explaining @lo1tuma. It still seems to me that this does not belong in the library because it is specific to your use case. At Airbnb, we have all sorts of overrides in our DataAdapter for this type of logic.

[c089]

Passing our own apiProxy would require us to copy the existing apiProxy just to add a bit of functionality. While I agree that the cookie handling probably shouldn't be in the default apiProxy, we were not able to extend on it because in the DataAdapter we only have the original req from the client, our request to the api and it's response, but not the res to the client.

Maybe we could change the default apiProxy to pass the res (responseToClient) object to the dataAdapter as well, so that we could handle cookies there?

Or could you think of a better way to make the apiProxy extensible?

[c089]

After toying with some ideas, @lo1tuma and I came up with the following proposal how we could make this extensible without copying the apiProxy. If @spikebrehm likes it we'd be happy to contribute the implementation, as well as rewrite the existing x-forwarded-for header and cookie forwarding stuff to make use of it:

---

Currently, when custom behavior for the apiProxy is required, users have to copy the whole apiProxy to make custom changes. While some custom behavior can be placed in the dataAdapter, this does not allow more complex things, which is why we proposed adding cookie handling to the core apiProxy where it does not belong (see #232). This proposal tries to make both the apiProxy and dataAdapter extensible using small, reusable modules. Once rendr supports configuring different data adapters per configured API, this will allow to enable a extension module for all configured APIs by setting it on the ApiProxy or just a specific API by setting it on a DataAdapter.

This would require two extension points: One before the DataAdapter is used to make the request and one when it receives the response. We propose implementing this using two events which will be emitted by both the apiProxy and the dataAdapter(s):

• one event sent before issuing the request to the API, where the handler function can access the request objects (and possible also the responseToClient though I'm not sure if that makes sense):

  apiProxy.on('before', function addXForwardedForHeader(requestFromClient, requestToApi) {
       requestToApi.headers['X-Forwarded-For'] = ...
       // ...
  });
  

• one event when the response is received, with access to the response objects (and possible also the request objects, though I'm not sure if that makes sense):

  apiProxy.on('after', function copyCookiesFromApiResToClientRes(responseToClient, responseFromApi) {
    //...
  })
  

• a shortcut for including extensions that use both before and after hooks:

  apiProxy.use(passthroughCookies))
  // equivalent to:
  //   apiProxy.on('before', passthroughCookies.before ))
  //   apiProxy.on('after', passthroughCookies.after ))
  

[c089]

this will no longer be necessary when #248 is merged

[bigethan]

I like this event extension a lot, as we'll rarely be changing the core apiProxy functionality, and just adding stuff to it.

[pjanuario]

👍

[pjanuario]

This was one of the reason why I created https://github.com/pjanuario/rendr-auth-rest-adapter and opened this issue #457