[Multi authentication] allow users with different auth method enter if the email is the same

[crazyserver]

When a user is already created and have another auth method (ex: LDAP or manual) the user cannot log in. And there is no error message.

We suggest to let the users login, of a setting to let the admins decide.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[crazyserver]

In the loginpage_hook function we use this hack:

               } else {
                    $username = $user->username;
                    //ADD THIS to save old method
                    $old_authmethod = $user->auth;
                    $DB->set_field('user', 'auth', 'googleoauth2', array('id'=>$user->id));
                    //END
                }

                //authenticate the user
                //TODO: delete this log later
                $userid = empty($user)?'new user':$user->id;
                add_to_log(SITEID, 'auth_googleoauth2', '', '', $username . '/' . $useremail . '/' . $userid);
                $user = authenticate_user_login($username, null);
                //ADD THIS to return the original state
                if (empty($newuser)) {
                    $DB->set_field('user', 'auth', $old_authmethod, array('id'=>$userid));
                }
                //END
                if ($user) {

[mouneyrac]

Thanks Pau. I thought about this hack when I created the plugin but I gave up on it because if ever something goes wrong in authenticate_user_login then the user can not login anymore with the original authentication method (as it is now set). It is too dangereous in my opinion.

However maybe with a sanity config that indicates that the auth must be reset if a use attempt to login, it may work...

There is also a potential security issue if a provider allows the users to register unverified email address - provider APIs do not return always the email verified information.

I am still unsure about that issue... How many users are going to use two different authentication methods? I am not sure it is worth to introduce such kind of hack for a user problem that may not exist...

[crazyserver]

This hack has to be done because the function authenticate_user_login only returns the userinfo if the auth selected method can validate user and password.

So, as the user cannot enter the password with this auth method, if the user is configured to authenticatewith another method with password this will fail.

I'll try to find another method to do that without using authenticate_user_login or this hack.

[crazyserver]

As I can see this can be done only changing

$user = authenticate_user_login($username, null);

by this

$user = get_complete_user_data('username', $username, $CFG->mnet_localhost_id);

To ensure email are verified we can validate it looking into the confirmed variable:

if ($user && !$user->confirmed) {
    throw new moodle_exception('usernotconfirmed');
}

[mouneyrac]

I understand the hack, I tried the same one when I first started to implement the plugin. As I mentioned, to avoid the problem of this hack, you need an additional config that you set to mytrueauth = 'manual' (or whatever it is) and in the googleoauth2 when you don't try to login (no code), then you do a check to see if the user that are trying to authenticate has the correct auth method. It's a sanity check in case something fail before you could revert the real auth method when you process the hack.

However as I mention this is a hack. Another solution I can see is to rewrite the core method in the plugin but that seems totally overkill.

I don't see why someone would want to login with two different methods. The only time it happened to me it's when I had to enter a login in the PS3 for crunchyroll and they didn't implement the facebook login on the PS3. But I would have prefer that they support the facebook login on PS3 instead to force my using two different methods. It looks to me the hack is not a solution to a real user problem.

PS: about your other suggestion, the goal of using authenticate_user_login is to trigger the whole authentication machinery to make sure no step is bypassed... not only retrieve some user info. For example, check if the user is suspended, trigger events...

[crazyserver]

There is one case that you have to consider: If you already have an account (manual login) and you want to login with Google. How can you do it? I think you cannot change the auth method of the user but you must let the user log in if the email address is the same.

Most of the platforms you can login with manual auth method and also Google, FB, Twitter...

Nevertheless, I undestand your position and the security thing. We must work hard :-)

Thanks for our time and your great work!

[mouneyrac]

I thought ab it more about this issue and I'll implement a similar solution with sanity checks later when I have a bit more of free time

[oscilate]

Hi mouneyrac and thanks for the great plugin. Are you still planning to implement this?

[mouneyrac]

Yes when I will have time :)

[dashohoxha]

I support the reason explained by crazyserver. I had some users with username/password login, and later I installed this plugin to allow them login through their Google+ account (I don't care about LinkedIn, Facebook, etc. and I don't understand why a plugin that is named googleoauth2 should support all the oauth2 logins that exist). But unfortunately they cannot switch to it.

Is there something that I can do to enable them to switch? I don't mind putting my hands on the DB (mysql) if needed.

@crazyserver, does your patch still work? Because it seems that later you propose a different solution. I am willing to try it.

@mouneyrac, I am willing to help with rewriting the core method, if you give me some ideas of what needs to be done and how to start.

[crazyserver]

@dashohoxha New code can be found on https://github.com/projectestac/moodle-auth_googleoauth2/commits/master (see my commits and @sarjona commits). The current maintainer of this is @sarjona feel free to ask her.

[dashohoxha]

@crazyserver Thanks for your quick reply and for your fixes/improvements. I will definitely try it.

But the repository says "This branch is 10 commits ahead, 5 commits behind mouneyrac:master." I wonder if you are planning to make any pull requests in order to merge both branches. Is it really neccessary to fork it?

If a fork/split is needed, in my opinion it should be a fork between a plugin that supports only Google+ authentication, and another plugin that supports any OAuth2 authentication providers (maybe renamed auth_oauth2). Such a split makes sense because different ouath2 providers implement the protocol in different ways. A plugin that supports only one of them knows exactly what to expect and can be efficient in its implementation. A plugin that tries to support all of them has to be more complex, and it has to make less assumptions about what it can expect. It may even become a nightmare to maintain it over time. I am in favour of having a different plugin for each oauth2 provider, each plugin having its own maintainer(s).

[dashohoxha]

@crazyserver and @sarjona It does not work with the latest version of Moodle (3.2), unless you apply this patch: https://github.com/mouneyrac/moodle-auth_googleoauth2/commit/fd754d778bb80456a92e0668aa45768397f0de52 I will also raise an issue about this at your fork.

Other than this, as far as I tested it, it works very well.

About the concern raised by mouneyrac, if something fails before you could revert to the real auth method, I think that the admin user should be able to fix it (revert back the auth method of the user). And also this should be a rare case, which does not happen frequently.

I will try to apply this fix to the main repo (this one) and then provide a pull request.

[mouneyrac]

Just a note I think the proper way to implement this hack is to have a sanity database table where you first store the user info that you are going to temporarily change. Then do sanity check on this table by cron to clean it up every 10mn (and revert the very unlikely broken user auth account). That said, the Moodle authentication core code may have changed since I work on this issue (can not remember but I think I created the plugin for Moodle 2.1/2.2) maybe there are better solution in Moodle since this time.

[rabser]

Hi all, i'm the new Lead Mantainer for this plugin and i'm reconsidering this pull-request left unresolved by Jerome. I was thinking too about this issue some weeks ago, and how overcome the strict separation between oauth2 authentication method and the other ones. Frequently i use a local manual account (or ldap-based) on my moodle platforms and i wish to login to the same user (= the same email) with my google university account...

but my thoughts are :

1. other auth plugins do not changes the authentication method of the user (so i believe that a double check with HQ should be done before any change in this way)
2. there is an intrinsic security problem in changing a user property when logging in from a social network, if someone on that social network opens an account with my email (And not tell me that they are the most secure identity providers in the world...) then my moodle account (e.g. manual or ldap or shibboleth) is at risk.

so i'm pretty convinced that this should not be done in a public plugin, but i'll contact the Moodle HQ for a comment on this.