Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11753 )
Change subject: WIP authz: verify tokens on scans ...................................................................... Patch Set 1: (3 comments) Just responding http://gerrit.cloudera.org:8080/#/c/11753/1/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/11753/1/src/kudu/tserver/tablet_service.cc@434 PS1, Line 434: is it a bug that we didn't verify the table id? > yes I'm inclined to agree, hence the TODO, though to add more color here, say we were to verify the table id and then we got no scan privileges, we'd still get FATAL_UNAUTHORIZED. Or say we were to attempt to verify the table id and it were wrong, still FATAL_UNAUTHORIZED. http://gerrit.cloudera.org:8080/#/c/11753/1/src/kudu/tserver/tablet_service.cc@435 PS1, Line 435: context->RespondRpcFailure(rpc::ErrorStatusPB::FATAL_UNAUTHORIZED, > What if all the columns are authorized? `authorized_column_ids` wouldn't be empty and we'd check them after this function. I don't think we can make the judgement at this point to say whether a set of given column IDs is "all the columns". http://gerrit.cloudera.org:8080/#/c/11753/1/src/kudu/tserver/tablet_service.cc@1589 PS1, Line 1589: !authorized_table_id.empty() > This is error prone, since it will cause bugs if VerifyAuthzTokenForScanOrR Yeah, makes sense. -- To view, visit http://gerrit.cloudera.org:8080/11753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7a5d81cf215a5d936f8853feba05778038764905 Gerrit-Change-Number: 11753 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 23 Oct 2018 04:37:30 +0000 Gerrit-HasComments: Yes
