Remove synthetic role names for legacy API keys (#119930)

Forward-ports: #119844
elastic · Jan 10, 2025 · b68fc8e · b68fc8e
1 parent 31f11c3
commit b68fc8e
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 2 deletions.
diff --git a/...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java
@@ -55,6 +55,7 @@
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import static org.elasticsearch.common.Strings.EMPTY_ARRAY;
 import static org.elasticsearch.transport.RemoteClusterPortSettings.TRANSPORT_VERSION_ADVANCED_REMOTE_CLUSTER_SECURITY;
 import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg;
 import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
@@ -170,21 +171,49 @@ public Authentication(StreamInput in) throws IOException {
             type = AuthenticationType.REALM;
             metadata = Map.of();
         }
+
         if (innerUser != null) {
-            authenticatingSubject = new Subject(innerUser, authenticatedBy, version, metadata);
+            authenticatingSubject = new Subject(
+                copyUserWithRolesRemovedForLegacyApiKeys(version, innerUser),
+                authenticatedBy,
+                version,
+                metadata
+            );
             // The lookup user for run-as currently doesn't have authentication metadata associated with them because
             // lookupUser only returns the User object. The lookup user for authorization delegation does have
             // authentication metadata, but the realm does not expose this difference between authenticatingUser and
             // delegateUser so effectively this is handled together with the authenticatingSubject not effectiveSubject.
+            // Note: we do not call copyUserWithRolesRemovedForLegacyApiKeys here because an API key is never the target of run-as
             effectiveSubject = new Subject(outerUser, lookedUpBy, version, Map.of());
         } else {
-            authenticatingSubject = effectiveSubject = new Subject(outerUser, authenticatedBy, version, metadata);
+            authenticatingSubject = effectiveSubject = new Subject(
+                copyUserWithRolesRemovedForLegacyApiKeys(version, outerUser),
+                authenticatedBy,
+                version,
+                metadata
+            );
         }
+
         if (Assertions.ENABLED) {
             checkConsistency();
         }
     }
 
+    private User copyUserWithRolesRemovedForLegacyApiKeys(TransportVersion version, User user) {
+        // API keys prior to 7.8 had synthetic role names. Strip these out to maintain the invariant that API keys don't have role names
+        if (type == AuthenticationType.API_KEY && version.onOrBefore(TransportVersions.V_7_8_0) && user.roles().length > 0) {
+            logger.debug(
+                "Stripping [{}] roles from API key user [{}] for legacy version [{}]",
+                user.roles().length,
+                user.principal(),
+                version
+            );
+            return new User(user.principal(), EMPTY_ARRAY, user.fullName(), user.email(), user.metadata(), user.enabled());
+        } else {
+            return user;
+        }
+    }
+
     /**
      * Get the {@link Subject} that performs the actual authentication. This normally means it provides a credentials.
      */

diff --git a/...st/java/org/elasticsearch/xpack/core/security/authc/AuthenticationSerializationTests.java b/...st/java/org/elasticsearch/xpack/core/security/authc/AuthenticationSerializationTests.java
@@ -13,16 +13,21 @@
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.TransportVersionUtils;
 import org.elasticsearch.transport.RemoteClusterPortSettings;
+import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer;
 import org.elasticsearch.xpack.core.security.user.ElasticUser;
 import org.elasticsearch.xpack.core.security.user.InternalUsers;
 import org.elasticsearch.xpack.core.security.user.KibanaSystemUser;
 import org.elasticsearch.xpack.core.security.user.KibanaUser;
 import org.elasticsearch.xpack.core.security.user.User;
 
+import java.io.IOException;
 import java.util.Arrays;
+import java.util.Map;
 
 import static org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationSerializationHelper;
+import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.emptyArray;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
@@ -171,4 +176,47 @@ public void testReservedUserSerialization() throws Exception {
 
         assertEquals(kibanaSystemUser, readFrom);
     }
+
+    public void testRolesRemovedFromUserForLegacyApiKeys() throws IOException {
+        TransportVersion transportVersion = TransportVersionUtils.randomVersionBetween(
+            random(),
+            TransportVersions.V_7_0_0,
+            TransportVersions.V_7_8_0
+        );
+        Subject authenticatingSubject = new Subject(
+            new User("foo", "role"),
+            new Authentication.RealmRef(AuthenticationField.API_KEY_REALM_NAME, AuthenticationField.API_KEY_REALM_TYPE, "node"),
+            transportVersion,
+            Map.of(AuthenticationField.API_KEY_ID_KEY, "abc")
+        );
+        Subject effectiveSubject = new Subject(
+            new User("bar", "role"),
+            new Authentication.RealmRef("native", "native", "node"),
+            transportVersion,
+            Map.of()
+        );
+
+        {
+            Authentication actual = AuthenticationContextSerializer.decode(
+                Authentication.doEncode(authenticatingSubject, authenticatingSubject, Authentication.AuthenticationType.API_KEY)
+            );
+            assertThat(actual.getAuthenticatingSubject().getUser().roles(), is(emptyArray()));
+        }
+
+        {
+            Authentication actual = AuthenticationContextSerializer.decode(
+                Authentication.doEncode(effectiveSubject, authenticatingSubject, Authentication.AuthenticationType.API_KEY)
+            );
+            assertThat(actual.getAuthenticatingSubject().getUser().roles(), is(emptyArray()));
+            assertThat(actual.getEffectiveSubject().getUser().roles(), is(arrayContaining("role")));
+        }
+
+        {
+            // do not strip roles for authentication methods other than API key
+            Authentication actual = AuthenticationContextSerializer.decode(
+                Authentication.doEncode(effectiveSubject, effectiveSubject, Authentication.AuthenticationType.REALM)
+            );
+            assertThat(actual.getAuthenticatingSubject().getUser().roles(), is(arrayContaining("role")));
+        }
+    }
 }