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

Remove JDBC connector allow-drop-table flag #588

Merged
merged 2 commits into from
Feb 2, 2022

Conversation

electrum
Copy link
Member

@electrum electrum commented Apr 5, 2019

Remove the legacy allow-drop-table flag. This defaulted to false, so this is a behavior change, but it was inconsistent since we didn't restrict any other operations. Users can enable security using https://trino.io/docs/current/security/built-in-system-access-control.html.

@cla-bot cla-bot bot added the cla-signed label Apr 5, 2019
@findepi findepi requested a review from kokosing April 5, 2019 07:11
Copy link
Member

@kokosing kokosing left a comment

Choose a reason for hiding this comment

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

% comment

@electrum electrum force-pushed the jdbc-security branch 2 times, most recently from 4b60499 to f660baf Compare April 6, 2019 05:15
@urbanfletch
Copy link
Contributor

@electrum @findepi it looks like this change was approved. can it be merged?

Copy link
Member

@kokosing kokosing left a comment

Choose a reason for hiding this comment

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

Thanks!


public static SecuritySystem fromString(String value)
{
return valueOf(value.toUpperCase(ENGLISH).replace("-", "_"));
Copy link
Member

Choose a reason for hiding this comment

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

See first commit of airlift/airlift#880

@findepi
Copy link
Member

findepi commented Mar 15, 2021

Remove the legacy allow-drop-table flag. This defaulted to false, so this is a behavior change, but it was inconsistent since we didn't restrict any other operations

Before the change, a user could not erase data easily, since we don't have DELETE support, and DROP TABLE was disabled.
A user could delete data by DROP COLUMN for every column.

Thus the data was not secured against a malicious user, acting intentionally, but was protected against a careless or irrepressible user.
Current default doesn't provide this protect.

I agree allow-all would be the more consistent default behavior from long term perspective. But maybe this is too late for this, and we need read-only as the default?

@willmostly
Copy link
Contributor

@findepi IMO the connector level default should be allow-all, and any default restrictions should be applied at the system level. This reduces the potential for confusion.

@urbanfletch
Copy link
Contributor

@findepi I agree with Will's assessment. Ranger is what the field is using - having multiple locations will confuse the issue and make things more complicated for us and the customer.

@electrum electrum merged commit 15f41e6 into trinodb:master Feb 2, 2022
@electrum electrum deleted the jdbc-security branch February 2, 2022 18:02
@github-actions github-actions bot added this to the 370 milestone Feb 2, 2022
@electrum electrum changed the title Update JDBC connector security Remove JDBC allow-drop-table flag Feb 3, 2022
@electrum electrum changed the title Remove JDBC allow-drop-table flag Remove JDBC connector allow-drop-table flag Feb 3, 2022
@electrum electrum mentioned this pull request Feb 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.

5 participants