Support to Uuid

[jimmylagp]

I have a problem with Passport and my Uuid users. Can you give me a solution for that problem?.

[nikkuang]

Since Passport uses integer for user id, If you want to use uuid for your users you have to modify the script for migration. If you have the latest version of Passport, you can add this code Passport::ignoreMigrations(); on your AuthServiceProvider@register then run

php artisan vendor:publish --tag=passport-migrations

and change the migration scripts, just change all user_id from integer to uuid and run your updated migration script

[livingos]

I think the client id should also be a Uuid by default and not just an integer.

If you are making your API available to external users, Uuid makes more sense.

[heisian]

Yes I'm looking for this as well - it seems pretty standard in any OAuth scheme to use UUID or some sort of random string instead of normal integers for a client ID.

[heisian]

Introduced UUID here: https://github.com/laravel/passport/pull/168

• Passes all unit tests
• Retains original primary key id for indexing performance
• add Passport::useClientUUIDs to app/AuthServiceProvider.php to enable
• works with existing installations by running passport:uuid command

[stefensuhat]

@heisian is this has been merge?. can't apply passport because of integer user_id

i tried to use your git when composer require heisian/passport got an error

[heisian]

@ssuhat no, not yet. My pull request is working from the master branch while the current stable release is 1.0. So there may be more changes Taylor makes to master that I'll be pulling in.

I also suspect there are some bugs present even though all the unit tests pass. I have to do some manual tests on the AuthorizationController and the ClientController, niether of which I use in my own implementation so I haven't had a chance to check yet.

Maybe this will make it to the next version. My advice is to take my pull request and merge it into your own fork for now.

[themsaid]

@ssuhat

can't apply passport because of integer user_id

You can alter the migration if you want.

[stefensuhat]

@themsaid i won't do that. it going take too much work to change my user_id to integer.

[themsaid]

@ssuhat No, I mean Passport's migrations, you can alter the key type to be string.

[aliasdoc]

Hi when I try to publish migration files, it fails:

php artisan vendor:publish --provider="Laravel\Passport\PassportServiceProvider" --tag=passport-migrations
Nothing to publish for tag [passport-migrations].

I added service provider in config/app.php :

/*
* Package Service Providers...
*/

 Laravel\Passport\PassportServiceProvider::class,

I forgot something ?? Thanks

[heisian]

I get the same issue too. not sure why the migrations won't publish. will try with a completely fresh installation

[jimmylagp]

We try to create alternative for Passport with uuid support, If you want to contribute to this alternative, we are happy to receive help. https://github.com/html5facil/passenger

[aliasdoc]

Hi @jimmylagp , maybe it's better to use id AND uuid for performance, no ?

[jimmylagp]

Hi @aliasdoc id is good but when i have a big project, is better to use uuid for security and integrity in data base. With uuid never you will have consecutives ids.

[heisian]

No offense, but I think @jimmylagp's solution is a bit over-engineered. The eval statements within the migrations are an instant turnoff for me personally.

I've created and maintain a fork of the Passport package that keeps intact the original oauth_clients.id column while adding a char(36) oauth_clients.uuid that's only used for public/external lookup. Internally, the package still uses the row id for indexing/performance, and as little impact as possible to the internal workings.

Additionally, you can easily enable or disable UUIDs by setting Passport::useClientUUIDs();. If you've already installed Passport, you can simply run the passport:uuid command to run an appropriate migration and generate version 4 UUIDs for your clients.

https://github.com/heisian/passport

[jimmylagp]

Thanks for your feedback @heisian.

[heisian]

Thank you for taking it well @jimmylagp - Passport is a relatively new package and we're not sure what changes will come.. best to keep any deviations as simple as possible!

[aliasdoc]

:) I probably spoke of it (my english is not very famous), I was just talking about keeping ids for internal processing (search, indexing, ...) and using the uuid for security (public), like the fork of @heisian mentioned above. My comment was not a negative review @jimmylagp, I was just asking the question since it seems to me that we can not create an index on a uuid field char (36). Thank you for your comments.

