Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is_unique is not considering the db prefix in checking for unique values in db #3741

Closed
paulbalandan opened this issue Oct 8, 2020 · 6 comments

Comments

@paulbalandan
Copy link
Member

Direction
I am testing a model where I've set the unique constraint on the migration file, and setting is_unique check on the name field. I am using SQLite3 as the test db driver.

Describe the bug
Migration

$this->forge
            ->addField([
                'id'          => ['type' => 'int', 'constraint' => 11, 'unsigned' => true, 'auto_increment' => true],
                'name'        => ['type' => 'varchar', 'constraint' => 100],
                'description' => ['type' => 'varchar', 'constraint' => 255, 'null' => true],
            ])
            ->addPrimaryKey('id')
            ->addUniqueKey('name')
            ->createTable('groups', true)
        ;

Model validation rules

$this->allowedFields = [
            'name',
            'description',
        ];
$this->validationRules = [
            'name'        => "required|max_length[100]|is_unique[groups.name,name,{name}]",
            'description' => 'permit_empty|max_length[255]',
        ];

I am developing a personal module and thus using the default test group of CI4 which uses sqlite3 and db_ as db prefix.

In my test case I have already seeded the groups table with ['name' => 'Admin', 'description' => 'Users with high powers']. Now, I am purposely inserting a new record with the new record's name to be the same Admin value. I am expecting the insert operation to be returning false as I believe this will fail validation, but instead got an ErrorException (SQLite3::exec(): UNIQUE constraint failed: db_groups.name). This means it passed validation and proceeded with the insert, which should not be the case.

If this is useful, the groups name is actually dynamic and can be set through a config file. But basically the value is default to groups.

CodeIgniter 4 version
develop

Affected module(s)
Validation

Context

  • OS: Win10
  • PHP version 7.4.11
@paulbalandan paulbalandan added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 8, 2020
@michalsn
Copy link
Member

michalsn commented Oct 8, 2020

If this is useful, the groups name is actually dynamic and can be set through a config file. But basically the value is default to groups.

Are you changing the $DBGroup property dynamically inside your model class / somewhere else? Or am I completely missing your point?

@paulbalandan
Copy link
Member Author

paulbalandan commented Oct 8, 2020

If this is useful, the groups name is actually dynamic and can be set through a config file. But basically the value is default to groups.

Are you changing the $DBGroup property dynamically inside your model class / somewhere else? Or am I completely missing your point?

This groups I'm referring is the groups table. Not the db group connection. And I'm using the default db connection here.

@michalsn
Copy link
Member

michalsn commented Oct 9, 2020

Ok, now I get it.

You are setting rule like this:

is_unique[groups.name,name,{name}]

but for insert it should look more like this:

is_unique[groups.name,name]

When you set this last parameter {name}, the variable value is treated as an exception for the query, which is usable for update only (it will produce query like: ... WHERE name != {name}).

@michalsn michalsn closed this as completed Oct 9, 2020
@paulbalandan
Copy link
Member Author

Ok. So that's how it is.

Honestly, I copied this validation rule from myth/auth's GroupModel thinking the is_unique check will apply for both insert and update operations. Am I wrong?

@michalsn
Copy link
Member

michalsn commented Oct 9, 2020

Yes, it would be applied for both but in createGroup() the rule is modified: https://github.com/lonnieezell/myth-auth/blob/e838cb8de6ffa118caf2b9909e71776a866c8973/src/Authorization/FlatAuthorization.php#L459. Technically speaking the validation in the model is skipped and we make separate validation.

In general, now it is possible to change the rules on the fly, I think.

@paulbalandan
Copy link
Member Author

Thanks for the enlightenment.

@paulbalandan paulbalandan removed the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants