silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
722 stars 821 forks source link

encryptWithUserSettings assumes there is a salt #11159

Closed lekoala closed 7 months ago

lekoala commented 7 months ago

Module version(s) affected

5.x

Description

I've encountered this issue while using https://github.com/emteknetnz/silverstripe-rest-api

My admin user doesn't have a password and no salt (but there is an PasswordEncryption field with a value)

If there is an attempt to encrypt with user settings (ie: an api token), the framework "assumes" there is a salt, while there is no guarantee there is one. Since php 8, this is breaking the blowfish encryption (a salt is required)

https://github.com/emteknetnz/silverstripe-rest-api/blob/924cd948c395f3fbf26c039af3f50f64a98b383d/src/Controllers/RestApiEndpoint.php#L153

Maybe it's a misuse of the method, but at any rate, I think that the framework should not assume anything: if the method is callable, it should work or throw a proper exception

How to reproduce

On a Member without a password, but with a PasswordEncryption Call $member->encryptWithUserSettings('somestring') You get a blowfish exception because it could not encrypt with an empty salt

Possible Solution

Additional Context

No response

Validations

PRs

GuySartorelli commented 7 months ago

PR merged. Will be automatically tagged by GitHub actions.