Doorkeeper: uid in oauth_applications table is required in mixin orm...

Created on 5 Mar 2020  路  8Comments  路  Source: doorkeeper-gem/doorkeeper

... but documentation says to remove it if you are using uuid.
See https://github.com/doorkeeper-gem/doorkeeper/blob/master/lib/doorkeeper/orm/active_record/mixins/application.rb
Lines 23, 23 and 82.

Also https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-PostgreSQL-UUIDs-as-primary-keys-with-Doorkeeper
The diff tells you to remove the column.

Encountered this problem while trying to add an app to my provider at localhost/oauth/applications.

EDIT:
I'm also wondering if doorkeeper should not use the uuid of the application record if the rails app use uuids. Feels weird to me to have both. Or is it alright since you don't want your record uuid to be shared around?

wontfix

Most helpful comment

Hi!

I was about to open the exact same issue. The documentation is very confusing on that matter.

I'm not sure removing the page from the wiki is a good thing but shouldn't we just rollback the last edit to keep the uid column? What harm would come from that?

All 8 comments

I think you need to use t.uuid type instead of t.string. Actually take your attention to id: :uuid definition. I think it will create required column itself, so you need to avoid t.string :uuid column. Also the docs could be outdated. I don't sure here

But this functionality could be an issue for you if you're using doorkeeper-jwt that must use token generator, not database UUIDs. So make this choice carefully

I'm also wondering if doorkeeper should not use the uuid of the application record if the rails app use uuids.

What do you mean by _Rails app use uuids_? It's not rails app, it's database tables. You can use any for your own tables, but (for example) predefined from the gem (like Doorkeeper). Also as stated above - you can actually switch to UUIDs in the migration generated by doorkeeper generator

I think you need to use t.uuid type instead of t.string. Actually take your attention to id: :uuid definition. I think it will create required column itself, so you need to avoid t.string :uuid column. Also the docs could be outdated. I don't sure here

Sorry if I was not clear. The point I was rasing was not on the column type, but on the column presence. The documentation says to remove the column if your database is using uuids, but it crashes when you are trying to create a new app, since the mixin is trying to assing the uid field - the column doesn't exist.

What do you mean by Rails app use uuids?

I used those words since I was following doorkeeper's rails tutorial, but I meant the postgreSQL database, sorry.

Yep, I checked the mixin and seems like it requires UUID, so you can't use postgres builtin.

# it's from mixin file:
before_create :generate_uid

def generate_uid
  self.uid = Doorkeeper::OAuth::Helpers::UniqueToken.generate if uid.blank?
end

The ways to solve it are:
1) Make your own Application class and override generate_uid method, configure Doorkeeper to use your class
2) Remove this outdated page from the Wiki
3) Introduce super-generic code for ORM to detect if UUIDs are used and reuse them

Hi!

I was about to open the exact same issue. The documentation is very confusing on that matter.

I'm not sure removing the page from the wiki is a good thing but shouldn't we just rollback the last edit to keep the uid column? What harm would come from that?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Not sure why the documentation is just not updated.

Just ran into the same issue, please update docs.

Was this page helpful?
0 / 5 - 0 ratings