Passport: The client id should not be an auto increment integer.

[tobiasthaden]

For sure that was already discussed nevertheless client ids should not be an auto increment integer for several reasons:

1. It shows information that should be hiddenable: The count of registered clients.
2. For security reasons: In the event of that a secret key would be public, it is very easy to get the connected client id.

Please, share your thoughts.

[Modelizer]

I think UUID should be added as an additional feature. Simply by running a command

php artisan passport:uuid activate

would now perform making client id as UUID. This way passport would be kept simple and whoever want to use this functionality can easily inherit.

[tobiasthaden]

Related Issues and PRs:

• 14

• 49

• 122

• 168

• 366

[chadhobson]

Mind-boggling that this STILL hasn't been done. Its a MAJOR breaking change if its really to replace Lucade's server.

[Modelizer]

The discussion is going on from Passport 0.1 version. I was expecting it to see it would be added in v2. But still, it is pending.

As per Taylor's comment, I remember he wants to keep Passport simple. But this is been demanded by a lot of people. That's why I said it should be added as an addition or optional feature.

I'll submit a PR if @taylorotwell accept it as an optional command to implement it.

[TimWolla]

For security reasons: In the event of that a secret key would be public, it is very easy to get the connected client id.

It's even worse. Quoting the email I sent to Taylor at the end of October without hearing back:

Hi

the /oauth/authorize route allows an attacker to enumerate all the
registered OAuth clients as the only value unknown to them (the
redirect_uri) is optional as of oauth2-server 5. See:
https://github.com/thephpleague/oauth2-server/issues/439

A URL as constructed by an attacker then would look like this:

/oauth/authorize?client_id=CLIENT_ID&response_type=code&scope=

The attacker simply counts up the CLIENT_IDs to retrieve information
about the clients registered in the application:

The route then exposes the redirect_uri (by authorizing) as well as the
Client Name (when using the default view).

Instead of using an sequence value a string from a secure random
generator should be used for the user facing client_id.

Applying the fix to existing clients causes breakage, because the
access_token and refresh_token JWTs encode the client_id.

For our own application I fixed the issue by migrating the clients to a
secure client ID, preserving the old client ID in the database rows and
adding a middleware rewriting the numeric client ID to the secure one
for a few weeks in order to allow consumers to catch up on the change. I
needed to forcefully revoke any existing access tokens in the database
as well.

Best regards
Tim Düsterhus

[mits87]

Hi, What is the current status of using UUID for client ID?

[josephxanderson]

@tobiasthaden @mits87 I just wrote up a Gist on using model observers for a custom ID for Passport Clients. Check it out on GitHub Gists.

However, I am unsure if UUIDs will work. They should, but I'm not sure if the Passport code checks for the ID to be an integer anywhere in the code. But give it a try and let me know if this works for you. I am using it in production currently and have no issues.

[bankorh]

@josephxanderson, your solution seems not to work correctly. From my understanding, although the value saved into the database is a UUID, but the Client model will return a wrong value for its ID. This is due to the data type of primary key is not overridden in the model so it is always be treated as an integer. Correct me if I'm wrong.

[josephxanderson]

@bankorh I have been using this for quite a while and have not had an issue. The Client model returns exactly what's in the database. I think it would only matter if you don't update the oauth_clients table to use the data type you want. Give it a try and let me know if you run into any issues.

[bankorh]

@josephxanderson in my case, the table data type is correct, and there's no error thrown. But do you notice that the client_id in oauth_personal_access_clients table didn't match any of the id in oauth_clients table?

I did run php artisan passport:install, and the result is always as following. It always show 0 or any random number, however, the Client ID should show UUID.

Personal access client created successfully.
Client ID: 0
Client Secret: pQg9jAd6SP3EwQ37UjTCXzj9ixTggdTinOaDdRF7
Password grant client created successfully.
Client ID: 0
Client Secret: IulAaS9EyLYjwWFtYMj4wRttqfrxw5pajDLFpSb3

[jh3er]

@josephxanderson me too , im having the same issue when changing all the client_id in migrations to string data type. I hope someone could help with this issue.

[JTallis]

@Modelizer 's idea would work perfectly here as by default, passport will still work with integers. This will avoid any breaking changes.

Plenty of valid points have repeatedly been made about how non-integer client IDs are preferred with the main one being If your client secret was leaked, it will be harder than going from 1 to N to find a match..

That said, you should have other concerns if your client secret managed to get leaked.

[diadal]

I create a simple package for this, it convert client id in all the table to Uuid.
https://github.com/diadal/passport

[bankorh]

Hey guys, I found a solution for this.

We can register Passport::useClientModel(YourUuidEnabledClient::class); and Passport::usePersonalAccessClientModel(Your UuidEnabledPersonalAccessClient::class).

Sample for my client class.

namespace App;

use Laravel\Passport\Client as BaseClient;

class Client extends BaseClient
{
    public function getIncrementing()
    {
        return false;
    }

    public function boot()
    {
        parent::boot();

        static::creating(function (Model $model) {
            if (!isset($model->attributes[$model->getKeyName()])) {
                $model->attributes[$model->getKeyName()] = Str::orderedUuid()->toString();
            }
        });
    }
}

I use a trait to generate the UUID, but I think the above code should work the same way.

NOTE: You need to use Passport::ignoreMigrations(); and publish the migrations, and update the IDs in migration tables to store UUID.

[mloberg]

You can also update the migrations and use model listeners. https://mlo.io/blog/2018/08/17/laravel-passport-uuid/

[driesvints]

Closing this as a duplicate of https://github.com/laravel/passport/issues/14. Please continue discussion there. Thanks.