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

Allow to retrieve the current client through the RequestGuard #882

Closed
driesvints opened this issue Nov 15, 2018 · 14 comments · Fixed by #1508
Closed

Allow to retrieve the current client through the RequestGuard #882

driesvints opened this issue Nov 15, 2018 · 14 comments · Fixed by #1508

Comments

@driesvints
Copy link
Member

driesvints commented Nov 15, 2018

We should enhance the RequestGuard so it's able to retrieve the current client that's being used as an addition to #854.

This probably requires some breaking changes and should target the next major release.

@telkins
Copy link

telkins commented Feb 28, 2019

Any traction on this one...? :-)

@driesvints
Copy link
Member Author

No sorry. Way too much other stuff on my plate atm.

@telkins
Copy link

telkins commented Feb 28, 2019

@driesvints Thx. Understandable.... :-)

@gerardnll
Copy link

gerardnll commented Mar 4, 2020

EDIT: I've noticed you explain a way to get it in #854. Would be great tho if you make this change for 9.x.

Today, while updating to Laravel 7 I've noticed that when I'm trying to get the Client (Client Credentials Grant) from the request, in TokenGuard there's a function called client() which would return the Client linked to the request but that function cannot be called in any way because TokenGuard is also not accessible.

I'm interested in this functionality because I'm trying to know which other 'server/machine' made the request. It looks like the functionality is there but it's not accessible. It would be great to be able to call something like auth()->guard('api')->client(). Hope this enhancement is relevant.

@billriess
Copy link
Contributor

billriess commented Apr 19, 2020

You can replicate what the token guard is doing as a macro on the request. It would involve a query so it may not be the cleanest solution but it would work.

Request::macro('client', function () {
    $server = app(ResourceServer::class);
    $psr = $server->validateAuthenticatedRequest((new PsrHttpFactory(
        new ServerRequestFactory,
        new StreamFactory,
        new UploadedFileFactory,
        new ResponseFactory
    ))->createRequest(request()));

    return \Laravel\Passport\Client::find($psr->getAttribute('oauth_client_id'));
});

Then you can just use $request->client().

@techenby
Copy link

Trying a variation of this with version 10 results in Error: Class 'Laminas\Diactoros\ServerRequestFactory' not found. Did Passport change some of its dependencies?

@techenby
Copy link

techenby commented Sep 15, 2020

Figured it out. Because of pull request #1330 the implementation above should be switched to:

use Nyholm\Psr7\Factory\Psr17Factory;

...

Request::macro('client', function () {
    $server = app(ResourceServer::class);
    $psr = $server->validateAuthenticatedRequest((new PsrHttpFactory(
        new Psr17Factory,
        new Psr17Factory,
        new Psr17Factory,
        new Psr17Factory
    ))->createRequest(request()));

    return \Laravel\Passport\Client::find($psr->getAttribute('oauth_client_id'));
});

@taftse
Copy link

taftse commented Apr 17, 2021

Any chance we could finally get this issue fixed?

@driesvints
Copy link
Member Author

@taftse as soon as someone sends in a PR

@axlon
Copy link
Contributor

axlon commented Nov 23, 2021

@driesvints I was looking at creating a PR for this. Does either of the following sound agreeable to you?

Option 1:

  • Convert TokenGuard to a full-fledged implementation of the Guard contract
    • Pass the request instance through the constructor
    • Remove the$request parameter from TokenGuard methods and use $this->request instead
  • Remove the wrapped RequestGuard and expose TokenGuard directly

Option 2:

  • Extend the RequestGuard and add a client() method to it
  • Change the guard's callback to return a TokenGuard instance rather than a Authenticatable
    • We should probably store the token guard instance in the request guard, to prevent it from being instantiated twice during a request

The first option is obviously very breaking, I initially thought the second option one was not, but the RequestGuard callback is protected, so changing that could break projects extending the RequestGuard as well.

@driesvints
Copy link
Member Author

@axlon gonna be real with you: I don't have the time atm to dig into that to review which would be better. It's best that you send in a PR according to your best insight at which one would be better.

@driesvints
Copy link
Member Author

Thanks to @axlon, this feature will be available in the next major Passport release.

@axlon
Copy link
Contributor

axlon commented Jun 13, 2022

Hey @driesvints, do you have any indication as to when the next major Passport release will be tagged? It seems it did not get one when Laravel 9 came out.

@driesvints
Copy link
Member Author

Not yet. It's on my list but right now other things are keeping me from checking in on it.

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

Successfully merging a pull request may close this issue.

7 participants