Do not set a NameID format in Policy by default

This commit changes the behavior of our SAML realm to not set a
Format element in the NameIDPolicy of a SAML Authentication
request if one has not been explicitly configured by the user
with `nameid_format`. We select to not include a format, rather
than setting it to
`urn:oasis:names:tc:SAML:2.0:nameid-format:unspecified` which would
have the same effect, in order to maximize interoperability with
IdP implementations. `AllowCreate` is not removed as this has a
default value (false) in the specification.

Relates: elastic#40353

docs/reference/settings/security-settings.asciidoc

@@ -938,11 +938,10 @@ As per `attribute_patterns.principal`, but for the _dn_ property.
 
 `nameid_format`::
 The NameID format that should be requested when asking the IdP to authenticate
-the current user. Defaults to requesting _transient_ names 
-(`urn:oasis:names:tc:SAML:2.0:nameid-format:transient`). 
+the current user. The default is to not  include the `nameid_format` attribute.
 
 `nameid.allow_create`:: The value of the `AllowCreate` attribute of the 
-`NameIdPolicy` element in an authentication request. Defaults to `false`. 
+`NameIdPolicy` element in an authentication request. The default value is false.
 
 `nameid.sp_qualifier`:: The value of the `SPNameQualifier` attribute of the 
 `NameIdPolicy` element in an authentication request. The default is to not 

x-pack/docs/en/security/authentication/saml-guide.asciidoc

@@ -266,6 +266,14 @@ additional names that can be used:
     mechanism that will cause an error if you attempt to map from a `NameID`
     that does not have a persistent value.
 
+NOTE: Identity Providers can be either statically configured to release a `NameID`
+with a specific format, or they can be configured to try to conform with the
+requirements of the SP. The SP declares its requirements as part of the
+Authentication Request, using an element which is called the `NameIDPolicy`. If
+this is needed, you can set the relevant <<saml-settings, settings>> named
+`nameid_format` in order to request that the IdP releases a `NameID` with a
+specific format.
+
 _friendlyName_::
     A SAML attribute may have a _friendlyName_ in addition to its URI based name.
     For example the attribute with a name of `urn:oid:0.9.2342.19200300.100.1.1`

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java

