Configurable VIEW security mode

[huw0]

Hi Rob,

I've just been made aware of this project via your appearance and comments in the latest Trino Community Broadcast.

Firstly, thanks! I have a project with a custom module that does something very similar to viewzoo. Being able to migrate away from our DIY module to something open-source would be really handy.

In #11 you mention...

both invoker modes are supported

What do you think about making the security mode of views configurable? e.g only supporting INVOKER mode?
Ideally when this is enabled this would be enforced both at the time of view being created/amended and when the view is run.

When storing views in git, the mode could be enforced as part of the PR process, but being a configuration option might be handy as an additional security control.

[robfromboulder]

Nice to meet you, and thanks so much for sharing this idea!

Having an option to enforce the security mode is really cool. Viewzoo could check the security mode when views are read from storage, and could check that modifications to view definitions use the intended security mode (and change the security mode by policy). The only caution flag I see is that viewzoo isn't actually in the loop when a view is executed, but enforcing this at the storage boundary seems totally doable.

Stupid question for you: if DEFINER mode is used accidentally, would you prefer viewzoo to throw an error (so the view doesn't get read or updated), or silently use INVOKER instead? (or have the option for either mode?)

ps. I know we're getting to implementation questions here pretty quick, but again I really appreciate you reaching out and would love to close any gaps that would make this more useful!

[huw0]

The only caution flag I see is that viewzoo isn't actually in the loop when a view is executed, but enforcing this at the storage boundary seems totally doable.

This is a good point and to some extent I wonder if doing this in the storage layer is bad practice since it is essentially handling authorization at the wrong layer?
It could probably be handled with access control implementing SystemSecurityContext#checkCanViewQueryOwnedBy - but only if Trino core was extended so that access control is aware of the security mode.

However at the same time doing this at the storage boundary enforces that the check does always take place and leaves less room for accidental configuration error.

Stupid question for you: if DEFINER mode is used accidentally, would you prefer viewzoo to throw an error (so the view doesn't get read or updated), or silently use INVOKER instead? (or have the option for either mode?)

I think throwing an error is probably the most sensible approach to avoid user confusion if the view description indicates one mode but the actual execution uses another.

In the git-sync scenario, it'd probably be best to throw the error at query time, rather than on-load to enable user visibility of the issue and allow them to fix it (e.g via another PR).
I'm working on the assumption that in many cases a user may not have access to Trino logs, but will be able to see errors for their own queries.