diff --git a/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/jwt/PortReservation.java b/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/jwt/PortReservation.java index 7df6b34..664c72e 100644 --- a/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/jwt/PortReservation.java +++ b/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/jwt/PortReservation.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.net.InetAddress; import java.net.ServerSocket; +import java.security.SecureRandom; import javax.net.ServerSocketFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,12 +17,17 @@ public class PortReservation { private static final int PORT_RANGE_START = 10000; private static final int PORT_RANGE_END = PORT_RANGE_MAX; + // seeded independently per JVM, so parallel test forks do not probe the same port sequence + private static final SecureRandom RANDOM = new SecureRandom(); + private final int portRangeStart; private final int portRangeEnd; private final String propertyName; private volatile int port = -1; private ServerSocket serverSocket; + // true when this instance picked the port itself; false when it was pinned via the property + private boolean portChosenByScan = false; public PortReservation(String portNames) { this(PORT_RANGE_START, PORT_RANGE_END, portNames); @@ -53,29 +59,62 @@ public synchronized boolean start() { throw new IllegalArgumentException( "Port range end must not be larger than " + PORT_RANGE_MAX + "."); } + if (serverSocket != null) { + return true; // already holding a reservation + } // check if the property already exists, if so it must be free String property = System.getProperty(propertyName); if (property != null) { - if (reserve(Integer.parseInt(property), true)) { + int preconfiguredPort = Integer.parseInt(property); + // distinguish a port this instance picked earlier from one pinned externally + boolean ownPort = portChosenByScan && preconfiguredPort == this.port; + if (reserve(preconfiguredPort, true)) { + portChosenByScan = ownPort; log.warn("Reserved previously configured port " + property); return true; - } else { + } + if (!ownPort) { throw new IllegalArgumentException("Preconfigured port " + property + " is not free"); } + // the port this reservation picked earlier has been taken by another process; + // abandon it and scan for a new one + System.clearProperty(propertyName); + this.port = -1; } - // systematically try ports in range - // starting at 'random' offset - int portRange = portRangeEnd - portRangeStart + 1; + scan(); + return true; + } - int offset = - (propertyName.hashCode() + (int) System.currentTimeMillis()) - % portRange; // more or less random per port name + /** + * Abandon the current reservation and reserve a fresh port from the range. Intended for recovery + * when another process binds the port between {@link #stop()} and the consumer's own bind + * attempt. + * + * @throws IllegalStateException if the port was preconfigured externally and therefore cannot be + * changed + */ + public synchronized void rescan() { + if (!portChosenByScan && System.getProperty(propertyName) != null) { + throw new IllegalStateException( + "Preconfigured port " + System.getProperty(propertyName) + " cannot be rescanned"); + } + stop(); + System.clearProperty(propertyName); + this.port = -1; + scan(); + } + + private void scan() { + // systematically try ports in range, starting at a random offset + int portRange = portRangeEnd - portRangeStart + 1; + int offset = RANDOM.nextInt(portRange); for (int i = 0; i < portRange; i++) { - int candidatePort = portRangeStart + (offset + portRange) % portRange; + int candidatePort = portRangeStart + (offset + i) % portRange; if (reserve(candidatePort, false)) { + portChosenByScan = true; log.warn("Reserved newly configured port " + candidatePort); - return true; + return; } } throw new IllegalArgumentException("Unable to reserve free port"); @@ -95,35 +134,34 @@ public synchronized void stop() { } private boolean reserve(int candidatePort, boolean retry) { - // Retry on failure, 10 times. - for (int i = 0; i < 10; i++) { + // While scanning, an occupied port is skipped immediately so the next candidate can be + // tried; only a specific (preconfigured or previously reserved) port is worth waiting for. + int attempts = retry ? 10 : 1; + for (int i = 0; i < attempts; i++) { if (i > 0) { log.debug("Waiting 1 second before try reserve port {}.", candidatePort); try { - wait(1000); // Wait 1 second + // Object.wait releases the monitor while sleeping, so other threads are not + // blocked for the full retry duration; a stray notify only shortens the wait + wait(1000); } catch (InterruptedException e) { Thread.currentThread().interrupt(); break; } } - try { - ServerSocket result = capturePort(candidatePort); - if (result != null) { - reserved(candidatePort, result); - return true; - } - } catch (Exception e) { - if (!retry) { - return false; - } + ServerSocket result = capturePort(candidatePort); + if (result != null) { + reserved(candidatePort, result); + return true; } } return false; } private void reserved(int port, ServerSocket serverSocket) { + stop(); // release any socket a concurrent caller reserved while this thread waited this.port = port; this.serverSocket = serverSocket; diff --git a/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactory.java b/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactory.java index 867beab..adb29ac 100644 --- a/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactory.java +++ b/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactory.java @@ -1,6 +1,7 @@ package org.entur.auth.junit.tenant; import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.common.FatalStartupException; import java.lang.annotation.Annotation; import java.time.Instant; import java.time.ZonedDateTime; @@ -42,6 +43,8 @@ */ @Slf4j public class TenantAnnotationTokenFactory implements AutoCloseable { + private static final int MAX_BIND_ATTEMPTS = 5; + private final Provider provider; private final PortReservation portReservation; private JwtTokenFactory jwtTokenFactory; @@ -60,8 +63,44 @@ public TenantAnnotationTokenFactory( this.portReservation = portReservation; /* Ensure the WireMock server is running, reserving the port if needed. */ - portReservation.stop(); - setServer(new WireMockAuthenticationServer(portReservation.getPort())); + setServer(startServer(portReservation)); + } + + /** + * Start a WireMock server on the reserved port. The reservation socket is released immediately + * before binding; if another process grabs the port in that window, a fresh port is reserved and + * the bind is retried instead of failing the whole test run. + */ + private static WireMockAuthenticationServer startServer(final PortReservation portReservation) { + if (portReservation.getPort() < 0) { + portReservation.start(); + } + + FatalStartupException lastFailure = null; + for (int attempt = 1; ; attempt++) { + if (attempt > MAX_BIND_ATTEMPTS) { + throw lastFailure; + } + if (lastFailure != null) { + // the previous bind failed; pick a fresh port before retrying + try { + portReservation.rescan(); + } catch (IllegalStateException pinnedPort) { + pinnedPort.initCause(lastFailure); + throw pinnedPort; + } + } + portReservation.stop(); + try { + return new WireMockAuthenticationServer(portReservation.getPort()); + } catch (FatalStartupException e) { + lastFailure = e; + log.warn( + "Failed to bind mock server to reserved port {}, rescanning for a new port", + portReservation.getPort(), + e); + } + } } /** diff --git a/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantJsonWebToken.java b/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantJsonWebToken.java index 638ee52..661b4b5 100644 --- a/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantJsonWebToken.java +++ b/oidc-rs-junit-tenant/src/main/java/org/entur/auth/junit/tenant/TenantJsonWebToken.java @@ -65,9 +65,17 @@ public class TenantJsonWebToken implements ParameterResolver, BeforeAllCallback /** Manages reservation of the network port for the WireMock authentication server. */ private static PortReservation portReservation; - /** Construct a new TenantJsonWebToken and initializes the reserved port */ + /** + * Construct a new TenantJsonWebToken, reserving the port and initializing the token factory and + * WireMock server. + * + *
This happens during extension construction (before any lifecycle callback) so the final port + * is published to the {@link #MOCKAUTHSERVER_PORT_NAME} system property before {@code + * SpringExtension} loads the context. This keeps the Spring context wired to the same port the + * mock server listens on, even when the port has to be retried because the original was stolen. + */ public TenantJsonWebToken() { - setupPortReservation(); + setupTokenFactory(); } /** diff --git a/oidc-rs-junit-tenant/src/test/java/org/entur/auth/junit/jwt/PortReservationTest.java b/oidc-rs-junit-tenant/src/test/java/org/entur/auth/junit/jwt/PortReservationTest.java new file mode 100644 index 0000000..fdc1cee --- /dev/null +++ b/oidc-rs-junit-tenant/src/test/java/org/entur/auth/junit/jwt/PortReservationTest.java @@ -0,0 +1,135 @@ +package org.entur.auth.junit.jwt; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.ServerSocket; +import org.junit.jupiter.api.Test; + +class PortReservationTest { + + @Test + void scanSkipsOccupiedPortsAndAdvancesToNextCandidate() throws IOException { + String propertyName = "PortReservationTest.scan"; + int base = findConsecutiveFreePorts(3); + try (ServerSocket occupiedFirst = bind(base); + ServerSocket occupiedSecond = bind(base + 1)) { + PortReservation reservation = new PortReservation(base, base + 2, propertyName); + try { + assertTrue(reservation.start()); + assertEquals(base + 2, reservation.getPort()); + assertEquals(Integer.toString(base + 2), System.getProperty(propertyName)); + } finally { + reservation.stop(); + System.clearProperty(propertyName); + } + } + } + + @Test + void rescanReservesNewPortWhenPreviousPortIsStolen() throws IOException { + String propertyName = "PortReservationTest.rescan"; + PortReservation reservation = new PortReservation(propertyName); + try { + reservation.start(); + int stolenPort = reservation.getPort(); + + reservation.stop(); + try (ServerSocket thief = bind(stolenPort)) { + reservation.rescan(); + + assertTrue(reservation.getPort() > 0); + assertNotEquals(stolenPort, reservation.getPort()); + assertEquals(Integer.toString(reservation.getPort()), System.getProperty(propertyName)); + } + } finally { + reservation.stop(); + System.clearProperty(propertyName); + } + } + + /** + * Simulates another process stealing the reserved port in the window after {@link + * PortReservation#stop()}. The stale port stays pinned until recovery is requested, but + * re-running {@link PortReservation#start()} (as {@code TenantAnnotationTokenFactory#close()} + * does) must abandon the stolen port and reserve a fresh one instead of failing. + * + *
Reproduces the upstream oidc-lib bug where start() instead throws {@code + * IllegalArgumentException: Preconfigured port ... is not free}, pinning the dead port forever. + * + *
Slow by design: start() retries the previously reserved port for ~10 seconds before giving
+ * it up, in case the thief is short-lived.
+ */
+ @Test
+ void restartReservesNewPortWhenPreviousPortIsStolen() throws IOException {
+ String propertyName = "PortReservationTest.restart";
+ PortReservation reservation = new PortReservation(propertyName);
+ try {
+ reservation.start();
+ int stolenPort = reservation.getPort();
+
+ reservation.stop();
+ try (ServerSocket thief = bind(stolenPort)) {
+ // the stale reservation still reports the stolen port...
+ assertEquals(stolenPort, reservation.getPort());
+
+ // ...but restarting recovers by abandoning it and scanning for a new one
+ assertTrue(reservation.start());
+ assertTrue(reservation.getPort() > 0);
+ assertNotEquals(stolenPort, reservation.getPort());
+ assertEquals(Integer.toString(reservation.getPort()), System.getProperty(propertyName));
+ }
+ } finally {
+ reservation.stop();
+ System.clearProperty(propertyName);
+ }
+ }
+
+ @Test
+ void externallyPreconfiguredPortIsReusedAndCannotBeRescanned() throws IOException {
+ String propertyName = "PortReservationTest.preconfigured";
+ int freePort;
+ try (ServerSocket probe = bind(0)) {
+ freePort = probe.getLocalPort();
+ }
+ System.setProperty(propertyName, Integer.toString(freePort));
+ PortReservation reservation = new PortReservation(propertyName);
+ try {
+ assertTrue(reservation.start());
+ assertEquals(freePort, reservation.getPort());
+ assertThrows(IllegalStateException.class, reservation::rescan);
+ } finally {
+ reservation.stop();
+ System.clearProperty(propertyName);
+ }
+ }
+
+ private static ServerSocket bind(int port) throws IOException {
+ return new ServerSocket(port, 1, InetAddress.getByName("localhost"));
+ }
+
+ private static int findConsecutiveFreePorts(int count) throws IOException {
+ for (int base = 24000; base < 64000; base += count) {
+ ServerSocket[] sockets = new ServerSocket[count];
+ try {
+ for (int i = 0; i < count; i++) {
+ sockets[i] = bind(base + i);
+ }
+ return base;
+ } catch (IOException e) {
+ // try the next block of ports
+ } finally {
+ for (ServerSocket socket : sockets) {
+ if (socket != null) {
+ socket.close();
+ }
+ }
+ }
+ }
+ throw new IOException("Unable to find " + count + " consecutive free ports");
+ }
+}
diff --git a/oidc-rs-junit-tenant/src/test/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactoryPortRaceTest.java b/oidc-rs-junit-tenant/src/test/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactoryPortRaceTest.java
new file mode 100644
index 0000000..51305cb
--- /dev/null
+++ b/oidc-rs-junit-tenant/src/test/java/org/entur/auth/junit/tenant/TenantAnnotationTokenFactoryPortRaceTest.java
@@ -0,0 +1,45 @@
+package org.entur.auth.junit.tenant;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.IOException;
+import java.net.ServerSocket;
+import org.entur.auth.junit.jwt.EnturProvider;
+import org.entur.auth.junit.jwt.PortReservation;
+import org.junit.jupiter.api.Test;
+
+class TenantAnnotationTokenFactoryPortRaceTest {
+
+ /**
+ * Simulates a parallel test fork stealing the reserved port in the window between the reservation
+ * socket being released and WireMock binding it. The factory must recover by reserving a fresh
+ * port instead of failing the whole run.
+ */
+ @Test
+ void recoversWhenReservedPortIsStolenBeforeWireMockBinds() throws IOException {
+ String propertyName = "TenantAnnotationTokenFactoryPortRaceTest.port";
+ PortReservation reservation = new PortReservation(propertyName);
+ try {
+ reservation.start();
+ int stolenPort = reservation.getPort();
+
+ reservation.stop();
+ // bind the wildcard address, like a WireMock server in a competing fork would
+ try (ServerSocket thief = new ServerSocket(stolenPort)) {
+ try (TenantAnnotationTokenFactory factory =
+ new TenantAnnotationTokenFactory(new EnturProvider(), reservation)) {
+ assertNotNull(factory.getServer());
+ assertNotEquals(stolenPort, factory.getServer().getPort());
+ assertEquals(reservation.getPort(), factory.getServer().getPort());
+ assertEquals(
+ Integer.toString(factory.getServer().getPort()), System.getProperty(propertyName));
+ }
+ }
+ } finally {
+ reservation.stop();
+ System.clearProperty(propertyName);
+ }
+ }
+}
diff --git a/oidc-rs-spring-boot-common/src/test/java/org/entur/auth/spring/common/server/DefaultAuthProvidersTest.java b/oidc-rs-spring-boot-common/src/test/java/org/entur/auth/spring/common/server/DefaultAuthProvidersTest.java
new file mode 100644
index 0000000..23364a7
--- /dev/null
+++ b/oidc-rs-spring-boot-common/src/test/java/org/entur/auth/spring/common/server/DefaultAuthProvidersTest.java
@@ -0,0 +1,72 @@
+package org.entur.auth.spring.common.server;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.List;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Locks the channel used to communicate the mock server port to Spring: the {@code
+ * MOCKAUTHSERVER_PORT} system property published by the JUnit extension is resolved into the mock
+ * issuer certificate (JWKS) URLs that the resource server uses to validate tokens.
+ */
+class DefaultAuthProvidersTest {
+
+ private static final String PORT_PROPERTY = "MOCKAUTHSERVER_PORT";
+ private static final List