Skip to content

Add run-views-as-invoker configuration property #23734

Closed
dejangvozdenac wants to merge 1 commit intotrinodb:masterfrom
dejangvozdenac:run-views-as-invoker
Closed

Add run-views-as-invoker configuration property #23734
dejangvozdenac wants to merge 1 commit intotrinodb:masterfrom
dejangvozdenac:run-views-as-invoker

Conversation

@dejangvozdenac
Copy link
Copy Markdown

@dejangvozdenac dejangvozdenac commented Oct 9, 2024

Description

This PR adds a new system session property run-views-as-invoker.

If set, this property makes it so that any view is granted access based on the permissions of the user who is querying the view, assuming that the security permission checks are enabled.

This is useful for platform administrators who don't want to enable DEFINER mode and want to force all view authorization to use INVOKER due to platform security decisions or changing access of definers.

Additional context and related issues

Something similar was already implemented for the Hive connector in #12221.
Some discussion in #14817 and #14790.

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# General
* Add  `run-views-as-invoker` configuration property to force all views to run in INVOKER mode.

@cla-bot cla-bot bot added the cla-signed label Oct 9, 2024
@dejangvozdenac dejangvozdenac changed the title [WIP] implement run-views-as-invoker Add run-views-as-invoker configuratino property Oct 11, 2024
@dejangvozdenac dejangvozdenac changed the title Add run-views-as-invoker configuratino property Add run-views-as-invoker configuration property Oct 11, 2024
@findepi findepi requested review from Praveen2112, dain and lukasz-walkiewicz and removed request for findepi October 12, 2024 18:40
@github-actions
Copy link
Copy Markdown

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions
Copy link
Copy Markdown

This pull request has gone a while without any activity. Tagging for triage help: @mosabua

@github-actions github-actions bot added the stale label Dec 27, 2024
Copy link
Copy Markdown
Member

@dain dain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will be accepted. This changes the behavior of views after they have been stored. This will cause the views to act very differently, and could cause a security vulnerability (because this changes the security checks we would perform).

The flag in Hive only exists because the handling of legacy views is not well defined in Hive, and to make matters worse, when Amazon implemented views in Athena they did it wrong. This is why there is an option to declare how legacy trino and translated hive views handle run-as-invoker vs run-as-definer, but this flag does not apply to modern Trino views. Instead there is a flag stored in the view that states the desired behavior of the view. When views were added to Iceberg and Delta definer views were properly supported, and if we were to add view support to any other table, we would do the same.

I think the best way to stop views from executing as the definer is to block them from being created in the underlying connector. We don't have an option to prevent the creation of definer view, so if you are interested, I suggest starting a converstion on Slack.

@github-actions github-actions bot removed the stale label Dec 31, 2024
@mosabua mosabua closed this Jan 10, 2025
@mosabua
Copy link
Copy Markdown
Member

mosabua commented Jan 10, 2025

Closing as follow up to the info from @dain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants