Bug: Duplicate username error

[mullernato]

PHP Version

8.2

CodeIgniter4 Version

4.3.3

Shield Version

latest dev

Shield OAuth Version?

dev-develop

Which operating systems have you tested for this bug?

Linux

Which server did you use?

fpm-fcgi

Database

Mariadb 10.6

Did you add customize OAuth?

No

What happened?

if there is another user with the same username, an error occurs when registering a new account with google login.
duplicate username error

CodeIgniter\Database\Exceptions\DatabaseException #1062

Duplicate entry 'Glad' for key 'username'

SYSTEMPATH/Database/BaseBuilder.php : 2309   —  CodeIgniter\Database\BaseConnection->query ()

2302             array_keys($this->QBSet),
2303             array_values($this->QBSet)
2304         );
2305 
2306         if (! $this->testMode) {
2307             $this->resetWrite();
2308 
2309             $result = $this->db->query($sql, $this->binds, false);
2310 
2311             // Clear our binds so we don't eat up memory
2312             $this->binds = [];
2313 
2314             return $result;
2315         }
2316 

SYSTEMPATH/Model.php : 330   —  CodeIgniter\Database\BaseBuilder->insert ()

323                 );
324             } else {
325                 $sql = 'INSERT INTO ' . $table . ' DEFAULT VALUES';
326             }
327 
328             $result = $this->db->query($sql);
329         } else {
330             $result = $builder->insert();
331         }
332 
333         // If insertion succeeded then save the insert ID
334         if ($result) {
335             $this->insertID = ! $this->useAutoIncrement ? $data[$this->primaryKey] : $this->db->insertID();
336         }
337 

SYSTEMPATH/BaseModel.php : 782   —  CodeIgniter\Model->doInsert ()

775 
776         $eventData = ['data' => $data];
777 
778         if ($this->tempAllowCallbacks) {
779             $eventData = $this->trigger('beforeInsert', $eventData);
780         }
781 
782         $result = $this->doInsert($eventData['data']);
783 
784         $eventData = [
785             'id'     => $this->insertID,
786             'data'   => $eventData['data'],
787             'result' => $result,
788         ];
789 

SYSTEMPATH/Model.php : 730   —  CodeIgniter\BaseModel->insert ()

723                 $this->tempPrimaryKeyValue = $data->{$this->primaryKey};
724             }
725         }
726 
727         $this->escape   = $this->tempData['escape'] ?? [];
728         $this->tempData = [];
729 
730         return parent::insert($data, $returnID);
731     }
732 
733     /**
734      * Updates a single record in the database. If an object is provided,
735      * it will attempt to convert it into an array.
736      *
737      * @param array|int|string|null $id

VENDORPATH/codeigniter4/shield/src/Models/UserModel.php : 253   —  CodeIgniter\Model->insert ()

246      * @throws ValidationException
247      */
248     public function insert($data = null, bool $returnID = true)
249     {
250         // Clone User object for not changing the passed object.
251         $this->tempUser = $data instanceof User ? clone $data : null;
252 
253         $result = parent::insert($data, $returnID);
254 
255         $this->checkQueryReturn($result);
256 
257         return $returnID ? $this->insertID : $result;
258     }
259 
260     /**

SYSTEMPATH/BaseModel.php : 692   —  CodeIgniter\Shield\Models\UserModel->insert ()

685         if (empty($data)) {
686             return true;
687         }
688 
689         if ($this->shouldUpdate($data)) {
690             $response = $this->update($this->getIdValue($data), $data);
691         } else {
692             $response = $this->insert($data, false);
693 
694             if ($response !== false) {
695                 $response = true;
696             }
697         }
698 
699         return $response;

VENDORPATH/codeigniter4/shield/src/Models/UserModel.php : 315   —  CodeIgniter\BaseModel->save ()

308      *
309      * @return true if the save is successful
310      *
311      * @throws ValidationException
312      */
313     public function save($data): bool
314     {
315         $result = parent::save($data);
316 
317         $this->checkQueryReturn($result);
318 
319         return true;
320     }
321 
322     /**

VENDORPATH/datamweb/shield-oauth/src/Controllers/OAuthController.php : 88   —  CodeIgniter\Shield\Models\UserModel->save ()

81 
82         if ($this->checkExistenceUser($find) === false) {
83             helper('text');
84             $users = model('ShieldOAuthModel');
85             // new user
86             $entitiesUser = new User($oauthClass->getColumnsName('newUser', $userInfo));
87 
88             $users->save($entitiesUser);
89             $userid = $users->getInsertID();
90             // To get the complete user object with ID, we need to get from the database
91             $user = $users->findById($userid);
92             $users->save($user);
93             // Add to default group
94             $users->addToDefaultGroup($user);
95         }

SYSTEMPATH/CodeIgniter.php : 934   —  Datamweb\ShieldOAuth\Controllers\OAuthController->callBack ()

927     protected function runController($class)
928     {
929         // This is a Web request or PHP CLI request
930         $params = $this->router->params();
931 
932         $output = method_exists($class, '_remap')
933             ? $class->_remap($this->method, ...$params)
934             : $class->{$this->method}(...$params);
935 
936         $this->benchmark->stop('controller');
937 
938         return $output;
939     }
940 
941     /**

SYSTEMPATH/CodeIgniter.php : 499   —  CodeIgniter\CodeIgniter->runController ()

492             if (! method_exists($controller, '_remap') && ! is_callable([$controller, $this->method], false)) {
493                 throw PageNotFoundException::forMethodNotFound($this->method);
494             }
495 
496             // Is there a "post_controller_constructor" event?
497             Events::trigger('post_controller_constructor');
498 
499             $returned = $this->runController($controller);
500         } else {
501             $this->benchmark->stop('controller_constructor');
502             $this->benchmark->stop('controller');
503         }
504 
505         // If $returned is a string, then the controller output something,
506         // probably a view, instead of echoing it directly. Send it along

SYSTEMPATH/CodeIgniter.php : 368   —  CodeIgniter\CodeIgniter->handleRequest ()

361             $this->response->send();
362             $this->callExit(EXIT_SUCCESS);
363 
364             return;
365         }
366 
367         try {
368             return $this->handleRequest($routes, $cacheConfig, $returnResponse);
369         } catch (RedirectException $e) {
370             $logger = Services::logger();
371             $logger->info('REDIRECTED ROUTE at ' . $e->getMessage());
372 
373             // If the route is a 'redirect' route, it throws
374             // the exception with the $to as the message
375             $this->response->redirect(base_url($e->getMessage()), 'auto', $e->getCode());

FCPATH/index.php : 67   —  CodeIgniter\CodeIgniter->run ()

60  *---------------------------------------------------------------
61  * LAUNCH THE APPLICATION
62  *---------------------------------------------------------------
63  * Now that everything is setup, it's time to actually fire
64  * up the engines and make this app do its thang.
65  */
66 
67 $app->run();
68 

