auth: Blank Shibboleth role means use existing WordPress access#37
Conversation
|
@michaelryanmcneill If you have any questions, please ask. I feel pretty good about the code, but I'd like a critical second opinion. |
|
I want to make sure that we still support the constants. Can you add that support in? I'll get it merged once that happens. |
michaelryanmcneill
left a comment
michaelryanmcneill
left a comment
There was a problem hiding this comment.
We need to support constants for the default role, if possible. Can you add that in?
|
Ok, I've added the check for SHIBBOLETH_DEFAULT_ROLE. It looks like the readme and admin-options already have this constant in place. Do you want me to modify this sentence?
It might be fine as is, but I get the feeling that "no role" is more accurate than "no access". |
|
Sorry for the delay, I've just been swamped. Trying to get eyes on this to get a release moving. |
2 participants
shibboleth_get_user_role()is currently used for three things:The third use is not valid.
shibboleth_create_new_user()already checks whether accounts can be created.shibboleth_authenticate_user()already checks to make sure an account exists. And most importantly, if the user account exists and is already tied to Shibboleth, our job as an authentication mechanism is to map the user to the user's account. Authorization should be done by WordPress based on roles that have been assigned to the user.With this change in place, a couple things start working:
shibboleth_create_accountsis disabled andshibboleth_update_rolesis enabled, the default role is correctly set.shibboleth_create_accountsis enabled andshibboleth_default_roleis blank, users can log in. (Fixes Blank shibboleth_default_role prevents login when shibboleth_create_accounts is enabled #36)