Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16113 )
Change subject: KUDU-3090 Restrict changing ownership of a table ...................................................................... Patch Set 10: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/16113/10/src/kudu/master/authz_provider.h File src/kudu/master/authz_provider.h: http://gerrit.cloudera.org:8080/#/c/16113/10/src/kudu/master/authz_provider.h@32 PS10, Line 32: lass TablePrivilegePB; : nit: drop the extra line? http://gerrit.cloudera.org:8080/#/c/16113/10/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/16113/10/src/kudu/master/catalog_manager.cc@2598 PS10, Line 2598: if (req.has_new_table_owner()) { : return SetupError(authz_provider_->AuthorizeChangeOwner(table_name, username, : username == owner), : resp, MasterErrorPB::NOT_AUTHORIZED); : } It seems easy to misconstrue this with a privilege escalation, since we're returning having only authorized the user for changing ownership. It's probably safe to do so because changing ownership requires ALL, and so it implies ALTER. That said, it'd be nice to add a comment addressing it here. -- To view, visit http://gerrit.cloudera.org:8080/16113 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75a8b24364572a84f93826ad670c543abd407bb1 Gerrit-Change-Number: 16113 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 Jul 2020 02:52:03 +0000 Gerrit-HasComments: Yes
