Skip to content
This repository has been archived by the owner on Oct 5, 2020. It is now read-only.

adds a new, explicitly white-listed REST API proxy #449

Merged
merged 1 commit into from
Feb 3, 2017

Conversation

joemfb
Copy link
Contributor

@joemfb joemfb commented Feb 1, 2017

using the http-proxy npm module, the express Router API, and custom middleware functions.

@grtjn
Copy link
Contributor

grtjn commented Feb 1, 2017

Good first step forward I think. It looks much cleaner than the old implementation, I like that a lot!

We did discuss abstracting more severely, but we can take multiple steps to get there. We hadn't thought about suggest, values, and rest extensions yet for instance, we need to embed those nicely into the abstract rest api we discussed internally too.

I do worry a little this is going to conflict with #443. If you could glance over that, and give some thought on how to merry those, that would be nice I think.

I also wonder if it won't be necessary to make front-end changes as well to make this work? That probably will be necessary once we move to the abstract rest api we discussed.

And is this proxy likely to behave pretty much 1:1 with the old one? Passing through all headers, handling binary back and forth, handling of timeout, etc?

Perhaps just matter of merging, and trying, but a bit of confirmation before I do so would be nice.. ;-)

@grtjn
Copy link
Contributor

grtjn commented Feb 1, 2017

@ryan321 Interested to take a look at this too?

@joemfb
Copy link
Contributor Author

joemfb commented Feb 1, 2017

My goal was to clean up as much as possible without requiring any frontend changes. I bound the routes that are directly supported in MLRest, but it should be very easy to add or remove routes here.

One interesting result of this proxy: if you remove the router.use(authed) middleware, DIGEST auth prompts are passed all the way back to the browser. Might come in handy at some point ...

I'm not sure about the current TLS approach, but the http-proxy module makes it easy to use TLS for ingoing or outgoing connections (or both): https://github.com/nodejitsu/node-http-proxy#using-https.

On a related note, I updated ml-common-ng so that requests can be bound to a different URL prefix:

angular.module('my.app', ['ml.common'])
.config(['MLRestProvider', function (provider) {
  // by default, the prefix is '/v1'
  provider.setPrefix('/ml')
}])

@grtjn grtjn added this to the 1.3.0 milestone Feb 3, 2017
@grtjn
Copy link
Contributor

grtjn commented Feb 3, 2017

OK, ready to merge..

@grtjn grtjn merged commit d19e35e into marklogic-community:production-hardening Feb 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants