From cb8316e45a0e97bbc07c1df23166e59e38fbb28c Mon Sep 17 00:00:00 2001
From: James Manger <james.h.manger@team.telstra.com>
Date: Tue, 1 Oct 2024 22:59:43 +1000
Subject: [PATCH] feat: Jwt -- don't force "upn" claim

---
 .../java/io/helidon/security/jwt/Jwt.java     | 20 +++++++++----------
 .../java/io/helidon/security/jwt/JwtTest.java | 14 +++++++++++++
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/security/jwt/src/main/java/io/helidon/security/jwt/Jwt.java b/security/jwt/src/main/java/io/helidon/security/jwt/Jwt.java
index b3e5a7e4b32..490b1e4e086 100644
--- a/security/jwt/src/main/java/io/helidon/security/jwt/Jwt.java
+++ b/security/jwt/src/main/java/io/helidon/security/jwt/Jwt.java
@@ -307,9 +307,7 @@ public class Jwt {
         this.cHash = JwtUtil.getByteArray(payloadJson, C_HASH, "c_hash value");
         this.nonce = JwtUtil.getString(payloadJson, NONCE);
         this.scopes = JwtUtil.toScopes(payloadJson);
-        this.userPrincipal = JwtUtil.getString(payloadJson, USER_PRINCIPAL)
-                .or(() -> preferredUsername)
-                .or(() -> subject);
+        this.userPrincipal = JwtUtil.getString(payloadJson, USER_PRINCIPAL);
     }
 
     private Jwt(Builder builder) {
@@ -357,9 +355,7 @@ private Jwt(Builder builder) {
         this.scopes = builder.scopes;
 
         this.userPrincipal = builder.userPrincipal
-                .or(() -> toOptionalString(builder.payloadClaims, USER_PRINCIPAL))
-                .or(() -> preferredUsername)
-                .or(() -> subject);
+                .or(() -> toOptionalString(builder.payloadClaims, USER_PRINCIPAL));
 
         this.userGroups = builder.userGroups;
     }
@@ -662,12 +658,15 @@ public Optional<String> subject() {
     }
 
     /**
-     * User principal claim ("upn" from microprofile specification).
+     * User principal claim ("upn" from microprofile specification). Falls back to
+     * "preferred_username" then "sub" claim.
      *
-     * @return user principal or empty if claim is not defined
+     * @return user principal or empty if claim and fallbacks are not defined
      */
     public Optional<String> userPrincipal() {
-        return userPrincipal;
+        return userPrincipal
+                .or(() -> preferredUsername)
+                .or(() -> subject);
     }
 
     /**
@@ -1682,8 +1681,7 @@ public Builder subject(String subject) {
          * User principal claim as defined by Microprofile JWT Auth spec.
          * Uses "upn" claim.
          *
-         * @param principal name of the principal, falls back to {@link #preferredUsername(String)} and then to
-         *                  {@link #subject(String)}
+         * @param principal name of the principal
          * @return updated builder instance
          */
         public Builder userPrincipal(String principal) {
diff --git a/security/jwt/src/test/java/io/helidon/security/jwt/JwtTest.java b/security/jwt/src/test/java/io/helidon/security/jwt/JwtTest.java
index 1f5eb1ba37b..fce6fe256af 100644
--- a/security/jwt/src/test/java/io/helidon/security/jwt/JwtTest.java
+++ b/security/jwt/src/test/java/io/helidon/security/jwt/JwtTest.java
@@ -28,7 +28,9 @@
 
 import org.junit.jupiter.api.Test;
 
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 /**
@@ -111,4 +113,16 @@ public void testOidcJwt() {
         errors.log(LOGGER);
         errors.checkValid();
     }
+
+    @Test
+    public void testUpnNotAddedAutomatically() {
+        String json = Jwt.builder().subject("a").build().payloadJson().toString();
+        assertThat(json, not(containsString("\"upn\"")));
+    }
+
+    @Test
+    public void testUserPrincipalFallsbackToSub() {
+        Jwt jwt = Jwt.builder().subject("a").build();
+        assertThat(jwt.userPrincipal(), is(Optional.of("a")));
+    }
 }