Add note in breaking changes for nameid_format #77785

jkakavas · 2021-09-15T14:17:57Z

We changed the default for nameid_format in 8.0 in #44090 but
did not add anything to the breaking changes in the release notes.
This change amends that.

Preview link: https://elasticsearch_77785.docs-preview.app.elstc.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_security_changes

We changed the default for `nameid_format` in 8.0 in elastic#44090 but did not add anything to the breaking changes in the release notes. This change amends that.

elasticmachine · 2021-09-15T14:18:00Z

Pinging @elastic/es-security (Team:Security)

elasticmachine · 2021-09-15T14:18:00Z

Pinging @elastic/es-docs (Team:Docs)

jrodewig

I left some non-blocking nits. The only major suggestion I have is about relocating this section. Feel free to ignore or cherry-pick the other stuff as wanted.

I'm also not as experienced with security or SAML as @lockewritesdocs. If there are any conflicts, I'll defer to him.

jrodewig · 2021-09-15T16:14:40Z

docs/reference/migration/migrate_8_0/security.asciidoc

+===== The default value of `nameid_format` setting has been removed.
+


I'd remove this heading (keep the anchor) and relocate these changes just below the section for The transport.profiles.*.xpack.security.type setting has been removed.

Suggested change

===== The default value of `nameid_format` setting has been removed.

Agreed! I'll implement that change.

docs/reference/migration/migrate_8_0/security.asciidoc

jrodewig · 2021-09-15T16:31:51Z

docs/reference/migration/migrate_8_0/security.asciidoc

+The default value has now been removed. This means that {es} will be default
+create SAML Authentication Requests that do not put forward such requirements
+to the Identity Provider.
+
+If you want to retain the previous behavior, you can set `nameid_format`
+to `urn:oasis:names:tc:SAML:2.0:nameid-format:transient`.


I wonder if we can remove the second sentence.

Suggested change

The default value has now been removed. This means that {es} will be default

create SAML Authentication Requests that do not put forward such requirements

to the Identity Provider.

If you want to retain the previous behavior, you can set `nameid_format`

to `urn:oasis:names:tc:SAML:2.0:nameid-format:transient`.

This default has been removed. To retain the previous default behavior, set

`nameid_format` to `urn:oasis:names:tc:SAML:2.0:nameid-format:transient`.

I don't think it makes sense to remove this. This is the essence of what changes and what this change means.

docs/reference/migration/migrate_8_0/security.asciidoc

jrodewig · 2021-09-15T17:03:15Z

🤦 Forgot to ask in my review:

Do we emit a deprecation warning for users that currently use the default?

If so, we should also add a deprecation notice to the breaking changes for that release like these:
https://www.elastic.co/guide/en/elasticsearch/reference/7.15/migrating-7.14.html#breaking_714_security_changes.

I didn't see one in the 7.x branch. This will let us link to the notice from the deprecation info API.

Thanks James! Co-authored-by: James Rodewig <[email protected]>

jkakavas

Appreciate the assistance @lockewritesdocs but please let the author of a PR have the chance to go through suggested changes and accept/decline/comment. I think this simplifies the flow for everyone involved 🙏

jkakavas · 2021-09-15T22:19:01Z

docs/reference/migration/migrate_8_0/security.asciidoc

+`urn:oasis:names:tc:SAML:2.0:nameid-format:transient`. This default created
+authentication requests that would require the IdP to release `NameID` with a
+transient format.
+The default value has now been removed. This means that {es} will be default


Suggested change

The default value has now been removed. This means that {es} will be default

The default value has now been removed. This means that {es} will by default

jkakavas · 2021-09-15T22:22:52Z

docs/reference/migration/migrate_8_0/security.asciidoc

+to `urn:oasis:names:tc:SAML:2.0:nameid-format:transient`.
+
+*Impact* +
+To avoid issues, explicitly configure `nameid_format`. If you don't configure


This is not what I am trying to say. If we instruct everyone to set this value, we might as well leave the default in place. What I am trying to say is that "this should probably be ok, but if you know you want the old behavior or you don't know how your IDP is configured but it was working so far and you don't care to try, set this setting to this value explicitly"

jkakavas · 2021-09-15T22:23:42Z

