Shibboleth Users

Minor rewording in description.

Updated description to elaborate on need for this module and to provide some information about my own background.

Added link to review of another project.

Adding "PAReview: review bonus" tag.

Thanks for the review (and the encouragement), Ken. I've fixed the two spacing issues and re-pushed. I had checked everything out in PAReview already and fixed a lot of stuff, but it didn't find this. I'll run my projects through Coder as well in the future.

I had thought about using shib_user_form_user_register_form_alter specifically instead of shib_user_form_alter; the reason I haven't made that change is that I'm intending to further develop the module to add Shibboleth provision controls on user edit as well (per the @todo on line 37), which will require that the same alterations be made to user_profile_form. I decided it was better to implement hook_form_alter once and check the form ID, but I'd be interested to know if it's a more accepted practice to implement the form-specific hooks (and call a shared function that implements the actual form changes, perhaps).

Thanks, Klaus. I had already run my code through the online PAReview.sh and cleaned up all the things it found before tagging this as "needs review." I just checked again and it doesn't report any problems, but I'll get these changes in.

The next revision of this project will have more than five functions (I'm not sure how long the code will need to be, though) when I've implemented the provision-on-edit and de-provision logic and the module configuration settings. Should I perhaps get that done now, if it will maybe give you enough code to assess my merits as a contributor?

The current revision (just tagged beta1) adds

• Shibboleth authentication provisioning on user update as well as create.
• Shibboleth authentication de- provisioning available on user update.
• Module configuration to select which attribute to use as unique ID from Shibboleth: username, email address, or an EPPN synthesized from the username and a configurable domain.

Klaus, implementing these changes brought the module past 200 lines of code, with 11 functions.

Added a third project review link.

I've updated the project page and reviewed three more project applications, so I'm adding the "PAReview: review bonus" tag once more. Provided that my changes don't raise any new concerns I'm hoping that this more mature module now represents a significant enough piece of functionality and code to establish contrib credibility.

Reviewed three more project applications.

Thanks, Klaus. The permission setting was a good catch -- I've switched it to use the shib_auth module's 'administer shibboleth authentication', since this module's configuration settings are dependent on shib_auth's configuration. I've also made the change to shib_user_is_provisioned(), but I'm still storing the query results in variables because I'll need them in the future if I want to check for the inconsistent and unexpected case where one an account has somehow been provisioned in only one of the two necessary tables (I've left myself a @todo comment about that).

This update resolves all the issues I'm aware of so I'm switching the status back from "needs work" to "needs review". I don't suppose I should put it back to "reviewed & tested by the community" myself, even if it was there before).

Thanks again, Ken. I did write to bajnokk on February 16 and asked him to weigh in when he can. He's otherwise busy at present but said he thought he'd be able to take a look in a few days.

Thanks for taking a look. My rationale for putting this in a separate module was that it's something useful to the subset of shib_auth sites which a) have a mixture of shib and non-shib users, b) need admins to be able to create shib_auth-provisioned accounts before a user logs in, and c) need to be able to view and alter shib_auth provisioning on existing accounts. I'm not sure how large a subset of shib_auth users this is but I'm not certain it constitutes a majority. At the very least I believe Shibboleth Users needs to be optional secondary functionality, and having it in a separate module makes this easy.

Another factor which made me lean toward a separate module was the fact that this shib_auth issue doesn't seem to have found traction with the shib_auth development community. If Shibboleth Users were, for example, made an optional sub-module of shib_auth it would raise the complexity and maintenance burden of shib_auth in an area where that simply may not be welcome.

What I propose is to release Shibboleth Users as a separate contrib module and see whether people find it useful. If they do, once it matures a little more as a result of user feedback perhaps we could consider rolling it into a future release of shib_auth itself.

Thanks, bajnokk, for your thorough analysis, for your kind words and thoughtful comments, and for your support. I've learned a lot going through this process, and I've been gratified by the helpful response of the people here. I also look forward to collaborating and contributing more with this and other projects.

Names are just names, as you said, but they're hard to choose. My intended short name for the project has been "shib_user" (and that's what used in the code files) but I could imagine changing it to "shib_prov". Either way do you think I should change the descriptive name to "Shibboleth User Provisioning"? Does anyone want to weigh in on this before I promote this to a full project?

Maybe I should move discussion of your other points to a different venue, but since we're here: the challenge I have with pre-populating the IdP field is knowing what string the provider's going to send along. In the application I'm working on there's a consortium of five different institutions any one of whom could provide accepted identities. I was expecting that when that happens each institution's Shibboleth users would end up a different IdP string, but maybe that's not the case. So far I've only turned on shib_auth access for the home institution. Do you think it would be better for shib_user-provisioned accounts to have some other value in the IdP field?

I would appreciate more detail from you at some point about the intended semantics of shib_authmap.targeted_id — it doesn't sound like my use of this field is getting in the way of shib_auth yet, but I certainly want to avoid that in the future.

As for documentation, you are absolutely right. I was dragging my heels a bit while things were changing quickly in the code, but I'll make sure that's much more thorough before the module gets out of beta. I'm planning, by the way, to stay in beta until at least a few other people have tried it in their own environments, or failing that, at least until I've run it in production for for some months.

Thank you, Tim — I hadn't seen your post when I was wrote my #19 above.

Thank you all again. I've changed the title to "Shibboleth User Provisioning", assigned shib_provision as the short name, and promoted to this to a full project at http://drupal.org/project/shib_provision.