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

Bug: ValidationRules::buildUserFromRequest() does not support json input #612

Closed
ValdisM opened this issue Jan 30, 2023 · 8 comments · Fixed by #747
Closed

Bug: ValidationRules::buildUserFromRequest() does not support json input #612

ValdisM opened this issue Jan 30, 2023 · 8 comments · Fixed by #747
Labels
bug Something isn't working waiting for info Issues or pull requests that need further clarification from the author

Comments

@ValdisM
Copy link

ValdisM commented Jan 30, 2023

PHP Version

8

CodeIgniter4 Version

4.3.0

Shield Version

dev-develop

Which operating systems have you tested for this bug?

macOS

Which server did you use?

apache

Database

MariaDB

Did you customize Shield?

   protected function buildUserFromRequest(): User
    {
        $fields = $this->prepareValidFields();
        $request = service('request');
        $data = $request->getJSON() ? (array) $request->getJSON() : $request->getPost($fields);
        return new User($data);
    }

What happened?

buildUserFromRequest support only form data inputs, not json input from service("request").
Before:

$data = $request->getPost($fields);

After:

$data = $request->getJSON() ? (array) $request->getJSON() : $request->getPost($fields);

Steps to Reproduce

$this->validation->setRules([
			"email" => ["label" => "Email", "rules" => "required|max_length[254]|valid_email"],
			"newPassword" => ["label" => "New password", "rules" => "required|strong_password"],
			"newPasswordAgain" => ["label" => "Repeat password", "rules" => "required|matches[newPassword]"],
		]);

it was catched when "strong_password" rule was applied.

Expected Output

/**
     * Builds a new user instance from the global request.
     */
    protected function buildUserFromRequest(): User
    {
        $fields = $this->prepareValidFields();

        /** @var IncomingRequest $request */
        $request = service('request');

        $data = $request->getJSON() ? (array) $request->getJSON() : $request->getPost($fields);

        return new User($data);
    }

Anything else?

No response

@ValdisM ValdisM added the bug Something isn't working label Jan 30, 2023
@jozefrebjak
Copy link
Contributor

jozefrebjak commented Jan 31, 2023

@ValdisM

Here is how you can do it. I'm not using it with User entity, but it can be the same.

You need to initialise User entity first and then use fill method to use provided data.

I'm using something like this within API

/**
 * Validate POST data
 */
if (! $this->validate($this->getUserValidationRules())) {
    log_message('error', implode(', ', $this->validator->getErrors()));
    return $this->fail($this->validator->getErrors());
}

$user = new User();

$user = $user->fill($this->request->getJSON() ? $this->request->getJSON(true) : $this->request->getVar());

@kenjis kenjis changed the title Bug: @ buildUserFromRequest Bug: ValidationRules::buildUserFromRequest() does not support json input Feb 1, 2023
@kenjis
Copy link
Member

kenjis commented Feb 1, 2023

I kind of dislike the implementation where the ValidationRule depends on the Request.

@ValdisM
Copy link
Author

ValdisM commented Feb 1, 2023

Can you explain? What do you mean by this?

There is a strong_password validation rule for the password input field available in shield. How else I should use the ValidationRule?

@kenjis
Copy link
Member

kenjis commented Feb 1, 2023

In my opinion a validation rule just validates value(s).
So it only requires data to validate. But this rule requires a Request object.

@kenjis
Copy link
Member

kenjis commented Feb 1, 2023

@ValdisM Please try:

$this->validation->setRules([
    "email" => ["label" => "Email", "rules" => "required|max_length[254]|valid_email"],
    "newPassword" => ["label" => "New password", "rules" => "required|strong_password[]"],
    "newPasswordAgain" => ["label" => "Repeat password", "rules" => "required|matches[newPassword]"],
]);

@datamweb datamweb added the waiting for info Issues or pull requests that need further clarification from the author label Feb 24, 2023
@sammyskills
Copy link
Contributor

This issue still persists.

I've tried to update this:

$data = $request->getPost($fields);

to:

$data = $request->getVar($fields);

and also tried @kenjis recommendation, i.e., using strong_password[], but in both cases, the error below is returned:

{
  "title": "Error",
  "type": "Error",
  "code": 500,
  "message": "Call to undefined function CodeIgniter\\Shield\\Authentication\\Passwords\\str_contains()",
  "file": "htdocs\\pbo\\vendor\\codeigniter4\\shield\\src\\Authentication\\Passwords\\NothingPersonalValidator.php",
  "line": 75,
  "trace": [
    {
      "file": "htdocs\\pbo\\vendor\\codeigniter4\\shield\\src\\Authentication\\Passwords\\NothingPersonalValidator.php",
      "line": 27,
      "function": "isNotPersonal",
      "class": "CodeIgniter\\Shield\\Authentication\\Passwords\\NothingPersonalValidator",
      "type": "->",
      "args": [
        "someotherrandomepasdword",
        {
          "username": "cucinszzz"
        }
      ]
    },
    {
      "file": "htdocs\\pbo\\vendor\\codeigniter4\\shield\\src\\Authentication\\Passwords.php",
      "line": 132,
      "function": "check",
      "class": "CodeIgniter\\Shield\\Authentication\\Passwords\\NothingPersonalValidator",
      "type": "->",
      "args": [
        "someotherrandomepasdword",
        {
          "username": "cucinszzz"
        }
      ]
    },
    {
      "file": "htdocs\\pbo\\vendor\\codeigniter4\\shield\\src\\Authentication\\Passwords\\ValidationRules.php",
      "line": 45,
      "function": "check",
      "class": "CodeIgniter\\Shield\\Authentication\\Passwords",
      "type": "->",
      "args": [
        "someotherrandomepasdword",
        {
          "username": "cucinszzz"
        }
      ]
    },
    {
      "file": "htdocs\\pbo\\vendor\\codeigniter4\\framework\\system\\Validation\\Validation.php",
      "line": 312,
      "function": "strong_password",
      "class": "CodeIgniter\\Shield\\Authentication\\Passwords\\ValidationRules",
      "type": "->",
      "args": [
        "someotherrandomepasdword",
        "",
        {
          "username": "cucinszzz",
          "email": "sammyskills@gmail.com",
          "password": "someotherrandomepasdword",
          "DBGroup": null
        },
        null,
        "password"
      ]
    },
    {
      "file": "htdocs\\pbo\\vendor\\codeigniter4\\framework\\system\\Validation\\Validation.php",
      "line": 170,
      "function": "processRules",
      "class": "CodeIgniter\\Validation\\Validation",
      "type": "->",
      "args": [
        "password",
        "Auth.password",
        "someotherrandomepasdword",
        [
          "required",
          "strong_password[]"
        ],
        {
          "username": "cucinszzz",
          "email": "sammyskills@gmail.com",
          "password": "someotherrandomepasdword",
          "DBGroup": null
        }
      ]
    }
}

@kenjis
Copy link
Member

kenjis commented May 15, 2023

str_contains() requires PHP 8.0 or later.
See https://www.php.net/manual/en/function.str-contains.php
It is a bug.

@sammyskills
Copy link
Contributor

The str_contains() will be fixed by #742.

How about this?

$data = $request->getPost($fields);

Is it not better written as:

$data = $request->getVar($fields);

so it can support all request types?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working waiting for info Issues or pull requests that need further clarification from the author
Projects
None yet
5 participants