[heisian]

@alias all being said, @jimmylagp's package will do just fine for their own purposees.. I don't think indexing really becomes a huge issue until you're serving hundreds of thousands/millions of requests per day, or something like that.

Suffice to say, my usage is definitely not that huge - but always good to plan for the future!

[jimmylagp]

@aliasdoc I agree with heisan's opinion and don't worry, my English is not good yet.

[nagaxiii]

@heisian You may want to change the Laravel\Passport\ClientRepository@find method to

public function find($id)
{
        if (!Passport::$useClientUUIDs) {
            return Client::find($id);
        } else {
             return Client::where('uuid', $id)->firstOrFail();
        }
}

since the Laravel\Passport\Http\Controllers\AuthorizationController@authorize with the following lines:

$request->session()->put(
    'authRequest', $authRequest = $this->server->validateAuthorizationRequest($psrRequest)
);

ends up using the repository's find($id) method regardless the UUID setting and you won't be able to call the oauth/authorize endpoint with the UUID.

[heisian]

that was the first thing i tried but theres a hidden issue with that, forgot what it is. thanks for pointing out, i'll investigate more when i can

On Feb 23, 2017 2:16 AM, "Gábor Nagy" notifications@github.com wrote:

@heisian https://github.com/heisian You may want to change the Laravel\Passport\ClientRepository@find method to

public function find($id) { if (!Passport::$useClientUUIDs) { return Client::find($id); } else { return Client::where('uuid', $id)->firstOrFail(); } }

since the Laravel\Passport\Http\Controllers\AuthorizationController@ authorize with the following lines:

$request->session()->put( 'authRequest', $authRequest = $this->server->validateAuthorizationRequest($psrRequest) );

ends up using the repository's find($id) method regardless the UUID setting and you won't be able to call the oauth/authorize endpoint with the UUID.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/laravel/passport/issues/122#issuecomment-281952265, or mute the thread https://github.com/notifications/unsubscribe-auth/AD8zBEfNS839jwzdcQocVWCoJV9HQEM2ks5rfVyOgaJpZM4KQmbl .

[nagaxiii]

@heisian In the Laravel\Passport\Http\Controllers\AccessTokenController@issueToken the following part actually negates the change in the Laravel\Passport\ClientRepository@find method what I've suggested... If you remove it you can easily use the repository overload.

if (Passport::$useClientUUIDs) {
    $rq = request();

    $client = Client::where('uuid', $rq->input('client_id'))->firstOrFail();

    $formData = $request->getParsedBody();

    array_set($formData, 'client_id', $client->id);

    $request = $request->withParsedBody($formData);
}

There might be more hidden issues though, but it is working for me so far. Thanks for taking care of that! :)

[heisian]

yup i believe that was added after finding that modifying the find method didn't work. my commit history might show the reasoning. either way its hacky and worth revisiting.. will take another look when I get a chance!

On Feb 23, 2017 6:44 AM, "Gábor Nagy" notifications@github.com wrote:

@heisian https://github.com/heisian In the Laravel\Passport\Http\Controllers\AccessTokenController@issueToken the following part actually negates the change in the Laravel\Passport\ ClientRepository@find method what I've suggested... If you remove it you can easily use the repository overload.

if (Passport::$useClientUUIDs) { $rq = request();

$client = Client::where('uuid', $rq->input('client_id'))->firstOrFail();

$formData = $request->getParsedBody();

array_set($formData, 'client_id', $client->id);

$request = $request->withParsedBody($formData);

}

There might be more hidden issues though, but it is working for me so far. Thanks for taking care of that! :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/laravel/passport/issues/122#issuecomment-282009555, or mute the thread https://github.com/notifications/unsubscribe-auth/AD8zBHFCTVlkmYVSFmxN0DtjKHK_eT48ks5rfZsvgaJpZM4KQmbl .

[heisian]

The issue there is that the Client instance at that point in the code doesn't have access to or is not in sync with the 'global' Passport instance and thus would always think useUUIDs was false. So that's why doing a simple modification to the find method didn't work. You're welcome to try it yourself!

