SAML user autocreation can become impossible

[Simon Story]

It is possible to put yourself in a situation where users having users auto-created by an authentication plugin is impossible.

By design, for auto-creation to happen, all institutions must be registerallowed = 0 .

By design, when an authentication plugin is added to an institution, registerallowed is set to 0. But it is not set for all institutions, if multiple exist.

Once an authentication plugin is added to an institution, via the web interface the control to toggle registerallowed for an institution is hidden.

To reproduce from a fresh installation of Mahara:
Create an institution
Set config item usersuniquebyusername = 1
Add and configure an authentication plugin
Attempt to login with with a new user that should autocreate, which will fail because the 'mahara' institution will still have registerallowed = 1

To workaround:
Connect to the database and set registerallowed = 0 for all institutions, eg 'UPDATE institution set registerallowed = 0 ;'.

[Kristina Hoeppner (kris-hoeppner)]

Can you please be more specific about which authentication method you talk? Is it the internal one? We use Mahara with multiple institutions and some have auto-creation of accounts switched on for Moodle or SAML SSO or LDAP but not necessarily for the rest.

So if you have a MNet auth method, you don't need to allow registration as you'll be placed into the institution automatically when you log in via Moodle.

The "Registration allowed" is also for moving between institutions and not just for registration of new accounts.

[Simon Story (simon-story)]

Fair question, it's the SAML module that has this restriction. I thought it was generic. Looking at the code, the XMLRPC probably also has this restriction.

See line 154 of htdocs/auth/xmlrpc/lib.php and line 124 of htdocs/auth/saml/lib.php . The plugin blocks auto-creation if any institution configured has registerallowed = 1 .

You don't encounter this bug if you add the SAML authentication to the default ('mahara') institution first, because after you do that, by default each subsequent institution you create has registerallowed = 0. Then the option to set registration is hidden and you can't get into this situation.

[Kristina Hoeppner (kris-hoeppner)]

Sorry, I don't quite follow as I am not a dev.

To understand the functionality better around the experimental setting of uniqubyusername, please take a look at http://manual.mahara.org/en/1.5_STABLE/site_admin/experimental.html#usersuniquebyusername-variable and around SAML and auto-creation of accounts see the warning at http://manual.mahara.org/en/1.5_STABLE/site_admin/institutions.html#saml-authentication

On one of our sites I know that we do not have registration allowed for the default institution (No institution), but subsequent institutions handle their registration themselves by deciding whether they allow or disallow it and also whether they have MNet auto-create accounts or not.

If you run a Mahara with multiple institutions where different SAML IdP can be set up, then SAML auth cannot be allowed to auto-create accounts. That's what the warning is about in the second link above.

[Simon Story (simon-story)]

The problem is the configuration ends up in two different states, depending on the order you do things in.

This is pretty minor, the docs do warn that you registerallowed needs to be 0 once you set usersuniquebyusername = 1.

People attempting to add SAML (And probably XMLRPC) authentication to an existing Mahara installation with multiple institutions configured will be frustrated because automatic user creation will not work for the via SAML. You shouldn't have to poke the database to make auto-creation work on an existing installation.

