-
Notifications
You must be signed in to change notification settings - Fork 784
Limiting scopes to certain oauth_clients #1272
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
Comments
Maybe I'm misunderstanding, but what is the goal of this? Without testing this myself are you saying a resource can use a scope that wasn't granted to it? Or are you trying to prevent certain clients from requesting some scopes as they are considered "internal use only"? |
@ReArmedHalo More of the latter. My application has many first-party "apps" used by various devices. So I had the need to make sure one specific app could not request a scope that it didn't need. I did this by setting an array of |
@billriess oh okay, so you have a scope that you don't want a third party developer to be able to request when asking for an access token? I'm guessing your flow for third party developers is how some services will have you select the scopes during application creation and then you can't ask for anything more than those? That way you can manually add "internal" scopes to "trusted" clients. If I am understanding you right, then yes I agree with this and would like to see that be an option in Passport. However, I believe I have also read that you shouldn't be using scopes for "authorizing" access to data that might be sensitive (I can't seem to find where I read that so I'm not sure if it was in Passport issues past or some OAuth docs so this may be way off basis as it was years ago). I believe the idea of protecting access to certain data was to mark a client as "trusted" using some mechanism like a column in the database. I do this method currently in an app I'm working on and have the I like the idea and might do it myself in an upcoming app. I would also be interested the thoughts of others as well. |
So in my case, we don't really have any third-party applications. Our business is around public transportation and parking. We have apps and devices (parking meters, bus validators, ticket vending machines, etc) deployed in various cities that all interact with our backend. We are using Passport's This change helps us better define what an |
Hi @billriess, I am facing a similar use case of this issue. What I am trying to do is to use an authorization server using passport that will only authorize access tokens to the clients that are registered on the table As far as I could understand, the client credentials flow it is used to communicate various services between each other, using an Authorization Server and each service would act as a Resource Server. For example, lets say we have three services: Service A, Service B and Service C and an Authorization Server that issues the tokens, so the following rule is applied to Service A:
So, when Service A requests a token to the Authorization Server, this will issue a token that internally has information (metadata of the token) of what services this service can retrieve information (resources) from, in this case it would only have access to data of Service B but not of Service C. My question here is that, how did you add the scopes to the token that I think would be the {
"aud": "939d4719-c1d0-4937-b279-58c8a3ba3dd2",
"jti": "9fc0f272368bde2a340d48b2ff31611754c38756570899a4c4fc3eca20fe638cbf50cea9ee6fbeb5",
"iat": 1623042730.238692,
"nbf": 1623042730.238695,
"exp": 1623043630.23013,
"sub": "",
"scopes": ["ServiceB"]
}
Why? According the documentation of laravel passport, it is a matter of a few lines of code which you can define global scopes for each client and token that it is issued by the package using: Passport::tokensCan([
'place-orders' => 'Place orders',
'check-status' => 'Check order status',
]); But what I want to do is to add the specific scopes to the issued token for an specific client and sadly I am stucked on that 😞 I hope you understand the scenario I explained and that you or someone else can help me out solving this "problem" or if I am miss-understanding the concept of Client Credential Grant Type. Thanks! |
This was previously attempted in https://github.com/laravel/passport/pull/747/files a few years back but the PR was rejected 😞 @xdiegom, if you follow the same approach as the PR I've linked then you should be able to set scopes for the client by populating the field in the database with some scopes. How you do that depends on your workflow for creating clients. It could via a UI that populates the clients table or a custom artisan command for example. |
Thanks @robbytaylor, Sad to read that. Although I'll look for that approach 😃 |
@billriess @robbytaylor short on time so don't have time to read all of this issue again or the related PR's. Taylor did mention he wanted to revisit this here: #747 (comment) But he mentions that he wants his original questions answered. What I suggest one of you do is resend in #747 (or the current-day equivalent) with a thorough explanation and answers to his questions to build up a good case. If the PR gets accepted all the better but if not then I think we're going to close this one off. |
Hi all, since there's no activity here anymore I'm going to close this one. Remember that we're open to PR's but for now we're not looking to actively work on this ourselves. |
Wow, not even a week between commenting and then closing the issue. I personally don't work with PHP anymore so I'm not intending to update my previous PR. I'm more than happy for someone else to update and resent it though. It's really not clear to me anyway what questions remain to be answered. The rationale and use-cases were discussed at length in #747 and #691. I'm not sure I could answer the questions now anyway since I haven't used this library for ~ 3 years. |
As OS maintainers aren't required to comply with your personal schedule 🤷♂️ This repo is open for PR's. Feel free to send one in if you want to put in the work. |
@driesvints can you help clarify what Taylor's questions are? Glancing through this and #747 I couldn't find what he was asking. |
@billriess I doubt Taylor knows himself since it's been so long. I suggest to simply re-try the PR. |
@driesvints Ok, just wanted to make sure I didn't miss it somewhere in one of the tickets. Unfortunately, we've been moving away from Passport and towards Sanctum in our project as it fits our needs better. Still trying to come up with a clean way to transition the two libraries though. So I don't know that I'll have time to really work on this one. |
I haven't gotten to a point in my current project that I needed this, but I will shortly, I would also throw my hat in the ring to try and submit a PR for this if no one else is able to do so right now. |
I found this package that partially solves my problem of scopes by clients, but still with limitations. I open an issue for a limitation on authorizing individual scopes. Because the clients scopes in database is acting as authorized scopes instead of allowed scopes. And maybe i contribute with a PR to that lib. But i think if this feature is native in the Passport, it will be much better. |
I've also run into this issue because in my mind a certain client I've given access to for example shouldn't be able to escalate to use the admin scope. To do this, I've updated my Passport::routes(function (RouteRegistrar $registrar) {
$registrar->all();
Route::group(
['middleware' => AuthorizationScopeMiddleware::class],
fn() => $registrar->forAuthorization()
);
}); Which in essence creates all the Passport routes and then re-initialises the authorization endpoint with the AuthorizationScopeMiddleware class. This class is an invokable class, as below: <?php
namespace App\Packages\Passport\Middleware;
use App\Packages\Passport\Exceptions\InvalidScopeException;
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Illuminate\Http\Request;
use Illuminate\Support\Str;
use Laravel\Passport\Http\Controllers\HandlesOAuthErrors;
use Laravel\Passport\Passport;
use League\OAuth2\Server\Exception\OAuthServerException;
use Psr\Http\Message\ServerRequestInterface;
class AuthorizationScopeMiddleware
{
use HandlesOAuthErrors;
public function __invoke(Request $request, \Closure $next)
{
$this->withErrorHandling(function () use ($request) {
$scopes = Str::of($request->get('scope'))->explode(' ');
try {
$client = Passport::clientModel()::query()
->select('permitted_scopes')
->findOrFail($request->get('client_id'));
} catch (ModelNotFoundException $e) {
throw OAuthServerException::invalidClient(app(ServerRequestInterface::class));
}
if (!Str::of($client->permitted_scopes)->explode(' ')->has($scopes->toArray())) {
throw InvalidScopeException::make();
}
});
return $next($request);
}
} I have then created a custom exception to handle the use case for invalid scopes - <?php
namespace App\Packages\Passport\Exceptions;
use League\OAuth2\Server\Exception\OAuthServerException as LeagueException;
class InvalidScopeException extends LeagueException
{
public static function make()
{
return new self(
'Unknown or unauthorized scopes were requested',
4,
'invalid_scopes',
401
);
}
} Finally, I have created a I think this is an elegant solution with the only downside (I can think of) being that it has to make one additional query on the authorization endpoints. |
I wanted to start a discussion regarding scopes and
oauth_clients
. I've found the need in my project to limit allowable scopes peroauth_client
. We have many first-party applications that useclient_credentials
grants. We protect our routes using theCheckClientCredentialsForAnyScope
middleware. Since scopes are passed via request, any request could use any scope it wanted.I've ended up adding an
allowed_scopes
column to myoauth_clients
table and casted it as an array. I then check if the requested scope is in the array when validating the grant. If it is, you're good to go. If not, you get an invalid scope exception.Would anyone else find this useful? Am I doing something wrong to not think this would be needed/useful for similar scenarios? I'm genuinely interested in some thoughts here. If it seems attractive enough of an add, I can issue a PR.
The text was updated successfully, but these errors were encountered: