SAML-Toolkits / wordpress-saml

OneLogin SAML plugin for Wordpress
MIT License
65 stars 75 forks source link

Authorization feature ... why is it missing ? #110

Closed becchett closed 3 years ago

becchett commented 3 years ago

Dear All, I've used this plugin for some weeks with the last Wordpress release. After some time I managed to link my website (Service Provider) to an IDP from my organization. I had also added some settings to allow the correct relation between the wordpress role and user profile. This can useful at the first login when wordpress creates new user automatically if it dosen't exist.

At the moment I'm using the "Regular expression for multiple role values" to match and extract the right role.

It seems that it always works fine but with this plugin I'm not forwarded to an authorization step.

In my case, after IDP identification I need to check if that user has got the authorization to login because not all users are allowed to access to my web site.

So I'd like to suggest a new feature: an authorization field that can be inserted to limit who can enter.

Actually I have made some test with your source PHP code. This is an easy example where I’ll show you my two changes. In this case I use email address to match authorization pattern.

The first change is to configuration.php:

-> from row number 239:

function plugin_setting_string_onelogin_saml_attr_authorization($network = false) { $value = $network ? get_site_option('onelogin_saml_attr_authorization') : get_option('onelogin_saml_attr_authorization'); echo '<input type="text" name="onelogin_saml_attr_authorization" id="onelogin_saml_attr_authorization" value= "'.esc_html($value).'" size="30">'; }

-> from row number 819:

    'onelogin_saml_attr_authorization' =>  array(
                     __('Authorization string', 'onelogin-saml-sso'),
                     'string'
             ) 

The second is to the function.php:

-> from row number 365:

            $idp_authorization = get_option('onelogin_saml_attr_authorization');
            if ( (!preg_match("$idp_authorization",$email)) ) {
                    echo __("The username provided by the IdP"). ' "'. esc_attr($username). '" '. __("hasn't got a valid mail address ".$email." ");
                    exit();
            }

-> from row number 414:

            $idp_authorization = get_option('onelogin_saml_attr_authorization');
            if ( (!preg_match("$idp_authorization",$email)) ) {
                    echo __("The username provided by the IdP"). ' "'. esc_attr($username). '" '. __("hasn't got a valid mail address ".$email." and can't the user at wordpress");
                    exit();
            }

This is just an example but If you agree with my Idea you could add the changes into the next release of onelogin.

Let me know what you think.

Very Best Regards Enrico

pitbulk commented 3 years ago

Not sure what have you included in this PR, I see 205 commits.

Most of the IdPs, like OneLogin, implements the authorization protection on its side so a valid SAMLResponse can't be generated at all to a user that does not have permission.

That said, I think its ok if you want to add an extra authorization process to the code.

You could enable the onelogin_saml_trigger_login_hook parameter so you could implement a hook for the wp_login method adding the authorization protection.

becchett commented 3 years ago

First thing, let me apologize for my incorrect use of Github. I understand now that I’ve opened an unwanted thread.

I just meant to give my contribution, that is, to offer my idea and suggest a code to achieve it: the program I’ve added was just an example.

What you say about the one authorization feature from IDPs is right, but bear in mind that this doesn’t always happen in other cases and, given that Onelogin SAML SSO is so versatile I was thinking that this new feature, optionally activated by the user, would be useful to many people and appliable to many websites.

I didn’t know the solution you suggest, of achieving this directly through a Wordpress hook, because essentially, I’m not so familiar with the whole WP software, but I think that, even though it’s the best procedure, it’s not easy for everybody.

It requires a software developer’s skills and a quite accurate knowledge of this content management system: not everybody has these, so not everybody will easily reach it.

For this reason I’m suggesting to integrate an authorization step inside this plugin and to add new fields, the first is “enable”, the second is which attribute received from IDP must be used, the third field is which pattern matches the attribute’s value to allow user login.

Very Best Regards Enrico