laravel / passport

Laravel Passport provides OAuth2 server support to Laravel.
https://laravel.com/docs/passport
MIT License
3.28k stars 779 forks source link

allow for automatic approval of authorization #243

Closed corbosman closed 7 years ago

corbosman commented 7 years ago

We use OAuth2 for internal web apps and user management and have no need for an authorization approval view. In lucadegasperi's oauth server you could just bypass it. I cant find any (easy) way to bypass it in Passport.

Would be nice to allow you to bypass this view.

craigpaul commented 7 years ago

@corbosman You could just use the Password Grant or the Fresh Api Token Middleware method instead since this is all internal use.

corbosman commented 7 years ago

@craigpaul It's multiple apps in multiple languages, using a central laravel server as single signon and identity provider. So I dont see how the fresh api token middleware is going to work. I also dont think that the password grant is going to work because of the single signon part. If I go to app 1, login in through oauth, then go to app 2, it should initiate the oauth cycle and immediately login the user into the second app without even providing a password. This worked fine with the now deprecated server from lucadegasperi's.

edit: actually, you may be right. I'll look into using the password grant.

craigpaul commented 7 years ago

@corbosman Ah ic what you mean about the middleware, that definitely isn't the right way. I assumed your apps were all in Laravel, so that's out the window.

If the password grant doesn't work out for you let me know and we can talk out how to get something working.

corbosman commented 7 years ago

@craigpaul I actually dont think the password grant is going to work. It would mean rewriting all our oauth client apps to add a login/pass screen and do a password grant instead of an authcode grant. That in itself is enough for me to try and find an alternative route to remove the approval view.

craigpaul commented 7 years ago

@corbosman Ah so you're pretty far into this. Well then, I would suggest you manually remap the route to your own controller and adjust the logic as needed.

web.php

Route::get('/oauth/authorize', [
    'uses' => 'AuthorizationController@authorize',
]);

That way you can take the logic from the existing controller and remove the view response that is returned in favour of what you need.

corbosman commented 7 years ago

Heh, was just doing that. Thanks for the help, hopefully in a future release there'll be a way to skip the approval view.

corbosman commented 7 years ago

Ok, that wasnt that difficult. For others that might want this functionality. Copied the AuthorizationController and some code from ApproveAuthorizationController and put them in my own Controller. Only gotcha is the route order, you cant add this route through web.php as the Passport::routes() is run after and overrides those routes. So I just added the route in the AuthServiceProvider.

Still, a way to just bypass the approval view would be appreciated.

damwhit commented 7 years ago

@corbosman looking to do the exact same thing, for the same reasons. Would you mind sending me your own controller code as well as the route added in AuthServiceProvider?

Any additional help is much appreciated. Also, agreed - this would be an excellent thing to have out of the box with passport.

corbosman commented 7 years ago

I use this. Had to replace the whole controller instead of extending the original controller because of the private methods.

https://laravel.io/bin/6LN6q

damwhit commented 7 years ago

This is great. Thanks @corbosman!

ReckeDJ commented 7 years ago

I have the same situation as I want to mark some of my clients as 'trusted' or 'first-party'. In this case, the approval screen should be skipped, otherwise the user can approve or deny the access in the approval view. I'm thinking of a simple boolean 'trusted' attached to a client in the database, as soon as the boolean is true it would redirect the user immediately with approved access. How do you think about this guys?

qrazi commented 7 years ago

A fyi, this now seems to be included in the 2.0 branch, specifically in version 2.0.4. See this commit: https://github.com/laravel/passport/commit/88e95971d59758f954e26686bf6d2e0563b85ab9

Edit: The fix above however determines grant was approved before based on existence of a valid access token. lucadegasperi/oauth2-server-laravel kept track of previous approvements by users through a separate grants table. That would be a better check here I think. It would also reflect better in de default AuthorizedClients-VueJS component to show which applications have a grant, instead of all (or complimentary to) valid access tokens, as it does now.

corbosman commented 7 years ago

I dont think so. This just makes sure you dont get the auth screen when you have a valid token. I would want to be able to never see the auth screen.

qrazi commented 7 years ago

@corbosman You are right, whilst implementing my use case I lost track of your exact use case, which the commit I referenced indeed does not support...

themsaid commented 7 years ago

Closing for lack of activity, hope you got the help you needed :)

JeanLucEsser commented 7 years ago

Well actually I’m in the exact same boat, and would love to have that option built in. There may be a lack of activity, but nonetheless this would be a very welcomed feature.

The Password Grant cannot be used for SSO, at least not if using SPAs, as the only secure way of keeping a token would be in an http only secure cookie. And using iframes is not a good practice. Unless I’m missing something, we cannot have SSO with a Password Grant.

