Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SAML complete logout API has a camel case request parameter #73543

Closed
ywangd opened this issue May 31, 2021 · 3 comments · Fixed by #73984
Closed

SAML complete logout API has a camel case request parameter #73543

ywangd opened this issue May 31, 2021 · 3 comments · Fixed by #73984
Assignees
@ywangd
Labels
>deprecation :Security/Security Security issues without another label Team:Security Meta label for security team

Comments

@ywangd
Copy link
Member

ywangd commented May 31, 2021

As discussed in https://github.com/elastic/elasticsearch/pull/72867/files#r629011079, the SAML complete logout API has a camel case request parameter, queryString. This is an anomaly because the rest of APIs use only snake case for request parameters.

This request parameter, queryString, should be deprecated and replaced with its snake case equivalent, query_string.
@ywangd ywangd added >deprecation :Security/Security Security issues without another label labels May 31, 2021
@ywangd ywangd self-assigned this May 31, 2021
@elasticmachine elasticmachine added the Team:Security Meta label for security team label May 31, 2021
@elasticmachine
Copy link
Collaborator

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

@ywangd ywangd mentioned this issue May 31, 2021
Merged
@ywangd
Copy link
Member Author

ywangd commented Jun 9, 2021

@tvernum I noticed the same queryString parameter (camel case) is also used for RestSamlInvalidateSessionAction. Do you think it should be deprecated and replaced with query_string as well? If so, I can raise a PR to deprecate both of them.

@tvernum
Copy link
Contributor

tvernum commented Jun 10, 2021

I guess we should. It's wrong, so we ought to fix it.

@ywangd ywangd mentioned this issue Jun 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ywangd ywangd

Labels
>deprecation :Security/Security Security issues without another label Team:Security Meta label for security team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deprecate camelCase parameters used by SAML APIs ywangd/elasticsearch
3 participants
@tvernum @ywangd @elasticmachine