Do the following on a fresh installation:
Create a new institution
Set config item usersuniquebyusername = 1
Add and configure the SAML authentication plugin, enable auto-creation of users.
Attempt to login with with a new user that should autocreate, this will fail.
Configure the default institution to use SAML OR edit the database and set registerallowed = 0 on the default institution (You can't do this via the web interface because the option is hidden because you set usersuniquebyusername = 1)
SAML users can now autocreate.

Do the following on a fresh installation:
For the default institution, add and configure the SAML authentication plugin, enable auto-creation of users.
Set config item usersuniquebyusername = 1
Create a new institution
Add and configure the SAML authentication plugin
Attempt to login with with a new SAML user that should auto-create, this will succeed.
You can de-configure the SAML plugin on the default institution now and auto-creation some SAML users will still work.

A possible solution is that when the SAML plugin is set to auto-create users it (After warning the user) disables registration for all other configured institutions.

[Kristina Hoeppner (kris-hoeppner)]

Do you want to propose a patch, Simon, to make it a setting instead of piking in the code?

[Richard Mansfield (richard-mansfield)]

"You shouldn't have to poke the database to make auto-creation work on an existing installation."

Simon, I don't think it's too bad. If the site admin is willing to turn on an undocumented option which is not recommended, has plenty of security warnings around it, and which you can't do via the web UI, then I reckon it's okay for us to assume that they know what they're doing and can handle poking around in the db.

[Melissa Draper (melissa)]

Error handling would be useful so when it falls over, there's some indication of why.

[Simon Story (simon-story)]

It kinda does, it logs it. But you only get the log message after you've already configured it any everything looks fine.

I was thinking I could create a patch that comes back with the red warning text the other options on that SAML screen can also come back with if there is something wrong. That'd be consistent.

[Simon Story (simon-story)]

Well, that wasn't hard. Attached is patch to master made with git format-patch.

[Richard Mansfield (richard-mansfield)]

Simon, awesome, thanks - just one thing - shouldn't it only be an error when usersuniquebyusername is on? Under normal circumstances it's fine to have one institution with weautocreateusers and another with registration on, isn't it?

[Simon Story (simon-story)]

As it is, the SAML library will forbid the login if any institution has registration enabled. Check link 124 of htdocs/auth/saml/lib.php .

I think Piers' was reasoning was that if registration is enabled there is not much stopping someone creating a user with the same username as a SAML authenticated user (Who comes along later) and messing things up for them.

[Richard Mansfield (richard-mansfield)]

Yes, that's right - the code there was copied out of the xmlrpc plugin, but it's only when usersuniquebyusername is on that it's necessary to forbid the login in this way.

If usersuniquebyusername is off, and registration is on, someone *can* create a user with the same username as a SAML authenticated user who comes along later, but that SAML authenticated user will get a fresh Mahara username (e.g. when SAML user bob comes along, he'll get bob1 in Mahara if we already have a bob).

The way I read your patch, it will stop the 99% of sites with usersuniquebyusername off from setting up SAML auth with auto-creation, even though username clashes will be handled properly for them.

[Richard Mansfield (richard-mansfield)]

On 01/06/12 21:30, Simon Story wrote:
> Hi Richard,
>
> As it is, you can't enable (SAML) user auto-creation without also
> setting usersuniquebyusername = 1. Honest. Please try it. I'm begging
> you. You must think I am crazy. Maybe I am.
>
> ...you get the error 'You can only choose user auto creation if
> you have not selected remoteuser'
> ...
> So therefore, you can't set have auto user creation of SAML users
> without usersuniquebyusername = 1. The manual says the same.

Damn, I'm the one who's crazy, I didn't know about that error message. I'll submit the patch.

Guess I just assumed it'd work the same as the xmlrpc plugin. It's a shame we are encouraging people to turn usersuniquebyusername on, because it really sucks.

Maybe there's no way around it, though, I'm not too sure. With other external id providers (e.g. ldap) you can make the ldap auth the 'parent method' of your SSO (xmlrpc), and that usually gives you enough to leave usersuniquebyusername off and autocreation on. But SAML is trying to do both the id provision & the SSO, which maybe makes it impossible.

[Richard Mansfield (richard-mansfield)]

Simon: see https://reviews.mahara.org/1300

Left the patch as you wrote it, but added a long-winded comment above your registerallowed check.

[Kristina Hoeppner (kris-hoeppner)]

Sorry if I add more to the confusion, but I just want to point out that you can use SAML without auto-creation as parent authority to XML-RPC and have accounts auto-created through XML-RPC and thus allow the users to still use the Mahara SSO login when they want to log in next.

[Simon Story (simon-story)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 05/06/12 00:24, Richard Mansfield wrote:
> On 01/06/12 21:30, Simon Story wrote:

>> So therefore, you can't set have auto user creation of SAML
>> users without usersuniquebyusername = 1. The manual says the
>> same.
> Damn, I'm the one who's crazy, I didn't know about that error
> message. I'll submit the patch. Guess I just assumed it'd work the
> same as the xmlrpc plugin. It's a shame we are encouraging people
> to turn usersuniquebyusername on, because it really sucks. Maybe
> there's no way around it, though, I'm not too sure. With other
> external id providers (e.g. ldap) you can make the ldap auth the
> 'parent method' of your SSO (xmlrpc), and that usually gives you
> enough to leave usersuniquebyusername off and autocreation on. But
> SAML is trying to do both the id provision & the SSO, which maybe
> makes it impossible.

Maybe talk to Piers and ask him what his thinking was. Surely
Username+institution is unique enough?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iF4EAREIAAYFAk/e7hUACgkQ0t2asVgsCgBenwEA1hK8TuPmljOw5mjLnP3saeD0
VePa7Si/3ZwYzgMuaSAA/1doghEXfN7Ibl2WtT9Dc1AF98DEFbi0RRcLT25gY/+O
=ljzj
-----END PGP SIGNATURE-----

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/1300
Committed: http://gitorious.org/mahara/mahara/commit/5c578a384dcfc2a7809b8d78b415b5082c84793b
Submitter: Richard Mansfield (<email address hidden>)
Branch: master

commit 5c578a384dcfc2a7809b8d78b415b5082c84793b
Author: Simon Story <email address hidden>
Date: Wed May 30 15:52:21 2012 +0100

    Add SAML config error for autocreation & registerallowed (bug #1003980)

    Display an error if a user tries to enable auto-creation at the same
    time as having registration enabled for an instition.

    Change-Id: I22cf0df27c5e4edc3d7f71e8870fe84620419279
    Signed-off-by: Richard Mansfield <email address hidden>

[Hugh Davenport (hugh-davenport)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 status fixreleased
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iJwEAQECAAYFAlCbHO8ACgkQuMoJ2LQ3zxH8TAP/YN4BiCJZsn5a899/0UzV31Qg
lM8LXAwZWa6zFv6t0BQUHCqe6eFK9wPp51qgCWWXjUZ3vvvVcsyeWp6626aBFKSU
pCQXI9E7huPw802nJQ9WcZXRBUmgw87ww72Tx4mybnu7SPSrkZgXdnPGSMwDs89N
oWvTpl7Xuac48e6p0lU=
=ouU+
-----END PGP SIGNATURE-----