@@ -24,7 +24,6 @@
 public class SamlRealmSettings {
 
     public static final String TYPE = "saml";
-    private static final String TRANSIENT_NAMEID_FORMAT = "urn:oasis:names:tc:SAML:2.0:nameid-format:transient";
 
     // these settings will be used under the prefix xpack.security.authc.realms.REALM_NAME.
     private static final String IDP_METADATA_SETTING_PREFIX = "idp.metadata.";
@@ -49,9 +48,8 @@ public class SamlRealmSettings {
     public static final Setting.AffixSetting<String> SP_ACS = RealmSettings.simpleString(TYPE, "sp.acs", Setting.Property.NodeScope);
     public static final Setting.AffixSetting<String> SP_LOGOUT = RealmSettings.simpleString(TYPE, "sp.logout", Setting.Property.NodeScope);
 
-    public static final Setting.AffixSetting<String> NAMEID_FORMAT = Setting.affixKeySetting(
-            RealmSettings.realmSettingPrefix(TYPE), "nameid_format",
-            key -> new Setting<>(key, s -> TRANSIENT_NAMEID_FORMAT, Function.identity(), Setting.Property.NodeScope));
+    public static final Setting.AffixSetting<String> NAMEID_FORMAT
+        = RealmSettings.simpleString(TYPE, "nameid_format", Setting.Property.NodeScope);
 
     public static final Setting.AffixSetting<Boolean> NAMEID_ALLOW_CREATE = Setting.affixKeySetting(
             RealmSettings.realmSettingPrefix(TYPE), "nameid.allow_create",

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthnRequestBuilder.java

@@ -31,7 +31,7 @@ class SamlAuthnRequestBuilder extends SamlMessageBuilder {
         super(idpDescriptor, spConfig, clock);
         this.spBinding = spBinding;
         this.idpBinding = idBinding;
-        this.nameIdSettings = new NameIDPolicySettings(NameID.TRANSIENT, false, null);
+        this.nameIdSettings = new NameIDPolicySettings(null, false, null);
     }
 
     SamlAuthnRequestBuilder forceAuthn(Boolean forceAuthn) {
@@ -80,7 +80,7 @@ private RequestedAuthnContext buildRequestedAuthnContext() {
 
     private NameIDPolicy buildNameIDPolicy() {
         NameIDPolicy nameIDPolicy = SamlUtils.buildObject(NameIDPolicy.class, NameIDPolicy.DEFAULT_ELEMENT_NAME);
-        nameIDPolicy.setFormat(nameIdSettings.format);
+        nameIDPolicy.setFormat(Strings.isNullOrEmpty(nameIdSettings.format) ? null : nameIdSettings.format);
         nameIDPolicy.setAllowCreate(nameIdSettings.allowCreate);
         nameIDPolicy.setSPNameQualifier(Strings.isNullOrEmpty(nameIdSettings.spNameQualifier) ? null : nameIdSettings.spNameQualifier);
         return nameIDPolicy;

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java

@@ -218,7 +218,7 @@ public static SamlRealm create(RealmConfig config, SSLService sslService, Resour
         this.idpDescriptor = idpDescriptor;
         this.serviceProvider = spConfiguration;
 
-        this.nameIdPolicy = new SamlAuthnRequestBuilder.NameIDPolicySettings(require(config, NAMEID_FORMAT),
+        this.nameIdPolicy = new SamlAuthnRequestBuilder.NameIDPolicySettings(config.getSetting(NAMEID_FORMAT),
                 config.getSetting(NAMEID_ALLOW_CREATE), config.getSetting(NAMEID_SP_QUALIFIER));
         this.forceAuthn = config.getSetting(FORCE_AUTHN, () -> null);
         this.useSingleLogout = config.getSetting(IDP_SINGLE_LOGOUT);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlSpMetadataBuilder.java

@@ -92,7 +92,7 @@ public SamlSpMetadataBuilder(Locale locale, String entityId) {
         this.attributeNames = new LinkedHashMap<>();
         this.contacts = new ArrayList<>();
         this.serviceName = "Elasticsearch";
-        this.nameIdFormat = NameID.TRANSIENT;
+        this.nameIdFormat = null;
         this.authnRequestsSigned = Boolean.FALSE;
     }
 
@@ -222,7 +222,7 @@ public EntityDescriptor build() throws Exception {
         spRoleDescriptor.setWantAssertionsSigned(true);
         spRoleDescriptor.setAuthnRequestsSigned(this.authnRequestsSigned);
 
-        if (Strings.hasLength(nameIdFormat)) {
+        if (Strings.isNullOrEmpty(nameIdFormat) == false) {
             spRoleDescriptor.getNameIDFormats().add(buildNameIdFormat());
         }
         spRoleDescriptor.getAssertionConsumerServices().add(buildAssertionConsumerService());
@@ -247,6 +247,9 @@ public EntityDescriptor build() throws Exception {
     }
 
     private NameIDFormat buildNameIdFormat() {
+        if (Strings.isNullOrEmpty(nameIdFormat)) {
+            throw new IllegalStateException("NameID format has not been specified");
+        }
         final NameIDFormat format = new NameIDFormatBuilder().buildObject();
         format.setFormat(this.nameIdFormat);
         return format;

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthnRequestBuilderTests.java

@@ -51,6 +51,26 @@ public void init() throws Exception {
         idpDescriptor.getRoleDescriptors().add(idpRole);
     }
 
+    public void testBuildRequestWithDefaultSettingsHasNoNameIdPolicy() {
+        SpConfiguration sp = new SpConfiguration(SP_ENTITY_ID, ACS_URL, null, null, null, Collections.emptyList());
+        final SamlAuthnRequestBuilder builder = new SamlAuthnRequestBuilder(
+            sp, SAMLConstants.SAML2_POST_BINDING_URI,
+            idpDescriptor, SAMLConstants.SAML2_REDIRECT_BINDING_URI,
+            Clock.systemUTC());
+
+        final AuthnRequest request = buildAndValidateAuthnRequest(builder);
+
+        assertThat(request.getIssuer().getValue(), equalTo(SP_ENTITY_ID));
+        assertThat(request.getProtocolBinding(), equalTo(SAMLConstants.SAML2_POST_BINDING_URI));
+
+        assertThat(request.getAssertionConsumerServiceURL(), equalTo(ACS_URL));
+
+        assertThat(request.getNameIDPolicy(), notNullValue());
+        assertThat(request.getNameIDPolicy().getFormat(), nullValue());
+        assertThat(request.getNameIDPolicy().getSPNameQualifier(), nullValue());
+        assertThat(request.getNameIDPolicy().getAllowCreate(), equalTo(Boolean.FALSE));
+    }
+
     public void testBuildRequestWithPersistentNameAndNoForceAuth() throws Exception {
         SpConfiguration sp = new SpConfiguration(SP_ENTITY_ID, ACS_URL, null, null, null, Collections.emptyList());
         final SamlAuthnRequestBuilder builder = new SamlAuthnRequestBuilder(

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlSpMetadataBuilderTests.java

@@ -71,7 +71,6 @@ public void testBuildMinimalistMetadata() throws Exception {
                 "<md:EntityDescriptor xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\" entityID=\"https://my.sp.example.net/\">" +
                 "<md:SPSSODescriptor AuthnRequestsSigned=\"false\" WantAssertionsSigned=\"true\"" +
                 " protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\">" +
-                "<md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:transient</md:NameIDFormat>" +
                 "<md:AssertionConsumerService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\"" +
                 " Location=\"https://my.sp.example.net/saml/acs/post\" index=\"1\" isDefault=\"true\"/>" +
                 "</md:SPSSODescriptor>" +
@@ -307,4 +306,4 @@ public void testAttributeNameIsRequired() {
     private void assertValidXml(String xml) throws Exception {
         SamlUtils.validate(new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)), SamlMetadataCommand.METADATA_SCHEMA);
     }
-}
+}