Steps to Reproduce

Manually register a user using Shield with the first name of a test gmail user, eg John. Then log in with a test Gmail account whose user is named John. This will generate a duplicate username error.

Expected Output

It was expected that the username would be registered with some suffix to differentiate it from an existing user.

Anything else?

No response

[datamweb]

Hi @mullernato ,
Thank you very much for your report.

The process of checking the existence of a user in the database is done by looking up the email in the table. If the user with the desired email is in the table (provided that config(ShieldOAuthConfig::class)->syncingUserInfo is active), the information will be updated in the table every time user login, and if there is no user with the email, a new user will be registered.

It was expected that the username would be registered with some suffix to differentiate it from an existing user.

Your solution is good, but as long as the shield settings are for user login with email and password, if login is with username and password, the user cannot login because he/she does not know that an suffix has been automatically added to username. to be Unless it is assumed that we will not have login with username and password, which is definitely not a good assumption.

Do you have any other opinion with the above explanation? thoughts?

[mullernato]

Here in my system the user has 2 login/registration options:

1. using email and password using Shield with only email and password. obs. Login with username is not allowed.
2. using google auth with your ShieldOAuth script.

The Shield requires a username in the register, and in the database table it cannot be duplicated, even if it is irrelevant for use, so I thought of putting a suffix in case of duplicity, as the user will not use this username to login.
And also the user will be able to change it in a user settings panel in my use case.

[mullernato]

even though it's not my case but if two people register using google oauth or any other oauth and they have the same first name it will result in this username already exists error.

to my case I change the Library GoogleOAuth.php setColumnsName function to:

protected function setColumnsName(string $nameOfProcess, $userInfo): array
    {
        if ($nameOfProcess === 'syncingUserInfo') {
            $usersColumnsName = [
                $this->config->usersColumnsName['first_name'] => $userInfo->given_name,
                $this->config->usersColumnsName['last_name']  => $userInfo->family_name,
                $this->config->usersColumnsName['avatar']     => $userInfo->picture,
            ];
        }

        if ($nameOfProcess === 'newUser') {
            $usersColumnsName = [
                // users tbl                                    // OAuth
                'username'                                    => $this->genUsername($userInfo->name),
                'email'                                       => $userInfo->email,
                'password'                                    => random_string('crypto', 32),
                'active'                                      => $userInfo->email_verified,
                $this->config->usersColumnsName['first_name'] => $userInfo->given_name,
                $this->config->usersColumnsName['last_name']  => $userInfo->family_name,
                $this->config->usersColumnsName['avatar']     => $userInfo->picture,
            ];
        }

        return $usersColumnsName;
    }

    protected function genUsername($name) : string
    {
        // change all spaces, underscores, dots, dashes, etc. to a single underscore
        // and make it lowercase
        $name = preg_replace('/\s+/', '_', $name);
        $name = preg_replace('/[^A-Za-z0-9\_]/', '', $name);
        $name = strtolower($name);

        //add 2 random numbers to the end of the username to prevent duplicates separate by underscore
        $name = $name . '_' . rand(10, 99);
        return 'g_'.$name;
    }

[datamweb]

I thought about this a bit, we can add a prefix to username.

In addition, we can add a session or event so that if the user's login is OAuth, the user will be reminded to change his username(or any data) if needed.

I have pinned this topic, I will wait for a while, maybe we will get another feedback, otherwise we will proceed as I explained above.

[datamweb]

@mullernato PR #58 solved this issue by replacing email for username. Check if you can.