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

Event Request on Changing Password #328

Closed
KiprotichKelly opened this issue Nov 7, 2023 · 9 comments · Fixed by #714
Closed

Event Request on Changing Password #328

KiprotichKelly opened this issue Nov 7, 2023 · 9 comments · Fixed by #714
Assignees
Labels
Approved The issue is approved

Comments

@KiprotichKelly
Copy link

🕮 Describe the issue
I would like to add an event that allows you to implement password policy on old and new password while changing your user passwords

🔧 To Reproduce

The current code only has only one integration event to modify password minimum length only

📣 Expected behavior

The current code works fine, this is an additional feature. You can then check that users don't reenter their old passwords asthe new password

Screenshots
image

Additional context

Add any other context about the problem here.

@JesperSchulz
Copy link
Contributor

Is the only ask to add the rule "You can then check that users don't reenter their old passwords as the new password"? Question is if we need an event, or if you should just add that rule to the base product. It's a common security best practice.
Adding an event here has security impact.
Also: moving to BCApps, which is the new repository for System Application related asks.

@JesperSchulz JesperSchulz transferred this issue from microsoft/ALAppExtensions Nov 13, 2023
@JesperSchulz JesperSchulz added the Follow Up The issue has an open question and must be followed up on label Nov 14, 2023
@JesperSchulz
Copy link
Contributor

@KiprotichKelly, could you look at the question above. Do you still need that event?

@KiprotichKelly
Copy link
Author

Hello @JesperSchulz, having mentioned the security concern, I think adding the rule to the base app will do for me.

@JesperSchulz JesperSchulz added Approved The issue is approved and removed Follow Up The issue has an open question and must be followed up on labels Nov 28, 2023
@JesperSchulz
Copy link
Contributor

JesperSchulz commented Nov 28, 2023

Based on the above, I will approve this issue for contribution :-)
Feel free to create a PR which adds the password rule that a user may not reuse the old password as the new one.

@Drakonian
Copy link
Contributor

Since this is still here, can I create a PR?

@JesperSchulz

@JesperSchulz
Copy link
Contributor

Absolutely 😊

@Drakonian
Copy link
Contributor

@JesperSchulz

Just wondering, in the current implementation the method takes two parameters OldPassword and Password. It is clear about Password, it is a new password that the user sets, but about OldPassword it is not so clear, this field always returns what the user fills in the Old Password field, but it is never set from outside, that is, this field is always empty when opening the password change dialog.

procedure OpenChangePasswordDialog(var OldPassword: SecretText; var Password: SecretText)

Which kind of implies that validation of whether the old password was entered correctly is done externally, in the place where this module is called.

Therefore, in order to implement validation inside PasswordDialogManagement it is necessary to pass OldPassword to the Password Dialog page. In such a scenario, the one who uses the OpenChangePasswordDialog method passes OldPassword with the value of the old password from the very beginning, at the moment it is just a Return Value for what the user will enter in the OldPassword field.

Do you agree with this addition?

@Drakonian
Copy link
Contributor

Drakonian commented Mar 7, 2024

@JesperSchulz in this case it will be two rules

1.Passed OldPassword same as what user fill to OldPassword field in Change Password dialog
2.New password is not equal to OldPassword

@JesperSchulz
Copy link
Contributor

@JesperSchulz in this case it will be two rules

1.Passed OldPassword same as what user fill to OldPassword field in Change Password dialog 2.New password is not equal to OldPassword

That's how I understand this issue, yes 😊

Drakonian added a commit to Drakonian/BCApps that referenced this issue Mar 9, 2024
…ch the OldPassword

Also in this case we check that the old password matches the entered old password microsoft#328
Drakonian added a commit to Drakonian/BCApps that referenced this issue Mar 11, 2024
Drakonian added a commit to Drakonian/BCApps that referenced this issue Mar 12, 2024
Drakonian added a commit to Drakonian/BCApps that referenced this issue Mar 25, 2024
JesperSchulz added a commit that referenced this issue May 24, 2024
<!-- Thank you for submitting a Pull Request. If you're new to
contributing to BCApps please read our pull request guideline below
* https://github.com/microsoft/BCApps/Contributing.md
-->
#### Summary <!-- Provide a general summary of your changes -->

If OldPassword is passed, we check that the new password should match
the OldPassword
Also in this case we check that the old password matches the entered old
password
New test

#### Work Item(s) <!-- Add the issue number here after the #. The issue
needs to be open and approved. Submitting PRs with no linked issues or
unapproved issues is highly discouraged. -->
Fixes #328 





Fixes
[AB#506715](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/506715)

---------

Co-authored-by: Jesper Schulz-Wedde <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved The issue is approved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants