michaelryanmcneill / shibboleth

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

Provisionning of account without email information #17

Closed ghost closed 4 years ago

ghost commented 6 years ago

Hello,

One of the feature I had added to my fork was the handling of account without email https://github.com/devanonyme/shibboleth/commit/cedf49875751ca7f315a008e2c84104acbc958f7 (The comments in code are in french, sorry).

The problems I had technically was that the existing filter hook was not providing enough context information and also another field synchronisation bug that was fixed since, so I had added this :

$email_managed = isset($shib_headers['email']['managed']) ? $shib_headers['email']['managed'] : false; if ($force_update && !$email_managed) { $user_data['user_email'] = apply_filters('shibboleth_override_email', $user_data); }

And using this hook (always replace the email).

public function on_filter_shibboleth_override_email( $user_data ) {
    $urlparts    = parse_url( site_url() );
    $domain      = $urlparts[ 'host' ];
    // Define a fake email using current domain and user_login

    $email       = $user_data[ 'user_login' ] . '@' . $domain; 

    return $email;
}

It this something that could be considered an acceptable extension or maybe there would be a better way to handle this in a more generic way? ( I will have the same problem if we use other integration like ldap plugin (if other organisation want to integrate our site with another identity provider), so I'm not sure I can go around and ask for this.

Thinking about it, another solution would be to generate the fake emails higher up (in the ldap accounts) but considering we have hundreds of other systems integrated (that could have handled the things differently), that would require more thoughts...

Thanks for any input!

ghost commented 6 years ago

Checking again, maybe I could have used the simple solution discussed here (ie. ignore errors on dashboard account update) https://wordpress.stackexchange.com/questions/22754/user-without-email

michaelryanmcneill commented 6 years ago

I think that this use case makes sense. I will plan to implement a filter like you suggested. My thought would be to implement the filter only if the field is managed. I'll add this as an enhancement and try to get it to ship with 2.0.

ghost commented 6 years ago

Nice, thanks!

ghost commented 6 years ago

Hello,

While creating an account today I realised that it was the real user email not the anonymised ones that where visibles, thepatch I had in my previous version got lost in getting back to official repo and I didnt noticed it.

What would be missing to make this functionnality a adequate candidate for integration? It's really just 3 lines to be callled before wp_update_user.

Maybe I should do a pull request from current code base and we will move from there?

Eric

michaelryanmcneill commented 6 years ago

Yes, if you would like to submit a PR for a filter to address this issue, that'd be great.

ghost commented 6 years ago

It does look like I need to give a further look, it looks like the attribute (user_login) I was using doesnt exist in user_data anymore, I'm trying to figure out what have changed.

ghost commented 6 years ago

Ok. I think I had a faulty logic, I will try a quick solution for my installation and rethink this.

michaelryanmcneill commented 4 years ago

Since there has been limited interest in this specific use case, I'm going to close this out as wontfix. If someone is interested in implementing this functionality, please provide a proposed PR with an associated fix.

ghost commented 4 years ago

The issue was actually indirectly solved for us as I decided to generate fake email using user another unique key (just for the registration). It was good enought in our context (no emails and notifications to users from the WordPress plaform).

Thanks for the plugin!