So the only way I see for that is using standard Authorization Grant (or Implicit for SPAs), but as the client is trusted, we should be able to ask to bypass the user confirmation dialog.

As I see it, the issue could be reopened. If I’m mistaking, I’d love to be shown wrong!

tschoffelen commented 7 years ago

Yes, this is something I would like to see as well. A simple way to make a client as trusted, thereby skipping the approval screen.

pleasela commented 7 years ago

+1 for this, can't find any other solutions out there.

hootener commented 7 years ago

@corbosman I was looking to do something similar and was hoping to follow what you linked to here: https://laravel.io/bin/6LN6q, but the page 404's.

Would you mind linking to a more permanent solution in this Issue since it seems like several people are looking for a good way to do this?

Thanks!

JeanLucEsser commented 7 years ago

@themsaid, also, I think this thread should be reopened. Maybe even tagged as an enhancement request.

Unless you think there are reasons this functionality is not worth it or goes against the OAuth2 specs.

Thanks!

tschoffelen commented 7 years ago

Just found the updated version of the link that @corbosman posted: https://paste.laravel.io/6LN6q

Note that that example only auto approves the request when the user already has a access token for this client (and is perhaps outdated - I didn't try it myself), but can easily be updated to also auto-approve certain client IDs (line 89 is the key to that).

Nonetheless I also agree the thread should be re-opened for a build-in solutions, because this solution (subclassing) is just a hack and breaks as soon as anything is changed in the class that we are subclassing.

corbosman commented 7 years ago

@tschoffelen I dont know what makes you say that but you're incorrect. The code around line 89 is untouched, because it deals with checking if the oauth2 client was previously approved for this user. You obviously dont want to have to approve it again.

The modification from the original code deals with not returning a view when the client hasnt been approved yet, but instead immediately calling ApproveRequest() (which would otherwise be called after the user clicked on ok in the approve form).

I recently upgraded our oauth2 server to passport 2.x and this code was unchanged so it's still working fine.

I do agree with you it's a hack, but it's the quickest way I could get this working right now. Im not sure if approving without user input is part of the spec, but several oauth2 server packages implement it. It seems to be a very obvious use case if you want to use oauth2 more like a SSO service within enterprises. In our case I had 2 reasons for needing the functionality.

  1. i didnt want to have to rewrite (or worse, having others rewrite) about 15 oauth2 clients to use a different grant.

  2. but even if I could, I wouldnt want to because a few oauth2 clients are not under our control and I dont want them to have access to the plaintext password which would be the case if I used the password grant.

tschoffelen commented 7 years ago

Oops, yep, my mistake. Stopped reading the code when I saw a line containing approveRequest 😄

Thanks for the snippet though - really helpful!

adaojunior commented 6 years ago

I agree that is a great future that should be part of Passport (actually essential), and it is not complicated at all, it is just a matter of adding is_first_party: boolean to oauth_clients and checking it on Laravel\Passport\Http\Controllers\AuthorizationController.

@themsaid if you need reference on other implementations check https://auth0.com/docs/api-auth/user-consent#types-of-clients

Sephster commented 6 years ago

The grants that should prompt a user to provide authorisation details are:

The grants that should not prompt for user auth are:

If you are using Auth Code Grant or Implicit grant with a trusted client, consider moving to one of the other grants. If Passport is not behaving in this manner, I think it needs changing but I suspect this is not the case.

cyberfly commented 6 years ago

Latest Passport you can easily extends the AuthorizationController since the method is protected instead of private. I modify @corbosman answer a little bit and use table column to mark the client as trusted

https://snippets.cacher.io/snippet/1ba633797af1951fb3a5

atrauzzi commented 6 years ago

@Sephster - Issue here that the password grant requires the secret. Because the client is internal, but not trusted, the consent should be bypassed and the redirect_uri is basically the mechanism of security.

It would be nice if clients could be flagged in the DB to bypass consent.

Sephster commented 6 years ago

@atrauzzi I'm not sure what you mean by bypassing consent. Can you explain further?

atrauzzi commented 6 years ago

This issue is about bypassing the consent screen when the client_id is first party or "trusted". Rather than prompting the user to reject or authorize, when the client is first party, consent is implied. So therefore, don't ask.

Sephster commented 6 years ago

I can't test this just now but there should be no reject/authorize step at all for the password grant. There doesn't need to be. It doesn't really matter if the client is first part or not. If there is a validation step after the user has provided their credentials, I think think this is a mistake. Are you sure this is what happens?

atrauzzi commented 6 years ago

Apologies - overall this is about the implicit grants as they're the only ones that can be used for SPAs "to spec". They're also the ones that people are being guided to use.

Generally, I agree with this as it preserves the server's control over any initial authentication that might be required. (I say this because you could just say to hell with it and deliberately have a client_secret where you don't care if it's published with an SPA. But that removes the server's ability to do things like delegate authentication to a 3rd party. Implicit login is also about ensuring html, browsers and redirects are in the mix as best I can tell.)

