relay-rails-blog
relay-rails-blog copied to clipboard
Doesn't this allow someone to delete anything?
There's a pretty serious hole in site security with this code.
Here: https://github.com/gauravtiwari/relay-rails-blog/blob/master/app/api/mutations/comment_mutations.rb#L52
comment = RelaySchema.object_from_id(inputs[:id], ctx)
comment.destroy
I could pass in the global id for an admin's User record and it would load then destroy it. There are no protections around whether it actually loaded a Comment or whether the User has permissions to delete that Comment. I can destroy any object in the DB with that code.
Hey Paul, Thanks for pointing out. You know, I intended to do another series of post on authorisation, but never got chance (apart from keeping the project up-to-date). Will fix this and let you know.
@nuclearspike - made some small updates. I might do this differently, but for now this is good enough - https://github.com/gauravtiwari/relay-rails-blog/blob/master/app/api/relay_schema.rb#L35
https://github.com/gauravtiwari/relay-rails-blog/blob/master/app/api/mutations/comment_mutations.rb#L50
What do you think?
Yeah, that looks like it would protect the objects from malicious deletion.
What I've done in my own project is have a function that wraps all 3 actions, it loads the object by id, validates its type and checks the permission of what you're about to do. It throws appropriate errors. Use is something like:
comment = GraphLoader.load(args[:commentId], Comment, ctx, :destroy)
You could make a "load_and_authorize" method.
@nuclearspike Yeah, that sounds like a good idea. However, have you checked graphql-ruby pro version, it provides authorisation and some other stuff if you pay for it. So, I was thinking perhaps I can patch the basic version of the Gem to include a authorise and validate method just like pro version so, we can do load_and_authorize
on fields and even on schemas
https://github.com/rmosolgo/graphql-ruby/tree/master/guides/pro
@gauravtiwari I like the graphql-ruby project, I have not looked at the pro version before. However, I still feel like I'm doing way too much wiring between GraphQL and ActiveRecord, stuff that should be way more automatic. I've written my own class that helps me a little where you just feed it a model class and an array of attributes to expose and it looks up their types for you. It's rather basic though.
I recently came across GraphAPI. Which is much more the direction I'd like to see people moving... however, it doesn't seem to have a ton of recent activity and it doesn't support cancan or similar yet. But it offers automatically generated mutations and queries of your models with very little effort, but also allows you to extend what it's doing.