Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Support dynamic salt lengths in ISPConfig password driver #2150

Closed
wants to merge 2 commits into from
Closed

Support dynamic salt lengths in ISPConfig password driver #2150

wants to merge 2 commits into from

Conversation

sguter90
Copy link

@sguter90 sguter90 commented Dec 23, 2021

This fixes the "CouldNotSaveNewPassword" error when changing password via settings for mailboxes created/updated in newer ISPConfig versions.

Additionally:

  • Splits logic into smaller methods
  • Adds log method to avoid same ifs
  • Adds more log output in case of errors
  • Updates cryptPassword to behave the same as in ISPConfig (more secure salt generation)
  • Fixes some code styles

This should fix:
#2072
#1946

Should include all fixes according ISPConfig solved in some other way in:
#2095

This still doesn't add support for multi server environments:
#733

I implemented the code with a PHP language level of 7.0. Everything above is supported.
Wasn't sure if there are any style rules but I did my best to match with the style used in other plugins.

I tested the code with a local environment and on production servers.
The password salt detection is backwards compatible with old passwords using a fixed salt length.
So this shouldn't have any side effects on existing installations.

This fixes the "CouldNotSaveNewPassword" error when changing password via settings for mailbox created/updated in newer ISPConfig versions.

Additionally:
* Splits logic into smaller methods
* Adds log method to avoid same ifs
* Adds more log output in case of errors
* Updates cryptPassword to behave the same as in ISPConfig (more secure salt generation)
* Fixes some code styles
the-djmaze pushed a commit to the-djmaze/snappymail that referenced this pull request Mar 10, 2022
@lol-deb
Copy link

lol-deb commented Mar 30, 2022

Hello,
I just wanted to confirm that the patch works for me.
Debian Bullseye - ISPConfig 3.2.8p1 - Rainloop 1.16.0
It solved #2072
Thank you.

@sguter90 sguter90 closed this by deleting the head repository May 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants