[
https://issues.apache.org/jira/browse/KNOX-3426?focusedWorklogId=1038564&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1038564
]
ASF GitHub Bot logged work on KNOX-3426:
----------------------------------------
Author: ASF GitHub Bot
Created on: 28/Aug/26 21:54
Start Date: 28/Aug/26 21:54
Worklog Time Spent: 10m
Work Description: Copilot commented on code in PR #1361:
URL: https://github.com/apache/knox/pull/1361#discussion_r3884291830
##########
gateway-spi/src/main/java/org/apache/knox/gateway/services/knoxidf/delegation/PolicyCheckRequest.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.services.knoxidf.delegation;
+
+import java.util.Set;
+
+/**
+ * Immutable input to {@link
DelegationPolicyService#evaluate(PolicyCheckRequest)}.
+ */
+public class PolicyCheckRequest {
+
+ private final String actorAuthority;
+ private final String actorId;
+ private final String subjectName;
+ private final String requestedResource;
+ private final Set<String> requestedScopes;
+ private final boolean headlessExchange;
+
+ public PolicyCheckRequest(String actorAuthority, String actorId, String
subjectName,
+ String requestedResource, Set<String> requestedScopes, boolean
headlessExchange) {
+ this.actorAuthority = actorAuthority;
+ this.actorId = actorId;
+ this.subjectName = subjectName;
+ this.requestedResource = requestedResource;
+ this.requestedScopes = requestedScopes;
+ this.headlessExchange = headlessExchange;
+ }
Review Comment:
PolicyCheckRequest is documented as immutable, but it stores the
caller-provided requestedScopes Set reference directly. That allows external
mutation after construction, which can change evaluate() outcomes (and can also
NPE later if null is passed). Consider defensively copying and wrapping the
scopes set (defaulting null to empty).
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/delegation/DelegationPolicyDatabase.java:
##########
@@ -0,0 +1,426 @@
+/*
+ * 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.services.knoxidf.delegation;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.knox.gateway.database.DatabaseType;
+import org.apache.knox.gateway.database.JDBCUtils;
+import org.apache.knox.gateway.database.KnoxDatabase;
+
+import javax.sql.DataSource;
+import java.io.InputStream;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * JDBC helper for the five DELEGATION_POLICIES tables.
+ * All SQL uses {@link PreparedStatement} with {@code ?} parameters only.
+ * Each public method manages its own {@link Connection} and, for multi-table
writes,
+ * its own transaction boundaries (setAutoCommit / commit / rollback).
+ */
+class DelegationPolicyDatabase extends KnoxDatabase {
+
+ static final String CORE_TABLE = "DELEGATION_POLICIES";
+
+ private static final String INSERT_REGISTRATION_SQL =
+ "INSERT INTO " + CORE_TABLE
+ + " (registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange) "
+ + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
+
+ private static final String UPDATE_CORE_SQL =
+ "UPDATE " + CORE_TABLE + " SET "
+ + "actor_authority = ?, actor_id = ?, name = ?, status = ?,
token_ttl_sec = ?, "
+ + "description = ?, created_by = ?, created_at = ?, updated_at = ?, "
+ + "allow_headless_exchange = ? "
+ + "WHERE registration_id = ?";
+
+ private static final String DELETE_REGISTRATION_SQL =
+ "DELETE FROM " + CORE_TABLE + " WHERE registration_id = ?";
+
+ private static final String SELECT_BY_ID_SQL =
+ "SELECT registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange "
+ + "FROM " + CORE_TABLE + " WHERE registration_id = ?";
+
+ private static final String SELECT_BY_ACTOR_SQL =
+ "SELECT registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange "
+ + "FROM " + CORE_TABLE + " WHERE actor_authority = ? AND actor_id =
?";
+
+ private static final String SELECT_ALL_BASE_SQL =
+ "SELECT registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange "
+ + "FROM " + CORE_TABLE;
+
+ // Built at construction time with limit+1 baked in as an integer literal
(Derby does not
+ // support ? parameters in FETCH FIRST n ROWS ONLY). Fetching one extra row
lets selectAll()
+ // detect truncation without a second COUNT query.
+ private final int listMaxTotal;
+ private final int listMaxPerAuthority;
+ private final String selectAllSql;
+ private final String selectAllFilteredSql;
+
+ private static final String INSERT_USER_SQL =
+ "INSERT INTO DELEGATION_POLICY_USERS (registration_id, username) VALUES
(?, ?)";
+
+ private static final String INSERT_GROUP_SQL =
+ "INSERT INTO DELEGATION_POLICY_GROUPS (registration_id, group_name)
VALUES (?, ?)";
+
+ private static final String INSERT_RESOURCE_SQL =
+ "INSERT INTO DELEGATION_POLICY_RESOURCES (registration_id, resource_uri)
VALUES (?, ?)";
+
+ private static final String INSERT_SCOPE_SQL =
+ "INSERT INTO DELEGATION_POLICY_RESOURCE_SCOPES (registration_id,
resource_uri, scope) VALUES (?, ?, ?)";
+
+ private static final String SELECT_USERS_SQL =
+ "SELECT username FROM DELEGATION_POLICY_USERS WHERE registration_id = ?";
+
+ private static final String SELECT_GROUPS_SQL =
+ "SELECT group_name FROM DELEGATION_POLICY_GROUPS WHERE registration_id =
?";
+
+ private static final String SELECT_RESOURCES_SQL =
+ "SELECT resource_uri FROM DELEGATION_POLICY_RESOURCES WHERE
registration_id = ?";
+
+ private static final String SELECT_SCOPES_SQL =
+ "SELECT scope FROM DELEGATION_POLICY_RESOURCE_SCOPES WHERE
registration_id = ? AND resource_uri = ?";
+
+ private static final String DELETE_USERS_SQL =
+ "DELETE FROM DELEGATION_POLICY_USERS WHERE registration_id = ?";
+
+ private static final String DELETE_GROUPS_SQL =
+ "DELETE FROM DELEGATION_POLICY_GROUPS WHERE registration_id = ?";
+
+ private static final String DELETE_RESOURCES_SQL =
+ "DELETE FROM DELEGATION_POLICY_RESOURCES WHERE registration_id = ?";
+
+ DelegationPolicyDatabase(DataSource dataSource, String dbType, int
listMaxTotal, int listMaxPerAuthority) throws Exception {
+ super(dataSource);
+ this.listMaxTotal = listMaxTotal;
+ this.listMaxPerAuthority = listMaxPerAuthority;
+ this.selectAllSql = SELECT_ALL_BASE_SQL + " FETCH FIRST " + (listMaxTotal
+ 1) + " ROWS ONLY";
+ this.selectAllFilteredSql = SELECT_ALL_BASE_SQL + " WHERE actor_authority
= ? FETCH FIRST " + (listMaxPerAuthority + 1) + " ROWS ONLY";
Review Comment:
selectAllSql/selectAllFilteredSql apply a hard row limit but don’t specify
an ORDER BY. Without deterministic ordering, list() results (and
truncation/hasMore behavior) can vary across executions and DB implementations,
which is risky for admin APIs/UI. Consider adding an explicit ORDER BY (e.g.,
actor_authority, actor_id) before applying FETCH FIRST.
##########
gateway-spi/src/main/java/org/apache/knox/gateway/services/knoxidf/delegation/DelegationPolicy.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * 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.services.knoxidf.delegation;
+
+import java.time.Instant;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Immutable representation of a stored delegation policy record.
+ */
+public class DelegationPolicy {
+
+ private final String registrationId;
+ private final String actorAuthority;
+ private final String actorId;
+ private final String name;
+ private final String status;
+ private final Integer tokenTtlSec;
+ private final String description;
+ private final String createdBy;
+ private final Instant createdAt;
+ private final Instant updatedAt;
+ private final boolean allowHeadlessExchange;
+ private final Set<String> canActForUsers;
+ private final Set<String> canActForGroups;
+ private final Map<String, Set<String>> resourcePolicy;
+
+ public DelegationPolicy(String registrationId, String actorAuthority, String
actorId,
+ String name, String status, Integer tokenTtlSec, String description,
String createdBy,
+ Instant createdAt, Instant updatedAt, boolean allowHeadlessExchange,
+ Set<String> canActForUsers, Set<String> canActForGroups,
+ Map<String, Set<String>> resourcePolicy) {
+ this.registrationId = registrationId;
+ this.actorAuthority = actorAuthority;
+ this.actorId = actorId;
+ this.name = name;
+ this.status = status;
+ this.tokenTtlSec = tokenTtlSec;
+ this.description = description;
+ this.createdBy = createdBy;
+ this.createdAt = createdAt;
+ this.updatedAt = updatedAt;
+ this.allowHeadlessExchange = allowHeadlessExchange;
+ this.canActForUsers = Collections.unmodifiableSet(new
HashSet<>(canActForUsers));
+ this.canActForGroups = Collections.unmodifiableSet(new
HashSet<>(canActForGroups));
+ Map<String, Set<String>> copy = new HashMap<>();
+ for (Map.Entry<String, Set<String>> entry : resourcePolicy.entrySet()) {
+ copy.put(entry.getKey(), Collections.unmodifiableSet(new
HashSet<>(entry.getValue())));
+ }
+ this.resourcePolicy = Collections.unmodifiableMap(copy);
Review Comment:
DelegationPolicy’s constructor assumes
canActForUsers/canActForGroups/resourcePolicy and each resourcePolicy value are
non-null (new HashSet<>(...) and entry.getValue()). Since this is a public SPI
POJO, a null input will currently throw a NullPointerException during
construction. Either document non-null requirements explicitly or make the
constructor null-safe (e.g., treat null collections as empty).
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/delegation/DelegationPolicyDatabase.java:
##########
@@ -0,0 +1,426 @@
+/*
+ * 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.services.knoxidf.delegation;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.knox.gateway.database.DatabaseType;
+import org.apache.knox.gateway.database.JDBCUtils;
+import org.apache.knox.gateway.database.KnoxDatabase;
+
+import javax.sql.DataSource;
+import java.io.InputStream;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * JDBC helper for the five DELEGATION_POLICIES tables.
+ * All SQL uses {@link PreparedStatement} with {@code ?} parameters only.
+ * Each public method manages its own {@link Connection} and, for multi-table
writes,
+ * its own transaction boundaries (setAutoCommit / commit / rollback).
+ */
+class DelegationPolicyDatabase extends KnoxDatabase {
+
+ static final String CORE_TABLE = "DELEGATION_POLICIES";
+
+ private static final String INSERT_REGISTRATION_SQL =
+ "INSERT INTO " + CORE_TABLE
+ + " (registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange) "
+ + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
+
+ private static final String UPDATE_CORE_SQL =
+ "UPDATE " + CORE_TABLE + " SET "
+ + "actor_authority = ?, actor_id = ?, name = ?, status = ?,
token_ttl_sec = ?, "
+ + "description = ?, created_by = ?, created_at = ?, updated_at = ?, "
+ + "allow_headless_exchange = ? "
+ + "WHERE registration_id = ?";
+
+ private static final String DELETE_REGISTRATION_SQL =
+ "DELETE FROM " + CORE_TABLE + " WHERE registration_id = ?";
+
+ private static final String SELECT_BY_ID_SQL =
+ "SELECT registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange "
+ + "FROM " + CORE_TABLE + " WHERE registration_id = ?";
+
+ private static final String SELECT_BY_ACTOR_SQL =
+ "SELECT registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange "
+ + "FROM " + CORE_TABLE + " WHERE actor_authority = ? AND actor_id =
?";
+
+ private static final String SELECT_ALL_BASE_SQL =
+ "SELECT registration_id, actor_authority, actor_id, name, status,
token_ttl_sec, "
+ + "description, created_by, created_at, updated_at,
allow_headless_exchange "
+ + "FROM " + CORE_TABLE;
+
+ // Built at construction time with limit+1 baked in as an integer literal
(Derby does not
+ // support ? parameters in FETCH FIRST n ROWS ONLY). Fetching one extra row
lets selectAll()
+ // detect truncation without a second COUNT query.
+ private final int listMaxTotal;
+ private final int listMaxPerAuthority;
+ private final String selectAllSql;
+ private final String selectAllFilteredSql;
+
+ private static final String INSERT_USER_SQL =
+ "INSERT INTO DELEGATION_POLICY_USERS (registration_id, username) VALUES
(?, ?)";
+
+ private static final String INSERT_GROUP_SQL =
+ "INSERT INTO DELEGATION_POLICY_GROUPS (registration_id, group_name)
VALUES (?, ?)";
+
+ private static final String INSERT_RESOURCE_SQL =
+ "INSERT INTO DELEGATION_POLICY_RESOURCES (registration_id, resource_uri)
VALUES (?, ?)";
+
+ private static final String INSERT_SCOPE_SQL =
+ "INSERT INTO DELEGATION_POLICY_RESOURCE_SCOPES (registration_id,
resource_uri, scope) VALUES (?, ?, ?)";
+
+ private static final String SELECT_USERS_SQL =
+ "SELECT username FROM DELEGATION_POLICY_USERS WHERE registration_id = ?";
+
+ private static final String SELECT_GROUPS_SQL =
+ "SELECT group_name FROM DELEGATION_POLICY_GROUPS WHERE registration_id =
?";
+
+ private static final String SELECT_RESOURCES_SQL =
+ "SELECT resource_uri FROM DELEGATION_POLICY_RESOURCES WHERE
registration_id = ?";
+
+ private static final String SELECT_SCOPES_SQL =
+ "SELECT scope FROM DELEGATION_POLICY_RESOURCE_SCOPES WHERE
registration_id = ? AND resource_uri = ?";
+
+ private static final String DELETE_USERS_SQL =
+ "DELETE FROM DELEGATION_POLICY_USERS WHERE registration_id = ?";
+
+ private static final String DELETE_GROUPS_SQL =
+ "DELETE FROM DELEGATION_POLICY_GROUPS WHERE registration_id = ?";
+
+ private static final String DELETE_RESOURCES_SQL =
+ "DELETE FROM DELEGATION_POLICY_RESOURCES WHERE registration_id = ?";
+
+ DelegationPolicyDatabase(DataSource dataSource, String dbType, int
listMaxTotal, int listMaxPerAuthority) throws Exception {
+ super(dataSource);
+ this.listMaxTotal = listMaxTotal;
+ this.listMaxPerAuthority = listMaxPerAuthority;
+ this.selectAllSql = SELECT_ALL_BASE_SQL + " FETCH FIRST " + (listMaxTotal
+ 1) + " ROWS ONLY";
+ this.selectAllFilteredSql = SELECT_ALL_BASE_SQL + " WHERE actor_authority
= ? FETCH FIRST " + (listMaxPerAuthority + 1) + " ROWS ONLY";
+ final DatabaseType databaseType = DatabaseType.fromString(dbType);
+
createDelegationTablesIfNotExists(databaseType.delegationPolicyTablesSql());
+ }
+
+ /**
+ * Multi-statement DDL runner: checks if DELEGATION_POLICIES exists, then
strips SQL line
+ * comments, splits on {@code ;}, and executes each non-empty statement
individually.
+ * {@link JDBCUtils#createTableFromSQL} handles only single statements;
delegation needs five.
+ * Comment stripping must happen before the split because the ASF license
header contains a
+ * semicolon inside a {@code --} comment line, which would otherwise produce
a spurious token.
+ */
+ private void createDelegationTablesIfNotExists(String sqlFileName) throws
Exception {
+ if (!JDBCUtils.tableExists(CORE_TABLE, dataSource)) {
+ try (InputStream is =
getClass().getClassLoader().getResourceAsStream(sqlFileName);
+ Connection connection = dataSource.getConnection()) {
+ final String script = IOUtils.toString(is, UTF_8);
+ final StringBuilder stripped = new StringBuilder();
Review Comment:
createDelegationTablesIfNotExists() reads the DDL resource stream without
checking for null. If the SQL file isn’t on the classpath (packaging
regression, typo, etc.), this will throw a NullPointerException from
IOUtils.toString() rather than a clear error. Add an explicit null check and
fail fast with a descriptive exception.
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/knoxidf/delegation/DelegationPolicySchemaTest.java:
##########
@@ -0,0 +1,230 @@
+/*
+ * 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.services.knoxidf.delegation;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.knox.gateway.database.AbstractDataSourceFactory;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.UUID;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Validates that the DELEGATION_POLICIES DDL scripts parse and execute
correctly
+ * against in-memory databases, and that schema constraints are enforced.
+ */
+public class DelegationPolicySchemaTest {
+
+ private static final String DERBY_DB = "delegationpolicies";
+ private static final String DERBY_URL = "jdbc:derby:memory:" + DERBY_DB +
";create=true";
+ private static final String DERBY_SHUTDOWN_URL = "jdbc:derby:memory:" +
DERBY_DB + ";shutdown=true";
+ private static final String HSQL_URL =
"jdbc:hsqldb:mem:delegationschema;ifexists=false";
+ private static final String HSQL_USER = "SA";
+ private static final String HSQL_PASSWORD = "";
+
+ private static Connection derbyConn;
+ private static Connection hsqlConn;
+
+ @BeforeClass
+ public static void setUp() throws Exception {
+ java.util.Locale.setDefault(java.util.Locale.US);
+ derbyConn = DriverManager.getConnection(DERBY_URL);
+ hsqlConn = DriverManager.getConnection(HSQL_URL, HSQL_USER, HSQL_PASSWORD);
+ // Run Derby DDL once - no IF NOT EXISTS, so run once at class level
+ runScript(derbyConn,
loadSql(AbstractDataSourceFactory.DERBY_KNOXIDF_DELEGATION_POLICY_TABLES_SQL));
+ // Run standard DDL on HSQLDB
+ runScript(hsqlConn,
loadSql(AbstractDataSourceFactory.KNOXIDF_DELEGATION_POLICY_TABLES_SQL));
+ }
Review Comment:
PR description says schema tests verify all three dialect DDL scripts, but
this test only executes Derby + the standard script. Oracle DDL isn’t currently
exercised/validated here, so either update the description or add at least a
classpath presence check for the Oracle script (full execution would require an
Oracle-backed test).
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/factory/DelegationPolicyServiceFactoryTest.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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.services.factory;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.knox.gateway.config.impl.GatewayConfigImpl;
+import org.apache.knox.gateway.database.DatabaseType;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.Service;
+import org.apache.knox.gateway.services.ServiceType;
+import
org.apache.knox.gateway.services.knoxidf.delegation.EmptyDelegationPolicyService;
+import
org.apache.knox.gateway.services.knoxidf.delegation.JdbcDelegationPolicyService;
+import org.apache.knox.gateway.services.knoxidf.delegation.PolicyCheckRequest;
+import org.apache.knox.gateway.services.knoxidf.delegation.PolicyDecision;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyService;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.topology.TopologyService;
+import org.apache.knox.gateway.topology.Topology;
+import org.easymock.EasyMock;
+import org.junit.After;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.io.File;
+import java.sql.DriverManager;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class DelegationPolicyServiceFactoryTest {
+
+ private final DelegationPolicyServiceFactory factory = new
DelegationPolicyServiceFactory();
+ private final Map<String, String> options = new HashMap<>();
+ private File tempDir;
+ private Service createdService;
+
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+ java.util.Locale.setDefault(java.util.Locale.US);
+
DriverManager.getConnection("jdbc:derby:memory:DelegationPolicyServiceFactoryTest_knoxidf;create=true").close();
+
DriverManager.getConnection("jdbc:derby:memory:DelegationPolicyServiceFactoryTest_knoxidf_admin;create=true").close();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ if (createdService != null) {
+ createdService.stop();
+ }
+ if (tempDir != null) {
+ FileUtils.forceDelete(tempDir);
+ }
+ }
+
+ // ------------------------------------------------------------------
+ // Empty (no KNOXIDF) cases
+ // ------------------------------------------------------------------
+
+ @Test
+ public void shouldSelectEmptyWhenNoTopologies() throws Exception {
+ final GatewayServices gws = servicesWithTopology(/* no topologies */);
+ final GatewayConfigImpl config =
EasyMock.createNiceMock(GatewayConfigImpl.class);
+ EasyMock.replay(config);
+ createdService = factory.create(gws,
ServiceType.DELEGATION_POLICY_SERVICE, config, options, "");
+ assertTrue(createdService instanceof EmptyDelegationPolicyService);
+ }
+
+ @Test
+ public void shouldSelectEmptyWhenNoKnoxIdfRole() throws Exception {
+ final GatewayServices gws =
servicesWithTopology(topologyWithRole("KNOXSSO"));
+ final GatewayConfigImpl config =
EasyMock.createNiceMock(GatewayConfigImpl.class);
+ EasyMock.replay(config);
+ createdService = factory.create(gws,
ServiceType.DELEGATION_POLICY_SERVICE, config, options, "");
+ assertTrue(createdService instanceof EmptyDelegationPolicyService);
+ }
+
+ @Test
+ public void shouldHonorExplicitEmptyImplEvenWhenKnoxIdfIsDeployed() throws
Exception {
+ final GatewayServices gws =
servicesWithTopology(topologyWithRole("KNOXIDF"));
+ final GatewayConfigImpl config =
EasyMock.createNiceMock(GatewayConfigImpl.class);
+ EasyMock.replay(config);
+ createdService = factory.create(gws,
ServiceType.DELEGATION_POLICY_SERVICE, config, options,
+ EmptyDelegationPolicyService.class.getName());
+ assertTrue(createdService instanceof EmptyDelegationPolicyService);
+ }
+
+ // ------------------------------------------------------------------
+ // JDBC when KNOXIDF deployed
+ // ------------------------------------------------------------------
+
+ @Test
+ public void shouldSelectJdbcWhenKnoxIdfDeployed() throws Exception {
+ tempDir =
org.apache.knox.test.TestUtils.createTempDir(getClass().getName());
+ final AliasService aliasService =
EasyMock.createNiceMock(AliasService.class);
+ EasyMock.replay(aliasService);
+
+ final TopologyService topologyService =
EasyMock.createNiceMock(TopologyService.class);
+ EasyMock.expect(topologyService.getTopologies()).andReturn(
+ Collections.singletonList(topologyWithRole("KNOXIDF"))).anyTimes();
+ EasyMock.replay(topologyService);
+
+ final GatewayServices gws = EasyMock.createNiceMock(GatewayServices.class);
+
EasyMock.expect(gws.getService(ServiceType.TOPOLOGY_SERVICE)).andReturn(topologyService).anyTimes();
+
EasyMock.expect(gws.getService(ServiceType.ALIAS_SERVICE)).andReturn(aliasService).anyTimes();
+ EasyMock.replay(gws);
+
+ final GatewayConfigImpl config =
EasyMock.createNiceMock(GatewayConfigImpl.class);
+
EasyMock.expect(config.getDatabaseType()).andReturn(DatabaseType.DERBY.type()).anyTimes();
+ EasyMock.expect(config.getDatabaseName())
+ .andReturn("memory:" + getClass().getSimpleName() +
"_knoxidf").anyTimes();
+
EasyMock.expect(config.getDelegationServiceTokenTtlSec()).andReturn(3600).anyTimes();
+ EasyMock.replay(config);
Review Comment:
These factory tests mock GatewayConfigImpl but don’t set expectations for
getDelegationServiceListMaxTotal()/getDelegationServiceListMaxPerAuthority().
JdbcDelegationPolicyService init uses those values, so the mock will return 0
by default and the service will be initialized with unintended list limits.
Setting explicit expectations to the defaults makes the test setup realistic
and prevents accidental behavior changes if list() is exercised later.
This issue also appears on line 154 of the same file.
Issue Time Tracking
-------------------
Worklog Id: (was: 1038564)
Time Spent: 20m (was: 10m)
> Delegation policy schema and JDBC implementation
> ------------------------------------------------
>
> Key: KNOX-3426
> URL: https://issues.apache.org/jira/browse/KNOX-3426
> Project: Apache Knox
> Issue Type: Task
> Components: JWT
> Reporter: Harrison Sheinblatt
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Persistent storage for delegation policies, with a service interface and JDBC
> implementation. After these tasks, delegation policies can be stored and
> retrieved.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
