oauth2-server icon indicating copy to clipboard operation
oauth2-server copied to clipboard

password grant username for number is not work

Open yangjisen opened this issue 2 years ago • 5 comments

    /**
     * @param ServerRequestInterface $request
     * @param ClientEntityInterface  $client
     *
     * @throws OAuthServerException
     *
     * @return UserEntityInterface
     */
    protected function validateUser(ServerRequestInterface $request, ClientEntityInterface $client)
    {
        $username = $this->getRequestParameter('username', $request);

        if (!\is_string($username)) {
            throw OAuthServerException::invalidRequest('username');
        }

        $password = $this->getRequestParameter('password', $request);

        if (!\is_string($password)) {
            throw OAuthServerException::invalidRequest('password');
        }

        $user = $this->userRepository->getUserEntityByUserCredentials(
            $username,
            $password,
            $this->getIdentifier(),
            $client
        );

        if ($user instanceof UserEntityInterface === false) {
            $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request));

            throw OAuthServerException::invalidCredentials();
        }

        return $user;
    }

yangjisen avatar Mar 23 '22 01:03 yangjisen

Strange. After having this release out for a good while now, this is the second report I've had of this.

As far as I can see, even if your username is an int, because it comes via the http request it should always be a string.

I can't replicate this issue. Are you able to share the contents of $request->getParsedBody() so I can investigate this further?

Sephster avatar Mar 23 '22 07:03 Sephster

call function createPasswordRequest(1, 1)

    protected function createPasswordRequest($username, $password, array $scopes = [])
    {
        return (new ServerRequest('POST', 'not-important'))
            ->withParsedBody([
                'grant_type' => 'password',
                'client_id' => config("passport.{$this->client_name}.id", 2),
                'client_secret' => config("passport.{$this->client_name}.secret", 'secret'),
                'scope' => implode(' ', $scopes),
                'username' => $username, // work: (string)$username
                'password' => $password, // work:  (string)$password
            ]);
    }

$request->getParsedBody()

array:6 [
  "grant_type" => "password"
  "client_id" => "2"
  "client_secret" => "Zu3aFcy6oQjcDqnm1npcTkxN4FzhXL0jIvfoMbiV"
  "scope" => ""
  "username" => 1
  "password" => 1
]

yangjisen avatar Mar 29 '22 03:03 yangjisen

The problem is here https://github.com/thephpleague/oauth2-server/blob/19dc5e47c465bc9f1804859660b7efe456a40713/src/Grant/AbstractGrant.php#L354

The request parameters do not cast to string|null and return as is. All helper methods should cast return value to string|null according to phpdocs.

eugene-borovov avatar Mar 29 '22 04:03 eugene-borovov

@eugene-borovov I don't think that line is causing the issue. Because we are receiving an http request, all values received should be strings as far as my testing goes unless I've made a mistake somewhere.

@yangjisen are you using any frameworks or a CMS with this package such as Laravel or Drupal? Out of the box, the request should always return strings but it might be the case that a system is converting numerical strings to an int. Just a guess at this stage but any extra information you can give would be appreciated.

Sephster avatar Apr 07 '22 21:04 Sephster

@Sephster Parsed body could be changed by some middleware in application. Therefore, we should not trust the type of request parameters.

I advise you to additionally cast the parameter type to the string. This will save application from unexpected mistakes.

protected function getRequestParameter($parameter, ServerRequestInterface $request, $default = null)
{
    $requestParameters = (array) $request->getParsedBody();

    $val = $requestParameters[$parameter] ?? $default;
   
   return $val === null ? null : (string)$val;
}

eugene-borovov avatar Apr 08 '22 04:04 eugene-borovov