Rest spec and documentation by jkakavas · Pull Request #54664 · elastic/elasticsearch

jkakavas · 2020-04-02T16:28:30Z

This change adds the spec for the new REST APIs that we
introduce for the IDP and documentation for each of the APIs. The
documentation pages are intentionally not included in the API
reference so as to minimize unnecessary exposure.

supersedes: #53858

This change adds the spec for the new REST APIs that we introduce for the IDP and documentation for each of the APIs. The documentation pages are intentionally not included in the API reference so as to minimize unnecessary exposure. supersedes: elastic#53858

elasticmachine · 2020-04-02T16:28:33Z

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

elasticmachine · 2020-04-02T16:28:34Z

Pinging @elastic/es-docs (>docs)

jkakavas · 2020-04-03T05:59:47Z

@elasticmachine test this please

tvernum · 2020-04-08T05:36:17Z

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-init.asciidoc

+This API generates a SAML Response message that should be sent to a Service Provider as part of an
+IDP initiated or SP initiated SAML Single Sign On. This API expects the caller to present
+credentials for the user that the SAML Response will be created for as "Secondary Authentication"
+using the `es-secondary-authorization` HTTP Request header.


FYI, @lcawl is working on some generic docs for secondary authentication, which we can link to when they're ready.

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-init.asciidoc

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-metadata.asciidoc

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-register-sp.asciidoc

tvernum · 2020-04-08T05:52:31Z

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-register-sp.asciidoc

+(Required, string) A name to identify this service provider. Used only for informational purposes
+
+`entity_id`::
+(Required, string) The SAML entity Id of the service provider.


This isn't required.
If it is not set, it will be populated from the URL parameter.
If it is set, it must match the URL parameter.

elasticsearch/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/action/PutSamlServiceProviderRequest.java

Lines 36 to 43 in 8552c5e

if (document.entityId == null) {

document.setEntityId(entityId);

} else if (entityId != null) {

if (entityId.equals(document.entityId) == false) {

throw new ElasticsearchParseException(

"Entity id [{}] inside request body and entity id [{}] from parameter do not match", document.entityId, entityId);

}

}

x-pack/plugin/identity-provider/src/test/resources/rest-api-spec/api/idp.saml_get_metadata.json

Co-Authored-By: Tim Vernum <tim@adjective.org>

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-init.asciidoc

jkakavas · 2020-04-15T08:14:24Z

ping @tvernum

Mpdreamz · 2020-04-15T08:31:51Z

cc @elastic/es-clients heads up, new API's incoming 😄

jkakavas · 2020-04-15T08:44:03Z

@Mpdreamz / @elastic/es-clients this is intentionally kept separate from the rest spec as we don't want our clients to support this cloud internal functionality.

Mpdreamz · 2020-04-15T09:05:39Z

Cool, the need private API;s for this has been discussed previously: #38413 (comment) but voted against.

I do think we need a private: true marker in the future but will chase this outside of this PR and circle back to this.

tvernum · 2020-04-16T05:34:54Z

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-register-sp.asciidoc

 `entity_id`::
-(Required, string) The SAML entity Id of the service provider.
+(Optional, string) The SAML entity Id of the service provider. If not set, it will be populated with the value from the URL parameter.
+If set, it musth match the value that is passed in the URL parameter.


Suggested change

If set, it musth match the value that is passed in the URL parameter.

If set, it must match the value that is passed in the URL parameter.

tvernum · 2020-04-16T05:36:06Z

x-pack/plugin/identity-provider/src/test/resources/rest-api-spec/api/idp.saml_get_metadata.json

      ]
+    },
+    "params": {
+      "acs": { ... }


My use of ... was just a placeholder - I think this should be populated with something meaningful.

Meh , I did a two step process of 1) all the suggestions make sense and later 2) lets merge these and forgot to take care of this . Will adjust

russcam

~~Suggestion to mark the stability of the specs as private. This signals to the language clients that these are internal APIs and not to be exposed. Current stability values supported are:~~

elasticsearch/rest-api-spec/src/main/resources/schema.json

Line 34 in 5920d1b

"enum": ["stable", "beta", "experimental", "private"]

EDIT

Removed the suggestions. Appears "private" is now not a stability value in specs

Mpdreamz · 2020-04-16T06:04:27Z

@russcam there is no private option:
https://github.com/elastic/elasticsearch/blob/master/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApi.java#L47

See also previous discussion: #38413 (comment)

