From c2a96d6b4662d4d5a275cceecc0de741fd847b25 Mon Sep 17 00:00:00 2001
From: Markus Strehle <11627201+strehle@users.noreply.github.com>
Date: Thu, 11 Jul 2024 08:35:54 +0200
Subject: [PATCH] Refactor: throw an exception if reference in uaa.yml is
 missing (#2952)

* Old: if global.jwt.entry.key is missing and reference in IdentityProviders configuration with ${global.jwt.entry.key} is missing, then default UAA key is used. This has side effect in error analysis, that it is not clear if sender or receive has an issue.
* New: if global.jwt.entry.key is missing the sender (UAA) throws the error but does not create any JWT which fails so that receiver is not involved in error handling
---
 .../oauth/jwt/JwtClientAuthentication.java    |  6 ++---
 .../jwt/JwtClientAuthenticationTest.java      | 22 +++++++------------
 2 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java
index eb9d625ed5f..1445d8e40d4 100644
--- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java
+++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthentication.java
@@ -202,12 +202,12 @@ private static Matcher getDynamicValueMatcher(String value) {
   }
 
   private static String getDynamicValue(Matcher m) {
+    ApplicationContext applicationContext = ApplicationContextProvider.getApplicationContext();
     /* return a reference from application environment only if in default zone */
-    if (!(new IdentityZoneManagerImpl().isCurrentZoneUaa())) {
+    if (applicationContext == null || !(new IdentityZoneManagerImpl().isCurrentZoneUaa())) {
       return null;
     }
-    ApplicationContext applicationContext = ApplicationContextProvider.getApplicationContext();
-    return applicationContext != null ? applicationContext.getEnvironment().getProperty(m.group("name")) : null;
+    return Optional.ofNullable(applicationContext.getEnvironment().getProperty(m.group("name"))).orElseThrow( () -> new BadCredentialsException("Missing referenced signing entry"));
   }
 
   private static String getDefaultValue(Matcher m) {
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java
index 71fc830344d..7af6e011dff 100644
--- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java
+++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/JwtClientAuthenticationTest.java
@@ -178,7 +178,7 @@ void testGetClientAssertionUnknownSingingKey() throws ParseException {
   }
 
   @Test
-  void testGetClientAssertionUsingCustomSingingKeyFromEnivoronment() throws ParseException, JOSEException {
+  void testGetClientAssertionUsingCustomSingingKeyFromEnvironment() throws ParseException, JOSEException {
     // Given: register 2 keys
     mockKeyInfoService("key-id-321", JwtHelperX5tTest.CERTIFICATE_1);
     HashMap customClaims = new HashMap<>();
@@ -201,7 +201,7 @@ void testGetClientAssertionUsingCustomSingingKeyFromEnivoronment() throws ParseE
   }
 
   @Test
-  void testGetClientAssertionUsingCustomSingingKeyFromEnivoronmentNoDefault() throws ParseException, JOSEException {
+  void testGetClientAssertionUsingCustomSingingKeyFromEnvironmentNoDefault() throws ParseException, JOSEException {
     // Given: register 2 keys
     mockKeyInfoService("key-id-321", JwtHelperX5tTest.CERTIFICATE_1);
     HashMap customClaims = new HashMap<>();
@@ -222,7 +222,7 @@ void testGetClientAssertionUsingCustomSingingKeyFromEnivoronmentNoDefault() thro
   }
 
   @Test
-  void testGetClientAssertionUsingCustomSingingKeyFromEnivoronmentUseDefault() throws ParseException, JOSEException {
+  void testGetClientAssertionUsingCustomSingingKeyFromEnvironmentButEntryIsMissing() throws ParseException, JOSEException {
     // Given: register 2 keys
     mockKeyInfoService("key-id-321", JwtHelperX5tTest.CERTIFICATE_1);
     HashMap customClaims = new HashMap<>();
@@ -231,20 +231,14 @@ void testGetClientAssertionUsingCustomSingingKeyFromEnivoronmentUseDefault() thr
     config.setJwtClientAuthentication(customClaims);
     // empty application context
     mockApplicationContext(Map.of());
-    // When
-    MultiValueMap<String, String> params = jwtClientAuthentication.getClientAuthenticationParameters(new LinkedMultiValueMap<>(), config);
     // Then
-    assertTrue(params.containsKey("client_assertion"));
-    assertTrue(params.containsKey("client_assertion_type"));
-    String clientAssertion = (String) params.get("client_assertion").get(0);
-    validateClientAssertionOidcComplaint(clientAssertion);
-    JWSHeader header = getJwtHeader(clientAssertion);
-    assertEquals(KEY_ID, header.getKeyID());
-    assertNull(header.getJWKURL());
+    Exception exception = assertThrows(BadCredentialsException.class, () ->
+        jwtClientAuthentication.getClientAuthenticationParameters(new LinkedMultiValueMap<>(), config));
+    assertEquals("Missing referenced signing entry", exception.getMessage());
   }
 
   @Test
-  void testGetClientAssertionUsingCustomSingingKeyFromEnivoronmentButNotInDefaultZone() throws JOSEException {
+  void testGetClientAssertionUsingCustomSingingKeyFromEnvironmentButNotInDefaultZone() throws JOSEException {
     // Given: register 2 keys
     mockKeyInfoService("key-id-321", JwtHelperX5tTest.CERTIFICATE_1);
     HashMap customClaims = new HashMap<>();
@@ -285,7 +279,7 @@ void testGetClientAssertionCustomSingingKeyButNoCertificate() throws ParseExcept
   }
 
   @Test
-  void testGetClientAssertionUsingCustomSingingPrivateKeyFromEnivoronment() throws ParseException, JOSEException {
+  void testGetClientAssertionUsingCustomSingingPrivateKeyFromEnvironment() throws ParseException, JOSEException {
     // Given: register 2 keys
     mockKeyInfoService("key-id-321", JwtHelperX5tTest.CERTIFICATE_1);
     // add reference in jwtClientAuthentication to customer one key-id-321