graphql-engine icon indicating copy to clipboard operation
graphql-engine copied to clipboard

Remote Schema Permission Validation is overly strict for enum's with _PLACEHOLDER

Open Kaelten opened this issue 2 months ago • 0 comments

Version Information

Server Version: v2.34.0

What is the current behaviour?

I have two hasura instances, one acts as the gateway and has a remote schema to the other.

The remote schema has multiple roles of permissions. Some are very small slices of permissions and don't inherit from a larger body of permissions.

This setup has worked fine until we added a new role that just has insert permissions into a table and no update permissions. This left us first with an issue where the remote schema permission validation was erroring because the role we were using for that check didn't have access to those resolvers.

I then tried to use the 'admin' role for the validation query. This started failing for a different reason. On the role specific schemas the on_conflict argument creates empty enums with _PLACEHOLDER as the only value. Again, this validated fine until I tried using admin, and now since admin can see all the values that the enum could have, I get a _PLACEHOLDER not in enum type validation error.

I've traced this back to this snippet: https://github.com/hasura/graphql-engine/blob/e891998ba2c82ccc6da90a69dd24d97b109eb624/server/src-lib/Hasura/RemoteSchema/SchemaCache/Permission.hs#L628-L663

What is the expected behaviour?

I should be able to leverage remote schema permissions in this context. To do this I can see two possible paths forward:

  1. Update the schema permission logic to be more permissive by determining if something is a logical subset versus a pure subset, in the case of the enum validation, I believe that could be accomplished by tweaking the logic slightly. While I'm not fluent in haskell, I think this would roughly be the 'fix' by replacing lines 651 and 652 with this:
  for_ (NE.nonEmpty $ S.toList fieldsDifference) $ \nonExistingEnumVals -> do
    when (nonExistingEnumVals /= "_PLACEHOLDER") $
      dispute $ pure $ NonExistingEnumValues providedName nonExistingEnumVals

There could be more spots that would need to be addressed like this, but I'm not aware of them.

  1. Allow for each remote schema permission role to define additional headers that get defined and if they're present do a per-role schema query to get the remote schema for just that query. This would provide a more 'perfect' solution in some ways since the schema would be validated against the exact role that you're testing.

A rough example of what this could look like:

    - role: role_manager
      definition:
        headers:
          x-hasura-role: role_manager
        schema: |
          ...

How to reproduce the issue?

  1. Have a remote schema role with only insert permissions and no update permissions
  2. Try to validate that remote schema permission set when the introspection query uses the admin role.

Any possible solutions/workarounds you're aware of?

Not so far. I've tried:

  • Using an inherited role that would only be used for schema validation, but due to the way inherited roles merge permissions this results in type mismatches.
  • Removing _PLACEHOLDER from the enum definitions, but then the remote schema definition is rejected with a syntax error despite being, I think, technically valid.

Keywords

remote schema, permissions, validation, _PLACEHOLDER

Kaelten avatar May 10 '24 19:05 Kaelten