From 7eb9f464e26aba8734a9966202078e47f9d28217 Mon Sep 17 00:00:00 2001 From: "Charles Graham, SWT" Date: Fri, 7 Aug 2026 10:20:25 -0500 Subject: [PATCH 1/2] Fix delayed OpenID provider activation Signed-off-by: Charles Graham, SWT --- .../java/cwms/cda/security/Authenticator.java | 28 ++++++++--- .../OpenIdConnectIdentitityProvider.java | 20 +++++--- .../cwms/cda/security/AuthenticatorTest.java | 50 +++++++++++++++++++ .../cwms/cda/security/OpenIDConfigTest.java | 19 +++++++ docker-compose.yml | 1 + 5 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java diff --git a/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java b/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java index 956ccc516d..92913f269a 100644 --- a/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java +++ b/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java @@ -3,6 +3,7 @@ import java.security.Principal; import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import com.google.common.flogger.FluentLogger; @@ -18,15 +19,22 @@ public final class Authenticator implements Handler { public Authenticator() { var surpressed = System.getenv("cwms.dataapi.access.providers.surpress"); - final var supressedList = surpressed == null ? List.of() : List.of(surpressed.split(",")); + final List supressedList = surpressed == null + ? Collections.emptyList() : List.of(surpressed.split(",")); - CdaIdentityProviders.providers().forEachRemaining(provider -> { - if (!supressedList.contains(provider.getName()) && provider.getScheme() != null) { + loadProviders(CdaIdentityProviders.providers(), supressedList); + } + + Authenticator(Iterator availableProviders, List suppressedProviders) { + loadProviders(availableProviders, suppressedProviders); + } + + private void loadProviders(Iterator availableProviders, List suppressedProviders) { + availableProviders.forEachRemaining(provider -> { + if (!suppressedProviders.contains(provider.getName())) { providers.add(provider); } else { - logger.atSevere() - .log("Unable to add Identity Provider %s. See earlier logs for specific error message.", - provider.getName()); + logger.atInfo().log("Suppressing configured Identity Provider %s.", provider.getName()); } }); } @@ -43,6 +51,12 @@ public void handle(Context ctx) throws Exception { } public List getActiveProviders() { - return Collections.unmodifiableList(providers); + ArrayList activeProviders = new ArrayList<>(); + for (IdentityProvider provider: providers) { + if (provider.getScheme() != null) { + activeProviders.add(provider); + } + } + return Collections.unmodifiableList(activeProviders); } } diff --git a/cwms-data-api/src/main/java/cwms/cda/security/OpenIdConnectIdentitityProvider.java b/cwms-data-api/src/main/java/cwms/cda/security/OpenIdConnectIdentitityProvider.java index 0f006eef7d..ea99b84e48 100644 --- a/cwms-data-api/src/main/java/cwms/cda/security/OpenIdConnectIdentitityProvider.java +++ b/cwms-data-api/src/main/java/cwms/cda/security/OpenIdConnectIdentitityProvider.java @@ -46,7 +46,7 @@ public final class OpenIdConnectIdentitityProvider implements IdentityProvider { private final ScheduledExecutorService executor = Executors.newScheduledThreadPool(1); - private AtomicReference config = new AtomicReference<>(null); + private final AtomicReference config = new AtomicReference<>(null); private final String wellKnownUrl; private final String issuer; @@ -65,9 +65,16 @@ public OpenIdConnectIdentitityProvider() { } else { timeout = 3600; } + if (wellKnownUrl == null || wellKnownUrl.isEmpty()) { + log.atInfo().log("OpenID Connect well-known URL is not set; provider will remain disabled."); + executor.shutdown(); + return; + } // try it once, then every 5 minutes until we get it. initializeProvider(); - executor.scheduleAtFixedRate(this::initializeProvider, 0, 5, TimeUnit.MINUTES); + if (config.get() == null) { + executor.scheduleAtFixedRate(this::initializeProvider, 5, 5, TimeUnit.MINUTES); + } } private void initializeProvider() @@ -79,10 +86,6 @@ private void initializeProvider() } try { log.atFine().log("Attempting to initalize OIDC provider for %s", wellKnownUrl); - if (wellKnownUrl == null || wellKnownUrl.isEmpty()) { - executor.shutdown(); // it won't be found, don't keep looking - throw new IOException("OpenID Connect well-known URL is not set."); - } URL wellKnown = new URL(wellKnownUrl); return OpenIDConfig.from(wellKnown, clientId, idpHint, timeout); } catch (IOException ex) { @@ -94,7 +97,7 @@ private void initializeProvider() } return c; }); - if (foundConfig != null) { + if (foundConfig != null || config.get() != null) { executor.shutdown(); // we have it, don't need to keep polling } } @@ -159,6 +162,9 @@ public String getName() { @Override public boolean canAuth(Context ctx) { + if (config.get() == null) { + return false; + } String header = ctx.header(AUTHORIZATION); if (header == null) { return false; diff --git a/cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java b/cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java new file mode 100644 index 0000000000..2266521f2e --- /dev/null +++ b/cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java @@ -0,0 +1,50 @@ +package cwms.cda.security; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.security.Principal; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.Test; + +import cwms.cda.spi.IdentityProvider; +import io.javalin.http.Context; +import io.swagger.v3.oas.models.security.SecurityScheme; + +class AuthenticatorTest { + + @Test + void providerBecomesActiveAfterSchemeIsAvailable() { + AtomicReference scheme = new AtomicReference<>(); + IdentityProvider provider = new IdentityProvider() { + @Override + public String getName() { + return "DelayedProvider"; + } + + @Override + public boolean canAuth(Context ctx) { + return false; + } + + @Override + public Principal authenticate(Context ctx) { + return null; + } + + @Override + public SecurityScheme getScheme() { + return scheme.get(); + } + }; + Authenticator authenticator = new Authenticator(List.of(provider).iterator(), List.of()); + + assertTrue(authenticator.getActiveProviders().isEmpty()); + + scheme.set(new SecurityScheme()); + + assertEquals(List.of(provider), authenticator.getActiveProviders()); + } +} diff --git a/cwms-data-api/src/test/java/cwms/cda/security/OpenIDConfigTest.java b/cwms-data-api/src/test/java/cwms/cda/security/OpenIDConfigTest.java index cdce8c5042..b7f46b2ff4 100644 --- a/cwms-data-api/src/test/java/cwms/cda/security/OpenIDConfigTest.java +++ b/cwms-data-api/src/test/java/cwms/cda/security/OpenIDConfigTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import io.swagger.v3.oas.models.security.SecurityScheme; @@ -12,6 +13,24 @@ class OpenIDConfigTest { + @Test + void providerRemainsDisabledWhenWellKnownUrlIsMissing() { + String previousWellKnown = System.getProperty(OpenIdConnectIdentitityProvider.WELL_KNOWN_PROPERTY); + try { + System.setProperty(OpenIdConnectIdentitityProvider.WELL_KNOWN_PROPERTY, ""); + + OpenIdConnectIdentitityProvider provider = new OpenIdConnectIdentitityProvider(); + + assertNull(provider.getScheme()); + } finally { + if (previousWellKnown == null) { + System.clearProperty(OpenIdConnectIdentitityProvider.WELL_KNOWN_PROPERTY); + } else { + System.setProperty(OpenIdConnectIdentitityProvider.WELL_KNOWN_PROPERTY, previousWellKnown); + } + } + } + @Test void buildSchemeUsesWellKnownDiscoveryUrlWithoutHttpAuthScheme() { SecurityScheme scheme = OpenIDConfig.buildScheme( diff --git a/docker-compose.yml b/docker-compose.yml index 663310b950..dd79581ef0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -79,6 +79,7 @@ services: - ./compose_files/api_entry.sh:/api_entry.sh:ro - ./compose_files/proxy_auth.sh:/proxy_auth.sh:ro environment: + - APP_PORT=${APP_PORT:-8081} - JAVA_OPTS=-Dproperties.file=/conf/features.properties -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 -Dorg.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true - CDA_JDBC_DRIVER=oracle.jdbc.driver.OracleDriver - CDA_JDBC_URL=jdbc:oracle:thin:@db/FREEPDB1 From c9c33b424921dc40885a7435a0b99ee61b2a31ac Mon Sep 17 00:00:00 2001 From: "Charles Graham, SWT" Date: Fri, 7 Aug 2026 10:28:30 -0500 Subject: [PATCH 2/2] Remove Authenticator follow-up changes Signed-off-by: Charles Graham, SWT --- .../java/cwms/cda/security/Authenticator.java | 28 +++-------- .../cwms/cda/security/AuthenticatorTest.java | 50 ------------------- 2 files changed, 7 insertions(+), 71 deletions(-) delete mode 100644 cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java diff --git a/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java b/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java index 92913f269a..956ccc516d 100644 --- a/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java +++ b/cwms-data-api/src/main/java/cwms/cda/security/Authenticator.java @@ -3,7 +3,6 @@ import java.security.Principal; import java.util.ArrayList; import java.util.Collections; -import java.util.Iterator; import java.util.List; import com.google.common.flogger.FluentLogger; @@ -19,22 +18,15 @@ public final class Authenticator implements Handler { public Authenticator() { var surpressed = System.getenv("cwms.dataapi.access.providers.surpress"); - final List supressedList = surpressed == null - ? Collections.emptyList() : List.of(surpressed.split(",")); + final var supressedList = surpressed == null ? List.of() : List.of(surpressed.split(",")); - loadProviders(CdaIdentityProviders.providers(), supressedList); - } - - Authenticator(Iterator availableProviders, List suppressedProviders) { - loadProviders(availableProviders, suppressedProviders); - } - - private void loadProviders(Iterator availableProviders, List suppressedProviders) { - availableProviders.forEachRemaining(provider -> { - if (!suppressedProviders.contains(provider.getName())) { + CdaIdentityProviders.providers().forEachRemaining(provider -> { + if (!supressedList.contains(provider.getName()) && provider.getScheme() != null) { providers.add(provider); } else { - logger.atInfo().log("Suppressing configured Identity Provider %s.", provider.getName()); + logger.atSevere() + .log("Unable to add Identity Provider %s. See earlier logs for specific error message.", + provider.getName()); } }); } @@ -51,12 +43,6 @@ public void handle(Context ctx) throws Exception { } public List getActiveProviders() { - ArrayList activeProviders = new ArrayList<>(); - for (IdentityProvider provider: providers) { - if (provider.getScheme() != null) { - activeProviders.add(provider); - } - } - return Collections.unmodifiableList(activeProviders); + return Collections.unmodifiableList(providers); } } diff --git a/cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java b/cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java deleted file mode 100644 index 2266521f2e..0000000000 --- a/cwms-data-api/src/test/java/cwms/cda/security/AuthenticatorTest.java +++ /dev/null @@ -1,50 +0,0 @@ -package cwms.cda.security; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.security.Principal; -import java.util.List; -import java.util.concurrent.atomic.AtomicReference; - -import org.junit.jupiter.api.Test; - -import cwms.cda.spi.IdentityProvider; -import io.javalin.http.Context; -import io.swagger.v3.oas.models.security.SecurityScheme; - -class AuthenticatorTest { - - @Test - void providerBecomesActiveAfterSchemeIsAvailable() { - AtomicReference scheme = new AtomicReference<>(); - IdentityProvider provider = new IdentityProvider() { - @Override - public String getName() { - return "DelayedProvider"; - } - - @Override - public boolean canAuth(Context ctx) { - return false; - } - - @Override - public Principal authenticate(Context ctx) { - return null; - } - - @Override - public SecurityScheme getScheme() { - return scheme.get(); - } - }; - Authenticator authenticator = new Authenticator(List.of(provider).iterator(), List.of()); - - assertTrue(authenticator.getActiveProviders().isEmpty()); - - scheme.set(new SecurityScheme()); - - assertEquals(List.of(provider), authenticator.getActiveProviders()); - } -}