[GitHub] yush1ga commented on a change in pull request #1087: Add basic authentication plugin

2018-01-24 Thread GitBox
yush1ga commented on a change in pull request #1087: Add basic authentication 
plugin
URL: https://github.com/apache/incubator-pulsar/pull/1087#discussion_r163719058
 
 

 ##
 File path: 
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
 ##
 @@ -81,12 +81,15 @@ public String authenticate(AuthenticationDataSource 
authData, String authMethodN
 }
 }
 
-public String authenticateHttpRequest(HttpServletRequest request) throws 
AuthenticationException {
+public String authenticateHttpRequest(HttpServletRequest request) throws 
PulsarHttpAuthenticationException {
+PulsarHttpAuthenticationException exception = new 
PulsarHttpAuthenticationException("Authentication required");
 
 Review comment:
   I made it create `realmInformation` string to avoid creating an exception.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] yush1ga commented on a change in pull request #1087: Add basic authentication plugin

2018-01-23 Thread GitBox
yush1ga commented on a change in pull request #1087: Add basic authentication 
plugin
URL: https://github.com/apache/incubator-pulsar/pull/1087#discussion_r163436049
 
 

 ##
 File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/AuthenticationFilter.java
 ##
 @@ -42,11 +43,18 @@
 private static final Logger LOG = 
LoggerFactory.getLogger(AuthenticationFilter.class);
 
 private final AuthenticationService authenticationService;
+private final boolean isBasic;
 
 public static final String AuthenticatedRoleAttributeName = 
AuthenticationFilter.class.getName() + "-role";
 
 public AuthenticationFilter(PulsarService pulsar) {
 this.authenticationService = 
pulsar.getBrokerService().getAuthenticationService();
+if (pulsar.getConfiguration().getAuthenticationProviders()
+.contains(AuthenticationProviderBasic.class.getName())) {
+isBasic = true;
 
 Review comment:
   Make sense.
   I added PulsarHttpAuthenticationException class, which has realm information 
for `WWW-Authentication` header and the information is gathered in 
`authenticateHttpRequest`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] yush1ga commented on a change in pull request #1087: Add basic authentication plugin

2018-01-23 Thread GitBox
yush1ga commented on a change in pull request #1087: Add basic authentication 
plugin
URL: https://github.com/apache/incubator-pulsar/pull/1087#discussion_r163435001
 
 

 ##
 File path: 
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderBasic.java
 ##
 @@ -0,0 +1,147 @@
+/**
+ * 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.pulsar.broker.authentication;
+
+import org.apache.commons.codec.digest.Crypt;
+import org.apache.commons.codec.digest.Md5Crypt;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.pulsar.broker.ServiceConfiguration;
+
+import javax.naming.AuthenticationException;
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.util.*;
+
+public class AuthenticationProviderBasic implements AuthenticationProvider, 
RealmInformationSupport {
+private final static String HTTP_HEADER_NAME = "Authorization";
+private final static String CONF_SYSTEM_PROPERTY_KEY = 
"pulsar.auth.basic.conf";
+private Map users;
+
+@Override
+public void close() throws IOException {
+// noop
+}
+
+@Override
+public void initialize(ServiceConfiguration config) throws IOException {
+File confFile = new File(System.getProperty(CONF_SYSTEM_PROPERTY_KEY));
+if (!confFile.exists()) {
+throw new IOException("The password auth conf file does not 
exist");
+} else if (!confFile.isFile()) {
+throw new IOException("The path is not a file");
+}
+BufferedReader reader = new BufferedReader(new FileReader(confFile));
+users = new HashMap<>();
+for (String line : reader.lines().toArray(s -> new String[s])) {
+List splitLine = Arrays.asList(line.split(":"));
+if (splitLine.size() != 2) {
+throw new IOException("The format of the password auth conf 
file is invalid");
+}
+users.put(splitLine.get(0), splitLine.get(1));
+}
+reader.close();
+}
+
+@Override
+public String getAuthMethodName() {
+return "basic";
+}
+
+@Override
+public String authenticate(AuthenticationDataSource authData) throws 
AuthenticationException {
+AuthParams authParams = new AuthParams(authData);
+String userId = authParams.getUserId();
+String password = authParams.getPassword();
+String msg = "Unknown user or invalid password";
+
+if (users.get(userId) == null) {
+throw new AuthenticationException(msg);
+}
+
+String encryptedPassword = users.get(userId);
+
+// For md5 algorithm
+if ((users.get(userId).startsWith("$apr1"))) {
+List splitEncryptedPassword = 
Arrays.asList(encryptedPassword.split("\\$"));
+if (splitEncryptedPassword.size() != 4 || !encryptedPassword
+.equals(Md5Crypt.apr1Crypt(password.getBytes(), 
splitEncryptedPassword.get(2 {
+throw new AuthenticationException(msg);
+}
+// For crypt algorithm
+} else if (!encryptedPassword.equals(Crypt.crypt(password.getBytes(), 
encryptedPassword.substring(0, 2 {
+throw new AuthenticationException(msg);
+}
+
+return userId;
+}
+
+@Override
+public String getRealmInformation() {
+return "Basic realm=\"Pulsar Web Service\"";
+}
+
+private class AuthParams {
+private String userId;
+private String password;
+
+public AuthParams(AuthenticationDataSource authData) throws 
AuthenticationException {
+String authParams;
+if (authData.hasDataFromCommand()) {
+authParams = authData.getCommandData();
+} else if (authData.hasDataFromHttp()) {
+String rawAuthToken = authData.getHttpHeader(HTTP_HEADER_NAME);
+// parsing and validation
+if (StringUtils.isBlank(rawAuthToken) || 
!rawAuthToken.toUpperCase().startsWith("BASIC ")) {
+throw new AuthenticationException("Authentication token 
has 

[GitHub] yush1ga commented on a change in pull request #1087: Add basic authentication plugin

2018-01-21 Thread GitBox
yush1ga commented on a change in pull request #1087: Add basic authentication 
plugin
URL: https://github.com/apache/incubator-pulsar/pull/1087#discussion_r162860524
 
 

 ##
 File path: 
pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/AuthenticationDataBasic.java
 ##
 @@ -0,0 +1,59 @@
+/**
+ * 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.pulsar.client.impl.auth;
+
+import org.apache.pulsar.client.api.AuthenticationDataProvider;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import java.util.Base64;
+
+public class AuthenticationDataBasic implements AuthenticationDataProvider {
+private final static String HTTP_HEADER_NAME = "Authorization";
+private String authToken;
+
+public AuthenticationDataBasic(String userId, String password) {
+authToken = "Basic " + Base64.getEncoder().encodeToString((userId + 
":" + password).getBytes());
 
 Review comment:
   I made command data not contain encoded params.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] yush1ga commented on a change in pull request #1087: Add basic authentication plugin

2018-01-21 Thread GitBox
yush1ga commented on a change in pull request #1087: Add basic authentication 
plugin
URL: https://github.com/apache/incubator-pulsar/pull/1087#discussion_r162860379
 
 

 ##
 File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/web/AuthenticationFilter.java
 ##
 @@ -42,11 +43,18 @@
 private static final Logger LOG = 
LoggerFactory.getLogger(AuthenticationFilter.class);
 
 private final AuthenticationService authenticationService;
+private final boolean isBasic;
 
 public static final String AuthenticatedRoleAttributeName = 
AuthenticationFilter.class.getName() + "-role";
 
 public AuthenticationFilter(PulsarService pulsar) {
 this.authenticationService = 
pulsar.getBrokerService().getAuthenticationService();
+if (pulsar.getConfiguration().getAuthenticationProviders()
+.contains(AuthenticationProviderBasic.class.getName())) {
+isBasic = true;
 
 Review comment:
   I think modifying `authenticateHttpRequest` is not efficient because the 
information is gathered every time authentication failed.
   I fixed AuthentcationFilter class to gather information from providers when 
it is initialized.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services