4.x: UPN claim should be optional (helidon-io#5151)

Captain1653 · Jan 8, 2025 · 039a2e7 · 039a2e7
1 parent 3c4bbd8
commit 039a2e7
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 13 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
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -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;
     }
@@ -663,11 +659,14 @@ public Optional<String> subject() {
 
     /**
      * User principal claim ("upn" from microprofile specification).
+     * Falls back "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
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2024 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2025 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,7 +20,6 @@
 import java.time.temporal.ChronoUnit;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 import java.util.UUID;
 
 import io.helidon.common.Errors;
@@ -29,6 +28,8 @@
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 /**
@@ -111,4 +112,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")));
+    }
 }