Remove synthetic role names for legacy API keys (elastic#119844)

This PR addresses an issue where legacy API keys fail consistency checks
because they include synthetic role names. 

We removed synthetic role names with
elastic#56005. We added
consistency checks sometime later to enforce no role names, with
elastic#93894 in `8.8.0`. 

Rather than relaxing our consistency checks, this PR tweaks
de-serialization logic to strip out role names when appropriate. This
has the advantage that we maintain the invariant the consistency check
is meant to enforce.

Note that this does not manifest in production: outside of RCS 2.0, we
only execute consistency checks with assertions enabled. For RCS 2.0, an
API key would require `remote_indices` privileges to ever be sent cross
cluster and go through consistency checks. These were introduced after
we've stopped including role names in API keys so it's not a real issue
either.

Closes: elastic#119259 Closes:
elastic#119435 Closes:
elastic#119434 Closes:
elastic#119433  Closes:
elastic#119424 Closes:
elastic#119423 Closes:
elastic#119422 Closes:
elastic#119396 Closes:
elastic#119395 Closes:
elastic#119394 Closes:
elastic#119393

Author: n1v0lg

muted-tests.yml

@@ -428,24 +428,9 @@ tests:
 - class: org.elasticsearch.index.engine.LuceneSyntheticSourceChangesSnapshotTests
   method: testSkipNonRootOfNestedDocuments
   issue: https://github.com/elastic/elasticsearch/issues/119553
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testWatcher {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119259
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testServiceAccountApiKey {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119435
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testSecurityNativeRealm {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119423
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testSingleDoc {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119434
 - class: org.elasticsearch.upgrades.SearchStatesIT
   method: testCanMatch
   issue: https://github.com/elastic/elasticsearch/issues/118718
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testTransformLegacyTemplateCleanup {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119395
 - class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT
   method: "test {p0=search.vectors/110_knn_query_with_filter/PRE_FILTER: knn query with internal filter as pre-filter}"
   issue: https://github.com/elastic/elasticsearch/issues/119804
@@ -469,21 +454,6 @@ tests:
 - class: org.elasticsearch.xpack.ml.integration.MlJobIT
   method: testGetJob_GivenJobExists
   issue: https://github.com/elastic/elasticsearch/issues/119811
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testSlmPolicyAndStats {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119393
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testApiKeySuperuser {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119424
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testDataStreams {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119394
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testDisableFieldNameField {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119422
-- class: org.elasticsearch.xpack.restart.FullClusterRestartIT
-  method: testRollupAfterRestart {cluster=UPGRADED}
-  issue: https://github.com/elastic/elasticsearch/issues/119433
 - class: org.elasticsearch.xpack.security.authc.ldap.ADLdapUserSearchSessionFactoryTests
   issue: https://github.com/elastic/elasticsearch/issues/119882
 - class: org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapperTests

x-pack/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.
      */

x-pack/plugin/core/src/test/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")));
+        }
+    }
 }