Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10135 )
Change subject: IMPALA-6802 (part 1): Clean up authorization tests ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/10135/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10135/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@95 PS2, Line 95: ); > I don't know if we want to go through every permutation in the error. It ca Maybe we don't need it for every test, but where is the test to ensure that "REFRESH" or other privileges do not unintentionally allow you to do select? Shouldn't that be somewhere with the select tests? http://gerrit.cloudera.org:8080/#/c/10135/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@509 PS2, Line 509: ) > Passing a full error string in expectedErrorString is essentially comparing The problem is, if I expect an error to be "... not authorized on functional", and the error is "... not authorized on functional.alltypes", I have no way to say I got the wrong error, i.e. there's information leakage on the error. -- To view, visit http://gerrit.cloudera.org:8080/10135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9cd5713607c423f644451af5ebb3494d3a728e3b Gerrit-Change-Number: 10135 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Adam Holley <g...@holleyism.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Comment-Date: Tue, 24 Apr 2018 15:37:00 +0000 Gerrit-HasComments: Yes
