michaeljmarshall commented on code in PR #19830: URL: https://github.com/apache/pulsar/pull/19830#discussion_r1139217585
########## 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: @nodece - regarding your list of potential combinations, are you already using these? I ask to understand if you know that they work. The first does not seem well supported given how the binary protocol proxy and the admin http proxy diverge. Notes inline: > * The proxy-server disables the authentication service, but broker-proxy's authentication was enabled This use case will work for both the binary protocol proxy as long as the proxy is not using a `proxyRole`. It will not work (and has never worked as far as I can tell) for the admin http proxy unless you're using mTLS from proxy to broker for auth and in that case, you'd need to not use a `proxyRole`. Non mTLS authentication solutions rely on the proxy forwarding the `Authorization` header to the broker. However, it is risky to not use a `proxyRole`. > * Only the proxy-server enables the authentication service This will work for both binary and http proxying since the broker only checks for authentication and authorization when each is enabled in the broker. > * Only the broker enables the authentication service How is this different from the first bullet? If the broker has authentication enabled, I would expect the proxy must authenticate. Perhaps the point is for the proxy to rely on the anonymous role. In that case, relying on the anonymous role would work here since the proxy would not be passing any auth data. -- 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]
