Hi there, just wanted to say great work!
I am currently working on a project that involves a multi-tenant graph logically partitioned using tenant labels on nodes. The autogenerated queries are great for allowing us to quickly mock out an api as we iterate, but there doesn't seem to be any way to support auth/access control for the multi-tenant graphs using labels for logical partitioning.
It would be great if there was a way to either:
a) Provide direct support for multi-tenancy via labels (i.e. attach labels from context on create, check on update and read) that restrict access/results in the automagically generated queries, or
b) Allow us to retrieve/set attributes of nodes not specified in the query, allowing a custom directive to implement auth (by requesting and checking the label). Seems like mutations might be a bit of an edge case, unless you only allow mutations by id.
option b) is great, because it would be nice for maintaining some other metadata we use on our nodes. But maybe what I really want is just a neo4j ORM that uses my graphql schema.....
https://community.neo4j.com/t/proper-way-to-implement-multi-tenancy-on-neo4j/625
First of all, as a cautionary note, I think multi-tenancy is going to be supported by neo4j v4.0. I don't know what the implementation will look like, but tenant labels have been one workaround in its absence. Not sure whether it makes sense to develop this feature based on the workaround or the future supported multi-tenancy.
Second, because adding multiple labels has more use cases than just multi-tenancy, I wonder if it would be possible to use Interfaces to accomplish that. You could add a directive that signifies whether to append the Interface as a secondary label on the node. More advanced implementation could even allow for dynamically-defined labels based on some logic supplied by the developer on the back-end; this seems like what would be needed for the multi-tenant support you describe.
Interesting. Could you provide a bit of detail on how that directive might be implemented? Based on my attempts it involves modifying the query AST, which isn't particularly well documented.
In index.js, both cypherQuery and cypherMutation get the typeName from the resolveInfo using the function typeIdentifiers() from utils.js. To implement this, you would basically just have to modify typeIdentifiers() to check the for the presence of a new directive (like @additionalLabels) and, if present, append the additional labels to the typeName that is returned for use in the auto-generated cypher.
However, I might be missing other uses of typeName that you wouldn't want to mess with, in which case this could be done one level below cypherQuery or cypherMutation in the translateQuery or translateMutation functions of translate.js. Pretty straightforward as I think through it.
You can use the existing code for the neo4j-graphql-js custom directives (like @cypher and @relation) as a model for how to add/use whatever new directive you would need.
(Edit: I took a quick peek, and it doesn't look like typeName is used for anything other than feeding into the safeLabel function, which would also have to be modified slightly to accommodate multiple labels. It currently wraps the single label name with back-ticks, with the leading colon hard-coded into the query -- not the safeLabelName itself; you would want to wrap each label separately and place colons between them all.)
(Edit 2: I'm going to start a new feature branch on my fork when I get home. After digging a little more, it seems like the murkiest part for me is going to be the relationships, mostly because it's one of the things I grokk least in this code base. The mutation & query of nodes seems straightforward -- famous last words -- but you don't want outside tenants to necessarily be able to view/create/destroy relationships between nodes they don't own. I'll post a link when I get somewhere with it.)
Cool, thanks! When I get around to taking a look, happy to play around and offer some feedback/input.
@imkleats What is the status of your branch? I'm starting a new project now where I need multi-tenant support. I read somewhere that if you implement multi-tenant with node labels the migration to neo4j version 4 (that should support multi-tenants) should be quite straight forward.
Hello! Good news! (I hope)
I've forked the project, updated it to new babel version + new ava version + implemented support for adding labels (can also use parameters from chyperParams) inside the schema. Tests are also updated!
Here is my fork
https://github.com/cmartin81/neo4j-graphql-js-1
That's awesome! Thanks. I've been MIA from making contributions because of
work/life.
Here's a different approach to tenancy that might work pretty well, too,
without the need for much modification of the base query generation for
read-only, update and relationship creation operations:
Leveraging the ability to make a filtered subgraph against which to apply
the otherwise unmodified auto-generated query.
You would still need to account for the addition of tenant labels in node
creation, which Christian's fork could hopefully accommodate. But if that
fork does everything anyway, you wouldn't really need this alternative
implementation.
On Wed, Jul 24, 2019, 9:45 AM Christian Martin notifications@github.com
wrote:
Hello! Good news! (I hope)
I've forked the project, updated it to new babel version + new ava version
- implemented support for adding labels (can also use parameters from
chyperParams) inside the schema. Tests are also updated!—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/neo4j-graphql/neo4j-graphql-js/issues/237?email_source=notifications&email_token=AFBH3PPPFZYYZOTV6JA3NZTQBB2LBA5CNFSM4HIWAVZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2WYJCY#issuecomment-514688139,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFBH3PIJ5I2SSHKSOC557LTQBB2LBANCNFSM4HIWAVZQ
.
Here is my fork
https://github.com/cmartin81/neo4j-graphql-js-1
Cool! When I get the chance, I'll try it out and let you know how it goes!
@mchong-teal I've tested til locally and it works great :)
Test result
schema
type Person @isAuthenticated @additionalLabels(labels: ["org_<%= $cypherParams.organizationId %>"]) {
...
...
The cypher that is created:
MATCH (`person`:`Person`:`org_86276b57-a39a-409a-be51-efd366dfb515`) RETURN `person` { .name } AS `person`
{
"offset": 0,
"first": -1,
.....
.....
But if you want to try it out before it is accepted, you have to clone and build it locally
@imkleats my code is only adding additional labels to the node queries (create, update, delete) and then you can decide yourself if you want to solve multi-tenant with extra labeling.
@mchong-teal
I've just published it to npm (I will unpublish when pull request is accepted)
`
npm install neo4j-graphql-with-additional-labels-directive-js --save-dev
@mchong-teal The PR is accepted now. So you should be able to use the original npm package :)
great news. thanks!
This feature is now available in v2.7.0, thanks to @cmartin81! And referenced in the docs here
Closing this issue, but please reopen if this new feature doesn't address the use case.
BTW @cmartin81 - it would be great if you'd like to write a blog post for blog.grandstack.io about this feature and how you use it :-)