I've made a couple of minor changes to the legal module so that it works with the LDAP integration modules. So far it has only required the addition of two if statements, though there is still a buggy interaction with the ldap groups module. My main question is how can I contribute a patch? I checked out the patch suggestions for Drupal, but it seems to be way overkill for such a minor fix.

What I found is that the 'legal_user' hook would create a 'legal_accept' record for any 'insert' operations, as long as it was not the admin account doing the operation. The status of the T&C accept checkbox was never checked. This is not as strange sounding as you might think. Essentially the module was relying on the form's required attribute to ensure the checkbox was checked before the form could be submitted. That is fine so long as every 'insert' (or 'update' - there is a similar bug in that operation) follows a form with that required checkbox. However the ldapauth module creates accounts on the fly so long as the user authenticates successfully against an ldap server.

When the LDAP module creates the new Drupal account to match the LDAP record, it is done as an anonymous user, so the one test that the legal_user function uses to test if it is the admin account inserting the new account failed. The result was that the user was created with the T&C already accepted.

So the simple solution I came up with is to check that the form checkbox is checked before calling 'legal_save_accept' from the 'insert' or 'update' operation block of 'legal_users'. Anyone see a problem with it?

       <b>if ($_POST['edit']['legal_accept'] == 1) {
            legal_save_accept($account->uid, $conditions['tc_id']);
        }</b>

The problem I'm having with the LDAP Groups part of it probably also has a simple solution. The problem is that ldapgroups_user function is not being called during the initial login. During that initial login, the ldapauth.module creates the drupal account if the user authenticates against the LDAP server, the legal.module displays and requires the T&C agreement (with my patch), but the group is not being set properly. If the user logs out and then re logs in, the group is set properly. I presume the problem is caused by the way that the legal_accept destroys the current session until the user agrees to the T&C. Once there is a legal_accept record for that user, the login is no longer hijacked so the ldapgroups_user function is called and sets the groups appropriately. What isn't clear to me is why after accepting the T&C, doesn't the next login operation (which I thought happens automatically) doesn't run the ldapgroups_user hook. Anyone have any thoughts?

Comments

Robert Castelo’s picture

Thanks.

Interested in adding LDAP integration, but don't have a LDAP server to develop this against. So anyone who steps up to code and test this feature will be very welcome.

scafmac’s picture

I'm happy to keep plugging away at it. Any suggestions for either submitting patches or insight into the sequence of events after the legal module destroys the current session?

Robert Castelo’s picture

Version: 4.7.x-1.x-dev » 5.x-1.x-dev

Feel welcome to post just the code that you've added to make this work.

Inna Klimbovskaia’s picture

scafmac, thank you for the fix you provided, it works for me.
I also solved the "groups" problem (groups were removed every time the legal page was used during login) by removing call to user_module_invoke('login', $edit, $user); in legal_login_submit function.
Not a hundred percent sure why it works, but it solved the problem.

Robert Castelo’s picture

Status: Needs review » Closed (outdated)