Document function rules for File-based Access Control #14774

huberty89 · 2022-10-26T17:53:25Z

Description

Update documentation for #13713

colebow

I've made an initial pass. Defer to @m57lyra if she suggests anything in the same places, as she's better at this than I am.

docs/src/main/sphinx/security/file-system-access-control.rst

m57lyra

Some things to take care of.

docs/src/main/sphinx/security/file-system-access-control.rst

m57lyra · 2022-10-31T17:59:47Z

docs/src/main/sphinx/security/file-system-access-control.rst

+    ``query`` polymorphic table function should be used by admins in a first
+    place.


I'm not quote sure what is meant here @huberty89 - Do you mean that only admins should have this privilege? If so:

" The query PTF is recommended for use only by admin or users.

Do you mean that only admins should have this privilege?

In short yes. query PTF bypass the whole access control and should be used carefully.
To make access more strict, admin should create a SECURITY DEFINER views which execute specific query using query PTF so user can use only those defined by admin.

docs/src/main/sphinx/security/file-system-access-control.rst

huberty89 · 2022-10-31T19:37:58Z

@colebow @m57lyra Please take a look, I applied fixed based on all comments

m57lyra

Just a couple of minor fixes needed.

docs/src/main/sphinx/security/file-system-access-control.rst

kokosing · 2022-10-31T21:01:59Z

docs/src/main/sphinx/security/file-system-access-control.rst

@@ -25,6 +25,13 @@ cluster in a single JSON file.
 Configuration
 -------------

+.. warning::


I think this should be part of the release notes. If we add warnings like that we may end up with messy docs.

Agreed, and it should be in a breaking change section, as this will break expected security behaviors.

I was told though that there are no special sections, and release notes are generated from issues?

Release notes are not generated. We write them .. but we do not have a dedicated breaking changes section. But this can be added .. probably was already .. @colebow ?

Also the warning is still appropriate .. maybe without the version info and past behavior though

I remove information about Trino versions

I think we still need this too in release notes.

docs/src/main/sphinx/security/file-system-access-control.rst

kokosing · 2022-10-31T21:06:02Z

docs/src/main/sphinx/security/file-system-access-control.rst

+.. note::
+
+    It is recommended to only allow admins to use the ``query`` polymorphic table
+    function.


Add "why"

Maybe it would be nice to have some best-practices somewhere. Where you would have config to grant access to PTF to admin, then create a (definer) view on top of it with admin as owner and granting access to view to regular users. Maybe we could explain that only somewhere.

I would even say .. remove that.. there is no reason for this to be best practice in general. Instead explain what the consequence is of this setup .. and earlier what happens by default

As in like .. if anyone is allowed to execute table functions.. then what?

query table function which works as a query pass-though - it's bypass the whole access control so IMO it's quite important to at least mention about it

Yeah .. mention.. but not recommend as best practice.. its just important to understand that it is kind of a security hole otherwise ;-) .. so we need to explain that and not just have a recommendation without detailing the reasoning

mosabua

Some changes like "regex" might have to wait and be done to the whole document later

mosabua · 2022-11-01T15:38:20Z

docs/src/main/sphinx/security/file-system-access-control.rst

@@ -25,6 +25,13 @@ cluster in a single JSON file.
 Configuration
 -------------

+.. warning::


Release notes are not generated. We write them .. but we do not have a dedicated breaking changes section. But this can be added .. probably was already .. @colebow ?

Also the warning is still appropriate .. maybe without the version info and past behavior though

docs/src/main/sphinx/security/file-system-access-control.rst

mosabua · 2022-11-01T15:43:21Z

docs/src/main/sphinx/security/file-system-access-control.rst

+.. note::
+
+    It is recommended to only allow admins to use the ``query`` polymorphic table
+    function.


I would even say .. remove that.. there is no reason for this to be best practice in general. Instead explain what the consequence is of this setup .. and earlier what happens by default

mosabua · 2022-11-01T15:44:56Z

docs/src/main/sphinx/security/file-system-access-control.rst

+.. note::
+
+    It is recommended to only allow admins to use the ``query`` polymorphic table
+    function.


As in like .. if anyone is allowed to execute table functions.. then what?

huberty89 · 2022-11-02T11:48:18Z

@mosabua @kokosing updated, please take a look

docs/src/main/sphinx/security/file-system-access-control.rst

mosabua

Let's update a bit more.

docs/src/main/sphinx/security/file-system-access-control.rst

mosabua

Looks good now. @martint could you merge this before the release since the feature already merged with the 401 release.

cla-bot bot added the cla-signed label Oct 26, 2022

huberty89 added the docs label Oct 26, 2022

