Copilot commented on code in PR #896: URL: https://github.com/apache/ranger/pull/896#discussion_r3005522777
########## pdp/src/main/java/org/apache/ranger/pdp/config/RangerPdpConfig.java: ########## @@ -0,0 +1,281 @@ +/* + * 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.pdp.config; + +import org.apache.commons.lang3.StringUtils; +import org.apache.ranger.plugin.util.XMLUtils; +import org.ietf.jgss.GSSCredential; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.util.Properties; + +/** + * Reads Ranger PDP configuration from {@code ranger-pdp-default.xml} (classpath) + * overridden by {@code ranger-pdp-site.xml} (classpath or filesystem). + * + * <p>Both files use the Hadoop {@code <configuration>} XML format, consistent + * with other Ranger server modules (tagsync, kms, etc.). + * The format is parsed directly using the JDK DOM API to avoid an early + * class-load dependency on Hadoop's {@code Configuration} class. + * + * <p>Authentication property names: + * <ul> + * <li>Kerberos/SPNEGO: {@code ranger.pdp.kerberos.spnego.*} + * <li>JWT bearer token: {@code ranger.pdp.jwt.*} + * <li>HTTP header: {@code ranger.pdp.authn.header.*} + * </ul> Review Comment: The class-level Javadoc lists authentication property prefixes like `ranger.pdp.kerberos.spnego.*` and `ranger.pdp.jwt.*`, but the actual configuration keys implemented in this module are under `ranger.pdp.authn.kerberos.*` and `ranger.pdp.authn.jwt.*` (see `RangerPdpConstants`). Please align the documentation with the real keys so operators don't configure the wrong properties. ########## pdp/src/main/java/org/apache/ranger/pdp/security/KerberosAuthNHandler.java: ########## @@ -0,0 +1,275 @@ +/* + * 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.pdp.security; + +import org.apache.commons.codec.binary.Base64; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.security.authentication.util.KerberosName; +import org.apache.ranger.pdp.config.RangerPdpConstants; +import org.ietf.jgss.GSSContext; +import org.ietf.jgss.GSSCredential; +import org.ietf.jgss.GSSException; +import org.ietf.jgss.GSSManager; +import org.ietf.jgss.GSSName; +import org.ietf.jgss.Oid; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.security.auth.Subject; +import javax.security.auth.callback.Callback; +import javax.security.auth.callback.CallbackHandler; +import javax.security.auth.callback.UnsupportedCallbackException; +import javax.security.auth.login.AppConfigurationEntry; +import javax.security.auth.login.Configuration; +import javax.security.auth.login.LoginContext; +import javax.security.auth.login.LoginException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.security.PrivilegedExceptionAction; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; + +/** + * Authenticates requests using Kerberos SPNEGO (HTTP Negotiate). + * + * <p>Uses the JDK's built-in GSSAPI/JGSS support – no external Kerberos library is required. + * The service principal and keytab must be configured via: + * <ul> + * <li>{@code ranger.pdp.kerberos.spnego.principal} – e.g. {@code HTTP/host.example.com@REALM} + * <li>{@code ranger.pdp.kerberos.spnego.keytab} – absolute path to the keytab file + * <li>{@code hadoop.security.auth_to_local} – Hadoop-style name rules (default: {@code DEFAULT}) + * </ul> Review Comment: The Kerberos handler Javadoc lists configuration keys without the `authn` segment (for example `ranger.pdp.kerberos.spnego.principal/keytab`). The actual keys consumed by `init()` are `ranger.pdp.authn.kerberos.spnego.principal` and `ranger.pdp.authn.kerberos.spnego.keytab` (see `RangerPdpConstants`). Please update the Javadoc so deployments configure the correct properties. ########## dev-support/ranger-docker/scripts/pdp/ranger-pdp-site.xml: ########## @@ -0,0 +1,240 @@ +<?xml version="1.0" encoding="UTF-8"?> +<?xml-stylesheet type="text/xsl" href="configuration.xsl"?> +<!-- + 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. +--> +<!-- Ranger PDP Server - Docker Configuration --> +<configuration> + <property> + <name>ranger.pdp.port</name> + <value>6500</value> + </property> + + <property> + <name>ranger.pdp.log.dir</name> + <value>/var/log/ranger/pdp</value> + </property> + + <property> + <name>ranger.pdp.service.*.delegation.users</name> + <value>ranger</value> + </property> + + <!-- Only delegation users are allowed call on behalf of other users --> + <property> + <name>ranger.pdp.service.dev_hdfs.delegation.users</name> + <value>hdfs</value> + </property> + + <property> + <name>ranger.pdp.service.dev_hive.delegation.users</name> + <value>hive</value> + </property> + + <property> + <name>ranger.pdp.service.dev_hbase.delegation.users</name> + <value>hbase</value> + </property> + + <property> + <name>ranger.pdp.service.dev_kafka.delegation.users</name> + <value>kafka</value> + </property> + + <property> + <name>ranger.pdp.service.dev_trino.delegation.users</name> + <value>trino</value> + </property> + + <property> + <name>ranger.pdp.service.dev_polaris.delegation.users</name> + <value>polaris</value> + </property> + + <!-- SSL/TLS: disabled for Docker dev environment --> + <property> + <name>ranger.pdp.ssl.enabled</name> + <value>false</value> + </property> + + <!-- HTTP/2 --> + <property> + <name>ranger.pdp.http2.enabled</name> + <value>true</value> + </property> + + <!-- Connector concurrency / queue limits --> + <property> + <name>ranger.pdp.http.connector.maxThreads</name> + <value>200</value> + </property> + + <property> + <name>ranger.pdp.http.connector.minSpareThreads</name> + <value>20</value> + </property> + + <property> + <name>ranger.pdp.http.connector.acceptCount</name> + <value>100</value> + </property> + + <property> + <name>ranger.pdp.http.connector.maxConnections</name> + <value>10000</value> + </property> + + <!-- Authentication for incoming REST requests --> + <property> + <name>ranger.pdp.authn.types</name> + <value>header,jwt,kerberos</value> + </property> + + <!-- HTTP Header authentication: disabled --> + <property> + <name>ranger.pdp.authn.header.enabled</name> + <value>true</value> + </property> Review Comment: The comment says "HTTP Header authentication: disabled" but the configuration sets `ranger.pdp.authn.header.enabled` to `true`. Please fix the comment (or the value) so the file is self-consistent and doesn't mislead operators. ########## intg/src/main/python/apache_ranger/exceptions.py: ########## @@ -33,14 +33,18 @@ def __init__(self, api, response): self.msgDesc = None self.messageList = None - print(response) - if api is not None and response is not None: if response.content: try: - respJson = response.json() - self.msgDesc = respJson['msgDesc'] if respJson is not None and 'msgDesc' in respJson else None - self.messageList = respJson['messageList'] if respJson is not None and 'messageList' in respJson else None + respJson = response.json() + + if respJson: + self.msgDesc = respJson['msgDesc'] if 'msgDesc' in respJson else None + self.messageList = respJson['messageList'] if 'messageList' in respJson else None + + if self.msgDesc is None: + self.msgDesc = respJson['message'] if 'message' in respJson else None + except Exception: self.msgDesc = response.content self.messageList = [ response.content ] Review Comment: Indentation in this block is inconsistent with the surrounding 4-space indentation used in the module (and makes the control flow hard to read). Please re-indent the nested `try/except` and inner `if` blocks using consistent 4-space increments. ########## pdp/src/main/java/org/apache/ranger/pdp/security/RangerPdpAuthNFilter.java: ########## @@ -0,0 +1,185 @@ +/* + * 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.pdp.security; + +import org.apache.commons.lang3.StringUtils; +import org.apache.ranger.pdp.config.RangerPdpConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Properties; + +import static org.apache.ranger.pdp.config.RangerPdpConstants.PROP_AUTHN_TYPES; + +/** + * Servlet filter that enforces authentication for all PDP REST endpoints. + * + * <p>Handlers are configured via the {@code ranger.pdp.auth.types} filter init parameter + * (comma-separated list of {@code header}, {@code jwt}, {@code kerberos}). Handlers are + * tried in the listed order; the first successful match wins. Review Comment: Javadoc references the init parameter as `ranger.pdp.auth.types`, but the actual config key used throughout the implementation is `ranger.pdp.authn.types` (see `RangerPdpConstants.PROP_AUTHN_TYPES`). Please update the Javadoc to the correct property name to avoid misconfiguration. ########## pdp/src/main/java/org/apache/ranger/pdp/rest/RangerPdpREST.java: ########## @@ -0,0 +1,507 @@ +/* + * 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.pdp.rest; + +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonInclude; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; +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.RangerAccessInfo; +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.RangerResourceInfo; +import org.apache.ranger.authz.model.RangerResourcePermissions; +import org.apache.ranger.authz.model.RangerResourcePermissionsRequest; +import org.apache.ranger.authz.model.RangerUserInfo; +import org.apache.ranger.pdp.RangerPdpStats; +import org.apache.ranger.pdp.config.RangerPdpConfig; +import org.apache.ranger.pdp.config.RangerPdpConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.PostConstruct; +import javax.inject.Inject; +import javax.inject.Singleton; +import javax.servlet.ServletContext; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.Consumes; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; + +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.FORBIDDEN; +import static javax.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR; +import static javax.ws.rs.core.Response.Status.UNAUTHORIZED; +import static org.apache.ranger.authz.api.RangerAuthzApiErrorCode.INVALID_REQUEST_USER_INFO_MISSING; +import static org.apache.ranger.pdp.config.RangerPdpConstants.PROP_PDP_SERVICE_PREFIX; +import static org.apache.ranger.pdp.config.RangerPdpConstants.PROP_SUFFIX_DELEGATION_USERS; +import static org.apache.ranger.pdp.config.RangerPdpConstants.WILDCARD_SERVICE_NAME; + +/** + * REST resource that exposes the three core {@link RangerAuthorizer} methods over HTTP. + * + * <p>All endpoints are under {@code /authz/v1} and produce/consume {@code application/json}. + * Authentication is enforced upstream by {@link RangerPdpAuthFilter}; the authenticated + * caller's identity is read from the {@link RangerPdpConstants#ATTR_AUTHENTICATED_USER} + * request attribute. Review Comment: The Javadoc links to `RangerPdpAuthFilter`, but that type doesn't exist in this module (the filter class is `RangerPdpAuthNFilter`). This produces broken Javadoc links and can fail builds that treat Javadoc warnings as errors. Update the reference to the correct filter class name. ########## pdp/src/main/java/org/apache/ranger/pdp/security/RangerPdpAuthNFilter.java: ########## @@ -0,0 +1,185 @@ +/* + * 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.pdp.security; + +import org.apache.commons.lang3.StringUtils; +import org.apache.ranger.pdp.config.RangerPdpConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Properties; + +import static org.apache.ranger.pdp.config.RangerPdpConstants.PROP_AUTHN_TYPES; + +/** + * Servlet filter that enforces authentication for all PDP REST endpoints. + * + * <p>Handlers are configured via the {@code ranger.pdp.auth.types} filter init parameter + * (comma-separated list of {@code header}, {@code jwt}, {@code kerberos}). Handlers are + * tried in the listed order; the first successful match wins. + * + * <p>On success the authenticated username is stored in the request attribute + * {@link RangerPdpConstants#ATTR_AUTHENTICATED_USER} so that REST resources can read it. + * + * <p>If all handlers return {@code SKIP} (no recognisable credentials found), the filter + * sends a {@code 401} response with {@code WWW-Authenticate} headers for every + * configured handler that provides a challenge. + */ +public class RangerPdpAuthNFilter implements Filter { + private static final Logger LOG = LoggerFactory.getLogger(RangerPdpAuthNFilter.class); + + private final List<PdpAuthNHandler> handlers = new ArrayList<>(); + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + Properties config = toProperties(filterConfig); + String authnTypes = filterConfig.getInitParameter(PROP_AUTHN_TYPES); + + for (String authnType : authnTypes.split(",")) { + PdpAuthNHandler handler = createHandler(authnType.trim().toLowerCase(), filterConfig); Review Comment: `authnTypes` is read from the `ranger.pdp.authn.types` init-param but isn't validated for null/blank before calling `split(",")`. If the filter is registered without this init-param (or an empty value), `init()` will throw a NullPointerException instead of a clear `ServletException`. Add a null/blank check and fail fast with an explicit message (or fall back to a default list). -- 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]