On Feb 23, 2017 9:05 AM, "Tim Huang" tim@revolverobotics.com wrote:

yup i believe that was added after finding that modifying the find method didn't work. my commit history might show the reasoning. either way its hacky and worth revisiting.. will take another look when I get a chance!

On Feb 23, 2017 6:44 AM, "Gábor Nagy" notifications@github.com wrote:

@heisian https://github.com/heisian In the Laravel\Passport\Http\Controllers\AccessTokenController@issueToken the following part actually negates the change in the Laravel\Passport\ClientRepository@find method what I've suggested... If you remove it you can easily use the repository overload.

if (Passport::$useClientUUIDs) { $rq = request();

$client = Client::where('uuid', $rq->input('client_id'))->firstOrFail();

$formData = $request->getParsedBody();

array_set($formData, 'client_id', $client->id);

$request = $request->withParsedBody($formData);

}

There might be more hidden issues though, but it is working for me so far. Thanks for taking care of that! :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/laravel/passport/issues/122#issuecomment-282009555, or mute the thread https://github.com/notifications/unsubscribe-auth/AD8zBHFCTVlkmYVSFmxN0DtjKHK_eT48ks5rfZsvgaJpZM4KQmbl .

[rotorsolutions]

Also looking for a solution to migrate from the LucaDegasperi package.

@heisian You could choose to remove the 'useUUIDs' setting and change it to a simple 'is_numeric' check. This would also allow a hybrid use of both the UUIDs and standaard Passport IDs.

This would simplify the change to just the 'find' method (apart from the creation part)

PS: Great work!

[lobermann]

@rotorsolutions I am hitting the same issue, migrating from LucaDegasperi. Did you find a solution? I also can not change how it worked with the LucaDegasperi package as there are a whole lot of clients using it already.

[robinvdvleuten]

Why not make the related model classes configurable like Laravel Spark? This way all users can extend models and customize to their needs. This prevents users from opening PRs for adding x/y/z id generation solutions.

[robinvdvleuten]

I've created a PR with this functionality => #715

[carlosafonso]

Hi,

My app uses UUIDv4 for user IDs, so I tried modifying the database schema used by Passport but even though the column now has the correct type, data is still being truncated somehow.

> describe oauth_access_tokens;
+------------+--------------+------+-----+---------+-------+
| Field      | Type         | Null | Key | Default | Extra |
+------------+--------------+------+-----+---------+-------+
| id         | varchar(100) | NO   | PRI | NULL    |       |
| user_id    | char(36)     | YES  | MUL | NULL    |       |
| client_id  | int(11)      | NO   |     | NULL    |       |
| name       | varchar(255) | YES  |     | NULL    |       |
| scopes     | text         | YES  |     | NULL    |       |
| revoked    | tinyint(1)   | NO   |     | NULL    |       |
| created_at | timestamp    | YES  |     | NULL    |       |
| updated_at | timestamp    | YES  |     | NULL    |       |
| expires_at | datetime     | YES  |     | NULL    |       |
+------------+--------------+------+-----+---------+-------+

> select id from users;
+--------------------------------------+
| id                                   |
+--------------------------------------+
| 42784bcd-9c7e-47e8-b8ac-0a46ab9c76d8 |
+--------------------------------------+

> select id, user_id from oauth_access_tokens;
+----------------------------------------------------------------------------------+---------+
| id                                                                               | user_id |
+----------------------------------------------------------------------------------+---------+
| 893257ad5b917f85d859eb50b2b36515df486854c3c5f0ff03c68aecc1037b8db9ed1d7cfa9baeee | 42784   |
+----------------------------------------------------------------------------------+---------+

Is there any additional step I may have missed?

[carlosafonso]

Please ignore my previous comment, as my User model did not set the $incrementing flag to false. Sorry for that. I can confirm that just changing the column type is enough.

[driesvints]

Think this question has been answered on how this can be solved.