mattisonchao commented on code in PR #19830: URL: https://github.com/apache/pulsar/pull/19830#discussion_r1139957106
########## pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authorization/AuthorizationServiceTest.java: ########## @@ -0,0 +1,135 @@ +/** + * 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.authorization; + +import static org.testng.AssertJUnit.assertFalse; +import static org.testng.AssertJUnit.assertTrue; +import java.util.HashSet; +import org.apache.pulsar.broker.PulsarServerException; +import org.apache.pulsar.broker.ServiceConfiguration; +import org.apache.pulsar.common.naming.NamespaceName; +import org.apache.pulsar.common.naming.TopicName; +import org.apache.pulsar.common.policies.data.NamespaceOperation; +import org.apache.pulsar.common.policies.data.PolicyName; +import org.apache.pulsar.common.policies.data.PolicyOperation; +import org.apache.pulsar.common.policies.data.TenantOperation; +import org.apache.pulsar.common.policies.data.TopicOperation; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class AuthorizationServiceTest { + + AuthorizationService authorizationService; + + @BeforeClass + void beforeClass() throws PulsarServerException { + ServiceConfiguration conf = new ServiceConfiguration(); + conf.setAuthorizationEnabled(true); + // Consider both of these proxy roles to make testing more comprehensive + HashSet<String> proxyRoles = new HashSet<>(); + proxyRoles.add("pass.proxy"); + proxyRoles.add("fail.proxy"); + conf.setProxyRoles(proxyRoles); + conf.setAuthorizationProvider(MockAuthorizationProvider.class.getName()); + authorizationService = new AuthorizationService(conf, null); + } + + /** + * See {@link MockAuthorizationProvider} for the implementation of the mock authorization provider. + */ + @DataProvider(name = "roles") + public Object[][] encryptionProvider() { + return new Object[][]{ + // Schema: role, originalRole, whether authorization should pass + + // Client conditions where original role isn't passed or is blank + {"pass.client", null, Boolean.TRUE}, + {"pass.client", " ", Boolean.TRUE}, + {"fail.client", null, Boolean.FALSE}, + {"fail.client", " ", Boolean.FALSE}, + + // Proxy conditions where original role isn't passed or is blank Review Comment: >If the proxy-server disables the authentication service, but the proxy-client connects the broker with the authentication, this means the original role is null. In this case, I think it should be passed. First, I checked the previous code. As @michaeljmarshall mentioned, the current pulsar behaviour doesn't allow that. [1]I think that isn't a breaking change. For the deep question, why do we do that? It makes no sense to disable proxy authentication and enable proxy2broker authentication. but it's logically wrong and depends on how we designed the `proxyRoles`. However, I think the best thing is to remove all the `proxy` concepts in the broker. They are different components, and we don't need to couple them. But all the above questions should discuss in another thread. @nodece Please correct me if I got something wrong, and please take a look at this PR again. Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
