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

[5.8] Support public clients #1052

Closed
wants to merge 1 commit into from
Closed

[5.8] Support public clients #1052

wants to merge 1 commit into from

Conversation

matt-allan
Copy link
Contributor

This pull request adds support for public clients.

The league/oauth2-server now supports using both public and confidential clients with the authorization code grant. The recommended flow for single page applications and native apps is the auth code grant with a public client and PKCE. Supporting public clients makes it possible for developers to use the authorization code grant with PKCE instead of the insecure implicit grant.

Previously all clients were required to have a secret. If you used the implicit grant the secret just wasn't checked. We can't just skip checking the secret for the auth code grant like we were doing with the implicit grant because we need to check the secret if it was issued.

Instead we need to offer the ability to create public clients, and only check the credentials if the client is confidential.

The UI looks like this:

Screenshot from 2019-07-17 16-28-13

The oauth_clients.secret column was updated to be nullable and the POST /clients endpoint was updated to allow passing a confidential param (defaults to true).

A public client can only be used with the implicit grant and the authorization code grant. The client credentials and personal access grants are only usable with a confidential client.

The password grant is supposed to be usable by public clients and this used to be possible but it's currently not working in league/oauth2-server, see https://github.com/thephpleague/oauth2-server/issues/889#issuecomment-512554693.g

If you attempt to use the auth code grant with a client was assigned a secret and don't provide the secret it will fail, because the League server calls ClientRepository::validateClient if Client::isConfidential returns true.

use Illuminate\Database\Schema\Blueprint;
use Illuminate\Database\Migrations\Migration;

class MakeClientSecretNullable extends Migration
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The migration isn't 100% necessary as we could just use the empty string for public clients but it seems more explicit to use NULL.

@@ -75,8 +75,6 @@ protected function handlesGrant($record, $grantType)
return $record->personal_access_client;
case 'password':
return $record->password_client;
case 'client_credentials':
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check was moved up to validateClient as the League server should only call validateClient for confidential clients, regardless of the grant type.

@driesvints driesvints changed the title Support public clients [5.8] Support public clients Jul 22, 2019
@matt-allan
Copy link
Contributor Author

I'm going to close this for now, I need to clarify some behavior with the League server first: thephpleague/oauth2-server#1034

@AlfaSchz
Copy link

Hello @matt-allan,
Are you still working on the public client for Passport? I am interested in a Oauth2 Authentication Code + PKCE solution and being able to use Passport for it would be great : )
Thanks for the awesome work!

@driesvints
Copy link
Member

@AlfaSchz Public Clients landed in Passport 8.0 thanks to @matt-allan

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

Successfully merging this pull request may close these issues.

3 participants