As mentioned in my comment will discuss and fix this outside this PR then circle back to it. We'll need to update the json schema in the meantime.

russcam · 2020-04-16T06:13:32Z

Ok @Mpdreamz. I would like to be involved in this discussion when it happens

codebrain · 2020-04-16T08:25:10Z

I think an is_private: true flag is more favourable to introducing the private "stability" option, on the basis that a developer can still use the experimental > beta > stable pathway and keep the is_private flag intact.

jkakavas · 2020-04-16T08:36:36Z

Can I kindly ask that we continue the flag related discussion in another medium/thread/issue so that it won't get lost in the comments of this PR ?

jkakavas · 2020-04-16T09:10:41Z

@elasticmachine update branch

tvernum

LGTM

This change adds the spec for the new REST APIs that we introduce for the IDP and documentation for each of the APIs. The documentation pages are intentionally not included in the API reference so as to minimize unnecessary exposure. supersedes: elastic#53858

This change adds the spec for the new REST APIs that we introduce for the IDP and documentation for each of the APIs. The documentation pages are intentionally not included in the API reference so as to minimize unnecessary exposure. supersedes: #53858

jkakavas added >docs General docs changes :Security/Security Security issues without another label labels Apr 2, 2020

jkakavas requested review from lloydmeta and tvernum April 2, 2020 16:28

jkakavas added 2 commits April 7, 2020 09:53

Merge remote-tracking branch 'origin/master' into rest-spec-idp-master

fa717bd

Adjust metadata API docs

8ae0bfb

tvernum reviewed Apr 8, 2020

View reviewed changes

jkakavas and others added 3 commits April 8, 2020 16:00

Apply suggestions from code review

ff35089

Co-Authored-By: Tim Vernum <tim@adjective.org>

address feedback

1103e58

Merge remote-tracking branch 'origin/master' into rest-spec-idp-master

8506582

jkakavas requested a review from tvernum April 8, 2020 13:06

lloydmeta reviewed Apr 9, 2020

View reviewed changes

x-pack/plugin/identity-provider/docs/en/rest-api/idp-saml-init.asciidoc Show resolved Hide resolved

tvernum reviewed Apr 16, 2020

View reviewed changes

russcam reviewed Apr 16, 2020

View reviewed changes

Address feedback

3216f31

Merge branch 'master' into rest-spec-idp-master

465ef8f

jkakavas requested a review from tvernum April 16, 2020 11:08

tvernum approved these changes Apr 16, 2020

View reviewed changes

jkakavas merged commit dfb3355 into elastic:master Apr 16, 2020

jkakavas mentioned this pull request Apr 16, 2020

[7.x] Rest spec and documentation (#54664) #55305

Merged

jkakavas mentioned this pull request Apr 16, 2020

[7.7] Rest spec and documentation (#54664) #55306

Merged

Mpdreamz mentioned this pull request May 4, 2020

Add visibility the to rest-spec-api #56104

Merged

	if (document.entityId == null) {
	document.setEntityId(entityId);
	} else if (entityId != null) {
	if (entityId.equals(document.entityId) == false) {
	throw new ElasticsearchParseException(
	"Entity id [{}] inside request body and entity id [{}] from parameter do not match", document.entityId, entityId);
	}
	}

	If set, it musth match the value that is passed in the URL parameter.
	If set, it must match the value that is passed in the URL parameter.

Conversation

jkakavas commented Apr 2, 2020

Uh oh!

elasticmachine commented Apr 2, 2020

Uh oh!

elasticmachine commented Apr 2, 2020

Uh oh!

jkakavas commented Apr 3, 2020

Uh oh!

tvernum Apr 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tvernum Apr 8, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jkakavas commented Apr 15, 2020

Uh oh!

Mpdreamz commented Apr 15, 2020

Uh oh!

jkakavas commented Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mpdreamz commented Apr 15, 2020

Uh oh!

tvernum Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

tvernum Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

jkakavas Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

russcam left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

EDIT

Uh oh!

Mpdreamz commented Apr 16, 2020

Uh oh!

russcam commented Apr 16, 2020

Uh oh!

codebrain commented Apr 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkakavas commented Apr 16, 2020

Uh oh!

jkakavas commented Apr 16, 2020

Uh oh!

tvernum left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tvernum Apr 8, 2020 •

edited

Loading

jkakavas commented Apr 15, 2020 •

edited

Loading

russcam left a comment •

edited

Loading

codebrain commented Apr 16, 2020 •

edited

Loading