-
-
Notifications
You must be signed in to change notification settings - Fork 204
Allow admin to manage password hash directely #1876
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're looking to be able to update hash values without the API re-hashing the existing hash. However, by changing the hash to not run if a value is provided, you're effectively breaking the hash type fields altogether..
There is no real solution to this, as we can't have it both hash, and not-hash at the same time..
I think the only way forwards would be to introduce some sort of flag (in a query param?) that indicates that you want to skip the hashing step, because you're already providing a hash yourself.
if (is_array($value) && isset($value['hashed'])) { | ||
$payload->set($key, $value['hashed']); | ||
continue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This'll break regular hash
type fields. They way they work right now is to hash the raw value on creation. By not-hashing it when a value already exists, we'll effectively break hash-type support.
@@ -1761,6 +1761,7 @@ protected function validateRecordArray(array $record) | |||
($field && is_array($columnValue) | |||
&& (!DataTypes::isJson($field->getType()) | |||
&& !DataTypes::isArray($field->getType()) | |||
&& !(DataTypes::isHashType($field->getType()) && isset($columnValue['hashed'])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this checking for the value of the column being the string hashed
?
if ($acl->isAdmin() && isset($payload['password_hashed'])) { | ||
$payload['password'] = [ 'hashed' => $payload['password_hashed']]; | ||
unset($payload['password_hashed']); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not add in a non-standard field like this 🤔
Returning the |
The rest is being read, but I'm running out of time :) |
Allow administrateur to get and set password hash directely on other user.
It will usefull to allow sync user between many project.