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 for automatic approval of authorization #243

Closed
corbosman opened this issue Jan 22, 2017 · 46 comments
Closed

allow for automatic approval of authorization #243

corbosman opened this issue Jan 22, 2017 · 46 comments

Comments

@corbosman
Copy link

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
Copy link

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

@corbosman
Copy link
Author

corbosman commented Jan 22, 2017

@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
Copy link

@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
Copy link
Author

@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
Copy link

@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
Copy link
Author

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
Copy link
Author

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
Copy link

damwhit commented Jan 26, 2017

@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
Copy link
Author

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
Copy link

damwhit commented Jan 26, 2017

This is great. Thanks @corbosman!

@ReckeDJ
Copy link

ReckeDJ commented Feb 15, 2017

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
Copy link
Contributor

qrazi commented Feb 27, 2017

A fyi, this now seems to be included in the 2.0 branch, specifically in version 2.0.4. See this commit: 88e9597

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
Copy link
Author

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
Copy link
Contributor

qrazi commented Feb 27, 2017

@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
Copy link
Member

themsaid commented Jul 5, 2017

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

@themsaid themsaid closed this as completed Jul 5, 2017
@JeanLucEsser
Copy link

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
Copy link

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
Copy link

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

@hootener
Copy link

@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
Copy link

@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
Copy link

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
Copy link
Author

@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
Copy link

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

Thanks for the snippet though - really helpful!

@adaojunior
Copy link

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
Copy link
Contributor

Sephster commented Apr 1, 2018

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

  • Auth Code Grant
  • Implicit Grant

The grants that should not prompt for user auth are:

  • Client Credentials Grant
  • Resource Owner Credentials (Password) Grant

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
Copy link

cyberfly commented May 4, 2018

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
Copy link

@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
Copy link
Contributor

Sephster commented Jun 21, 2018

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

@atrauzzi
Copy link

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
Copy link
Contributor

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
Copy link

atrauzzi commented Jun 23, 2018

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
Copy link

brutto commented Jul 10, 2018

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
Copy link

@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
Copy link

JeanLucEsser commented Sep 18, 2018

@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
Copy link
Author

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
Copy link

JeanLucEsser commented Sep 18, 2018

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
Copy link
Author

I think we just said exactly the same thing.

@jgardezi
Copy link

jgardezi commented Oct 9, 2018

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

@JeanLucEsser
Copy link

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
Copy link

jgardezi commented Oct 9, 2018

@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
Copy link

@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
Copy link

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
Copy link

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
Copy link

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

@pokrenz
Copy link

pokrenz commented Nov 15, 2021

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
Copy link
Author

This is no longer necessary. As of 7.0 passport has a skipsAuthorization option. See this PR: #1022

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

No branches or pull requests