rpitv / glimpse

Monorepo for the RPI TV Glimpse project
MIT License
3 stars 1 forks source link

Bug: Field-based permission checks do not apply to automatically generated fields #9

Open robere2 opened 1 year ago

robere2 commented 1 year ago

When a user creates/updates an object, we check that they have permission to mutate the specific fields they passed in, and then we make sure they have permission to mutate those fields on the specific object, however we don't check for automatically generated values.

Consider a user with the following permission:

{
  "action": "create",
  "subject": ["Production"],
  "fields": ["name", "description"]
}

When the user submits a mutation to create a Production and supplies the "name" and "description" values, the permission checks will pass and the Production will be created. However, in our current schema, the database/Prisma also generates values for two additional fields: id and startTime. The user does not have permission to create objects with these fields, and thus, the request should fail. It does not, however, since the API is only checking that the user has permission to create the name and description fields.

This is not a major issue, as usually it is likely implied that the user has permission to "create" these fields via automatic generation. Arguably, it's not an issue at all. One advantage to the current implementation is that the absence of the startTime field permission requires that the value always be equal to the current time. If these changes are made, then to require this would mean adding a condition to the permission that requires the startTime value to be equal to the current time. I cannot come up with a great way to solve this problem (the "current time" will vary by 10-1000+ milliseconds throughout the entire lifetime of the request).

Regardless, this is something worth thinking about, and at the very least, worth taking into account when creating your permissions. If we decide not to fix this "issue", then something should be added to the wiki about it.