Merge branch 'brazil-fixes' into 'master'

Various small fixes based on feedback from Brazil

Closes #984

See merge request openid/conformance-suite!1073

src/main/java/net/openid/conformance/condition/as/AddSecondAudValueToIdToken.java → src/main/java/net/openid/conformance/condition/as/AddUntrustedSecondAudValueToIdToken.java

@@ -7,7 +7,7 @@
 import net.openid.conformance.condition.PreEnvironment;
 import net.openid.conformance.testmodule.Environment;
 
-public class AddSecondAudValueToIdToken extends AbstractCondition {
+public class AddUntrustedSecondAudValueToIdToken extends AbstractCondition {
 
 	@Override
 	@PreEnvironment(required = "id_token_claims")

src/main/java/net/openid/conformance/condition/as/ExtractClientCertificateFromTokenEndpointRequestHeaders.java

@@ -30,7 +30,12 @@ public Environment evaluate(Environment env) {
 
 		String certStr = env.getString("token_endpoint_request", "headers.x-ssl-cert");
 		if (certStr == null) {
-			throw error("Client certificate not found");
+			throw error("Client certificate not found; likely the non-mtls version of the endpoint was called");
+		}
+		if (certStr.equals("(null)")) {
+			// "(null)" is particular behaviour of apache's request header, as used in our ingress via:
+			// "RequestHeader set X-Ssl-Cert "%{SSL_CLIENT_CERT}s"
+			throw error("Client certificate not found; the client did not supply a MTLS certification to the endpoint. In some cases this may be because the client is, incorrectly, configured to supply a TLS certificate only if the server explicitly requires a certificate at the TLS level.");
 		}
 
 		try {

src/main/java/net/openid/conformance/condition/client/AbstractJsonUriIsValidAndHttps.java

@@ -10,9 +10,9 @@
 
 public abstract class AbstractJsonUriIsValidAndHttps extends AbstractCondition {
 
-	private static final String requiredProtocol = "https";
+	protected static final String requiredProtocol = "https";
 	private static final String errorMessageNotJsonPrimitive = "Specified value is not a Json primitive";
-	private static final String errorMessageInvalidURL = "Invalid URL. Unable to parse.";
+	protected static final String errorMessageInvalidURL = "Invalid URL. Unable to parse.";
 
 	private JsonElement ServerValue = null;
 	private URL extractedUrl = null;

src/main/java/net/openid/conformance/condition/client/CallPAREndpoint.java

@@ -66,9 +66,21 @@ public Environment evaluate(Environment env) {
 				HttpMethod.POST : HttpMethod.valueOf(env.getString(HTTP_METHOD_KEY));
 
 			try {
-				final String parEndpointUri = env.getString("pushed_authorization_request_endpoint") != null ?
-					env.getString("pushed_authorization_request_endpoint") :
-					env.getString("server", "pushed_authorization_request_endpoint");
+				String parEndpointUri = null;
+				if (env.containsObject("mutual_tls_authentication")) {
+					// the MTLS aliased endpoint if we have MTLS authentication available.
+					// This is to cater for private_key_jwt (where we should not use the alias) and mtls client auth
+					// (where we should); it assumes the caller only supplies mutual_tls_authentication for the calls
+					// it is required for.
+					// I think here we could just call env.getString("server", "mtls_endpoint_aliases.pushed_authorization_request_endpoint");
+					// but https://gitlab.com/openid/conformance-suite/-/issues/914 is open to reconsider the overall
+					// mechanism.
+					parEndpointUri = env.getString("pushed_authorization_request_endpoint");
+				}
+
+				if (parEndpointUri == null) {
+					parEndpointUri = env.getString("server", "pushed_authorization_request_endpoint");
+				}
 				if (Strings.isNullOrEmpty(parEndpointUri)) {
 					throw error("Couldn't find pushed_authorization_request_endpoint in server discovery document. This endpoint is required as you have selected to test pushed authorization requests.");
 				}

src/main/java/net/openid/conformance/condition/client/ExtractClientManagementCredentials.java

@@ -1,15 +1,15 @@
 package net.openid.conformance.condition.client;
 
 import com.google.common.base.Strings;
-import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
-import net.openid.conformance.condition.AbstractCondition;
-import net.openid.conformance.condition.PostEnvironment;
 import net.openid.conformance.condition.PreEnvironment;
 import net.openid.conformance.testmodule.Environment;
 import net.openid.conformance.testmodule.OIDFJSON;
 
-public class ExtractClientManagementCredentials extends AbstractCondition {
+import java.net.MalformedURLException;
+import java.net.URL;
+
+public class ExtractClientManagementCredentials extends AbstractJsonUriIsValidAndHttps {
 
 	@Override
 	@PreEnvironment(required = "client")
@@ -37,6 +37,18 @@ public Environment evaluate(Environment env) {
 		if (Strings.isNullOrEmpty(registrationClientUri)) {
 			throw error("registration_client_uri must not be an empty string");
 		}
+
+		URL url;
+		try {
+			url = new URL(registrationClientUri);
+		} catch (MalformedURLException invalidURL) {
+			throw error(errorMessageInvalidURL);
+		}
+		if (!url.getProtocol().equals(requiredProtocol)) {
+			throw error("URL for client management point does not use "+requiredProtocol+" scheme",
+				args("required", requiredProtocol, "actual", registrationClientUri));
+		}
+
 		if (Strings.isNullOrEmpty(registrationAccessToken)) {
 			throw error("registration_access_token must not be an empty string");
 		}

src/main/java/net/openid/conformance/fapi1advancedfinal/AbstractFAPI1AdvancedFinalClientTest.java

@@ -266,6 +266,8 @@ public void configure(JsonObject config, String baseUrl, String externalUrlOverr
 			// Certificate-Bound Access Tokens the token_endpoint, registration_endpoint and userinfo_endpoint;
 			callAndStopOnFailure(FAPIBrazilGenerateServerConfiguration.class);
 		} else {
+			// We should really create the 'Brazil' configuration that contains mtls_endpoint_aliases in at least some
+			// cases - it's mandatory for clients to support it as per https://datatracker.ietf.org/doc/html/rfc8705#section-5
 			callAndStopOnFailure(GenerateServerConfigurationMTLS.class);
 		}
 
@@ -429,6 +431,13 @@ protected Object handleClientRequestForPath(String requestId, String path){
 		} else if (path.equals(".well-known/openid-configuration")) {
 			return discoveryEndpoint();
 		} else if (path.equals("par") && authRequestMethod == FAPIAuthRequestMethod.PUSHED) {
+			if(profile == FAPI1FinalOPProfile.OPENBANKING_BRAZIL) {
+				throw new TestFailureException(getId(), "In Brazil, the PAR endpoint must be called over an mTLS " +
+					"secured connection using the pushed_authorization_request_endpoint found in mtls_endpoint_aliases.");
+			}
+			if (clientAuthType == ClientAuthType.MTLS) {
+				throw new TestFailureException(getId(), "The PAR endpoint must be called over an mTLS secured connection.");
+			}
 			return parEndpoint(requestId);
 		} else if (path.equals(ACCOUNT_REQUESTS_PATH) && profile == FAPI1FinalOPProfile.OPENBANKING_UK) {
 			return accountRequestsEndpoint(requestId);
@@ -677,13 +686,19 @@ protected Object discoveryEndpoint() {
 
 	protected void checkMtlsCertificate() {
 		callAndContinueOnFailure(ExtractClientCertificateFromTokenEndpointRequestHeaders.class, ConditionResult.FAILURE);
-		callAndContinueOnFailure(CheckForClientCertificate.class, ConditionResult.FAILURE, "FAPI1-ADV-5.2.2-5");
+		callAndStopOnFailure(CheckForClientCertificate.class, ConditionResult.FAILURE, "FAPI1-ADV-5.2.2-5");
 		callAndContinueOnFailure(EnsureClientCertificateMatches.class, ConditionResult.FAILURE);
 	}
 	protected void authenticateParEndpointRequest(String requestId) {
 		call(exec().mapKey("token_endpoint_request", requestId));
 
-		checkMtlsCertificate();
+		if(clientAuthType == ClientAuthType.MTLS || profile == FAPI1FinalOPProfile.OPENBANKING_BRAZIL) {
+			// there is no requirement to present an MTLS certificate at the PAR endpoint when using private_key_jwt.
+			// (This differs to the token endpoint, where an MTLS certificate must always be presented, as one is
+			// required to bind the issued access token to.)
+			// The exception is Brazil, where a TLS client certificate must be presented to all endpoints in all cases.
+			checkMtlsCertificate();
+		}
 
 		if(clientAuthType == ClientAuthType.PRIVATE_KEY_JWT) {
 			call(new ValidateClientAuthenticationWithPrivateKeyJWT().

src/main/java/net/openid/conformance/fapi1advancedfinal/AbstractFAPI1AdvancedFinalMultipleClient.java

@@ -5,7 +5,12 @@
 import net.openid.conformance.condition.client.CallProtectedResourceWithBearerTokenExpectingError;
 import net.openid.conformance.condition.client.CreateRedirectUri;
 import net.openid.conformance.condition.client.RedirectQueryTestDisabled;
+import net.openid.conformance.variant.FAPI1FinalOPProfile;
+import net.openid.conformance.variant.VariantConfigurationFields;
 
+@VariantConfigurationFields(parameter = FAPI1FinalOPProfile.class, value = "openbanking_brazil", configurationFields = {
+	"client2.org_jwks"
+})
 public abstract class AbstractFAPI1AdvancedFinalMultipleClient extends AbstractFAPI1AdvancedFinalServerTestModule {
 
 	@Override

src/main/java/net/openid/conformance/fapi1advancedfinal/AbstractFAPI1AdvancedFinalServerTestModule.java

@@ -61,7 +61,6 @@
 })
 @VariantConfigurationFields(parameter = FAPI1FinalOPProfile.class, value = "openbanking_brazil", configurationFields = {
 	"client.org_jwks",
-	"client2.org_jwks",
 	"resource.consentUrl",
 	"resource.brazilCpf",
 	"resource.brazilCnpj",
@@ -858,8 +857,23 @@ protected void performPARRedirectWithRequestUri() {
 
 	protected void performParAuthorizationRequestFlow() {
 
+		// we only need to (and only should) supply an MTLS authentication when using MTLS client auth;
+		// there's no need to pass mtls auth when using private_key_jwt (except in some of the banking
+		// profiles that explicitly require TLS client certs for all endpoints).
+		boolean mtlsRequired = getVariant(ClientAuthType.class) == ClientAuthType.MTLS ||
+			getVariant(FAPI1FinalOPProfile.class) != FAPI1FinalOPProfile.PLAIN_FAPI;
+		JsonObject mtls = null;
+		if (!mtlsRequired) {
+			mtls = env.getObject("mutual_tls_authentication");
+			env.removeObject("mutual_tls_authentication");
+		}
+
 		callAndStopOnFailure(CallPAREndpoint.class, "PAR-2.1");
 
+		if (!mtlsRequired) {
+			env.putObject("mutual_tls_authentication", mtls);
+		}
+
 		processParResponse();
 	}
 

src/main/java/net/openid/conformance/fapi1advancedfinal/FAPI1AdvancedFinalClientTestInvalidSecondaryAud.java

@@ -1,13 +1,12 @@
 package net.openid.conformance.fapi1advancedfinal;
 
-import net.openid.conformance.condition.as.AddSecondAudValueToIdToken;
+import net.openid.conformance.condition.as.AddUntrustedSecondAudValueToIdToken;
 import net.openid.conformance.testmodule.PublishTestModule;
-import net.openid.conformance.testmodule.TestFailureException;
 
 @PublishTestModule(
 	testName = "fapi1-advanced-final-client-test-invalid-secondary-aud",
-	displayName = "FAPI1-Advanced-Final: client test - multiple aud values in id_token from authorization_endpoint, should be rejected",
-	summary = "This test should end with the client displaying an error message that there are multiple aud values in the id_token from the authorization_endpoint, and this behaviour is not expected",
+	displayName = "FAPI1-Advanced-Final: client test - untrusted aud value in id_token from authorization_endpoint, must be rejected",
+	summary = "This test issues an id_token where the 'aud' is an array which contains both the correct client_id and the id for a non-existent client. This test should end with the client displaying an error message that there is an untrusted aud value in the id_token from the authorization_endpoint, or that the 'azp' claim is missing.\n\nAs per OpenID Connect section 3.1.3.7 clause 3:\n\n'The ID Token MUST be rejected if the ID Token does not list the Client as a valid audience, or if it contains additional audiences not trusted by the Client.'\n\n and clause 4:\n\n'If the ID Token contains multiple audiences, the Client SHOULD verify that an azp Claim is present.'",
 	profile = "FAPI1-Advanced-Final",
 	configurationFields = {
 		"server.jwks",
@@ -25,11 +24,11 @@ public class FAPI1AdvancedFinalClientTestInvalidSecondaryAud extends AbstractFAP
 	@Override
 	protected void addCustomValuesToIdToken() {
 
-		callAndStopOnFailure(AddSecondAudValueToIdToken.class, "OIDCC-3.1.3.7-8");
+		callAndStopOnFailure(AddUntrustedSecondAudValueToIdToken.class, "OIDCC-3.1.3.7-3");
 	}
 
 	@Override
 	protected String getIdTokenFaultErrorMessage() {
-		return "multiple aud values";
+		return "aud is an array that contains an untrusted value";
 	}
 }

src/main/java/net/openid/conformance/fapi1advancedfinal/FAPI1AdvancedFinalUserRejectsAuthentication.java

@@ -14,7 +14,7 @@
 @PublishTestModule(
 	testName = "fapi1-advanced-final-user-rejects-authentication",
 	displayName = "FAPI1-Advanced-Final: user rejects authentication",
-	summary = "This test requires the user to reject the authentication or consent, for example by pressing the 'cancel' button on the login screen. It verifies that the user is redirected back to the relying party's redirect_uri with an 'access_denied' error. (You may need to clear any cookies for the authorization server before running this test, to remove any existing login session and hence ensure the user is offered the opportunity to reject the authentication.)",
+	summary = "This test requires the user to reject the authentication or consent, for example by pressing the 'cancel' button on the login screen. It verifies that the user is redirected back to the relying party's redirect_uri with an 'access_denied' error.\n\nYou may need to clear any cookies for the authorization server before running this test, to remove any existing login session and hence ensure the user is offered the opportunity to reject the authentication.",
 	profile = "FAPI1-Advanced-Final",
 	configurationFields = {
 		"server.discoveryUrl",

src/main/java/net/openid/conformance/fapirwid2/FAPIRWID2ClientTestInvalidSecondaryAud.java

@@ -1,13 +1,13 @@
 package net.openid.conformance.fapirwid2;
 
-import net.openid.conformance.condition.as.AddSecondAudValueToIdToken;
+import net.openid.conformance.condition.as.AddUntrustedSecondAudValueToIdToken;
 import net.openid.conformance.testmodule.PublishTestModule;
 import net.openid.conformance.testmodule.TestFailureException;
 
 @PublishTestModule(
 	testName = "fapi-rw-id2-client-test-invalid-secondary-aud",
-	displayName = "FAPI-RW-ID2: client test - multiple aud values in id_token from authorization_endpoint, should be rejected",
-	summary = "This test should end with the client displaying an error message that there are multiple aud values in the id_token from the authorization_endpoint, and this behaviour is not expected",
+	displayName = "FAPI-RW-ID2: client test - untrusted aud value in id_token from authorization_endpoint, must be rejected",
+	summary = "This test issues an id_token where the 'aud' is an array which contains both the correct client_id and the id for a non-existent client. This test should end with the client displaying an error message that there is an untrusted aud value in the id_token from the authorization_endpoint, or that the 'azp' claim is missing.\n\nAs per OpenID Connect section 3.1.3.7 clause 3:\n\n'The ID Token MUST be rejected if the ID Token does not list the Client as a valid audience, or if it contains additional audiences not trusted by the Client.'\n\n and clause 4:\n\n'If the ID Token contains multiple audiences, the Client SHOULD verify that an azp Claim is present.'",
 	profile = "FAPI-RW-ID2",
 	configurationFields = {
 		"server.jwks",
@@ -24,13 +24,13 @@ public class FAPIRWID2ClientTestInvalidSecondaryAud extends AbstractFAPIRWID2Cli
 	@Override
 	protected void addCustomValuesToIdToken() {
 
-		callAndStopOnFailure(AddSecondAudValueToIdToken.class, "OIDCC-3.1.3.7-8");
+		callAndStopOnFailure(AddUntrustedSecondAudValueToIdToken.class, "OIDCC-3.1.3.7-3");
 	}
 
 	@Override
 	protected Object authorizationCodeGrantType(String requestId) {
 
-		throw new TestFailureException(getId(), "Client has incorrectly called token_endpoint after receiving an id_token with multiple aud values from the authorization_endpoint.");
+		throw new TestFailureException(getId(), "Client has incorrectly called token_endpoint after receiving (from the authorization endpoint) an id_token where aud is an array that contains an untrusted value.");
 
 	}
 

src/main/java/net/openid/conformance/fapirwid2/FAPIRWID2UserRejectsAuthentication.java

@@ -14,7 +14,7 @@
 @PublishTestModule(
 	testName = "fapi-rw-id2-user-rejects-authentication",
 	displayName = "FAPI-RW-ID2: user rejects authentication",
-	summary = "This test requires the user to reject the authentication or consent, for example by pressing the 'cancel' button on the login screen. It verifies that the user is redirected back to the relying party's redirect_uri with an 'access_denied' error. (You may need to clear any cookies for the authorization server before running this test, to remove any existing login session and hence ensure the user is offered the opportunity to reject the authentication.)",
+	summary = "This test requires the user to reject the authentication or consent, for example by pressing the 'cancel' button on the login screen. It verifies that the user is redirected back to the relying party's redirect_uri with an 'access_denied' error.\n\nYou may need to clear any cookies for the authorization server before running this test, to remove any existing login session and hence ensure the user is offered the opportunity to reject the authentication.",
 	profile = "FAPI-RW-ID2",
 	configurationFields = {
 		"server.discoveryUrl",

src/main/java/net/openid/conformance/runner/TestDispatcher.java

@@ -3,6 +3,7 @@
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
 import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParser;
 import net.openid.conformance.condition.ConditionError;
@@ -135,7 +136,7 @@ public Object handle(
 			}
 
 			Object response;
-			logIncomingHttpRequest(test, restOfPath, requestParts);
+			logIncomingHttpRequest(test, path, requestParts);
 
 			if (TestModule.Status.CREATED.equals(test.getStatus())) {
 				if (path.endsWith("/client1_jwks")) {
@@ -288,14 +289,38 @@ protected MultiValueMap<String, String> convertQueryStringParamsToMap(String que
 	}
 
 	protected void logIncomingHttpRequest(TestModule test, String path, JsonObject requestParts) {
+		// make sure the http headers our proxy adds aren't exposed to the user
+		JsonObject originalHeaders = (JsonObject) requestParts.get("headers");
+		JsonObject headers = originalHeaders.deepCopy();
+		JsonElement tlsCipher = originalHeaders.get("x-ssl-cipher");
+		JsonElement tlsVersion = originalHeaders.get("x-ssl-protocol");
+		JsonElement tlsCert = originalHeaders.get("x-ssl-cert");
+
+		// see chart/templates/ingress.yaml for the apache config that adds these
+		List<String> proxyHeaders = List.of(
+			"X-Ssl-Cipher",
+			"X-Ssl-Protocol",
+			"X-Forwarded-Proto", // no need to log this as it's only acted upon by spring/tomcat
+			"X-Forwarded-Port", // no need to log this as it's only acted upon by spring/tomcat
+			"x-forwarded-host", // automatically added by apache, but only acted upon by spring/tomcat
+			"x-forwarded-server", // automatically added by apache, but only acted upon by spring/tomcat
+			"X-Ssl-Cert",
+			"X-Ssl-Verify"); // this doesn't appear to be used so we don't log it
+		for (String h : proxyHeaders) {
+			headers.remove(h.toLowerCase());
+		}
+
 		eventLog.log(test.getId(), test.getName(), test.getOwner(), args(
 			"msg", "Incoming HTTP request to test instance " + test.getId(),
 			"http", "incoming",
 			"incoming_path", path,
 			"incoming_query_string_params", requestParts.get("query_string_params"),
 			"incoming_body_form_params", requestParts.get("body_form_params"),
 			"incoming_method", requestParts.get("method"),
-			"incoming_headers", requestParts.get("headers"),
+			"incoming_headers", headers,
+			"incoming_tls_cipher", tlsCipher,
+			"incoming_tls_version", tlsVersion,
+			"incoming_tls_cert", tlsCert,
 			"incoming_body", requestParts.get("body"),
 			"incoming_body_json", requestParts.get("body_json")));
 	}

src/main/resources/application.properties

@@ -1,5 +1,5 @@
 # Current framework version
-fintechlabs.version=4.1.37
+fintechlabs.version=4.1.38
 
 spring.data.mongodb.uri=mongodb://mongodb:27017/test_suite
 fintechlabs.base_url=https://localhost:8443

src/main/resources/static/js/fapi.ui.js

@@ -428,8 +428,8 @@ var FAPI_UI = {
 	    // Each url must have the fragment necessary to form a link to a particular section of document
 		// When you add a new value to this list, also update net.openid.conformance.export.LogEntryHelper
 		specLinks : {
-			"BrazilOB-" : "https://openbanking-brasil.github.io/specs-seguranca/open-banking-brasil-financial-api-1_ID1.html#section-",
-			"BrazilOBDCR-" : "https://openbanking-brasil.github.io/specs-seguranca/open-banking-brasil-dynamic-client-registration-1_ID1.html#section-",
+			"BrazilOB-" : "https://openbanking-brasil.github.io/specs-seguranca/open-banking-brasil-financial-api-1_ID3.html#section-",
+			"BrazilOBDCR-" : "https://openbanking-brasil.github.io/specs-seguranca/open-banking-brasil-dynamic-client-registration-1_ID2.html#section-",
 			"FAPI-R-" : "https://openid.net/specs/openid-financial-api-part-1-ID2.html#rfc.section.",
 			"FAPI-RW-" : "https://openid.net/specs/openid-financial-api-part-2-ID2.html#rfc.section.",
 			"FAPI1-BASE-" : "https://openid.net/specs/openid-financial-api-part-1-1_0-final.html#rfc.section.",