docs/reference/migration/migrate_8_0/security.asciidoc

+In SAML, Identity Providers (IdPs) either release a `NameID` or attempt to
+conform with the requirements of a Service Provider (SP). The SP declares its
+requirements in the `NameIDPolicy` of an authentication request. In {es}, the
+`nameid_format` SAML realm setting controls the `NameIDPolicy`.


Suggested change

In SAML, Identity Providers (IdPs) either release a `NameID` or attempt to

conform with the requirements of a Service Provider (SP). The SP declares its

requirements in the `NameIDPolicy` of an authentication request. In {es}, the

`nameid_format` SAML realm setting controls the `NameIDPolicy`.

In SAML, Identity Providers (IdPs) can be either statically configured to release a `NameID`

with a specific format, or configured to try to conform with the requirements of Service Provider (SP)

The SP declares its requirements in the `NameIDPolicy` element of a SAML Authentication Request.

In {es}, the `nameid_format` SAML realm setting controls the `NameIDPolicy` value.

jkakavas · 2021-09-15T22:26:22Z

facepalm Forgot to ask in my review:

Do we emit a deprecation warning for users that currently use the default?

If so, we should also add a deprecation notice to the breaking changes for that release like these:
https://www.elastic.co/guide/en/elasticsearch/reference/7.15/migrating-7.14.html#breaking_714_security_changes.

I didn't see one in the 7.x branch. This will let us link to the notice from the deprecation info API.

This is where this PR is coming from: #77276

lockewritesdocs · 2021-09-15T22:43:22Z

Appreciate the assistance @lockewritesdocs but please let the author of a PR have the chance to go through suggested changes and accept/decline/comment. I think this simplifies the flow for everyone involved 🙏

I'm sorry @jkakavas. I'm trying to work too quickly. I'll incorporate your changes and push an update.

jrodewig · 2021-09-16T00:45:46Z

Thanks @jkakavas @lockewritesdocs!

@lockewritesdocs Can you ensure we also add a deprecation notice for this change to the related 7.x branch?
We should linking be there in the deprecation info API.

docs/reference/migration/migrate_8_0/security.asciidoc

Dipping out to defer to Adam. Thanks!

docs/reference/migration/migrate_8_0/security.asciidoc

Co-authored-by: Tim Vernum <[email protected]>

lockewritesdocs

One minor nit, but LGTM otherwise. Thanks for weathering the review storm on this one 🌩️

docs/reference/migration/migrate_8_0/security.asciidoc

Co-authored-by: Ioannis Kakavas <[email protected]>

