[
https://issues.apache.org/jira/browse/KNOX-3279?focusedWorklogId=1010944&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1010944
]
ASF GitHub Bot logged work on KNOX-3279:
----------------------------------------
Author: ASF GitHub Bot
Created on: 23/Mar/26 15:41
Start Date: 23/Mar/26 15:41
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1182:
URL: https://github.com/apache/knox/pull/1182#discussion_r2975797210
##########
gateway-service-restcatalog/src/main/java/org/apache/knox/gateway/service/restcatalog/TokenMetadataHeaderHandler.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.knox.gateway.service.restcatalog;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.security.token.UnknownTokenException;
+
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletRequest;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public class TokenMetadataHeaderHandler {
+ static final String TOKEN_METADATA_PARAM = "token-metadata-headers";
+
+ static final String HEADER_PREFIX = "X-Knox-Meta-";
+
+ static final String GRANT_TYPE = "grant_type";
+ static final String CLIENT_CREDENTIALS = "client_credentials";
+ static final String CLIENT_SECRET = "client_secret";
Review Comment:
I know we have constants for these Strings in `JWTFederationFilter`. I also
realize these constants are not available without adding that Maven module as a
dependency (which is way too broad here and may result in circular deps).
What we could do, IMO, is to introduce a new cross-service constant
interface in a commonly available Maven module (`gateway-spi[-common]` looks
like a good candidate) and start gathering these constants there.
What do you think?
##########
gateway-service-restcatalog/src/main/java/org/apache/knox/gateway/service/restcatalog/TokenMetadataHeaderHandler.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.knox.gateway.service.restcatalog;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.security.token.UnknownTokenException;
+
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletRequest;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public class TokenMetadataHeaderHandler {
+ static final String TOKEN_METADATA_PARAM = "token-metadata-headers";
+
+ static final String HEADER_PREFIX = "X-Knox-Meta-";
+
+ static final String GRANT_TYPE = "grant_type";
+ static final String CLIENT_CREDENTIALS = "client_credentials";
+ static final String CLIENT_SECRET = "client_secret";
+ static final String INVALID_CLIENT_SECRET = "Error while parsing the
received client secret";
+
+ TokenStateService tss;
+
+ Set<String> metadataHeaderConfig = new HashSet<>();
+
+ Cache<String, TokenMetadata> metadataCache =
Caffeine.newBuilder().maximumSize(100).build();
Review Comment:
All should be `private final`.
##########
gateway-service-restcatalog/src/main/java/org/apache/knox/gateway/service/restcatalog/TokenMetadataHeaderHandler.java:
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.knox.gateway.service.restcatalog;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.security.token.UnknownTokenException;
+
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletRequest;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+public class TokenMetadataHeaderHandler {
+ static final String TOKEN_METADATA_PARAM = "token-metadata-headers";
+
+ static final String HEADER_PREFIX = "X-Knox-Meta-";
+
+ static final String GRANT_TYPE = "grant_type";
+ static final String CLIENT_CREDENTIALS = "client_credentials";
+ static final String CLIENT_SECRET = "client_secret";
+ static final String INVALID_CLIENT_SECRET = "Error while parsing the
received client secret";
+
+ TokenStateService tss;
+
+ Set<String> metadataHeaderConfig = new HashSet<>();
+
+ Cache<String, TokenMetadata> metadataCache =
Caffeine.newBuilder().maximumSize(100).build();
+
+ TokenMetadataHeaderHandler(final FilterConfig filterConfig) {
+ ServletContext sc = filterConfig.getServletContext();
+ GatewayServices gatewayServices = (GatewayServices)
sc.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+ tss = gatewayServices.getService(ServiceType.TOKEN_STATE_SERVICE);
+
+ metadataHeaderConfig = getMetadataHeaderConfig(filterConfig);
+ }
+
+ void applyHeadersToRequest(final HttpServletRequest inboundRequest,
+ final HttpUriRequest outboundRequest) {
+ final String clientId = getClientID(inboundRequest);
+ if (clientId != null) {
+ Map<String, String> metadata = getMetadata(tss, clientId);
+ for (String key : metadataHeaderConfig) {
+ if (metadata.containsKey(key)) {
+ outboundRequest.setHeader(HEADER_PREFIX + key,
metadata.get(key));
+ }
+ }
+ }
+ }
+
+ private Set<String> getMetadataHeaderConfig(final FilterConfig
filterConfig) {
+ Set<String> metadataForHeaders = new HashSet<>();
+
+ // Add the default metadata element to be included in the outbound
request headers
+ metadataForHeaders.add("userName");
+
+ // Parse the configured token metadata elements which should be
included as outbound request headers
+ String tokenMetadataHeadersConfig =
filterConfig.getInitParameter(TOKEN_METADATA_PARAM);
+ String[] tokenMetadataHeaderNames =
tokenMetadataHeadersConfig.split(",");
+ for (String metadataName : tokenMetadataHeaderNames) {
+ metadataForHeaders.add(metadataName.trim());
+ }
+ return metadataForHeaders;
+ }
+
+ /**
+ * Get the clientId from the client credentials in the request body
content.
+ *
+ * @param request The ServletRequest.
+ *
+ * @return The clientId from the request.
+ */
+ String getClientID(ServletRequest request) {
+ String clientId = null;
+
+ String clientSecret = getClientSecretFromRequestBody(request);
+ if (clientSecret != null) {
+ try {
+ final String[] decodedSecret =
decodeBase64(clientSecret).split("::");
+ clientId = decodeBase64(decodedSecret[0]);
+ } catch (Exception e) {
+ throw new SecurityException(INVALID_CLIENT_SECRET, e);
+ }
+ }
Review Comment:
I'm not sure I understand why we fetch the `client secret` from the request
and not `client_id`. AFAIK they are both part of the request, the client ID
should be fetched by `request.getParameter(CLIENT_ID)`.
##########
gateway-service-restcatalog/src/main/java/org/apache/knox/gateway/service/restcatalog/RestCatalogHaDispatch.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * 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.knox.gateway.service.restcatalog;
+
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.knox.gateway.ha.dispatch.ConfigurableHADispatch;
+
+import javax.servlet.FilterConfig;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+
+public class RestCatalogHaDispatch extends ConfigurableHADispatch {
+
+ static final String SERVICE_ROLE = "ICEBERG-REST";
+
+ private TokenMetadataHeaderHandler headerHandler;
Review Comment:
nit: should be `final`.
##########
gateway-service-restcatalog/src/main/java/org/apache/knox/gateway/service/restcatalog/RestCatalogDispatch.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * 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.knox.gateway.service.restcatalog;
+
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.knox.gateway.dispatch.ConfigurableDispatch;
+
+import javax.servlet.FilterConfig;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+
+/**
+ * A Dispatch implementation that supports adding token metadata to the
outbound request headers.
+ */
+public class RestCatalogDispatch extends ConfigurableDispatch {
+
+ TokenMetadataHeaderHandler headerHandler;
Review Comment:
nit: should be `private final`.
Issue Time Tracking
-------------------
Worklog Id: (was: 1010944)
Time Spent: 1h 20m (was: 1h 10m)
> REST Catalog dispatch implementation for including configurable metadata as
> outbound request headers
> ----------------------------------------------------------------------------------------------------
>
> Key: KNOX-3279
> URL: https://issues.apache.org/jira/browse/KNOX-3279
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Reporter: Philip Zampino
> Assignee: Philip Zampino
> Priority: Major
> Time Spent: 1h 20m
> Remaining Estimate: 0h
>
> For Iceberg REST Catalog proxying, Knox should support the ability to convey
> a configurable set of token metadata elements associated with the client
> credentials from the inbound request as headers in the outbound (dispatch)
> request.
> A custom dispatch for the ICEBERG-REST service should be implemented to
> provide this support.
> Proposed topology contents (example):
> {code:java}
> <service>
> <role>ICEBERG-REST</role>
> <param>
> <name>token-metadata-headers</name>
> <value>email,category</value>
> </param>
> </service> {code}
> If the configured metadata items don't exist for a given client_id, then no
> headers for those items should be conveyed in the outbound request (i.e.,
> they should be ignored).
>
> It's not clear whether the standard {{userName}} metadata item should be
> included by default.
> The resulting header names can be of the form {{X-Knox-Meta-<ITEM_NAME>}}
> where {{<ITEM_NAME>}} is the token metadata item name.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