But yeah, speaking to the implicit grant specifically, by default the attempt to authorize will present a login screen (if no session/cookie/auth already exists), and then after successful authentication, forward to the consent screen. All the code people are sharing above adds a column to the client that allows the grant to not require user consent because the authorization falls under the terms/privacy/consent/security context of an application they're already established with.

brutto commented 6 years ago

This issue already successfully are solved by adding prompt to authorization query with none if you do not want to ask user with creds with existen session.

atrauzzi commented 6 years ago

@brutto - Not quite. The prompt none is only going to allow for authorization to pass if it's already been granted. It's a subtle difference.

The issue here is to allow authorization to bypass prompting even when it hasn't been granted yet.

JeanLucEsser commented 6 years ago

@atrauzzi I didn't know about this prompt none thing (for non trusted clients). Where do you pass it? As a query param on the authorize endpoint? How does it work? Does it check if an access token as already been issued in the past, no matter if it is now revoked or expired? Imagine you have an access token valid for 10 minutes and a refresh one for 24h, I don't see asking the user to confirm a prompt every single day (or every 10 minutes if I don't use a refresh strategy).

In my tests, prompt=none doesn't do anything. For non trusted clients (ie requiring user consent via prompt), how would you implement an Always Approve option, so that you can get future access tokens without asking for new user consent?

@cyberfly Thank you for this updated snippet, still waiting for official support from Passport for trusted clients. As a matter of fact, would be nice to have the trusted client option (never ask user consent) and an Always Approve option (ask consent only once) for non trusted clients.

corbosman commented 6 years ago

This is about the prompt that says:

"Client Foobar wants to access your name and e-mail address, is that ok?"

This is a useless prompt in many environments as all clients and user data are internal, and authorisation for these pieces of data is implicit. But you still went a single signon, so the passport service should provide the login, not the client. Think of it more as a single signon service.

Ive been working with a modified authorizationcontroller ever since passport came out, so that route works fine.

JeanLucEsser commented 6 years ago

Yes it is about what is called the User Consent Prompt.

It is not useless. It informs the user. For non trusted clients it is even required. In that case an Always Approve option would be nice, so that an expired access_token (or refresh_token) won't force this prompt on the user if he has already approved it in the past (or somehow recently).

For trusted clients, it is useless. In a company intranet for exemple. In this case an option to flag clients as trusted and never show the prompt should be the correct behavior.

Note that I'm talking about the Authorization Grant here. The discussion is not about using the Password Grant or which grant is for what. We want the user to use the provider login page, we do not want the client to use its own login page as we don't want the client to be able to "see" the credentials.

corbosman commented 6 years ago

I think we just said exactly the same thing.

jgardezi commented 6 years ago

Is there any proper solution for this post or we have to write the provided snippets?

JeanLucEsser commented 6 years ago

No there is not.

I ended up overriding the AuthorizationController as in the modified @cyberfly answer to add the trusted client option.

For non trusted clients, I added a way to bypass the prompt with a ?prompt=none query parameter on the condition that a non revoked access token has been issued in the past for the client no matter if it has expired.

jgardezi commented 6 years ago

@JeanLucEsser thank you for your quick response, I wish there is a better way or open up to pull to resolve this issue.

A-Ghorab commented 5 years ago

@jgardezi you can fork it and point your app to the forked version it will be better and you don't need to put the code every time in every project.

I added to my fork the ability to add a trusted client using command.

I will share all changes tomorrow for every one (it's same code used by cyberfly)

A-Ghorab commented 5 years ago

Here is all the files changed https://snippets.cacher.io/snippet/eab6c308d4ca5b16392f

This is already your work nothing new just added the client command maybe @themsaid can check it now and see.

English isn't my original language so if any one sees something wrong just tell me

atrauzzi commented 5 years ago

I feel like a new ticket has to be made and pointed at this one to get attention.

Coding for "lack of activity" may have been premature. At least if I'm going by votes and recurrence...

glennjacobs commented 5 years ago

@AhmedGoGo that update looks great, why not submit a pull request?

pokrenz commented 2 years ago

hi all, I'm new to this. so, any newest method to skip the authorization page? any help would be appreciated. btw, i use latest version

corbosman commented 2 years ago

This is no longer necessary. As of 7.0 passport has a skipsAuthorization option. See this PR: https://github.com/laravel/passport/pull/1022