Posted by rj on May 30, 2009 at 7:33am
| Project: | Salesforce Suite |
| Version: | 6.x-2.x-dev |
| Component: | salesforce_api |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Currently, salesforce_api_password & salesforce_api_token are currently saved as plain text passwords in the variable table. Is it possible to hash those values?
Comments
#1
Actually, I was just about to request this hashing myself (preferably a strong/secure hash). I imagine this is a real quickie feature request, best done by JoshK/Aaron/Bevan.
However, if not, I could have a different "credential encryption module" (that I had produced for the Ubercart Authorize.net CIM module) re-purposed into an add-on for this module. This add-on module would be called "Salesforce API Encryption" or something. Having a whole separate little module for this feature might be overkill-- but if you decide you want this help, I will have it done asap.
#2
Patches welcome as well as integration with other modules providing this API
#3
In this patch:
salesforce_api_encrypt,salesforce_api_decrypt, andsalesforce_api_encryption_availableto abstract AES dependency and make pluggable encryption easier in the future.Notes:
#4
Updated patch to current dev (2010-Jun-09) code and README.txt styling. Works as described.
#5
Fixed some trailing spaces and formatting.
#6
#7
Subscribing...maybe will test/re-roll in next few weeks.
#8
The patch no longer applies cleanly to salesforce_api.admin.inc, so the variables aren't getting encrypted. I'll try to work on this soon.
In hook_requirements, line 175:
<?phpelseif (!salesforce_api_encryption_available(array('check_config' => FALSE))) {
$description = t('Encryption is unavailable. Using encryption is <strong>highly recommended</strong> in order to better secure your data.');
$severity = REQUIREMENT_ERROR;
}
?>
If we are requiring encryption to use the Salesforce module, then the wording should change to "Using encryption is required" (rather than just highly recommended).
Personally, I would prefer to change the severity to REQUIREMENT_WARNING, because I don't think this should be a requirement for using the module.
#9
@kostajh: Sounds good (in re: making it a warning).
#10
Attached patch applies cleanly against 6.x-2.x-dev, and changes the severity level to a warning (instead of error) if encryption is unavailable.
I tested and it works great.
#11
OK, this is in.
Thanks for contribs and testing.
related: another advantage of #990700: Support the new REST API would be that credentials wouldn't be stored on the server at all, only the REST auth token.
#12
Automatically closed -- issue fixed for 2 weeks with no activity.