diff --git a/src/main/java/net/openid/conformance/condition/client/WaitFor5Seconds.java b/src/main/java/net/openid/conformance/condition/client/WaitFor5Seconds.java new file mode 100644 index 0000000000..3955bafe07 --- /dev/null +++ b/src/main/java/net/openid/conformance/condition/client/WaitFor5Seconds.java @@ -0,0 +1,12 @@ +package net.openid.conformance.condition.client; + +import net.openid.conformance.condition.common.AbstractWaitForSpecifiedSeconds; +import net.openid.conformance.testmodule.Environment; + +public class WaitFor5Seconds extends AbstractWaitForSpecifiedSeconds { + + @Override + protected long getExpectedWaitSeconds(Environment env) { + return 5; + } +} diff --git a/src/main/java/net/openid/conformance/fapiciba/FAPICIBAID1AuthReqIdExpired.java b/src/main/java/net/openid/conformance/fapiciba/FAPICIBAID1AuthReqIdExpired.java index 90082c34d5..6fa9ff69aa 100644 --- a/src/main/java/net/openid/conformance/fapiciba/FAPICIBAID1AuthReqIdExpired.java +++ b/src/main/java/net/openid/conformance/fapiciba/FAPICIBAID1AuthReqIdExpired.java @@ -5,6 +5,7 @@ import net.openid.conformance.condition.client.CheckTokenEndpointHttpStatusNot200; import net.openid.conformance.condition.client.SleepUntilAuthReqExpires; import net.openid.conformance.condition.client.TellUserToIgnoreCIBAAuthentication; +import net.openid.conformance.condition.client.WaitFor5Seconds; import net.openid.conformance.testmodule.PublishTestModule; import net.openid.conformance.variant.CIBAMode; @@ -50,11 +51,12 @@ protected void waitForAuthenticationToComplete(long delaySeconds) { callAndStopOnFailure(TellUserToIgnoreCIBAAuthentication.class); setStatus(Status.WAITING); + callAndStopOnFailure(SleepUntilAuthReqExpires.class); if (testType == CIBAMode.PING) { - // test resumes when notification endpoint called - return; + // a ping notification may or may not be issued; allow an extra 5 seconds to make sure any ping arrives + // before we continue + callAndStopOnFailure(WaitFor5Seconds.class); } - callAndStopOnFailure(SleepUntilAuthReqExpires.class); setStatus(Status.RUNNING); callTokenEndpointAndFinishTest(); @@ -64,7 +66,8 @@ protected void waitForAuthenticationToComplete(long delaySeconds) { protected void processNotificationCallback(JsonObject requestParts) { if (testType == CIBAMode.PING) { verifyNotificationCallback(requestParts); - callTokenEndpointAndFinishTest(); + setStatus(Status.WAITING); + // test continues when the above sleep/wait completes } else { super.processNotificationCallback(requestParts); } diff --git a/src/main/java/net/openid/conformance/testmodule/AbstractTestModule.java b/src/main/java/net/openid/conformance/testmodule/AbstractTestModule.java index 075a8e439b..055bdf0a74 100644 --- a/src/main/java/net/openid/conformance/testmodule/AbstractTestModule.java +++ b/src/main/java/net/openid/conformance/testmodule/AbstractTestModule.java @@ -8,6 +8,7 @@ import net.openid.conformance.condition.Condition; import net.openid.conformance.condition.ConditionError; import net.openid.conformance.condition.client.SleepUntilAuthReqExpires; +import net.openid.conformance.condition.client.WaitFor5Seconds; import net.openid.conformance.frontChannel.BrowserControl; import net.openid.conformance.info.ImageService; import net.openid.conformance.info.TestInfoService; @@ -248,7 +249,12 @@ protected void call(ConditionCallBuilder builder) { // We skip these checks for a condition that we deliberately call without the lock held so as not to block // other threads; I suspect it means that this condition should have the ability to call setStatus or that // their functionality should be in a method in AbstractTestModule instead - if (builder.getConditionClass() != SleepUntilAuthReqExpires.class) { + // Not all the 'WaitFor' functions are listed here; that means some of them we are calling with the lock held + // which may be problematic (e.g. it prevents any incoming connections being process and I suspect may prevent + // the test being aborted until the sleep expires). It would probably be preferable to always release the + // lock whilst sleeping (which is probably best achieve by one of the ways outlined in the previous paragraph.) + if (builder.getConditionClass() != SleepUntilAuthReqExpires.class && + builder.getConditionClass() != WaitFor5Seconds.class) { if (getStatus() != Status.CREATED) { // We don't run this check for 'CREATED' as the lock is currently not held during 'configure'; see // https://gitlab.com/openid/conformance-suite/issues/688