Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test access control functionality in the context of table redirections #11488

Merged

Conversation

findinpath
Copy link
Contributor

@findinpath findinpath commented Mar 15, 2022

Description

With the addition of the new functionality related to adding table redirection awareness to multiple SQL commands in Trino there needs to be invested additional effort in making sure that the access control works as expected when performing table redirections.

Note that the new tests from this PR don't aim to replace the tests from the classio.trino.security.TestAccessControl, but rather enrich the suite of tests for the access control.

Any new table redirection awareness change on a SQL command should add a corresponding test to the class io.trino.security.TestAccessControlTableRedirection.

Is this change a fix, improvement, new feature, refactoring, or other?

Test

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

This test is ensuring that the access control functionality which is part of the core query engine works as expected.

How would you describe this change to a non-technical end user or system administrator?

This PR aims to add basic access control tests for the existing commands doing table redirects.
Depending on the connector, a table redirect can point to another table in the same schema or to a table in a different connector. This PR makes sure that the access control checks for the appropriate permissions on the redirected table and not on the source table.

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Mar 15, 2022
@findinpath findinpath requested review from findepi, kokosing and phd3 March 15, 2022 07:22
@findinpath findinpath force-pushed the table-redirection-access-control-tests branch from df554e5 to 5a72611 Compare March 15, 2022 07:31
@findinpath findinpath force-pushed the table-redirection-access-control-tests branch from 5a72611 to 5a80d07 Compare March 23, 2022 04:45
@findinpath findinpath force-pushed the table-redirection-access-control-tests branch from 5a80d07 to a5b9265 Compare March 28, 2022 09:05
@findinpath findinpath force-pushed the table-redirection-access-control-tests branch from a5b9265 to f6dec74 Compare March 31, 2022 05:59
@kokosing kokosing merged commit 7e552f9 into trinodb:master Apr 3, 2022
@kokosing
Copy link
Member

kokosing commented Apr 3, 2022

Merged, thanks!

@github-actions github-actions bot added this to the 376 milestone Apr 3, 2022
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.

2 participants