[ 
https://issues.apache.org/jira/browse/HDFS-15765?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17697229#comment-17697229
 ] 

ASF GitHub Bot commented on HDFS-15765:
---------------------------------------

saxenapranav commented on code in PR #5447:
URL: https://github.com/apache/hadoop/pull/5447#discussion_r1127353729


##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/URLConnectionFactory.java:
##########
@@ -120,7 +136,7 @@ public HttpURLConnection configure(HttpURLConnection 
connection)
       }
     }
 
-    return conn;
+    return new BasicAuthConfigurator(conn, basicAuthCredentials);

Review Comment:
   should we have something like:
   ```
   private ConfigurationConfigurator getConfigurator(ConfigurationConfigurator 
configurator, String basicAuthCred) {
   if(basicAuthCred != null && !basicAuthCred.isEmpty()) {
   return new BasicAuthConfigurator(configurator, basicAuthCred);}
    else {return configurator;}
   }
   ```
   and instead of creating new object of basicAuthConfigurator, we call this 
new method. And if an object of BasicAuthConfigurator can be made, method will 
return that new object.
   What you say?



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/test/java/org/apache/hadoop/hdfs/web/TestBasicAuthConfigurator.java:
##########
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.web;
+
+import org.apache.hadoop.security.authentication.client.ConnectionConfigurator;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+
+public class TestBasicAuthConfigurator {
+  @Test
+  public void testNullCredentials() throws IOException {
+    ConnectionConfigurator conf = new BasicAuthConfigurator(null, null);
+    HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+    conf.configure(conn);
+    Mockito.verify(conn, Mockito.never()).setRequestProperty(Mockito.any(), 
Mockito.any());
+  }
+
+  @Test
+  public void testEmptyCredentials() throws IOException {
+    ConnectionConfigurator conf = new BasicAuthConfigurator(null, "");
+    HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+    conf.configure(conn);
+    Mockito.verify(conn, Mockito.never()).setRequestProperty(Mockito.any(), 
Mockito.any());
+  }
+
+  @Test
+  public void testCredentialsSet() throws IOException {
+    ConnectionConfigurator conf = new BasicAuthConfigurator(null, "user:pass");
+    HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+    conf.configure(conn);
+    Mockito.verify(conn, Mockito.times(1)).setRequestProperty(
+        "AUTHORIZATION",
+        "Basic dXNlcjpwYXNz"
+    );
+  }
+
+  @Test
+  public void testParentConfigurator() throws IOException {
+    ConnectionConfigurator parent = Mockito.mock(ConnectionConfigurator.class);
+    ConnectionConfigurator conf = new BasicAuthConfigurator(parent, 
"user:pass");
+    HttpURLConnection conn = Mockito.mock(HttpURLConnection.class);
+    conf.configure(conn);
+    Mockito.verify(conn, Mockito.times(1)).setRequestProperty(
+        "AUTHORIZATION",
+        "Basic dXNlcjpwYXNz"

Review Comment:
   should we have` "Basic " + Base64.getEncoder().encodeToString(
                 credentials.getBytes(StandardCharsets.UTF_8)`
   
   as its not clear from test how user:pass is converting to `dXNlcjpwYXNz`





> Add support for Kerberos and Basic Auth in webhdfs
> --------------------------------------------------
>
>                 Key: HDFS-15765
>                 URL: https://issues.apache.org/jira/browse/HDFS-15765
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: hadoop-client
>            Reporter: Pushpendra Singh
>            Priority: Minor
>              Labels: pull-request-available
>
> webhdfs's HTTP operation like get ( GetOpParam.java) operation and other HTTP 
> operation has 'requireAuth' set to false and expected to work with Delegation 
> token only. However, when working with webhdfs over Apache Knox, delegation 
> token authentication is not supported, we should support Kerberos 
> authentication (SPNEGO) or Basic authentication for WebHdfsFileSystem if user 
> turns on a configuration.
> Further webhdfs (WebHDFSFileSystem.java)  is calling 'public URLConnection 
> openConnection(URL url)' and providing no way to use the kerberos 
> authentication, if configured.
> Even after setting the UserGroupInformation with user name and keytab, 
> openConnection is not using the keytab for authentication.
> Also WebHdfsFileSystem doesn't provide any support for HTTP BASIC 
> authentication (username/password). Provide support to read the password via 
> environment variable.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to