Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1888,4 +1888,24 @@ public List<String> getLDAPSSLEnabledCipherSuites() {
public boolean getGroupUIServicesOnHomepage() {
return getBoolean(KNOX_HOMEPAGE_GROUP_UI_SERVICES, DEFAULT_GROUP_UI_SERVICES);
}

@Override
public int getTrustedOidcIssuerMaxTrustedIssuers() {
return getInt(TRUSTED_OIDC_ISSUER_MAX_TRUSTED_ISSUERS, TRUSTED_OIDC_ISSUER_MAX_TRUSTED_ISSUERS_DEFAULT);
}

@Override
public int getTrustedOidcIssuerDiscoveryCacheTtlSecs() {
return getInt(TRUSTED_OIDC_ISSUER_DISCOVERY_CACHE_TTL_SECS, TRUSTED_OIDC_ISSUER_DISCOVERY_CACHE_TTL_SECS_DEFAULT);
}

@Override
public int getTrustedOidcIssuerDiscoveryConnectTimeoutMs() {
return getInt(TRUSTED_OIDC_ISSUER_DISCOVERY_CONNECT_TIMEOUT_MS, TRUSTED_OIDC_ISSUER_DISCOVERY_CONNECT_TIMEOUT_MS_DEFAULT);
}

@Override
public int getTrustedOidcIssuerDiscoveryReadTimeoutMs() {
return getInt(TRUSTED_OIDC_ISSUER_DISCOVERY_READ_TIMEOUT_MS, TRUSTED_OIDC_ISSUER_DISCOVERY_READ_TIMEOUT_MS_DEFAULT);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@
*/
package org.apache.knox.gateway.services.knoxidf.trustedoidcissuer;

import org.apache.hadoop.conf.Configuration;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.database.DataSourceProvider;
import org.apache.knox.gateway.i18n.messages.MessagesFactory;
import org.apache.knox.gateway.services.ServiceLifecycleException;
import org.apache.knox.gateway.services.security.AliasService;
import org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants;

import java.sql.SQLException;
import java.util.ArrayList;
Expand Down Expand Up @@ -55,8 +53,6 @@ public class JdbcTrustedOidcIssuerService implements TrustedOidcIssuerService {
private static final TrustedOidcIssuerServiceMessages LOG =
MessagesFactory.get(TrustedOidcIssuerServiceMessages.class);

static final String MAX_TRUSTED_ISSUERS_CONFIG = "gateway.trustedoidcissuer.max.issuers";
private static final int DEFAULT_MAX_TRUSTED_ISSUERS = 10_000;

private final AtomicBoolean initialized = new AtomicBoolean(false);
private final Lock initLock = new ReentrantLock(true);
Expand All @@ -78,27 +74,11 @@ public void init(GatewayConfig config, Map<String, String> options) throws Servi
throw new ServiceLifecycleException("The required AliasService reference has not been set.");
}
try {
int maxIssuers = DEFAULT_MAX_TRUSTED_ISSUERS;
long cacheTtlSecs = KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DEFAULT_DISCOVERY_CACHE_TTL_SECS;
int connectTimeoutMs = KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DEFAULT_DISCOVERY_CONNECT_TIMEOUT_MS;
int readTimeoutMs = KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DEFAULT_DISCOVERY_READ_TIMEOUT_MS;

if (config instanceof Configuration) {
final Configuration conf = (Configuration) config;
maxIssuers = conf.getInt(MAX_TRUSTED_ISSUERS_CONFIG, DEFAULT_MAX_TRUSTED_ISSUERS);
cacheTtlSecs = conf.getLong(KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DISCOVERY_CACHE_TTL_SECS,
KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DEFAULT_DISCOVERY_CACHE_TTL_SECS);
connectTimeoutMs = conf.getInt(KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DISCOVERY_CONNECT_TIMEOUT_MS,
KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DEFAULT_DISCOVERY_CONNECT_TIMEOUT_MS);
readTimeoutMs = conf.getInt(KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DISCOVERY_READ_TIMEOUT_MS,
KnoxIDFConstants.TRUSTED_OIDC_ISSUER_DEFAULT_DISCOVERY_READ_TIMEOUT_MS);
}

this.maxTrustedIssuers = maxIssuers;
this.maxTrustedIssuers = config.getTrustedOidcIssuerMaxTrustedIssuers();
this.database = new TrustedOidcIssuerDatabase(
DataSourceProvider.getDataSource(config, aliasService), config.getDatabaseType());
this.discoveryHelper = new OIDCDiscoveryHelper(this, cacheTtlSecs,
OIDCDiscoveryHelper.buildHttpClient(connectTimeoutMs, readTimeoutMs));
this.discoveryHelper = new OIDCDiscoveryHelper(this, config.getTrustedOidcIssuerDiscoveryCacheTtlSecs(),
OIDCDiscoveryHelper.buildHttpClient(config.getTrustedOidcIssuerDiscoveryConnectTimeoutMs(), config.getTrustedOidcIssuerDiscoveryReadTimeoutMs()));
reloadRegistrySnapshot();
initialized.set(true);
} catch (ServiceLifecycleException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.apache.knox.gateway.services.knoxidf.trustedoidcissuer;

import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.config.impl.GatewayConfigImpl;
import org.apache.knox.gateway.database.AbstractDataSourceFactory;
import org.apache.knox.gateway.database.DatabaseType;
import org.apache.knox.gateway.services.ServiceLifecycleException;
Expand Down Expand Up @@ -48,9 +47,8 @@ public class JdbcTrustedOidcIssuerServiceTest {
private static final String DERBY_URL = "jdbc:derby:memory:" + DB_NAME;
private static final String DERBY_SHUTDOWN_URL = "jdbc:derby:memory:" + DB_NAME + ";shutdown=true";

private static GatewayConfig gatewayConfig;
private static AliasService aliasService;

private GatewayConfig gatewayConfig;
private AliasService aliasService;
private JdbcTrustedOidcIssuerService service;

@BeforeClass
Expand All @@ -59,18 +57,6 @@ public static void setUpClass() throws Exception {
java.util.Locale.setDefault(java.util.Locale.US);
// Create the Derby in-memory DB so DerbyDataSourceFactory can connect to it
DriverManager.getConnection(DERBY_CREATE_URL).close();

gatewayConfig = EasyMock.createNiceMock(GatewayConfig.class);
EasyMock.expect(gatewayConfig.getDatabaseType()).andReturn(DatabaseType.DERBY.type()).anyTimes();
EasyMock.expect(gatewayConfig.getDatabaseName()).andReturn("memory:" + DB_NAME).anyTimes();
EasyMock.replay(gatewayConfig);

aliasService = EasyMock.createNiceMock(AliasService.class);
EasyMock.expect(aliasService.getPasswordFromAliasForGateway(
AbstractDataSourceFactory.DATABASE_USER_ALIAS_NAME)).andReturn(null).anyTimes();
EasyMock.expect(aliasService.getPasswordFromAliasForGateway(
AbstractDataSourceFactory.DATABASE_PASSWORD_ALIAS_NAME)).andReturn(null).anyTimes();
EasyMock.replay(aliasService);
}

@AfterClass
Expand All @@ -86,7 +72,7 @@ public static void tearDownClass() {
}

@Before
public void setUp() throws ServiceLifecycleException, SQLException {
public void setUp() throws Exception {
// Clear table between tests
try (Connection conn = DriverManager.getConnection(DERBY_URL);
PreparedStatement ps = conn.prepareStatement("DELETE FROM TRUSTED_OIDC_ISSUERS")) {
Expand All @@ -95,6 +81,19 @@ public void setUp() throws ServiceLifecycleException, SQLException {
// Table may not exist yet on first setUp; service.init() will create it
}

gatewayConfig = EasyMock.createNiceMock(GatewayConfig.class);
EasyMock.expect(gatewayConfig.getDatabaseType()).andReturn(DatabaseType.DERBY.type()).anyTimes();
EasyMock.expect(gatewayConfig.getDatabaseName()).andReturn("memory:" + DB_NAME).anyTimes();
EasyMock.expect(gatewayConfig.getTrustedOidcIssuerMaxTrustedIssuers() ).andReturn(10).anyTimes();
EasyMock.replay(gatewayConfig);

aliasService = EasyMock.createNiceMock(AliasService.class);
EasyMock.expect(aliasService.getPasswordFromAliasForGateway(
AbstractDataSourceFactory.DATABASE_USER_ALIAS_NAME)).andReturn(null).anyTimes();
EasyMock.expect(aliasService.getPasswordFromAliasForGateway(
AbstractDataSourceFactory.DATABASE_PASSWORD_ALIAS_NAME)).andReturn(null).anyTimes();
EasyMock.replay(aliasService);

service = new JdbcTrustedOidcIssuerService();
service.setAliasService(aliasService);
service.init(gatewayConfig, null);
Expand Down Expand Up @@ -217,12 +216,13 @@ public void testReloadAfterMutation() {
assertTrue(service.list().isEmpty());
}

@Test
@Test(expected = IllegalStateException.class)
public void testMaxTrustedIssuers() throws ServiceLifecycleException {
final GatewayConfigImpl limitedConfig = new GatewayConfigImpl();
limitedConfig.set(JdbcTrustedOidcIssuerService.MAX_TRUSTED_ISSUERS_CONFIG, "2");
limitedConfig.set(GatewayConfigImpl.GATEWAY_DATABASE_TYPE, DatabaseType.DERBY.type());
limitedConfig.set(GatewayConfigImpl.GATEWAY_DATABASE_NAME, "memory:" + DB_NAME);
final GatewayConfig limitedConfig = EasyMock.createNiceMock(GatewayConfig.class);
EasyMock.expect(limitedConfig.getDatabaseType()).andReturn(DatabaseType.DERBY.type()).anyTimes();
EasyMock.expect(limitedConfig.getDatabaseName()).andReturn("memory:" + DB_NAME).anyTimes();
EasyMock.expect(limitedConfig.getTrustedOidcIssuerMaxTrustedIssuers() ).andReturn(2).anyTimes();
EasyMock.replay(limitedConfig);

final JdbcTrustedOidcIssuerService limitedService = new JdbcTrustedOidcIssuerService();
limitedService.setAliasService(aliasService);
Expand All @@ -234,13 +234,9 @@ public void testMaxTrustedIssuers() throws ServiceLifecycleException {
limitedService.register(issuer("https://b.example.com", false));
assertEquals("Second registration must succeed", 2, limitedService.list().size());

try {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I prefer to use the try/catch with fail rather than an expected exception in any test that makes more than one call. In this case there are several calls to limitedService. With the general expected exception, you don't know where the exception came from, it could have happened well before you intend.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, let me fix this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened a follow-up PR for this: #1326

limitedService.register(issuer("https://c.example.com", false));
fail("Expected IllegalStateException when exceeding max issuers limit");
} catch (IllegalStateException e) {
assertEquals("Prior registrations must be unaffected by the rejected call",
2, limitedService.list().size());
}
// this one should fail (see expected error on the test annotation)
limitedService.register(issuer("https://c.example.com", false));
fail("Expected IllegalStateException when exceeding max issuers limit");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1359,4 +1359,25 @@ public List<String> getLDAPSSLEnabledCipherSuites() {
public boolean getGroupUIServicesOnHomepage() {
return false;
}

@Override
public int getTrustedOidcIssuerMaxTrustedIssuers() {
return 0;
}

@Override
public int getTrustedOidcIssuerDiscoveryCacheTtlSecs() {
return 0;
}

@Override
public int getTrustedOidcIssuerDiscoveryConnectTimeoutMs() {
return 0;
}

@Override
public int getTrustedOidcIssuerDiscoveryReadTimeoutMs() {
return 0;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,18 @@ public interface GatewayConfig {
String LDAP_SSL_KEYSTORE_PASSWORD_ALIAS = "gateway.ldap.ssl.keystore.password.alias";
String LDAP_SSL_ENABLED_CIPHER_SUITES = "gateway.ldap.ssl.enabled.cipher.suites";

// TrustedOidcIssuerService gateway-level params and their default values
String TRUSTED_OIDC_ISSUER_PREFIX = "gateway.trusted.oidc.issuer.";
String TRUSTED_OIDC_ISSUER_MAX_TRUSTED_ISSUERS = TRUSTED_OIDC_ISSUER_PREFIX + "max.issuers";
int TRUSTED_OIDC_ISSUER_MAX_TRUSTED_ISSUERS_DEFAULT = 10_000;
String TRUSTED_OIDC_ISSUER_DISCOVERY_PREFIX = TRUSTED_OIDC_ISSUER_PREFIX + "discovery.";
String TRUSTED_OIDC_ISSUER_DISCOVERY_CACHE_TTL_SECS = TRUSTED_OIDC_ISSUER_DISCOVERY_PREFIX + "cache.ttl.secs";
int TRUSTED_OIDC_ISSUER_DISCOVERY_CACHE_TTL_SECS_DEFAULT = 600;
String TRUSTED_OIDC_ISSUER_DISCOVERY_CONNECT_TIMEOUT_MS = TRUSTED_OIDC_ISSUER_DISCOVERY_PREFIX + "connect.timeout.ms";
int TRUSTED_OIDC_ISSUER_DISCOVERY_CONNECT_TIMEOUT_MS_DEFAULT = 3000;
String TRUSTED_OIDC_ISSUER_DISCOVERY_READ_TIMEOUT_MS = TRUSTED_OIDC_ISSUER_DISCOVERY_PREFIX+ "read.timeout.ms";
int TRUSTED_OIDC_ISSUER_DISCOVERY_READ_TIMEOUT_MS_DEFAULT = 10000;

/**
* The location of the gateway configuration.
* Subdirectories will be: topologies
Expand Down Expand Up @@ -1217,4 +1229,13 @@ public interface GatewayConfig {
Set<String> getPropertyNames();

boolean getGroupUIServicesOnHomepage();

int getTrustedOidcIssuerMaxTrustedIssuers();

int getTrustedOidcIssuerDiscoveryCacheTtlSecs();

int getTrustedOidcIssuerDiscoveryConnectTimeoutMs();

int getTrustedOidcIssuerDiscoveryReadTimeoutMs();

}
Loading