mitcho / shibboleth

WordPress Shibboleth plugin
24 stars 23 forks source link

Problem with $force_update in shibboleth_update_user_data #28

Open ghost opened 7 years ago

ghost commented 7 years ago

I was asked to generate a semi-random (need to be unique) nickname on initial account creation and the implementation of Shibboleth seems to conflict with user_register hook.

I was able to work around this by using shibboleth_user_nickname but I was actually surprised to discover the $force_update flag in the fonction shibboleth_update_user_data.

2 observations :

1) You absolutely need to set a filed mapping value (in admin option) even if "managed" is check to false (otherwise it generate index error visible in debug).

2) wp_update_user($user_data) write over the value that is set by my user_register hook (not surprisingly as it's called after). Considering the value is not checked "managed" I would have assumed that it would have been handled as such even in account creation.

I would consider this as a bug but I dont see the use case of $force_update? Maybe it's just a missing check to skip over undefined mapping?

ghost commented 7 years ago

I stumbled on another problem that is more or less related. In our system, we have case where the email data would not be available at mapping time (the value is not empty, it simply doesnt exist and it crash).

I will probably patch it with something like this :

            if ( $force_update || $managed ) {
                    if (isset($_SERVER[$shib_headers[$header]['name']])) {
                            $filter = 'shibboleth_' . ( strpos($field, 'user_') === 0 ? '' : 'user_' ) . $field;
                            $user_data[$field] = apply_filters($filter, $_SERVER[$shib_headers[$header]['name']]);
                    }
            }
michaelryanmcneill commented 6 years ago

Hello, thank you for reporting this. I released version 1.8 today to resolve issue 1 above, as well as other issues. I am the new maintainer of the plugin and all further work on the plugin will be done in a new GitHub repository. Please review the latest updated and let me know if it resolves your issue. If it doesn't, please open up a issue in the new repository and reference this issue.