glFusion / glfusion

glFusion CMS - Advanced Content Management with Style
https://www.glfusion.org
GNU General Public License v2.0
14 stars 15 forks source link

Remote login replaces user profile image #538

Open leegarner opened 2 years ago

leegarner commented 2 years ago

Logging in via remote service, e.g. Google, replaces the user's profile image with the remote service image.

  1. Upload a profile image. The image is saved as X_YYYYY.jpg where "X" is the user ID and "YYYYY" is some number of digits.
  2. Log out
  3. Log back in using the remote service.
  4. Note that the remote profile image has been uploaded as X.jpg and the gl_users table updated. X_YYYYY.jpg remains in userphotos.
  5. Re-upload the personal profile image.
  6. Note that X.jpg has been deleted and a new X_YYYYY.jpg has been uploaded.

I've started looking but haven't found yet where the image is updated from the remote authenticator.

Recommended fix, get the image from Google et al only if the profile image field is empty for the user.

walterrowe commented 2 years ago

Think I found the ref in private/system/classes/oauthconsumer.class.php.

https://github.com/glFusion/glfusion/blob/df4bd6daa60a3462909801d2028154978eb1023e/private/system/classes/oauthconsumer.class.php#L255

leegarner commented 2 years ago

I've traced the issue to the part starting on line 221 in private/system/classes/oauthconsumer.class.php

        if ($userRow !== false && $userRow !== null) { // existing user...
            $uid    = $userRow['uid'];
            $status = $userRow['status'];
            $retval = true;
            $this->_DBupdate_users($uid, $users);
            $this->_DBupdate_userinfo($uid, $userinfo);
        } else {
            // new user

In _DBupdate_users(), the user's remoteservice, remoteusername, email, fullname, homepage and image are updated from the oauth provider, if they're in the response. I suppose remoteservice and remoteusername make sense but don't see the need to update anything else for existing users.

leegarner commented 2 years ago

For now I've commented out the call to _DBupdate_users(). In a perfect world glFusion would know if each field was updated from the remote service or overridden by the user, and update the non-overridden ones from remote during login. That's a whole lot to keep track of though so it's probably sufficient to update from remote when creating an account, otherwise leaving it alone.