* upstream/master: (24 commits) Implement framework for migrating system indices (elastic#78951) Improve transient settings deprecation message (elastic#79504) Remove getValue and getValues from Field (elastic#79516) Store Template's mappings as bytes for disk serialization (elastic#78746) [ML] Add queue_capacity setting to start deployment API (elastic#79433) [ML] muting rest compat test issue elastic#79518 (elastic#79519) Avoid redundant available indices check (elastic#76540) Re-enable BWC tests TEST Ensure password 14 chars length on Kerberos FIPS tests (elastic#79496) [DOCS] Temporarily remove APM links (elastic#79411) Fix CCSDuelIT for skipped shards (elastic#79490) Add other time accounting in HotThreads (elastic#79392) Add deprecation info API entries for deprecated monitoring settings (elastic#78799) Add note in breaking changes for nameid_format (elastic#77785) Use 'migration' instead of 'upgrade' in GET system feature migration status responses (elastic#79302) Upgrade lucene version 8b68bf60c98 (elastic#79461) Use Strings#EMPTY_ARRAY (elastic#79452) Quicker shared cache file preallocation (elastic#79447) [ML] Removing some code that's obsolete for 8.0 (elastic#79444) Ensure indexing_data CCR requests are compressed (elastic#79413) ...

Add note in breaking changes for nameid_format

a4a2a09

We changed the default for `nameid_format` in 8.0 in elastic#44090 but did not add anything to the breaking changes in the release notes. This change amends that.

jkakavas added >docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 labels Sep 15, 2021

jkakavas requested review from tvernum, lockewritesdocs and jrodewig September 15, 2021 14:17

elasticmachine added Team:Docs Meta label for docs team Team:Security Meta label for security team labels Sep 15, 2021

jkakavas mentioned this pull request Sep 15, 2021

Adding a deprecation info API check for saml nameid_format setting #77276

Merged

jkakavas and others added 2 commits September 15, 2021 17:39

remove reference to settings

345008d

Fix docs build

82e2a87

jrodewig previously approved these changes Sep 15, 2021

View reviewed changes

Accepting most of James' suggested changes

306a786

Thanks James! Co-authored-by: James Rodewig <[email protected]>

jkakavas commented Sep 15, 2021

View reviewed changes

Incorporating changes from Ioannis

50ca91a

ywangd reviewed Sep 16, 2021

View reviewed changes

docs/reference/migration/migrate_8_0/security.asciidoc Outdated Show resolved Hide resolved

tvernum reviewed Sep 16, 2021

View reviewed changes

docs/reference/migration/migrate_8_0/security.asciidoc Outdated Show resolved Hide resolved

docs/reference/migration/migrate_8_0/security.asciidoc Outdated Show resolved Hide resolved

jkakavas commented Sep 16, 2021

View reviewed changes

docs/reference/migration/migrate_8_0/security.asciidoc Outdated Show resolved Hide resolved

Apply suggestions from code review

f8bffd5

Co-authored-by: Tim Vernum <[email protected]>

lockewritesdocs approved these changes Sep 16, 2021

View reviewed changes

docs/reference/migration/migrate_8_0/security.asciidoc Outdated Show resolved Hide resolved

Apply suggestions from code review

88c7f75

Co-authored-by: Ioannis Kakavas <[email protected]>

lockewritesdocs merged commit d6ef299 into elastic:master Oct 19, 2021

lockewritesdocs mentioned this pull request Oct 19, 2021

[7.X] [DOCS] Add deprecation notice for nameid_format #79497

Merged

jakelandis added v8.0.0-beta1 and removed v8.0.0 labels Oct 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add note in breaking changes for nameid_format #77785

Add note in breaking changes for nameid_format #77785

jkakavas commented Sep 15, 2021 •

edited by lockewritesdocs

Loading

elasticmachine commented Sep 15, 2021

elasticmachine commented Sep 15, 2021

jrodewig left a comment

jrodewig Sep 15, 2021

lockewritesdocs Sep 15, 2021

jrodewig Sep 15, 2021

jkakavas Sep 15, 2021

jrodewig commented Sep 15, 2021

jkakavas left a comment

jkakavas Sep 15, 2021

jkakavas Sep 15, 2021

jkakavas Sep 15, 2021

jkakavas commented Sep 15, 2021

lockewritesdocs commented Sep 15, 2021

jrodewig commented Sep 16, 2021 •

edited

Loading

lockewritesdocs left a comment

		===== The default value of `nameid_format` setting has been removed.

	The default value has now been removed. This means that {es} will be default
	The default value has now been removed. This means that {es} will by default

Add note in breaking changes for nameid_format #77785

Add note in breaking changes for nameid_format #77785

Conversation

jkakavas commented Sep 15, 2021 • edited by lockewritesdocs Loading

elasticmachine commented Sep 15, 2021

elasticmachine commented Sep 15, 2021

jrodewig left a comment

Choose a reason for hiding this comment

jrodewig Sep 15, 2021

Choose a reason for hiding this comment

lockewritesdocs Sep 15, 2021

Choose a reason for hiding this comment

jrodewig Sep 15, 2021

Choose a reason for hiding this comment

jkakavas Sep 15, 2021

Choose a reason for hiding this comment

jrodewig commented Sep 15, 2021

jkakavas left a comment

Choose a reason for hiding this comment

jkakavas Sep 15, 2021

Choose a reason for hiding this comment

jkakavas Sep 15, 2021

Choose a reason for hiding this comment

jkakavas Sep 15, 2021

Choose a reason for hiding this comment

jkakavas commented Sep 15, 2021

lockewritesdocs commented Sep 15, 2021

jrodewig commented Sep 16, 2021 • edited Loading

lockewritesdocs left a comment

Choose a reason for hiding this comment

jkakavas commented Sep 15, 2021 •

edited by lockewritesdocs

Loading

jrodewig commented Sep 16, 2021 •

edited

Loading