Vulcan
Vulcan copied to clipboard
UserId based permissions not behaving correctly
From Olivier Cado in the Slack:
Hi, I'm still playing with a SmartForm. When passing an existing documentId to edit a document, no fields show in the form.I traced a bit and found out that the CollectionPropsWrapper around the SmartForm generated a fragment that included all my fields except the userId, which resulted in the code considering that my user (an admin) was not owning the document ie. I had no permissions to edit it ie. no fields were relevant to be displayed.
So I added a queryFragment argument. At first it was the version from the getting-started tutorial with user { displayName } but Users.owns() was expecting userId. So I modified the fragment to include userId, then it worked.Is it something expected, should I always explicitly pass fragments to SmartForm? Automatic generation is a time-saver so I'd prefer it to include everything needed for checking permissions.
I could also reproduce the issue in getting-started by setting canUpdate to ['owners'] instead of ['members'] in some fields of the schema: the fields don't appear when I edit the movies I added.Also this means that another workaround, instead of passing a queryFragment, is to set the per-field canUpdate property to ['members'] while keeping it to ['owners'] in the permissions of the collection.
=> I suspect the issue is "userId" being removed to soon when not explicitely queried by the user query/mutation, so it makes permissions checking fail for other fields that uses "owners" permission
CollectionPropsWrapper do not really generate a fragment but get the collection based on either a "collection" props or the "collectionName".
So I guess the faulty fragment is just the default one?
If the permission check is client side, we have no other way but correctly fetching userId (not fetching it is a valid use case so we can't force it systematically eg for custom fragents), so I guess the fix is adding userId to some automated fragment generation, or find why it is not there in the first place (wrong read permissions?).
Maybe here: packages/getting-started/lib/modules/schema.js, in the userId schema I am not sure whether we preserve the userId field when adding the user resolver => that may break permissions?