| Project: | LDAP integration |
| Version: | master |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | ldapgroups |
Issue Summary
This patch does 3 things. I can separate them into 3 patches if you like. Just tell me which functionality you want.
1). Performance: Changes ldapgroups.php to include ldapgroups.inc such that ldap groups code is only parsed when used during login; not on every drupal page. Also moved $ldapgroups_mappings = _ldapgroups_ldap_info($account, 'ldapgroups_mappings'); to a single location.
2). Security. I change the eval to use the php.module functionality and respect whether it was enabled or not. A common practice is just to remove the php.module folder altogether on sites for security practices. This is just a best practices adjustment.
3). I added some validation and additional directions to the ldapgroups_filter_php field on the ldap groups admin page.
| Attachment | Size |
|---|---|
| ldap_groups_php_and_perf.patch | 27.43 KB |
Comments
#1
Below is the patch without the change in the file structure. It better illustrates the changes:
#2
I agree on moving code to a separate file and load it on demand. This was and still is om my todo list for the next refactoring of ldap_intagration bundle.
However, I don't see a reason of bringing in a dependency of the php module. It is sufficient to change eval() to drupal_eval() or am I missing anything?
#3
I think drupal_eval() will do it. I misunderstood the convention; I thought there was a way to disable execution of php code stored in the database.
I'll submit this as 3 patches:
- validation addtions
- drupal_eval()
- moving of $ldapgroups_mappings
John
#4
Moved most of the functions to the separate inc file and substituted eval with the drupal_eval.
#5
Automatically closed -- issue fixed for 2 weeks with no activity.