[GitHub] yush1ga commented on a change in pull request #1087: Add basic authentication plugin
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
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
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 Mapusers; + +@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
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
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