Add setting to require existing password when changing password

Sytten commented 2 years ago

Preflight checklist

[X] I could not find a solution in the existing issues, docs, nor discussions.
[X] I agree to follow this project's Code of Conduct.
[X] I have read and am following this repository's Contribution Guidelines.
[ ] This issue affects my Ory Cloud project.
[ ] I have joined the Ory Community Slack.
[ ] I am signed up to the Ory Security Patch Newsletter.

Describe your problem

This is a follow-up to https://github.com/ory/kratos/discussions/1971 since I don't think the solution is enough for certain uses cases (including regulated industries). The privileged session is a good idea, but since it's not a manual operation from the user (aka they get a privileged session automatically on login), this means that a client side vulnerability can still trigger a password change and an account take over.

Further more, since you get a privilege session from a social login (most of which won't ask for a password on login if the user is already logged-in with the provider), it is possible to change the password of a user without ever knowing any password of the targeted user.

Describe your ideal solution

Adding a setting to require the current password for this password change flow mitigates the issue entirely.

Workarounds or alternatives

None at the moment.

Version

0.10.1

Additional Context

No response

alantbarlow commented 2 years ago

I agree with this, there should be a way to require password to update any of the user details. If this feature is enabled, you can have a self-service page that lets the customer update their info but only be able to save the changes if the password is correct. It could even be implemented in one of a couple ways...

asking for current password after "saving" the changes (but before persisting the changes) as a way of verification.
adding a current password field that activates the "save changes" button, then it could check the password for a match before persisting the changes.

This feature should be completely optional as it would not really be needed if going the passwordless route.

vinckr commented 2 years ago

You can change the Authenticator Assurance Level required for settings/profile updates.

So if you set the aal level to strict, users dont get a privileged session from login automatically - they have to use a second factor.

client side vulnerability can still trigger a password change and an account take over.

This is prevented when you use a second factor - as are many other attack vectors.

If security is the motivation for this feature, I don't really see how entering the password (again) would be an improvement over using a second factor - but open to have my mind changed :)

alantbarlow commented 2 years ago

it sounds like the ability to change the aal level to prevent the automatic privileged session could solve the issue. The scenario @Sytten mentioned of someone maliciously hacking your account trough the use of a connected social login method, can also be solved by including a second factor for authentication.

But this brings up two questions.

If you don't want to require a second factor for login, is it possible to only require the second factor when the user does actions that need privileged sessions (like when modifying account info or adding/removing login methods)?
if the user has multiple login methods assigned to their account, is it possible to require them to use a different method than the one they used to sign-in to grant a privileged session?

here is an example scenario... A company requires email/password sign-up and doesn't want to bother the user to use a second factor to login. Instead, the user is prompted to connect their social providers as additional options to login and as a security precaution to prevent account takeover.

In this scenario, if the user setup an additional login method and someone hacked their account by way of stealing their password. is it possible for the system to know what login method was used for the session and require an alternative method before granting a privileged session?

github-actions[bot] commented 1 year ago

Hello contributors!

I am marking this issue as stale as it has not received any engagement from the community or maintainers for a year. That does not imply that the issue has no merit! If you feel strongly about this issue

open a PR referencing and resolving the issue;
leave a comment on it and discuss ideas on how you could contribute towards resolving it;
leave a comment and describe in detail why this issue is critical for your use case;
open a new issue with updated details and a plan for resolving the issue.

Throughout its lifetime, Ory has received over 10.000 issues and PRs. To sustain that growth, we need to prioritize and focus on issues that are important to the community. A good indication of importance, and thus priority, is activity on a topic.

Unfortunately, burnout has become a topic of concern amongst open-source projects.

It can lead to severe personal and health issues as well as opening catastrophic attack vectors.

The motivation for this automation is to help prioritize issues in the backlog and not ignore, reject, or belittle anyone.

If this issue was marked as stale erroneously you can exempt it by adding the backlog label, assigning someone, or setting a milestone for it.

Thank you for your understanding and to anyone who participated in the conversation! And as written above, please do participate in the conversation if this topic is important to you!

Thank you 🙏✌️

Sytten commented 1 year ago

Not stale

Sytten commented 8 months ago

@vinckr While I agree that in theory the aal is code without https://github.com/ory/kratos/issues/3668 I don't see how we would use it. Because we don't want to mandate all users to setup a 2FA.

ory / kratos