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.

Sign up for GitHub

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

Jump to bottom

Roles PR #90

Merged
merged 61 commits into from
Jan 29, 2019
Merged

Roles PR #90

merged 61 commits into from
Jan 29, 2019

Conversation

sopel39
Copy link
Member

@sopel39 sopel39 commented Jan 28, 2019 •
edited
Loading

Extracted from: prestodb/presto#11645

@cla-bot
Copy link

cla-bot bot commented Jan 28, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@sopel39
Copy link
Member Author

sopel39 commented Jan 28, 2019

@martint @electrum please take a look at the PR and commits structure if they are fine with CLA

@cla-bot
Copy link

cla-bot bot commented Jan 28, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@sopel39
Copy link
Member Author

sopel39 commented Jan 28, 2019

For future reference this code was initially created by @arhimondr @cawallin @anusudarsan while at TD and massaged a lot by @kokosing, @sopel39, @anusudarsan, @findepi during many PR incarnations to prestodb

@kokosing kokosing mentioned this pull request Jan 29, 2019
Merged
Andrii Rosa and others added 24 commits January 29, 2019 12:00
@sopel39
Introduce CREATE ROLE and DROP ROLE statements
d177e3b 
Extracted-From: prestodb/presto#10904
@sopel39
Move PrincipalType to presto-spi
de48dc1 
Extracted-From: prestodb/presto#10904
@sopel39
Expose Create/Drop/List roles methods in SPI
3f7f4b7 
Extracted-From: prestodb/presto#10904
@sopel39
Introduce <catalog>.information_schema.roles table
dd359c5 
Extracted-From: prestodb/presto#10904
@sopel39
Assign admin role to subset of users in FileHiveMetastore
da86c1e 
Extracted-From: prestodb/presto#10904
@sopel39
Implement Create/Drop/List roles in Hive connector
22de2d8 
Extracted-From: prestodb/presto#10904
@sopel39
Introduce GRANT/REVOKE roles statements
c587c8f 
Extracted-From: prestodb/presto#10904
@sopel39
Add Grant/Revoke/List roles authorization to the SPI
03101af 
Extracted-From: prestodb/presto#10904
@sopel39
Introduce APPLICABLE_ROLES view
74f90b5 
Extracted-From: prestodb/presto#10904
@sopel39
Implement Grant/Revoke/ListApplicableRoles in Hive
006216d 
Extracted-From: prestodb/presto#10904
@sopel39
Refactor GRANT/REVOKE in Hive
35b85b4 
Leverage newly introduced method for recursive role grants traversal

Extracted-From: prestodb/presto#10904
@sopel39
Introduce access control for GRANT/REVOKE ROLE
487fe85 
Extracted-From: prestodb/presto#10904
@sopel39
Prepare metastore interface to accept ROLE for GRANT/REVOKE
f892c99 
Extracted-From: prestodb/presto#10904
@sopel39
Introduce SET ROLE statement
7ec759d 
Extracted-From: prestodb/presto#10904
@sopel39
Introduce ConnectorIdentity
1e4ff65 
Identity must hold all the selected roles for all the catalogs.
ConnectorIdentity holds only the role selected for some particular catalog.

Extracted-From: prestodb/presto#10904
@sopel39
Implement SET ROLE
e02bdab 
Extracted-From: prestodb/presto#10904
@sopel39
Make HiveQueryRunner default user have admin role
fb63425 
Extracted-From: prestodb/presto#10904
@sopel39
Set admin role before setting system properties in storage format test
dbad00d 
Extracted-From: prestodb/presto#10904
@sopel39
Store catalog selected roles in Identity
67b86c6 
Extracted-From: prestodb/presto#10904
@sopel39
Introduce ENABLED_ROLES view
7baabd1 
Extracted-From: prestodb/presto#10904
@sopel39
Implement SET ROLE in Hive Connector
f97f880 
Extracted-From: prestodb/presto#10904
@sopel39
Accept ROLE in GRANT/REVOKE Privileges statements
8707576 
Extracted-From: prestodb/presto#10904
@cawallin @sopel39
Add SHOW ROLES to the parser
7e8e3f0 
Extracted-From: prestodb/presto#10904
@cawallin @sopel39
Rewrite SHOW ROLES as a select query
eb49521 
For SHOW ROLES, issue the query:
select role_name as "Role" from catalog.information_schema.roles;

Extracted-From: prestodb/presto#10904
Andrii Rosa and others added 19 commits January 29, 2019 12:00
@sopel39
Consider enabled roles for permissions
8141cd3 
Extracted-From: prestodb/presto#10904
@sopel39
Refactor HivePrivilegeInfo
95f0e69 
Extracted-From: prestodb/presto#10904
@sopel39
Reorder methods in HivePrivilegeInfo
aec1458 
Extracted-From: prestodb/presto#10904
@sopel39
Move parsePrivilege to MetastoreUtil
8642d4e 
Extracted-From: prestodb/presto#10904
@sopel39
Add grantor to HivePrivilegeInfo
8e4c4e4 
Extracted-From: prestodb/presto#10904
@sopel39
Add grantor_type and grantee_type columns to table_privileges
6735d23 
Extracted-From: prestodb/presto#10904
@sopel39
More product tests for SET ROLE
aec0671 
Verify that role set with `SET ROLE` is considering during the access check.

Extracted-From: prestodb/presto#10904
@sopel39
Document role management
7d106b9 
Extracted-From: prestodb/presto#10904
@sopel39
Catalog access control for roles
33298dd 
Extracted-From: prestodb/presto#10904
@kokosing @sopel39
Remove unused metastore statistics beans
0bb22e4 
Extracted-From: prestodb/presto#10904
@kokosing @sopel39
Extract role name constants
07d80dc 
Extracted-From: prestodb/presto#10904
@kokosing @sopel39
Enumerate roles until enabled role is found
674285f 
Previously when SqlStandardAccessControl was checking if given role is
enabled, it listed all role grants and check if that role is is among
all listed role grants.
Now it list all role grants until it finds that role.

Extracted-From: prestodb/presto#10904
@kokosing @sopel39
Make ThriftMetastoreUtil.list*Roles methods to return Stream
7b73ba0 
That way roles are enumerated lazily.

Extracted-From: prestodb/presto#10904
@kokosing @sopel39
Make list*TablePrivileges to return Stream
c29e458 
This way table privileges are enumerated lazily.

Extracted-From: prestodb/presto#10904
@anusudarsan @sopel39
Fix listing privileges of tables owned by an user
d769507 
Currently Presto shows that the owner of a table
has ALL privileges, even after some privileges are revoked.
This commit fixes this issue by listing only privileges
actually present in the metastore.

Extracted-From: prestodb/presto#10904
@anusudarsan @sopel39
Fix listing privileges when admin role is set
0cf6e5b 
Presto currently lists only privilges of the
tables owned by the current user, even after the
admin role is set. This commit fixes this and lists all
privileges for admins.

Extracted-From: prestodb/presto#10904
@kokosing @sopel39
Fix rewrite SHOW GRANTS as a SELECT query
cabbfe6 
When tables of the same name exist across different schemas, Presto lists privileges
of the table from all schemas instead of the single schema mentioned in the
SHOW GRANTS query. This commit fixes the issue.

Extracted-From: prestodb/presto#10904
@kokosing @sopel39
Remove 'IN catalog' syntax from role management commands
b302214 
Extracted-From: prestodb/presto#10904
@sopel39
Refactor RecordingHiveMetastore to make it consistent between releases
9d717dc 
Extracted-From: prestodb/presto#10904
@cla-bot
Copy link

cla-bot bot commented Jan 29, 2019

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to [email protected]. If you are contributing on behalf of someone else (e.g., your employer), the individual CLA may not be sufficient and your employer may need the Corporate CLA signed.

@sopel39
Copy link
Member Author

sopel39 commented Jan 29, 2019

Our internal infra passed

@sopel39 sopel39 merged commit f958f81 into trinodb:master Jan 29, 2019
@sopel39 sopel39 deleted the epic/roles/7 branch January 29, 2019 12:26
@luohao luohao mentioned this pull request Jan 30, 2019
Closed
@tooptoop4
Copy link
Contributor

gentle ping for prestodb merge

@arhimondr arhimondr mentioned this pull request Feb 25, 2019
Merged
wenleix
wenleix reviewed Feb 26, 2019
View reviewed changes
presto-parser/src/main/java/io/prestosql/sql/parser/AstBuilder.java
@@ -11,6 +11,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

Copy link
Contributor

Choose a reason for hiding this comment

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

unrelated change

wenleix
wenleix reviewed Feb 26, 2019
View reviewed changes
Copy link
Contributor

@wenleix wenleix left a comment •
edited
Loading

Choose a reason for hiding this comment

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

[removed]

@ebyhr ebyhr mentioned this pull request Feb 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wenleix wenleix wenleix left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sopel39 @tooptoop4 @wenleix @cawallin @kokosing @anusudarsan