graphile / crystal

🔮 Graphile's Crystal Monorepo; home to Grafast, PostGraphile, pg-introspection, pg-sql2 and much more!
https://graphile.org/
Other
12.63k stars 572 forks source link

Hapi plugin usage #150

Closed mytototo closed 6 years ago

mytototo commented 8 years ago

Hey! I saw it's possible to use postgraphql as a middleware for express. Is it possible to use it as a hapi plugin? Thanks.

calebmer commented 8 years ago

Not currently 😞

PostGraphQL uses the native Node.js HTTP IncomingRequest/ServerResponse cycle which Hapi intentionally bypasses (https://github.com/hapijs/hapi/issues/80). If you wanted to open a PR that adds support, I’d merge it. You’d probably take a similar approach to Jake Pruitt in his article “Using Express Middleware with Hapi.js.”

mytototo commented 8 years ago

Thank you for your quick answer @calebmer. Either I use Koa, Express or Hapi I don't really care as long as it works. It was more by curiosity.

I just tried the next branch with Koa 2 and it looks like it works fine. Thanks!

calebmer commented 8 years ago

Awesome :blush:, I’m looking forward to trying Koa 2 :+1:

If you want, the next branch is published under version 2.0.0-beta.2. You can install the latest version with:

npm install postgraphql@next

I’m going to close this issue for now. If I hear a need for this a few more times I’ll consider adding it :+1:

ghost commented 8 years ago

Koa is the future ™️

PierBover commented 7 years ago

If I hear a need for this a few more times I’ll consider adding it

Here's another vote for Hapi integration.

ugiacoman commented 7 years ago

It'd be pretty cool to see a Hapi integration :)

flippidippi commented 7 years ago

+1 for Hapi integration :)

benjie commented 7 years ago

The schema generation is separate in v4; so it would be cool to see a "postgraphql-hapi" module that uses postgraphile-core under the hood. The hapi module would be responsible for handling the request lifecycle, setting up the postgres connection (including setting the various settings from JWTs etc) and then handing off to the GraphQL schema generated by postgraphile-core for schema introspection and query execution. Is anyone interested in taking this on? I'm interested in creating a higher level shared module too by extracting even more functionality from core (e.g. the PG connection setup/cleanup) so even less of this would need to be in the hapi module.

newswim commented 7 years ago

I'm quite interested in this becoming a thing. I'd like to contribute any way that I can!

benjie commented 7 years ago

If you look at the code on benjie/graphql-build you’ll see that it’s much reduced from v3. Would you like to experiment with building a similar codebase for HAPI? Basically it’s just the HTTP handling and pg connection code you need (plus optionally GraphiQL)

newswim commented 7 years ago

That does sound reasonable given the new architecture, but unfortunately I think it's a little beyond my current skill level. I will look into it more this week and try to get a better picture of the scope--another consideration is Hapi's own plugin architecture having changed quite a bit with their v17 release.. I'll see if I can make any headway 👷

benjie commented 7 years ago

Good luck! Feel free to drop messages in our gitter chat, we might be able to help if you get stuck.

mshick commented 6 years ago

@newswim @benjie I've put together a simple HAPI plugin for this. It provides the /graphql endpoint and supports JWT headers. It's at a very early state, but working for me.

https://github.com/mshick/hapi-postgraphile

If you try it out please let me know what you think.

benjie commented 6 years ago

Awesome!

If you think this is ready for other users (or when you are) feel free to send a PR adding a small comment to the relevant part of the README. Something like "If you are using HAPI, check out the experimental hapi-postgraphile module." or whatever works.

By the way, Node.js requirement is >= 8.6 rather than 8.0.

🙏

mshick commented 6 years ago

@benjie will do!

mshick commented 6 years ago

@benjie Added one brief line in. I'd say the plugin is simple enough that it doesn't need major caveats. My main concern right now is tests...

I also included some configurable features for storing the JWT in a cookie, and for creating an operation whitelist for caching discrete queries. Hopefully folks will find the project useful!

benjie commented 6 years ago

Putting the JWT in a cookie makes your API vulnerable to CSRF attacks; have you worked around that somehow?

mshick commented 6 years ago

@benjie Good question. The mix of the cookie having the httpOnly and secure flags set (defaults), coupled with CORS restrictions should prevent CSRF attacks that exploit the cookie in any uncompromised browser. The endpoint in this case is POST only, so CORS should always be triggered in the browser.

I realize there are situations that could compromise the browser and forge an origin header, but if your browser is compromised it seems like you have bigger problems, and I don't think Bearer tokens in the header protect against that more effectively.

hapi defaults to cors: false, so you'd be allowed same-origin only. When setting up CORS I'd expect the developer to use best practices.

I can add a note to the README about the cookie option and the CSRF vulnerabilities.

Please let me know if you think I'm off base with my assumptions. I would definitely like to know if I've missed something.

(I'll also add, I default the cookie behavior to off in the plugin)

benjie commented 6 years ago

You don’t need CORS to POST to a different origin; just submit a form for example. CORS is only required if you want to receive the result; you can do a lot of damage with GraphQL mutations without receiving the result - e.g. deleting records.

mshick commented 6 years ago

@benjie Doh, yes, of course... I've been in CORS / AJAX land for so long I sometimes forget.

I'm going to do some more research on this one, and see if hapi has a suggested approach. I know about crumb which is a plugin somebody could easily layer on to the route for anti-CSRF token support.

I'd like to see if hapi has origin / referer header verification options for all requests, or if I could / should bake some sort of allowedOrigins option into this plugin.

Based on the Prevention Cheat Sheet I would assume that if I can verify these headers I should be good against browser-based CSRF attacks.

Thanks so much for bringing this up.

mshick commented 6 years ago

@benjie I just pushed up a new version incorporating this feedback. I've added a number of configuration features and have secure defaults in place. By default everything behaves quite similar to your reference implementation, and the cookie behavior is entirely optional.

Essentially, when using cookies my solution is to check the hapi provided CORS match boolean (request.info.cors.isOriginMatch), and if it doesn't match I throw an error. Of course this doesn't protect against poorly configured CORS on the server, so I've added a section to the README to call it out.

Again, I appreciate your help bringing this up. Please let me know if you see any other issues.

benjie commented 6 years ago

I'm going to close this as I'm not keen to add another server to support; anyone who needs it should check out @mshick's module 👍

@mshick if you want I'd be happy for you to move that module under https://github.com/graphile-contrib - you're welcome to leave it where it is though! Completely up to you.