UUID for account ID

[saul-data]

Is there a way to rather store the account ID or user ID as UUID instead of integers? I worry about iterating over integers where we would have to separately map UUID to an account/user.

[cainlevy]

I could imagine making it a default configuration for new setups. We can't guarantee backwards compatibility on existing installs though.

[saul-data]

That would be amazing, we are a new project so that would be perfect.

…

On Sun, 5 Jul 2020 at 22:36, Lance Ivy ***@***.***> wrote: I could imagine making it a default configuration for new setups. We can't guarantee backwards compatibility on existing installs though. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#160 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APGDMKLSLXX2UYZ35HINJLLR2DW45ANCNFSM4OQ6HTIA> .

[saul-data]

Oh and an audit table showing all the logins, logouts, registrations etc would also be very good. I love what you guys are building.

[cainlevy]

You mentioned using Gorm and Kubernetes on another issue, so I presume you're looking at github.com/keratin/authn-go for an integration. That library mostly treats the ID as a string (because of this possibility) but at one point did in fact encode the ID as an integer: https://github.com/keratin/authn-go/blob/master/authn/models.go#L10. I believe it only affects the return value of GetAccount, which is not critical.

[saul-data]

So when it decodes the JWT, we get account id: INT - that worries me. My focus isnt about gorm/kubernetes integration here. If you look at Fusionauth or Ory Keratos, they are both storing user IDs as UUID. For good reason, you cant iterate over UUID. I have integrated both those solutions btw and I dont like them for various reasons that I discovered.

I had a very scary incidence once where a security engineer managed to download my entire customer database by iterating over integer IDs! I learnt a hard lesson there and I've never used integers for IDs again. Lucky he found it before others did!

I typically use the uuid.New() from (https://github.com/google/uuid) which generates UUID V4. Then when unpacking the JWT as a user ID thats safer to use across the application. I could map it in another table but I'd prefer simple and relying on the authentication service for this.

Your best bet is to use varchar(36) in the database or something similar to have compatibility across databases. Btw there is an insert performance benefit with UUID too because the database doesnt have to work out the next integer which is actually a complex algorithm at scale.

[cainlevy]

The ID decoded from the JWT is thankfully a string:

https://github.com/keratin/authn-go/blob/45602e8424095484881debf5e40e2ded222e0206/authn/authn.go#L53-L55

The GetAccount API is used for reading account status information, like whether the account is currently locked or deleted. Furthermore the authn-go client requires you to submit the ID as a string before it returns the ID as an int. That's a minor bug and an easy one to overlook.

[cainlevy]

I should also mention that it's not possible for an end-user to enumerate accounts by taking advantage of incrementing IDs. The only AuthN API endpoints that use account IDs are private admin actions. The worst result of using an incrementing ID, I think, is that it's possible for an end-user to learn how many accounts exist in your system by creating an account and decoding their own JWT.

[saul-data]

I think also you can guess user ids like someone else in the system is ID 7. I do think it provides a level of protection one way or the other.

[saul-data]

just thinking about it many routes in an application will be based on the user ID. e.g. /account/settings/:id

so:
/account/settings/7
/account/settings/a69b6292-e817-4d2d-be00-485f74ed3a88

I guess instead of using routes, we could rely on the header but that too could be forged and iterated over.

[cainlevy]

Your approach may differ, but I've been advocating for applications to maintain their own users dataset in a 1:1 relationship with the AuthN accounts. Your URLs would then be /account/settings/:app_user_id instead of /account/settings/:authn_account_id.

[madbbb]

Such a good library and having integer as a primary key in the user table is insecure. We can see the number of users after registration and iteration through them quite easily.

[cainlevy]

@madbbb i'd be happy to look at pull requests that switch to UUID. it's primarily a challenge with legacy data.

in the meantime, i don't believe that iteration is possible in a standard setup. there shouldn't be any user-facing routes that rely on the account id.

[AlexCuse]

I think the best way to tackle this in v2 is probably to keep the sequence and add a new string column for UUID. Its easy to backfill with UUIDs on migration. Then we can make UUID the default and have a config option like USE_LEGACY_IDS thats noted in migration docs. Maybe we could provide some tooling to help with migration as well if someone wanted to migrate to using UUIDs.

I guess the UUID should be the primary key but we'll want to make sure good indexes remain available on the sequence column.