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.

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

Change password API authenticating with a bearer token #48752

Closed
jkakavas opened this issue Oct 31, 2019 · 7 comments · Fixed by #49694
Closed

Change password API authenticating with a bearer token #48752

jkakavas opened this issue Oct 31, 2019 · 7 comments · Fixed by #49694
Labels
:Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0-alpha1

Comments

@jkakavas
Copy link
Member

We should disallow requests to Change Password API authenticated with a bearer token from our Token Service. The reasoning for this is that it is best practice from a security perspective to mandate the proof of knowledge of the current password at the time the password is changed.

Kibana ( ping @azasypkin ) needs to adjust for this change too when the Token authentication provider is in use. The two available options that we originally discussed :

  • Kibana makes the request to the change password API on behalf of the user using the current password and a basic auth header ( preferable from our perspective )
  • Kibana makes the request to the change password API , passing the current password as a parameter in the call. This would require changing the API to support the extra parameter and investigation to see if/how it is possible to use the principal from the bearer token and the password from the API request to authenticate the request.
@jkakavas jkakavas added :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0 v7.6.0 labels Oct 31, 2019
@elasticmachine
Copy link
Collaborator

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

@jkakavas
Copy link
Member Author

jkakavas commented Nov 4, 2019

@original-brownbear I guess this was closed by mistake:)

original-brownbear added a commit to original-brownbear/elasticsearch that referenced this issue Nov 4, 2019
@original-brownbear
Copy link
Member

Yea, sorry about that!

@jkakavas
Copy link
Member Author

@azasypkin I went ahead and opened #49694

I feel that the proper way forward with this is option 1 from above where we disallow password change requests in elasticsearch when the user authenticates with a bearer token. Thinking through option 2, it feels like that would be a big workaround where we would in principal perform authentication of the request using a password that is passed in the request body which is rather strange for an API. Let me know what you think

@azasypkin
Copy link
Member

Thanks for explaining, I think I understand where it gets complicated for ES internally, but let me share my thoughts on that as well.

Option 1 feels like a workaround from the consumer (Kibana) point of view since we have a session with authentication information (Bearer: x-token-x) that we provide for every API action to authenticate the user. And if one particular API action has additional requirement (provide the proof of knowledge of the current password), changing the way we authenticate the user completely instead of satisfying this requirement through additional API parameter and enforcing it on the API action level feels a bit strange.

Is there a precedent maybe or I just look at it at the wrong angle?

Thinking through option 2, it feels like that would be a big workaround where we would in principal perform authentication of the request using a password that is passed in the request body which is rather strange for an API

I was thinking about it (I admit, very naively) as not authentication through request body parameter, but rather as enforcing of the API action specific requirement at the API action level 🙈

@jkakavas
Copy link
Member Author

jkakavas commented Nov 29, 2019

Option 1 feels like a workaround from the consumer (Kibana) point of view since we have a session with authentication information (Bearer: x-token-x) that we provide for every API action to authenticate the user.

Honestly, this is not about throwing the problem over the fence or about washing ES hands clean of a possible non-simple solution implementation. The requirement we're trying to impose from the API perspective with requiring proof of the current password is that the caller of the API can at this time authenticate with the username and current password. Instead of using a bearer token that they might got 10 days ago and keep refreshing since, or even worse found in client-side log 10 mins ago.

If Kibana was the only consumer of the API, I would think it would be fine if kibana guarantees that it will re-authenticate the user with the current password right before it issues a change password request and use that token as a Bearer token.

And if one particular API action has additional requirement (provide the proof of knowledge of the current password), changing the way we authenticate the user completely instead of satisfying this requirement through additional API parameter and enforcing it on the API action level feels a bit strange.

As such, I don't think that the current password is another API specific parameter in this case, but rather that an authentication based on the current password is required in order to call that API.

Is there a precedent maybe or I just look at it at the wrong angle?

This is a rather unique case so the only precedent I can think of is the fact that we don't allow requests to the change password API when authenticating with an API Key.

If you think it would be better to discuss this live, let's set up a short call for Monday and go over pros and cons of approaches and our thoughts about it !

@azasypkin
Copy link
Member

If you think it would be better to discuss this live, let's set up a short call for Monday and go over pros and cons of approaches and our thoughts about it !

I'm not sure if I agree with the reasoning completely but that's fine. If you all feel strongly about option 1, let's not spin our wheels here and see how this plays out 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.0.0-alpha1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants