mneethiraj commented on code in PR #928:
URL: https://github.com/apache/ranger/pull/928#discussion_r3140247987
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerPdpClient.java:
##########
@@ -82,52 +88,64 @@ class RangerPdpClient implements Closeable {
RangerAuthzResult authorize(RangerAuthzRequest request) throws
RangerAuthzException {
String endpoint = config.getEndpointUrl(PATH_AUTHORIZE);
- String payload = RangerRemoteJson.writeAuthzRequest(request);
- String response = post(endpoint, payload);
- return RangerRemoteJson.readAuthzResult(response, endpoint);
+ return post(endpoint, request, RangerAuthzResult.class);
}
RangerMultiAuthzResult authorize(RangerMultiAuthzRequest request) throws
RangerAuthzException {
String endpoint = config.getEndpointUrl(PATH_AUTHORIZE_MULTI);
- String payload = RangerRemoteJson.writeMultiAuthzRequest(request);
- String response = post(endpoint, payload);
- return RangerRemoteJson.readMultiAuthzResult(response, endpoint);
+ return post(endpoint, request, RangerMultiAuthzResult.class);
}
RangerResourcePermissions
getResourcePermissions(RangerResourcePermissionsRequest request) throws
RangerAuthzException {
String endpoint = config.getEndpointUrl(PATH_RESOURCE_PERMISSIONS);
- String payload =
RangerRemoteJson.writeResourcePermissionsRequest(request);
- String response = post(endpoint, payload);
- return RangerRemoteJson.readResourcePermissions(response, endpoint);
+ return post(endpoint, request, RangerResourcePermissions.class);
}
- private String post(String endpoint, String payload) throws
RangerAuthzException {
+ private <T> T post(String endpoint, Object payload, Class<T> responseType)
throws RangerAuthzException {
+ final String requestBody;
+
+ try {
+ requestBody = OBJECT_MAPPER.writeValueAsString(payload);
+ } catch (IOException e) {
+ throw new RangerAuthzException(REMOTE_RESPONSE_INVALID, e,
"request-body");
Review Comment:
REMOTE_RESPONSE_INVALID => FAILED_TO_SERIALIZE_REQUEST
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerPdpClient.java:
##########
@@ -82,52 +88,64 @@ class RangerPdpClient implements Closeable {
RangerAuthzResult authorize(RangerAuthzRequest request) throws
RangerAuthzException {
String endpoint = config.getEndpointUrl(PATH_AUTHORIZE);
- String payload = RangerRemoteJson.writeAuthzRequest(request);
- String response = post(endpoint, payload);
- return RangerRemoteJson.readAuthzResult(response, endpoint);
+ return post(endpoint, request, RangerAuthzResult.class);
}
RangerMultiAuthzResult authorize(RangerMultiAuthzRequest request) throws
RangerAuthzException {
String endpoint = config.getEndpointUrl(PATH_AUTHORIZE_MULTI);
- String payload = RangerRemoteJson.writeMultiAuthzRequest(request);
- String response = post(endpoint, payload);
- return RangerRemoteJson.readMultiAuthzResult(response, endpoint);
+ return post(endpoint, request, RangerMultiAuthzResult.class);
}
RangerResourcePermissions
getResourcePermissions(RangerResourcePermissionsRequest request) throws
RangerAuthzException {
String endpoint = config.getEndpointUrl(PATH_RESOURCE_PERMISSIONS);
- String payload =
RangerRemoteJson.writeResourcePermissionsRequest(request);
- String response = post(endpoint, payload);
- return RangerRemoteJson.readResourcePermissions(response, endpoint);
+ return post(endpoint, request, RangerResourcePermissions.class);
}
- private String post(String endpoint, String payload) throws
RangerAuthzException {
+ private <T> T post(String endpoint, Object payload, Class<T> responseType)
throws RangerAuthzException {
+ final String requestBody;
+
+ try {
+ requestBody = OBJECT_MAPPER.writeValueAsString(payload);
+ } catch (IOException e) {
+ throw new RangerAuthzException(REMOTE_RESPONSE_INVALID, e,
"request-body");
+ }
+
HttpPost request = new HttpPost(endpoint);
try {
request.setHeader("Content-Type",
ContentType.APPLICATION_JSON.getMimeType());
request.setHeader("Accept",
ContentType.APPLICATION_JSON.getMimeType());
- request.setEntity(new StringEntity(payload,
ContentType.APPLICATION_JSON));
+ request.setEntity(new StringEntity(requestBody,
ContentType.APPLICATION_JSON));
for (Map.Entry<String, String> header :
config.getHeaders().entrySet()) {
request.setHeader(header.getKey(), header.getValue());
}
+ String responseBody;
+
if (authType == KERBEROS) {
- return
kerberosContext.doAs((PrivilegedExceptionAction<String>) () -> execute(request,
endpoint));
+ responseBody =
kerberosContext.doAs((PrivilegedExceptionAction<String>) () -> execute(request,
endpoint));
+ } else {
+ responseBody = execute(request, endpoint);
}
- return execute(request, endpoint);
+ try {
+ return OBJECT_MAPPER.readValue(responseBody, responseType);
+ } catch (IOException e) {
+ throw new RangerAuthzException(REMOTE_RESPONSE_INVALID, e,
endpoint);
Review Comment:
REMOTE_RESPONSE_INVALID => FAILED_TO_DESERIALIZE_RESPONSE
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerRemoteAuthorizer.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.ranger.authz.remote;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.ranger.authz.api.RangerAuthorizer;
+import org.apache.ranger.authz.api.RangerAuthzException;
+import org.apache.ranger.authz.model.RangerAccessContext;
+import org.apache.ranger.authz.model.RangerAuthzRequest;
+import org.apache.ranger.authz.model.RangerAuthzResult;
+import org.apache.ranger.authz.model.RangerMultiAuthzRequest;
+import org.apache.ranger.authz.model.RangerMultiAuthzResult;
+import org.apache.ranger.authz.model.RangerResourcePermissions;
+import org.apache.ranger.authz.model.RangerResourcePermissionsRequest;
+
+import java.util.Properties;
+
+import static
org.apache.ranger.authz.api.RangerAuthzApiErrorCode.INVALID_REQUEST_SERVICE_NAME_OR_TYPE_MANDATORY;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.INVALID_PROPERTY_VALUE;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.REMOTE_REQUEST_FAILED;
+
+public class RangerRemoteAuthorizer extends RangerAuthorizer {
+ private final RangerRemoteAuthzConfig config;
+
+ private volatile RangerPdpClient client;
+
+ public RangerRemoteAuthorizer(Properties properties) {
+ super(properties);
+
+ this.config = new RangerRemoteAuthzConfig(properties);
+ }
+
+ @Override
+ public void init() throws RangerAuthzException {
+ config.getPdpUrl();
Review Comment:
Are lines 52, 53, 54 necessary?
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerRemoteAuthorizer.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.ranger.authz.remote;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.ranger.authz.api.RangerAuthorizer;
+import org.apache.ranger.authz.api.RangerAuthzException;
+import org.apache.ranger.authz.model.RangerAccessContext;
+import org.apache.ranger.authz.model.RangerAuthzRequest;
+import org.apache.ranger.authz.model.RangerAuthzResult;
+import org.apache.ranger.authz.model.RangerMultiAuthzRequest;
+import org.apache.ranger.authz.model.RangerMultiAuthzResult;
+import org.apache.ranger.authz.model.RangerResourcePermissions;
+import org.apache.ranger.authz.model.RangerResourcePermissionsRequest;
+
+import java.util.Properties;
+
+import static
org.apache.ranger.authz.api.RangerAuthzApiErrorCode.INVALID_REQUEST_SERVICE_NAME_OR_TYPE_MANDATORY;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.INVALID_PROPERTY_VALUE;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.REMOTE_REQUEST_FAILED;
+
+public class RangerRemoteAuthorizer extends RangerAuthorizer {
+ private final RangerRemoteAuthzConfig config;
+
+ private volatile RangerPdpClient client;
+
+ public RangerRemoteAuthorizer(Properties properties) {
+ super(properties);
+
+ this.config = new RangerRemoteAuthzConfig(properties);
+ }
+
+ @Override
+ public void init() throws RangerAuthzException {
+ config.getPdpUrl();
+ config.getConnectTimeoutMs();
+ config.getReadTimeoutMs();
+
+ this.client = new RangerPdpClient(config);
+ }
+
+ @Override
+ public void close() throws RangerAuthzException {
+ RangerPdpClient client = this.client;
+
+ this.client = null;
+
+ if (client != null) {
+ try {
+ client.close();
+ } catch (Exception e) {
+ throw new RangerAuthzException(REMOTE_REQUEST_FAILED, e,
RangerRemoteAuthzConfig.PROP_REMOTE_URL);
Review Comment:
REMOTE_REQUEST_FAILED here? Consider using a new error code.
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerRemoteAuthorizer.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * 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.ranger.authz.remote;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.ranger.authz.api.RangerAuthorizer;
+import org.apache.ranger.authz.api.RangerAuthzException;
+import org.apache.ranger.authz.model.RangerAccessContext;
+import org.apache.ranger.authz.model.RangerAuthzRequest;
+import org.apache.ranger.authz.model.RangerAuthzResult;
+import org.apache.ranger.authz.model.RangerMultiAuthzRequest;
+import org.apache.ranger.authz.model.RangerMultiAuthzResult;
+import org.apache.ranger.authz.model.RangerResourcePermissions;
+import org.apache.ranger.authz.model.RangerResourcePermissionsRequest;
+
+import java.util.Properties;
+
+import static
org.apache.ranger.authz.api.RangerAuthzApiErrorCode.INVALID_REQUEST_SERVICE_NAME_OR_TYPE_MANDATORY;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.INVALID_PROPERTY_VALUE;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.REMOTE_REQUEST_FAILED;
+
+public class RangerRemoteAuthorizer extends RangerAuthorizer {
+ private final RangerRemoteAuthzConfig config;
+
+ private volatile RangerPdpClient client;
+
+ public RangerRemoteAuthorizer(Properties properties) {
+ super(properties);
+
+ this.config = new RangerRemoteAuthzConfig(properties);
+ }
+
+ @Override
+ public void init() throws RangerAuthzException {
+ config.getPdpUrl();
+ config.getConnectTimeoutMs();
+ config.getReadTimeoutMs();
+
+ this.client = new RangerPdpClient(config);
+ }
+
+ @Override
+ public void close() throws RangerAuthzException {
+ RangerPdpClient client = this.client;
+
+ this.client = null;
+
+ if (client != null) {
+ try {
+ client.close();
+ } catch (Exception e) {
+ throw new RangerAuthzException(REMOTE_REQUEST_FAILED, e,
RangerRemoteAuthzConfig.PROP_REMOTE_URL);
+ }
+ }
+ }
+
+ @Override
+ public RangerAuthzResult authorize(RangerAuthzRequest request) throws
RangerAuthzException {
+ validateRequest(request);
+
+ return getClient().authorize(request);
+ }
+
+ @Override
+ public RangerMultiAuthzResult authorize(RangerMultiAuthzRequest request)
throws RangerAuthzException {
+ validateRequest(request);
+
+ return getClient().authorize(request);
+ }
+
+ @Override
+ public RangerResourcePermissions
getResourcePermissions(RangerResourcePermissionsRequest request) throws
RangerAuthzException {
+ validateRequest(request);
+
+ return getClient().getResourcePermissions(request);
+ }
+
+ @Override
+ protected void validateAccessContext(RangerAccessContext context) throws
RangerAuthzException {
+ super.validateAccessContext(context);
+
+ String serviceName = context.getServiceName();
+ String serviceType = context.getServiceType();
+
+ if (StringUtils.isBlank(serviceName)) {
+ if (StringUtils.isBlank(serviceType)) {
+ throw new
RangerAuthzException(INVALID_REQUEST_SERVICE_NAME_OR_TYPE_MANDATORY);
+ }
+
+ serviceName =
config.getDefaultServiceNameForServiceType(serviceType);
Review Comment:
lines 107 to 123 are unnecessary at the client side; the client should
either know the service name or rely on the service name configured at the PDP
server side for the given service type. Please remove corresponding methods and
fields as well, like:
- `RangerRemoteAuthzConfig.getDefaultServiceNameForServiceType()`
- `RangerRemoteAuthzConfig.getServiceTypeForService()`
- `RangerRemoteAuthzConfig.PROP_PREFIX_SERVICE`
- `RangerRemoteAuthzConfig.PROP_PREFIX_SERVICE_TYPE`
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerRemoteAuthzConfig.java:
##########
@@ -58,9 +59,26 @@ public class RangerRemoteAuthzConfig {
private static final String DEFAULT_STORE_TYPE = "PKCS12";
private final Properties properties;
+ private final Map<String, String> headers;
public RangerRemoteAuthzConfig(Properties properties) {
this.properties = properties != null ? properties : new Properties();
+ Map<String, String> ret = new LinkedHashMap<>();
+
+ for (String propName : this.properties.stringPropertyNames()) {
Review Comment:
Consider replacing this for loop with use of streams.
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerRemoteAuthzConfig.java:
##########
@@ -58,9 +59,26 @@ public class RangerRemoteAuthzConfig {
private static final String DEFAULT_STORE_TYPE = "PKCS12";
private final Properties properties;
+ private final Map<String, String> headers;
public RangerRemoteAuthzConfig(Properties properties) {
this.properties = properties != null ? properties : new Properties();
+ Map<String, String> ret = new LinkedHashMap<>();
Review Comment:
ret => headers
##########
authz-remote/src/main/java/org/apache/ranger/authz/remote/RangerPdpClient.java:
##########
@@ -0,0 +1,267 @@
+/*
+ * 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.ranger.authz.remote;
+
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.http.HttpEntity;
+import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.config.Registry;
+import org.apache.http.config.RegistryBuilder;
+import org.apache.http.conn.socket.ConnectionSocketFactory;
+import org.apache.http.conn.socket.PlainConnectionSocketFactory;
+import org.apache.http.conn.ssl.DefaultHostnameVerifier;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.client.HttpClients;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.http.ssl.SSLContextBuilder;
+import org.apache.http.util.EntityUtils;
+import org.apache.ranger.authz.api.RangerAuthzException;
+import org.apache.ranger.authz.model.RangerAuthzRequest;
+import org.apache.ranger.authz.model.RangerAuthzResult;
+import org.apache.ranger.authz.model.RangerMultiAuthzRequest;
+import org.apache.ranger.authz.model.RangerMultiAuthzResult;
+import org.apache.ranger.authz.model.RangerResourcePermissions;
+import org.apache.ranger.authz.model.RangerResourcePermissionsRequest;
+import org.apache.ranger.authz.remote.authn.RangerRemoteKerberosContext;
+
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLContext;
+
+import java.io.Closeable;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.security.KeyStore;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.util.Map;
+
+import static org.apache.ranger.authz.remote.RangerRemoteAuthType.KERBEROS;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.REMOTE_CALL_UNSUCCESSFUL;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.REMOTE_REQUEST_FAILED;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.REMOTE_RESPONSE_INVALID;
+import static
org.apache.ranger.authz.remote.RangerRemoteAuthzErrorCode.TLS_CONFIGURATION_FAILED;
+
+class RangerPdpClient implements Closeable {
+ private static final ObjectMapper OBJECT_MAPPER = new
ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES,
false);
+
+ private static final String PATH_AUTHORIZE = "/authorize";
+ private static final String PATH_AUTHORIZE_MULTI = "/authorizeMulti";
+ private static final String PATH_RESOURCE_PERMISSIONS = "/permissions";
+
+ private final RangerRemoteAuthzConfig config;
+ private final CloseableHttpClient httpClient;
+ private final RangerRemoteAuthType authType;
+ private final RangerRemoteKerberosContext kerberosContext;
+
+ RangerPdpClient(RangerRemoteAuthzConfig config) throws
RangerAuthzException {
+ this.config = config;
+ this.authType = config.getAuthType();
+ this.kerberosContext = authType == KERBEROS ?
RangerRemoteKerberosContext.create(config) : null;
+ this.httpClient = createHttpClient(config, kerberosContext);
+ }
+
+ RangerAuthzResult authorize(RangerAuthzRequest request) throws
RangerAuthzException {
+ String endpoint = config.getEndpointUrl(PATH_AUTHORIZE);
+
+ return post(endpoint, request, RangerAuthzResult.class);
+ }
+
+ RangerMultiAuthzResult authorize(RangerMultiAuthzRequest request) throws
RangerAuthzException {
+ String endpoint = config.getEndpointUrl(PATH_AUTHORIZE_MULTI);
Review Comment:
Consider caching API endpoints in the constructor , to avoid repeated lookup
in each call.
```
private final String apiEndPointAuthorize;
private final String apiEndpointMultiAuthorize;
private final String apiEndpointResourcePermissions;
// constructor
{
...
this.apiEndPointAuthorize = config.getEndpointUrl(PATH_AUTHORIZE);
this.apiEndpointMultiAuthorize =
config.getEndpointUrl(PATH_AUTHORIZE_MULTI);;
this.apiEndpointResourcePermissions =
config.getEndpointUrl(PATH_RESOURCE_PERMISSIONS);;
}
```
--
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]