kokosing mentioned this pull request Oct 26, 2022

Add Trino 401 release notes #14675

Merged

colebow requested review from mosabua and colebow October 31, 2022 17:33

jhlodin requested review from m57lyra and removed request for mosabua and colebow October 31, 2022 17:36

huberty89 mentioned this pull request Oct 31, 2022

Add table functions support to File-based access control #13713

Merged

colebow suggested changes Oct 31, 2022

View reviewed changes

m57lyra reviewed Oct 31, 2022

View reviewed changes

huberty89 force-pushed the hubert/fbac-functions-doc branch 2 times, most recently from 93c1b5d to 475c417 Compare October 31, 2022 19:33

m57lyra approved these changes Oct 31, 2022

View reviewed changes

docs/src/main/sphinx/security/file-system-access-control.rst Outdated Show resolved Hide resolved

docs/src/main/sphinx/security/file-system-access-control.rst Outdated Show resolved Hide resolved

docs/src/main/sphinx/security/file-system-access-control.rst Outdated Show resolved Hide resolved

huberty89 force-pushed the hubert/fbac-functions-doc branch from 475c417 to 729ded8 Compare October 31, 2022 19:43

colebow approved these changes Oct 31, 2022

View reviewed changes

kokosing reviewed Oct 31, 2022

View reviewed changes

mosabua requested changes Nov 1, 2022

View reviewed changes

huberty89 force-pushed the hubert/fbac-functions-doc branch from 729ded8 to 195091a Compare November 2, 2022 11:44

mosabua reviewed Nov 2, 2022

View reviewed changes

docs/src/main/sphinx/security/file-system-access-control.rst Outdated Show resolved Hide resolved

mosabua reviewed Nov 2, 2022

View reviewed changes

docs/src/main/sphinx/security/file-system-access-control.rst Show resolved Hide resolved

mosabua reviewed Nov 2, 2022

View reviewed changes

docs/src/main/sphinx/security/file-system-access-control.rst Show resolved Hide resolved

mosabua reviewed Nov 2, 2022

View reviewed changes

docs/src/main/sphinx/security/file-system-access-control.rst Outdated Show resolved Hide resolved

mosabua requested changes Nov 2, 2022

View reviewed changes

huberty89 force-pushed the hubert/fbac-functions-doc branch 2 times, most recently from 177a916 to a679b25 Compare November 2, 2022 16:34

mosabua reviewed Nov 2, 2022

View reviewed changes

docs/src/main/sphinx/security/file-system-access-control.rst Show resolved Hide resolved

mosabua reviewed Nov 2, 2022

View reviewed changes

docs/src/main/sphinx/security/file-system-access-control.rst Outdated Show resolved Hide resolved

Document function rules for File-based Access Control

c658368

huberty89 force-pushed the hubert/fbac-functions-doc branch from a679b25 to c658368 Compare November 2, 2022 17:22

mosabua approved these changes Nov 2, 2022

View reviewed changes

kokosing approved these changes Nov 4, 2022

View reviewed changes

kokosing merged commit bc7f2a7 into trinodb:master Nov 4, 2022

github-actions bot added this to the 403 milestone Nov 4, 2022

colebow mentioned this pull request Nov 9, 2022

Add Trino 403 release notes #14977

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document function rules for File-based Access Control #14774

Document function rules for File-based Access Control #14774

huberty89 commented Oct 26, 2022

colebow left a comment

m57lyra left a comment

m57lyra Oct 31, 2022

huberty89 Oct 31, 2022

huberty89 commented Oct 31, 2022

m57lyra left a comment

kokosing Oct 31, 2022

m57lyra Oct 31, 2022

m57lyra Oct 31, 2022

mosabua Nov 1, 2022

huberty89 Nov 2, 2022

kokosing Nov 2, 2022

kokosing Oct 31, 2022

mosabua Nov 1, 2022

mosabua Nov 1, 2022

huberty89 Nov 2, 2022

mosabua Nov 2, 2022

mosabua left a comment

mosabua Nov 1, 2022

mosabua Nov 1, 2022

mosabua Nov 1, 2022

huberty89 commented Nov 2, 2022

mosabua left a comment

mosabua left a comment

		``query`` polymorphic table function should be used by admins in a first
		place.

Document function rules for File-based Access Control #14774

Document function rules for File-based Access Control #14774

Conversation

huberty89 commented Oct 26, 2022

Description

colebow left a comment

Choose a reason for hiding this comment

m57lyra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huberty89 commented Oct 31, 2022

m57lyra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mosabua left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huberty89 commented Nov 2, 2022

mosabua left a comment

Choose a reason for hiding this comment

mosabua left a comment

Choose a reason for hiding this comment