search

michaelryanmcneill / shibboleth

Shibboleth plugin for WordPress
https://wordpress.org/plugins/shibboleth/
19 stars 11 forks source link

Possible problem with Yoast SEO #16

Closed ghost closed 6 years ago

ghost commented 6 years ago

Hello,

I tentatively report this issue that I'm still investigating (and I still use the code from my fork as I didnt had the time yet to see if I could get back to the official repo).

So, Yoast in lastest version seems to have added new roles, from the readme.txt

Since this update I get a crash in Shibboleth

Notice: Undefined index: wpseo_manager in /var/www/html/wp-content/plugins/shibboleth/shibboleth.php

The problem occur in shibboleth_get_user_role, $wp_roles contains the new wpseo_manager role but it's not defined in $shib_roles.

$role_header = $shib_roles[$key]['header']; $role_value = $shib_roles[$key]['value'];

Could be a plugin order activation issue (if shibboleth is registered before yoast?).

I will try to reproduce the issue in a vanilla Wordpress with only yoast and shibboleth.

To my knowledge there is no easy way to changer plugin activation order...

ghost commented 6 years ago

I did a quick fix by adding a isset so that I can at least get to admin without having to desactivate plugins.

ghost commented 6 years ago

Oh, I get it.

The roles are saved in a option, I saw the new roles in the shibboleth settings panel and thought about saving them again, which also resolved the issue.

So the question is how should the plugin detect / handle such changes a bit more gracefully...

Maybe the isset check would be enough?

jrchamp commented 6 years ago

Yeah, isset would be fine. It's doing an empty check later already, but that's too late for the the PHP Notice.

@michaelryanmcneill already fixed that in 1.9-alpha.

michaelryanmcneill commented 6 years ago

Yep, I believe this should be fixed when 2.0 gets submitted to the repo (you can take a look at the code, it is in the 1.9-alpha branch). I've just got a few more kinks that I need to work out before I can release it. In addition, I plan to create some documentation to add configuration information. I'm hoping to have that released by the middle of October.

ghost commented 6 years ago

Nice, I was not expecting such active work. I'll do my best to get back to the common repo as soon as possible and contribute to the effort.

michaelryanmcneill commented 6 years ago

This should be resolved by the 2.0 release that will be released either today or tomorrow. Closing this issue.