CVS edit link for abulte

Contribution of a cas_ldap module, as discussed in this thread : http://drupal.org/node/533762. We developed the module for internal needs and we want to share it with the community.
CommentFileSizeAuthor
#7 ldap_cas-0.3.zip4.02 KBabulte
#3 ldap_cas-0.2.zip2.6 KBabulte
#1 ldap_cas.zip2.59 KBabulte

Comments

abulte’s picture

abulte commented
Assigned: Unassigned » abulte
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new ldap_cas.zip2.59 KB

Attached is an alpha version of the ldap_cas module.

abulte’s picture

abulte commented
Assigned: abulte » Unassigned
abulte’s picture

abulte commented
StatusFileSize
new ldap_cas-0.2.zip2.6 KB

New version, with dependencies and an annoying white line removed.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work
Issue tags: +Module review

Hello, and thanks for applying for a CVS account.

As per requirements, the motivation message needs to be expanded to include more details about the features of the proposed module, and a comparison with the existing projects.
As you took the code from an issue queue of an existing project, what would happen if the maintainer of that project decides to accept the patch, and create the new sub-module?

abulte’s picture

abulte commented

Hello and thanks for replying.

The features of the module are :
- implements the hooks that could require ldap functionality defined in cas module
- call ldap_authentication functions in those hooks in order to perform ldap stuff

I didn't take the code from an issue queue of an existing project, I used the ideas provided in it.

I'm pretty convinced the new module does not belong in any of the two other module (ldap_integration or cas). As a matter of fact, CAS functionality should not care if there is LDAP involved and LDAP functionalities should not care if there is CAS involved. Further more, the maintainers (at least the one from CAS) may prefer not maintaining something they don't really need or master. That's why I think the idea of a "link" module is the best.

abulte’s picture

abulte commented
Status: Needs work » Needs review
abulte’s picture

abulte commented
StatusFileSize
new ldap_cas-0.3.zip4.02 KB

New version attached. Mostly code cleaning and documentation.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno
Status: Needs review » Fixed 
    if (LDAPAUTH_LOGIN_PROCESS == LDAPAUTH_AUTH_MIXED && LDAPAUTH_SYNCHRONIZE_PASSWORDS)

What is the purpose of comparing two constants?

    if (($code = _ldapauth_ldap_info($row->sid, 'filter_php')) && !eval($code))
      continue;

The code should written as 

  if (($code = _ldapauth_ldap_info($row->sid, 'filter_php')) && !eval($code)) {
    continue;
  }

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

dgeo’s picture

dgeo commented

Thanks to abulte for working on this... Is there any news for those trying to use CAS *and* LDAP together ?

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: Miscellaneous » new project application
Issue summary: View changes
Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.