Local API: Where statement against access-limited fields throws an error

[chladog]

Link to reproduction

https://github.com/chladog/payload/tree/where-access-limited-error

To Reproduce

Following code is working fine until version 1.6.32 including, version 1.7.0 and newer throws an error.

1. I have custom endpoint returning an collection item.
2. Some fields have limited access (readonly group), but the where statement of the query is needs to to run against these restricted fields (readonly__domain, readonly__status).

The Local API should find the correct collection item - ignoring the access rules in where statement, but then it should strip the item of all properties that user doesn't have access to.

This behaves as described until version 1.6.32 - the collection item is found based on the where statement, and the object returned is correctly stripped of all fields that user doesn't have access to - in this case collection item without readonly group.
In versions 1.7.0. and newer this throws an error. I musn't set overrideAccess to true as that would return collecton item including fields that user shouldn't have access to (readonly group).

Proposed Solution

I believe access control shouldn't apply to where statements of the query.
If for some reason you believe it should apply to where statements, then there should be additional mechanism to turn off such behavior something like overrideAccessForWhere, that would allow for described behavior.

My Setup

mycollection endpoints config

endpoints: [
    {
      path: "/hostname/:hostname",
      method: "get",
      handler: async (req, res, next) => {
        try {
          console.log(req.params.hostname);
          res.json(await req.payload.find({
            collection: "posts",
            overrideAccess: false,
            req,
            where: {
              and: [
                {
                  readonly__domain: {
                    equals: req.params.hostname,
                  },
                },
                {
                  readonly__status: {
                    equals: "active",
                  },
                },
              ],
            },
          }));
        } catch (err) {
          console.error(err);
        }
      }
    }
  ],

mycollection limited field

  type: "group",
  name: "readonly",
  access: {
    read: fieldIsOwner,
  ...
 },
 ...

access function

export const fieldIsOwner: FieldAccess = ({ req: { user }, doc }) => {
  if (userIsAdmin(user)) {
    return true;
  }
  if (user) {
    return user.id === doc.owner;
  }
  return false;
};

Describe the Bug

QueryError: The following paths cannot be queried: readonly, readonly
    at new ExtendableError (...\node_modules\payload\src\errors\APIError.ts:26:11)
    at new APIError (...\node_modules\payload\src\errors\APIError.ts:43:5)
    at new QueryError (...\node_modules\payload\src\errors\QueryError.ts:8:5)
    at Function.buildQuery (...\node_modules\payload\src\mongoose\buildQuery.ts:570:15)
    at find (...\node_modules\payload\src\collections\operations\find.ts:125:17)
    at handler (...\src\collection.ts:177:40) {
  status: 400,
  data: [
    { path: 'readonly' },
    { path: 'readonly' },
  ],
  isPublic: false,
  isOperational: true
}

Payload Version

1.7.0+

[DanRibbens]

Hey @chladog,

For more info on the security concerns and why this was changed you can read thye CVE: GHSA-35jj-vqcf-f2jf

TLDR; by querying data you can reveal private information so you're correct that we need a mechanism to override allow access or some other way you might do it specific to your project.

[chladog]

hey @DanRibbens,
Ah, I understand why it was changed.
And indeed some extra mechanism for these cases when the developer is in control of the where statement would be perfect. Currently my only option is to overrideAccess completely and then manually erase the fields somehow, which wasn't comfortable as can be error-prone in the future and cannot afford leaking sensitive data therefore I got stuck at version 1.6.32 o:-) )

As said Local API parameter would be ideal IMHO and here are two ideas:

• new whereOverrideAccess or queryOverrideAccess option
• extend the overrideAccess option to allow pass an object specifying the behavior like:

overrideAccess: { 
        query: true,  // or "where"  -> this would allow filter on restricted fields
        fields: false  // or "object" or "data" or "doc" -> this would control visiblity of whole object and/or fields
}

[DanRibbens]

Wouldn't this be best to be done in your own collection afterRead hook to remove the property from the returned data after running your access control logic?

That way querying would still be possible on restricted fields the way you want it but the data wouldn't be able to be read directly.

I think this satisfies your requirements, what do you think @chladog?

[chladog]

I'm unsure that the needed behavior is possible to implement without being "too hacky" if you know what I mean @DanRibbens. It's quite similiar to the alternative I mentioned - that is running a query with overrideAccess: true, but then the need to manually shape the response object field by field based on user's access to it.
This solution would be viable, if there are exported utility functions that would shape the item based on the User and Collection Config automatically, but I couldn't find any in the docs, something as simple as ```shapeForUser(doc, user, collection);````.

Simply said I need to filter object against fields that are strictly private from users POV, but then return the filtered-out object shaped based on their rights.
I still believe such a simple use-case should have a simple solution as in terms of the complexity and maintainability of the app-code updating over 1.6.32 would be a regression and I honstely believe that such a feature would be useful to other devs as well, hopefully you understand my current restraint due to these reasons.

[jmikrut]

I think I see the value here. Just so I am clear - when you are suggesting this:

overrideAccess: { 
        query: true,  // or "where"  -> this would allow filter on restricted fields
        fields: false  // or "object" or "data" or "doc" -> this would control visiblity of whole object and/or fields
}

You're looking to use the Local API already, but just need more granularity. Right? If so, I think we could add this to our roadmap. Might need to do a bit more thinking through it, but I don't see why this would be impossible / or a bad idea.

Let me know. I will convert this to a Feature Request discussion and add it to our Roadmap in the meantime. 👍

[chladog]

@jmikrut
Exactly, I'm looking for granular access-control on Local API.
The whole point is that need of filtering of items by particular fields the does not necessarily intersect with particular user-access and since in my case the Local API function is used internally and its where property is not set by the user user is unable to reverse-engineer it with brute force CVE: GHSA-35jj-vqcf-f2jf.

I need to ignore access-restriction for Local API's where to get the corresponding doc (based on hidden/invisible-to-user values)
and at the same time I still need to respect the user access to the found data.

• If user does not have access to the returned object then return nothing
• If user does have the access to the object then shape it to have only those field values he has access to.