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

Invalid request error with MySQL & emulated prepares OFF #1089

Closed
strongholdmedia opened this issue Sep 29, 2019 · 7 comments · Fixed by #1091
Closed

Invalid request error with MySQL & emulated prepares OFF #1089

strongholdmedia opened this issue Sep 29, 2019 · 7 comments · Fixed by #1091
Labels

Comments

@strongholdmedia
Copy link
Contributor

  • Passport Version: 7.5.0
  • Laravel Version: 5.8.35
  • PHP Version: 7.3.9
  • Database Driver & Version: PDO over mysqlnd, emulated prepares off; MySQL 5.7.29 & MariaDB 10.3.16

Description:

When not using emulated prepares on MySQL, you will always get "invalid_request" error because the client_id should be string, but is returned (properly) as an integer from the database

Steps To Reproduce:

  • Create any Passport project that otherwise "works"
  • Use PHP 7.3 (earlier versions have other issues with e.g. microseconds truncated and so on, with native statements)
  • Turn off emulated prepares (so that the native database driver is actually used) using
   PDO::ATTR_EMULATE_PREPARES => false

in the database configuration

Note that I am somewhat certain that this issue should be present with other database drivers, I just have no means currently to check this.

NB: I tried to "fix" this issue in thephpleague/oauth2-server#1054 but @Sephster insisted on that this is primarily an implementation issue, which it actually is.
Thus I turned up here.

Please also see details of the underlying issue there.

@strongholdmedia
Copy link
Contributor Author

strongholdmedia commented Sep 29, 2019

I am glad to send a PR given consensus that there is

  • willingness to fix, and
  • preferred/excluded methods of doing so

Otherwise I currently use the "fix" linked in the pull request above, and encourage anyone to do so, even that it is somewhat hardcore to override the used grant(s).
(Basically one needs to override the whole chain above starting from the provider.)

@driesvints
Copy link
Member

Has this changed in v8 of oauth2-server? This never was giving issues before.

@Sephster
Copy link
Contributor

Hey @driesvints. This hasn't changed for v8. I am assuming most people don't bother with turning off emulated prepares so this hasn't been raised as an issue before because no one has noticed it.

@driesvints
Copy link
Member

Okay, let's see how we can get this fixed then. Probably type casting before sending in the parameter?

@Sephster
Copy link
Contributor

Yeah I think that is the easiest way. Just ensure it is cast to an string in advance. Cheers Dries.

@strongholdmedia
Copy link
Contributor Author

Exactly. I tested it yesterday with Laravel 5.6.39, Passport 5.0.3 and OAuth2 Server 6.1.1 - same results.

The thing why nobody bothered with turning emulated prepares off may well be the decade-old microsecond bug with native driver.
(That here: laravel/framework#24214)
But since that got fixed in PHP 7.3 that is now well into mainline, I expect this to become more and more frequent over time.

@strongholdmedia
Copy link
Contributor Author

strongholdmedia commented Oct 1, 2019

It seems work was done on this back in Laravel 4.0 (#550), but it was not completely bulletproof.
I am not completely sure, but I suppose that Eloquent hydration does not invoke the constructor when returning an entity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants