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. -- 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]
