[
https://issues.apache.org/jira/browse/KNOX-3368?focusedWorklogId=1031879&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1031879
]
ASF GitHub Bot logged work on KNOX-3368:
----------------------------------------
Author: ASF GitHub Bot
Created on: 23/Jul/26 14:56
Start Date: 23/Jul/26 14:56
Worklog Time Spent: 10m
Work Description: smolnar82 commented on code in PR #1321:
URL: https://github.com/apache/knox/pull/1321#discussion_r3639088265
##########
gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/deploy/KnoxIDFAdminServiceDeploymentContributor.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.knoxidf.deploy;
+
+import org.apache.knox.gateway.jersey.JerseyServiceDeploymentContributorBase;
+
+/**
+ * Deployment contributor for the KNOXIDF_ADMIN service role, which hosts the
+ * Knox IDF admin REST API. This contributor initially registers
+ * {@link org.apache.knox.gateway.service.knoxidf.TrustedOidcIssuersResource};
+ * it will be extended in a later task to also register
DelegationAdminResource.
+ */
+public class KnoxIDFAdminServiceDeploymentContributor extends
JerseyServiceDeploymentContributorBase {
+
+ @Override
+ public String getRole() {
+ return "KNOXIDF_ADMIN";
+ }
+
+ @Override
+ public String getName() {
+ return "KNOXIDF_ADMIN";
+ }
+
+ @Override
+ protected String[] getPackages() {
+ return new String[] { "org.apache.knox.gateway.service.knoxidf" };
+ }
+
+ @Override
+ protected String[] getPatterns() {
+ return new String[] { "knoxidf/api/**?**" };
+ }
Review Comment:
`KNOXIDF_ADMIN` and `KNOXIDF` share the same package AND URL pattern. Both
contributors return:
- `getPackages()` → `{"org.apache.knox.gateway.service.knoxidf"}`
- `getPatterns()` → `{"knoxidf/api/**?**"}`
Because Jersey scans the whole package, the `TrustedOidcIssuersResource`
admin endpoints are served by the `KNOXIDF` role too, not just `KNOXIDF_ADMIN`.
If the intent is for admin operations to sit behind stricter authorization
(a separate role usually implies a separate authz/ACL binding in the topology),
that intent is defeated: the same resource is reachable through the non-admin
role. Conversely, deploying both roles in one topology with the identical
knoxidf/api/**?** pattern invites a routing conflict. The test even notes this
(`"Same prefix … so both roles can coexist … Jersey disambiguates by @Path"`),
but path-based disambiguation doesn't give you role-based access control.
Is `KNOXIDF_ADMIN` meant to be independently protected, and if so, shouldn't
its package/pattern be scoped to the admin resources only?
##########
gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TrustedOidcIssuersResource.java:
##########
@@ -0,0 +1,227 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.knoxidf;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.knox.gateway.audit.api.Action;
+import org.apache.knox.gateway.audit.api.ActionOutcome;
+import org.apache.knox.gateway.audit.api.AuditServiceFactory;
+import org.apache.knox.gateway.audit.api.Auditor;
+import org.apache.knox.gateway.audit.api.ResourceType;
+import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.commons.lang3.StringUtils;
+import
org.apache.knox.gateway.services.knoxidf.trustedoidcissuer.TrustedOidcIssuer;
+import
org.apache.knox.gateway.services.knoxidf.trustedoidcissuer.TrustedOidcIssuerService;
+import org.apache.knox.gateway.util.JsonUtils;
+
+import javax.annotation.PostConstruct;
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.security.Principal;
+import java.time.Instant;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static
org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESORCE_PATH;
+
+@Path(TrustedOidcIssuersResource.RESOURCE_PATH)
+@Produces(MediaType.APPLICATION_JSON)
+public class TrustedOidcIssuersResource {
+
+ static final String RESOURCE_PATH = BASE_RESORCE_PATH +
"/admin/trusted-oidc-issuers";
+
+ // Non-final and package-private to allow test injection of a mock Auditor.
+ static Auditor auditor = AuditServiceFactory.getAuditService()
+ .getAuditor(AuditConstants.DEFAULT_AUDITOR_NAME,
+ AuditConstants.KNOX_SERVICE_NAME,
AuditConstants.KNOX_COMPONENT_NAME);
+
+ @Context
+ private ServletContext servletContext;
+
+ @Context
+ private HttpServletRequest request;
+
+ private TrustedOidcIssuerService trustedIssuers;
+
+ @PostConstruct
+ public void init() {
+ final GatewayServices services = (GatewayServices)
+
servletContext.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+ trustedIssuers =
services.getService(ServiceType.TRUSTED_OIDC_ISSUER_SERVICE);
+ }
+
+ @POST
+ @Consumes(MediaType.APPLICATION_JSON)
+ public Response registerIssuer(String body) {
+ String issuerUrl = "INVALID_REQUEST";
+ final String operatorId = getOperatorId();
+ String outcome = ActionOutcome.FAILURE;
+
+ try {
+ final Map<String, Object> parsed;
+ try {
+ parsed = new ObjectMapper().readValue(body, new
TypeReference<Map<String, Object>>() {});
Review Comment:
`ObjectMapper` is expensive to construct and thread-safe to reuse: make it a
`private static final`, or parse via `JsonUtils` for consistency with the rest
of the class.
##########
pom.xml:
##########
@@ -259,6 +259,7 @@
<mina.version>2.2.8</mina.version>
<netty.version>4.1.135.Final</netty.version>
<nimbus-jose-jwt.version>10.9.1</nimbus-jose-jwt.version>
+ <oauth2-oidc-sdk.version>11.37.2</oauth2-oidc-sdk.version>
Review Comment:
This must be left here because of the recent changes in #1320
Issue Time Tracking
-------------------
Worklog Id: (was: 1031879)
Time Spent: 50m (was: 40m)
> Trusted OIDC Issuer admin API for Knox IDF
> ------------------------------------------
>
> Key: KNOX-3368
> URL: https://issues.apache.org/jira/browse/KNOX-3368
> Project: Apache Knox
> Issue Type: Task
> Components: JWT
> Reporter: Harrison Sheinblatt
> Assignee: Harrison Sheinblatt
> Priority: Major
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Implement the admin API for trusted OIDC issuers as part of knox IDF. Include
> the contributor needed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
