[8.0] Allow public clients

[matt-allan]

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:

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, the authorization code grant, and the password grant. The client credentials and personal access grants are only usable with a confidential client.

I pulled in changes from #1064 so the tests would pass. This may need to be rebased once that is merged.

[taylorotwell]

The UI is very unclear as to what this even does. As an end user, I would be very confused.

[matt-allan]

The terms come from the OAuth 2.0 spec. The user needs to declare if the client is confidential or public so the server can know if it should require the client to authenticate with a secret. Do you have any ideas for alternative wording to explain that?

Keycloak (the only other open source OAuth server I know of with a UI for this) uses a 'client type' dropdown with 'public' and 'confidential' options (screenshot).

Auth0 has you select an application type (native app, Single page web app, regular web app, or machine to machine app) and uses that info to determine if the client should be confidential or public (screenshot).

Do you think either of those approaches would be less confusing?

The UI didn't seem unclear to me but I've read the OAuth specs and I'm familiar with the terminology. I would love to make this less confusing for end users that aren't as familiar with OAuth but I'm not sure how to succinctly explain everything in a single sentence.

[taylorotwell]

Yeah, but I mean typical end-users of SaaS applications don't know or care about the OAuth 2.0 spec, so we need to specify what this actually does?

[matt-allan]

These are end users that are creating a OAuth 2.0 client though right? I would think they need to understand OAuth to know what an 'authorization callback URL' is too.

I'm all for simplifying this but it isn't clear to me what you would like this to say. It currently says exactly what it actually does - checking the box causes the server to require the client to authenticate with a client secret.

Do you have any thoughts on the alternatives I proposed previously? I added links to screenshots so it's easier to see what I'm talking about. If you want this to be usable by users that don't know anything about OAuth the Auth0 approach might be a good fit?

[fh-jashmore]

If you're creating a client for the first time, you haven't seen a secret on the screen yet. They may not know what a secret is.

[matt-allan]

@fh-jashmore sure, I'm not arguing that all users will know what a secret is. My question is if they don't know what a secret is how should the UI prompt them to decide if the client should have a secret or not?

It sounds like the general idea of the feedback so far is that the end user might not know what a confidential vs public client is or what a client secret is, but nobody has proposed an alternative way to ask the user if the client should be confidential or not.

Because of the way the League server works in 8.0 it's necessary to know if the client is confidential or not, and if we want to support public clients we need to allow creating public clients.

I've proposed two alternatives above that don't require the end user to know what a secret is but nobody has offered feedback on those options yet. It takes a non trivial amount of time to rewrite the code and test everything with a real app so it would be really helpful if someone could provide feedback before I update the PR.

[fh-jashmore]

What about focusing on a Public description instead of Confidential?

[taylorotwell]

I'm a developer and I would have no idea what this checkbox does.

[matt-allan]

🤦‍♂️ @taylorotwell I understand that you think it's unclear. I'm not sure where the miscommunication is happening here, but I'm just trying to ask for suggestions on how to make it more clear.

I've asked three times now for suggestions on better wording and proposed two alternative UIs and directly asked for feedback. In response I have just received three variations of 'the original UI is unclear'.

To be perfectly clear: I understand you think the original UI is unclear. I am not trying to argue that. All I said was it's not unclear to me. That isn't me trying to argue that it's clear for everybody. I'm not trying to state opinion as fact.

Is there something I said that makes you think I am trying to argue with you about whether or not you think this is confusing/unclear?

As I said before, I would love to update this PR with a UI that is less confusing. I am asking for suggestions for how to make the UI more clear.

'unclear' and 'confusing' are subjective and since I don't personally find the UI unclear I need some input on what would make it less unclear. That is all I'm asking for here. The only concrete feedback I have gotten is "we need to specify what this actually does" which doesn't make any sense to me because it literally says exactly what it does.

This whole experience has been really frustrating. I was hoping for collaboration and feedback; I'm not trying to argue. After asking this many direct questions and receiving the same quality of response it doesn't seem likely that is going to happen.

[driesvints]

@matt-allan hey Matt, I'm sorry you feel that way. Taylor has a lot on his plate atm with Laracon and Vapor. Maybe it's best that we took this discussion back to the issue for a bit to figure out the best approach to tackle this? Then we can try again with a fresh approach 👍

[matt-allan]

Thanks for the non-apology apology I guess? I'm not really interested in continuing to work on Passport at this point; it hasn't really been an enjoyable experience for me. Someone else can take over this PR if they are interested.

[taylorotwell]

Matt, sorry you feel that way. If you feel up to it, I would just augment the wording with maybe a full paragraph explanation of what the checkbox actually does. I don't know what it does myself, so there isn't much for me to contribute 😅. If you are able to add a few more sentences then it might get the ball rolling.

[taylorotwell]

As a follow up, do other SaaS apps like GitHub have this option? If so, what is their wording - maybe we can just pull from there?

[fh-jashmore]

Heres the spec on client types https://tools.ietf.org/html/rfc6749#section-2.1

   confidential
      Clients capable of maintaining the confidentiality of their
      credentials (e.g., client implemented on a secure server with
      restricted access to the client credentials), or capable of secure
      client authentication using other means.

   public
      Clients incapable of maintaining the confidentiality of their
      credentials (e.g., clients executing on the device used by the
      resource owner, such as an installed native application or a web
      browser-based application), and incapable of secure client
      authentication via any other means.

[taylorotwell]

Thanks

[taylorotwell]

I supplemented the wording a bit with help from the Auth0 docs. Removed the migration as I think we would just provide that in the upgrade notes.

[driesvints]

Thanks @matt-allan!

[htunnicliff]

Are there any issues or PRs out there that add either of the following?

• Additions to the passport docs
• Updates to the artisan passport:client

[driesvints]

@htunnicliff not yet I'm afraid

[taylorotwell]

@htunnicliff is this something you can help us with? We personally do not use this type of SPA flow and need to talk to someone who does and understands it well. You can ping me on Discord or email me.

[Sephster]

@taylorotwell if there are no takers for writing the docs, give me a shout. I'm on my Christmas leave soon so should have some time free if you are stuck

[driesvints]

@Sephster that would be super helpful. Thank you!

[htunnicliff]

@taylorotwell, @Sephster, @driesvints – I'm implementing an OAuth flow for the first time, so I can only offer a learner's first-time perspective. The reason I was hoping updates were coming to the Passport docs was for clarification of best practices for issuing OAuth clients.

After reviewing "Which OAuth 2.0 grant should I implement?" from the League OAuth 2.0 Server docs, they list these clients as best practice:

• Client Credentials Grant
• Authorization Code Grant
• Authorization Code Grant with PCKE (specifically for browser and native apps)

The doc also mentions:

The Password Grant and Implicit Grant are not included in our recommendation diagram as these grants have several drawbacks and/or are no longer considered to be best practice.

Following the League's recommendations here points to using an Authorization Code Grant with a PCKE challenge. The trouble I'm running into is figuring out how to implement a PCKE challenge using Passport.

Since I haven't used this kind of OAuth flow before, I am hesitant to write documentation for it.

Do you think there is a need to update the Passport docs to recommend legacy clients to avoid and to provide examples for how to use PCKE correctly?

[htunnicliff]

I did some more research, and it appears that the OAuth 2.0 Security Best Current Practice and OAuth 2.0 for Browser-Based Apps IETF documents officially discourage usage of Implicit Grant and Password Grant.

[driesvints]

We definitely need to update the docs for PKCE. We're welcoming prs for that.