Copilot commented on code in PR #899: URL: https://github.com/apache/ranger/pull/899#discussion_r3007712573
########## pdp/src/main/java/org/apache/ranger/pdp/security/RangerPdpAuthNFilter.java: ########## @@ -0,0 +1,189 @@ +/* + * 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 javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; + +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.authn.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); + + if (StringUtils.isNotBlank(authnTypes)) { + for (String authnType : authnTypes.split(",")) { + PdpAuthNHandler handler = createHandler(authnType.trim().toLowerCase(), filterConfig); + + if (handler == null) { + continue; + } + + try { + handler.init(config); + + handlers.add(handler); + + LOG.info("{}: successfully registered authentication handler", authnType); + } catch (Exception excp) { + LOG.error("{}: failed to initialize authentication handler. Handler disabled", authnType, excp); + } + } + } + + if (handlers.isEmpty()) { + throw new ServletException("No valid authentication handlers configured"); + } + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + HttpServletRequest httpReq = (HttpServletRequest) request; + HttpServletResponse httpResp = (HttpServletResponse) response; + + for (PdpAuthNHandler handler : handlers) { + PdpAuthNHandler.Result result = handler.authenticate(httpReq, httpResp); + + switch (result.getStatus()) { + case AUTHENTICATED: + httpReq.setAttribute(RangerPdpConstants.ATTR_AUTHENTICATED_USER, result.getUserName()); + httpReq.setAttribute(RangerPdpConstants.ATTR_AUTHN_TYPE, result.getAuthType()); + + LOG.debug("doFilter(): authenticated user={}, type={}", result.getUserName(), result.getAuthType()); + + chain.doFilter(request, response); + return; + + case CHALLENGE: + // handler has already written the 401; stop processing + return; + + case SKIP: + default: + // try the next handler + break; + } + } + + // No handler could authenticate the request; send a 401 with all challenge headers + LOG.debug("doFilter(): no handler authenticated request from {}", httpReq.getRemoteAddr()); + + sendUnauthenticated(httpResp); + } Review Comment: Authentication failures handled in this filter (no credentials / invalid JWT/SPNEGO challenge) are not recorded in `RangerPdpStats`, so `/metrics` counters like `ranger_pdp_auth_failures_total` will miss authN-level failures (requests rejected before hitting the REST resource). Consider capturing start time in `doFilter()` and updating the runtime stats (from `ServletContext` attribute `ranger.pdp.runtime.state`) when returning 401/403 from the filter. ########## authz-embedded/src/main/java/org/apache/ranger/authz/embedded/RangerEmbeddedAuthorizer.java: ########## @@ -131,6 +133,10 @@ public RangerMultiAuthzResult authorize(RangerMultiAuthzRequest request, RangerA return authorize(request, plugin, auditHandler); } + public Set<String> getLoadedServices() { + return Collections.unmodifiableSet(this.plugins.keySet()); + } Review Comment: `getLoadedServices()` returns an unmodifiable view backed by the internal `plugins` HashMap keySet. Since `plugins` can still be mutated during lazy service initialization, callers (e.g., readiness/metrics) can observe concurrent modifications and potentially hit `ConcurrentModificationException` or inconsistent results. Consider synchronizing access around `plugins` and/or returning a snapshot copy (e.g., new HashSet of keys) instead of a live view. ########## dev-support/ranger-docker/Dockerfile.ranger-pdp: ########## @@ -0,0 +1,36 @@ +# 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. + +ARG RANGER_BASE_IMAGE=apache/ranger-base +ARG RANGER_BASE_VERSION=20260123-2-8 + +FROM ${RANGER_BASE_IMAGE}:${RANGER_BASE_VERSION} + +ARG PDP_VERSION + +COPY ./dist/ranger-${PDP_VERSION}-pdp.tar.gz /home/ranger/dist/ +COPY ./scripts/pdp/ranger-pdp.sh ${RANGER_SCRIPTS}/ + +RUN tar xvfz /home/ranger/dist/ranger-${PDP_VERSION}-pdp.tar.gz --directory=${RANGER_HOME} \ + && ln -s ${RANGER_HOME}/ranger-${PDP_VERSION}-pdp ${RANGER_HOME}/pdp \ + && rm -f /home/ranger/dist/ranger-${PDP_VERSION}-pdp.tar.gz \ + && mkdir -p /var/log/ranger/pdp /etc/ranger/cache \ + && ln -s ${RANGER_HOME}/pdp/ranger-pdp-services.sh /usr/bin/ranger-pdp-services.sh \ + && chown -R ranger:ranger ${RANGER_HOME}/pdp/ ${RANGER_SCRIPTS}/ /var/log/ranger/ /etc/ranger/ \ Review Comment: In the Docker image, the container runs as the non-root `ranger` user, but this Dockerfile doesn't create/chown the PID directory used by `ranger-pdp-services.sh` (defaults to `/var/run/ranger`). This will likely cause startup failures when the script tries `mkdir -p /var/run/ranger` or writes `/var/run/ranger/pdp.pid`. Consider creating `/var/run/ranger` during image build and chowning it to `ranger`, or overriding `RANGER_PDP_PID_DIR_PATH` to a writable path inside `$RANGER_HOME`. ```suggestion && mkdir -p /var/log/ranger/pdp /etc/ranger/cache /var/run/ranger \ && ln -s ${RANGER_HOME}/pdp/ranger-pdp-services.sh /usr/bin/ranger-pdp-services.sh \ && chown -R ranger:ranger ${RANGER_HOME}/pdp/ ${RANGER_SCRIPTS}/ /var/log/ranger/ /etc/ranger/ /var/run/ranger/ \ ``` -- 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]
