Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16833 )
Change subject: [WiP] SAML implementation in Impala ...................................................................... Patch Set 9: (55 comments) http://gerrit.cloudera.org:8080/#/c/16833/9/be/src/rpc/authentication.cc File be/src/rpc/authentication.cc: http://gerrit.cloudera.org:8080/#/c/16833/9/be/src/rpc/authentication.cc@1160 PS9, Line 1160: callbacks.get_saml_redirect_fn = std::bind(GetSaml2Redirect, connection_ptr.get()); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/be/src/transport/THttpServer.h File be/src/transport/THttpServer.h: http://gerrit.cloudera.org:8080/#/c/16833/9/be/src/transport/THttpServer.h@160 PS9, Line 160: // 2. validating an authNRespone from the SSO provider line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/16833/9/be/src/transport/THttpServer.h@192 PS9, Line 192: // Used to collect all information about the http request. Can be passed to the Frontend. line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/AuthTokenGenerator.java File fe/src/main/java/org/apache/impala/authentication/saml/AuthTokenGenerator.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/AuthTokenGenerator.java@20 PS9, Line 20: // copy of https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/AuthTokenGenerator.java line too long (135 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java File fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java@32 PS9, Line 32: // https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlAuthTokenGenerator.java line too long (135 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java@52 PS9, Line 52: private static final Logger LOG = LoggerFactory.getLogger(HiveSamlAuthTokenGenerator.class); line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlGroupNameFilter.java File fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlGroupNameFilter.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlGroupNameFilter.java@31 PS9, Line 31: // https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlGroupNameFilter.java line too long (132 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateInfo.java File fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateInfo.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateInfo.java@20 PS9, Line 20: // copy of https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlRelayStateInfo.java line too long (139 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStore.java File fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStore.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStore.java@32 PS9, Line 32: // slightly modified copy of https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlRelayStateInfo.java line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStore.java@32 PS9, Line 32: // slightly modified copy of https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlRelayStateInfo.java line too long (158 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java File fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@42 PS9, Line 42: // modified version of https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSaml2Client.java line too long (144 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@65 PS9, Line 65: //TODO handle the replayCache as described in http://www.pac4j.org/docs/clients/saml.html line too long (93 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@146 PS9, Line 146: // This is done to keep original structure by Vihang + keep ImpalaSamlClient as the only line too long (92 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@191 PS9, Line 191: // https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java line too long (129 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@192 PS9, Line 192: private String doSamlAuth(WrappedWebContext webContext) throws HttpSamlAuthenticationException { line too long (98 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/service/BackendConfig.java File fe/src/main/java/org/apache/impala/service/BackendConfig.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/service/BackendConfig.java@245 PS9, Line 245: public String getSaml2GroupAttibuteName() { return backendCfg_.saml2_group_attribute_name; } line too long (94 > 90) http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/16833/9/fe/src/main/java/org/apache/impala/service/JniFrontend.java@688 PS9, Line 688: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py File test_saml.py: http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@6 PS9, Line 6: class NoRedirection(urllib2.HTTPErrorProcessor): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@11 PS9, Line 11: opener = urllib2.build_opener(NoRedirection) flake8: E305 expected 2 blank lines after class or function definition, found 1 http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@15 PS9, Line 15: #print result.geturl() flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@16 PS9, Line 16: #print result.info() flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@21 PS9, Line 21: #print query flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@22 PS9, Line 22: = flake8: E225 missing whitespace around operator http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@23 PS9, Line 23: #print saml_req flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@25 PS9, Line 25: #saml_req = urlparse.unquote(saml_req) flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@26 PS9, Line 26: #print "unquoted " + saml_req flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@80 PS9, Line 80: = flake8: E225 missing whitespace around operator http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@82 PS9, Line 82: #req = urllib2.Request("http://localhost:28000/SAML2/SSO/POST";, "SAMLResponse=" + samlResponse + "&RelayState=1234") flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@82 PS9, Line 82: o flake8: E501 line too long (116 > 90 characters) http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@83 PS9, Line 83: #result = opener.open(req) flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@84 PS9, Line 84: #print result.headers flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@85 PS9, Line 85: #new_url = result.info().getheader("location") flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@86 PS9, Line 86: #print new_url flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@87 PS9, Line 87: #bearer_token = result.info().getheader("WWW-Authenticate").split(" ")[1]; flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@88 PS9, Line 88: #print bearer_token flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@90 PS9, Line 90: #bearer_token="u=_02c81a85938a2639ed9b7fb9bdae7b697e76b80bdc;id=7016226164652493581;time=1608051695185;rs=e9d08eac-458b-44e3-afbd-a3c3870d7c20;sg=eFOkCkMXTRoj7QrmaLFeZP7pGtxmcj2+5izt7qWwnz8=" flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@90 PS9, Line 90: 6 flake8: E501 line too long (191 > 90 characters) http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@91 PS9, Line 91: #req = urllib2.Request("http://localhost:28000";, " ") flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@92 PS9, Line 92: #req.add_header('Authorization', 'Bearer ' + bearer_token) flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@93 PS9, Line 93: #result = opener.open(req) flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@94 PS9, Line 94: #print result.info() flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/test_saml.py@95 PS9, Line 95: #print result.getcode() flake8: E265 block comment should start with '# ' http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py File tests/custom_cluster/test_saml2_sso.py: http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@20 PS9, Line 20: import logging flake8: F401 'logging' imported but unused http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@28 PS9, Line 28: from tests.common.impala_service import ImpaladService flake8: F401 'tests.common.impala_service.ImpaladService' imported but unused http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@31 PS9, Line 31: from tests.shell.util import run_impala_shell_cmd, run_impala_shell_cmd_no_expect, \ flake8: F401 'tests.shell.util.run_impala_shell_cmd_no_expect' imported but unused http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@31 PS9, Line 31: from tests.shell.util import run_impala_shell_cmd, run_impala_shell_cmd_no_expect, \ flake8: F401 'tests.shell.util.ImpalaShell' imported but unused http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@34 PS9, Line 34: class NoRedirection(urllib2.HTTPErrorProcessor): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@41 PS9, Line 41: class TestClientSsl(CustomClusterTestSuite): flake8: E303 too many blank lines (3) http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@46 PS9, Line 46: flake8: W291 trailing whitespace http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@46 PS9, Line 46: SP_URL = "https://localhost:8443/simplesaml/saml2/idp/SSOService.php"; line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@82 PS9, Line 82: r flake8: F841 local variable 'relay_state' is assigned to but never used http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@97 PS9, Line 97: d flake8: F841 local variable 'decoded_saml_req' is assigned to but never used http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@103 PS9, Line 103: flake8: W293 blank line contains whitespace http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@103 PS9, Line 103: line has trailing whitespace http://gerrit.cloudera.org:8080/#/c/16833/9/tests/custom_cluster/test_saml2_sso.py@104 PS9, Line 104: flake8: W391 blank line at end of file -- To view, visit http://gerrit.cloudera.org:8080/16833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia0c026cba1b90e7ff6ec5ae49be78b0d1edd8dfa Gerrit-Change-Number: 16833 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Wed, 13 Jan 2021 21:42:42 +0000 Gerrit-HasComments: Yes
