Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11753 )
Change subject: authz: verify tokens on scans ...................................................................... Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/11753/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11753/2//COMMIT_MSG@17 PS2, Line 17: if uses pk: > Does "if uses pk" mean "if projection includes pk"? That, and if we're doing a fault-tolerant (i.e. ORDERED) scan, since that passes around encoded primary keys as well. Added a note about it. http://gerrit.cloudera.org:8080/#/c/11753/2//COMMIT_MSG@13 PS2, Line 13: Scans or checksum scans require: : if no projected columns: : SCAN ON TABLE || foreach (column): SCAN ON COLUMN : else: : if uses pk: : foreach(primary key column): SCAN ON COLUMN : foreach(projected column): SCAN ON COLUMN : foreach(predicated column): SCAN ON COLUMN : : Split-key requests require: : if uses pk: : foreach(primary key column): SCAN ON COLUMN : foreach(requested column): SCAN ON COLUMN > Put this in a comment somewhere. Done http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/common/schema.h File src/kudu/common/schema.h: http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/common/schema.h@588 PS2, Line 588: std::vector<ColumnId> get_key_column_ids() const { > This could be simpler a la CreateKeyProjection: Done http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/integration-tests/authz_token-itest.cc File src/kudu/integration-tests/authz_token-itest.cc: http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/integration-tests/authz_token-itest.cc@110 PS2, Line 110: > warning: using decl 'SecureShortDebugString' is unused [misc-unused-using-d Done http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/integration-tests/authz_token-itest.cc@111 PS2, Line 111: using cluster::InternalMiniCluster; > warning: using decl 'ErrorStatusPB' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/integration-tests/authz_token-itest.cc@846 PS2, Line 846: > warning: the parameter 'send_req' is copied for each invocation but only us Done http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/integration-tests/authz_token-itest.cc@858 PS2, Line 858: // output based on 'is_authorized'. > update this Done http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/11753/2/src/kudu/tserver/tablet_service.cc@1495 PS2, Line 1495: if (PREDICT_FALSE(req->has_scanner_id() && > Splitting the authz checks into two chunks makes these RPC implementations Done I don't think we need to worry about leaking information in these cases. AFAIK tablet IDs and tablet locations aren't sensitive information? -- 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: 3 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[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-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 12 Mar 2019 07:21:44 +0000 Gerrit-HasComments: Yes
