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

Update plugin ldap-mail-accounts: use SensitiveString for passwords #1492

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

cm-schl
Copy link

@cm-schl cm-schl commented Mar 13, 2024

This updates the plugin ldap-mail-accounts to use the new SensitiveString class introduced with Snappymail v2.30.0

uses the new SensitiveString class introduced with Snappymail v2.30.0
@cm-schl
Copy link
Author

cm-schl commented Mar 14, 2024

@the-djmaze a doubt: the ldap-mail-accounts plugin needs to store a password for authentication against the ldap server.
Now as there exists SensitiveString class is there any need to use this also for password fields inside the plugin configuration?

@the-djmaze the-djmaze added duplicate This issue or pull request already exists enhancement New feature or request and removed duplicate This issue or pull request already exists labels Mar 14, 2024
@the-djmaze
Copy link
Owner

the-djmaze commented Mar 14, 2024

Now as there exists SensitiveString class is there any need to use this also for password fields inside the plugin configuration?

Currently: no
The SensitiveString class is not for storage, it does:

  1. make sure the data in memory is scrambled
  2. any (php) error log will show scrambled data

Mainly because of PHP < 8.2 not supporting #[\SensitiveParameter]

In the future this might change in some way

@the-djmaze the-djmaze merged commit a0e7bf7 into the-djmaze:master Mar 14, 2024
1 check passed
@cm-schl
Copy link
Author

cm-schl commented Mar 15, 2024

Currently: yes

So you mean "currently: no", right? 😊

@the-djmaze
Copy link
Owner

Right 😉

the-djmaze pushed a commit that referenced this pull request Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants