[D7] LDAP Login

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxrafaelferreir4274103...

Fixed the git clone URL in the issue summary for non-maintainer users.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Automated Review

http://pareview.sh/pareview/httpsgitdrupalorgsandboxrafaelferreir42741035git

Note that perfect adherence to Drupal Coding Standard is NOT a reason to block an application, except for total disregard of them. However, modules should follow them as closely as possible.

Manual Review

Individual user account

[Yes: Follows ] the guidelines for individual user accounts.

No duplication

[Yes: Does not cause] module duplication and/or fragmentation.

Master Branch

[Yes: Follows ] the guidelines for master branch.

Licensing

[Yes: Follows] the licensing requirements.

3rd party assets/code

[Yes: Follows ] the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes: Follows ] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[Yes: Follows ] the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements.

Coding style & Drupal API usage

This review uses the Project Application Review Template.

Manual review:
* Variables set by the admin form should be deleted on uninstall.

Module duplication

What is the use case for this module versus the ldap module? I get that it's a lot smaller but if I want to authenticate with an LDAP server or create users that don't exist in Drupal yet I can already do that pretty quickly in that module.

• info file: package should be removed, see "In general, this property should only be used by large multi-module packages, or by modules meant to extend these packages, such as Fields, Views, Commerce, Organic Groups, and the like." from https://www.drupal.org/node/542202#package
• README.md is probably not what you want when the markdown is rendered.
  - The numbered lists does not render as sub-lists (eg: 1, 1.1, 1.1.1 are all on one line).
  - Headings like "For Developers" and "Similar modules:" could be prefixed with one or more '#' to render as varying sizes of heading text.

ldap_login.module
_ldap_login_validate_ldap_user()

    // Allow other modules to modify the user entity in case of custom fields.
    if (count(module_implements('ldap_login_user_alter')) > 0) {
      // Call all modules that implement the hook,
      // and let them make changes to $drupal_user.
      $drupal_user = module_invoke_all('ldap_login_user_alter', $drupal_user);
    }
    

It's not necessary to wrap this in count(module_implements(...)).

    // TODO: remove this mail creation once all the users from LDAP have the respective email.
    $user_mail = isset($ldap_user->mail) ? $ldap_user->mail : rand('1', '1000') . '@drupalized.co';

* Can you explain the random-number@drupalized.co thing or remove it if it's no longer necessary? $user_mail is used to load the user and is passed in to user_save() and I don't think that a random email would be a desirable thing.

_ldap_login_decode_objectsid()
Is this function named appropriately? It's called decode_objectsid but returns a hash and that's not what I'd think of as "decoded".
This function also appears to be unused - is it stil necessary?

Could you also compare this module to Simple LDAP?

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.