Doorkeeper 5.5 upgrade

[MacTwister]

Trying again the v5.5 upgrade, with the default scope, tested with some existing apps that broke last time.

The updated tests now set a default scope for the access token factory. I guess I could have used the config default of doorkeeper, thought a fixed value better, in future if testing different scopes.

@viktorsmari Could you point me how-to/what it's called in Rails to make a cli/command/script to update db records, to be run after deploying this PR, only needs to be run once? Need to set a "default" scope on all existing OAuth grant records, otherwise existing users will be required to re-authorize the app on the next login (because they haven't yet authorized that scope).

[viktorsmari]

I think you might be talking about rails db:migrate

If you look at line 5 in the deploy script, it checks if migrations needs to be run: https://github.com/fablabbcn/fablabs.io/blob/c79b96d3d61274486f7cc96b15b62bf29bddecef/scripts/deploy.sh#L4-L7

It does not run them, but I think it should be fine to make the script also run migrations So we could simply add docker-compose exec app rails db:migrate after line 5

https://guides.rubyonrails.org/active_record_migrations.html#writing-a-migration

If you want to create your own migration script you can do that. For example rails g migration MyCustomMigration which generates a file that you can edit and put commands inside. This file will only be run once, even though you run rails db:migrate 10x

[MacTwister]

ah yeah, I know migration, was curious on the Rails ways to update records. Though migrations make sense for what I need to do. 👍 Thanks for the docs/hints.

Then thinking something like: rails g migration SetDefaultScopeForExistingOauthGrants

[viktorsmari]

Yes exactly, inside that file you can do something like

Client.all.each do |c|
  c.update! name: 'John'
end

You can also use the rails console to do this directly on the production server docker-compose exec app rails console

[MacTwister]

Oh yeah true, I don't have opinion here what would be preferred.

Did this, through tests, I probably only need to fetch the most recent record perapplication_id & resource_owner pair https://github.com/fablabbcn/fablabs.io/blob/a304fc67c19dacb1d280d52b993dcf04da5adb92/db/migrate/20220128091016_set_default_scope_for_existing_oauth_grants.rb#L3-L7

[MacTwister]

@pral2a Checking with you if you have any opinions on setting the default scope for existing logins.

Scenario

Since we now ask for confirmation, I thought it would be weird to suddenly ask existing users, when they next attempt to log in (ie with GitLab).

Options

Was planning on running an SQL query to set a default scope, either:

• For all existing login tokens (inc old ones)
• For the past year of logins (this would be my preference, people might get confused with the page)
• [NEW] Do nothing, then all new logins from the deploy time will show the confirmation dialog

Example query:

UPDATE oauth_access_tokens SET scopes = 'public' WHERE scopes IS NULL AND revoked_at IS NULL;

Example of first login check (per app):