-
Notifications
You must be signed in to change notification settings - Fork 57
User.search_ldap has Yale phonebook hard-coded into it #653
Comments
Good catch, let's definitely add this as an administrative setting. I'm wondering if it needs to be something our current admin role deals with; it seems to me like it's more of a production setting. Is it worth making some of these IT-level settings accessible to superusers via either a settings page or ActiveAdmin? We should definitely document this in our wiki either way. |
At the very least we should incorporate it into |
(sorry, mouse jumped and closed this issue by mistake) We should also per #2 consider situations in which LDAP service isn't available. Large institutions like universities are likely to have it, but we shouldn't code our app in such a way that a smaller institution can't use it. Granted, it's a fairly moot point until we enable non-CAS login in #2, because small orgs are less likely to use an industrial CAS service, but we should consider making LDAP a soft (optional) dependency of the app, part of |
This is necessary for #399, at least an initial implementation. Moving to the Wish List for now. |
This is actually critical to exporting the app, moving to 4.0.1. |
Let's store the relevant parameters in an initializer / |
I'm actually merging this into #683 since it already includes a lot of the necessary code reorganization. |
resolved via #1048 |
Status quo: whatever CAS login gets looked up in the Yale phonebook, as it's hard-coded in the method.
Expected behavior: app_setup / AppConfig should allow the user to configure their own LDAP, or skip it entirely if the institution lacks LDAP. At the very least, we should list this behavior in documentation, so that any adopter who is not Yale-based will know to change the code in their instance.
Making the code more modular is relevant to #2.
The text was updated successfully, but these errors were encountered: