Auto create account with "none" as default role should deny access to unmapped users

[Alhrath]

As I understand when I read the config page in "If there is no default role, the user will not be able to log in with Shibboleth.", unmapped roles should deny access when default role is set to "None". But it's not the case : an account is still created with no role and the user is logged in, which can be a security issue in some structures, like ours.

To solve this, I temporarily edited the code in the function shibboleth_create_new_user, moving "$user_role = shibboleth_get_user_role();" from line 636 to line 617 and adding " || empty( $user_role )" to the condition at line 619.

Is it possible to implement a fix in the next release ? I actually use the latest version (2.1.1).

[jrchamp]

The config page does need to be updated. Currently, the code does:

"If there is no default role, the user will not be assigned a role when creating an account with Shibboleth."

It sounds like you want:

"If there is no default role, the user will not be able to create an account with Shibboleth."

Such a change would mean:

• If you want users to be able to create accounts on their own, you must have an attribute with a known, static value to check.
• It is no longer possible to create a no-role account with Shibboleth.
• If you do not have an attribute to check (e.g. if you only receive the targeted-id) then the user managing the WordPress site/network must must both create the user accounts and assign roles to users.

Philosophically, Shibboleth is primarily about authentication, not authorization. In that sense, using the WordPress authorization functions is the appropriate security mechanism. The security guarantee that Shibboleth gives you is that the user being created has logged in with Shibboleth.

I like part of this change, but I would want it to be optional to avoid forcing a large maintenance burden on our users who have less control over the attributes they receive.

[michaelryanmcneill]

I agree with @jrchamp that I'd welcome this change as so long as it is optional. Adding this as a enhancement, but would welcome PRs.

[jrchamp]

This came up again here: https://wordpress.org/support/topic/clarification-of-default-role-none-accounts-still-created-with-no-role-mapping/

My idea that aligns with the discussion here is: Modify shibboleth_create_new_user() so that when the result of shibboleth_get_user_role() is blank, the user create is dependent upon a new setting, such as shibboleth_allow_user_create_without_role (which would default to true so as to not break backward compatibility)

I'm open to other suggestions.

[Alhrath]

This solution sound good for our use case.

[dandalpiaz]

I created a PR which I think addresses this issue #59

I added an additional option to the "Default Role" drop-down, which allows "Do NOT created an account" to be selected. I kept "(none)" as option to still allow no-role accounts to be created.

Any feedback welcome!

[jrchamp]

Thank you @dandalpiaz for getting this moving!

@Alhrath, if you can, please try the version that @dandalpiaz provided and let us know if this solution behaves as expected.

[indrek-k]

Hi everyone!

@jrchamp what's the status on this issue? Can I help in any way to get this moving?

[jrchamp]

Hi @indrek-k! @dandalpiaz has what I believe to be a fully working solution in Pull Request #59. At this time, I believe only @michaelryanmcneill has the ability to merge code and create a new release version. To make it easier for @michaelryanmcneill to review, it helps to have a myriad of users of the plugin try out proposed changes and provide feedback on whether it works and how well it works. The more positive results helps build confidence for the maintainer (who may not have the same needs or ability to dedicate time as those of us who are directly affected).

[indrek-k]

Got it, thank you. I will test the PR and report back.

[Alhrath]

Hi all, thank you for your work. I just tested the PR on our test server and I confirm it behave like intended.

[michaelryanmcneill]

This will be released in 2.2. Sorry for the delay and thanks for the PR @dandalpiaz!