From e02d23b5499318432981b16d8968e109ebeca18c Mon Sep 17 00:00:00 2001 From: Emelia Smith Date: Thu, 6 Jun 2024 09:30:10 +0200 Subject: [PATCH] Change `read:me` scope to `profile` scope (#30357) Co-authored-by: Claire --- .../api/v1/accounts/credentials_controller.rb | 2 +- .../settings/applications_controller.rb | 2 +- app/lib/scope_transformer.rb | 3 ++ config/initializers/doorkeeper.rb | 4 +-- config/locales/doorkeeper.en.yml | 3 +- ...3195202_change_read_me_scope_to_profile.rb | 23 +++++++++++++++ db/schema.rb | 2 +- lib/tasks/tests.rake | 28 ++++++++++++++++++- spec/lib/scope_transformer_spec.rb | 6 ++++ .../api/v1/accounts/credentials_spec.rb | 4 +-- 10 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 db/post_migrate/20240603195202_change_read_me_scope_to_profile.rb diff --git a/app/controllers/api/v1/accounts/credentials_controller.rb b/app/controllers/api/v1/accounts/credentials_controller.rb index e8f712457e..a378425183 100644 --- a/app/controllers/api/v1/accounts/credentials_controller.rb +++ b/app/controllers/api/v1/accounts/credentials_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Api::V1::Accounts::CredentialsController < Api::BaseController - before_action -> { doorkeeper_authorize! :read, :'read:accounts', :'read:me' }, except: [:update] + before_action -> { doorkeeper_authorize! :profile, :read, :'read:accounts' }, except: [:update] before_action -> { doorkeeper_authorize! :write, :'write:accounts' }, only: [:update] before_action :require_user! diff --git a/app/controllers/settings/applications_controller.rb b/app/controllers/settings/applications_controller.rb index 6849979b11..d6573f9b49 100644 --- a/app/controllers/settings/applications_controller.rb +++ b/app/controllers/settings/applications_controller.rb @@ -13,7 +13,7 @@ class Settings::ApplicationsController < Settings::BaseController def new @application = Doorkeeper::Application.new( redirect_uri: Doorkeeper.configuration.native_redirect_uri, - scopes: 'read:me' + scopes: 'profile' ) end diff --git a/app/lib/scope_transformer.rb b/app/lib/scope_transformer.rb index adcb711f8a..7dda709229 100644 --- a/app/lib/scope_transformer.rb +++ b/app/lib/scope_transformer.rb @@ -11,6 +11,9 @@ class ScopeTransformer < Parslet::Transform @namespace = scope[:namespace]&.to_s @access = scope[:access] ? [scope[:access].to_s] : DEFAULT_ACCESS.dup @term = scope[:term]&.to_s || DEFAULT_TERM + + # # override for profile scope which is read only + @access = %w(read) if @term == 'profile' end def key diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 1e8f9ad506..83100b1cf5 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -74,7 +74,8 @@ Doorkeeper.configure do # For more information go to # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes default_scopes :read - optional_scopes :write, + optional_scopes :profile, + :write, :'write:accounts', :'write:blocks', :'write:bookmarks', @@ -89,7 +90,6 @@ Doorkeeper.configure do :'write:reports', :'write:statuses', :read, - :'read:me', :'read:accounts', :'read:blocks', :'read:bookmarks', diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index 98776f2193..b623cc7135 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -135,6 +135,7 @@ en: media: Media attachments mutes: Mutes notifications: Notifications + profile: Your Mastodon profile push: Push notifications reports: Reports search: Search @@ -165,6 +166,7 @@ en: admin:write:reports: perform moderation actions on reports crypto: use end-to-end encryption follow: modify account relationships + profile: read only your account's profile information push: receive your push notifications read: read all your account's data read:accounts: see accounts information @@ -174,7 +176,6 @@ en: read:filters: see your filters read:follows: see your follows read:lists: see your lists - read:me: read only your account's basic information read:mutes: see your mutes read:notifications: see your notifications read:reports: see your reports diff --git a/db/post_migrate/20240603195202_change_read_me_scope_to_profile.rb b/db/post_migrate/20240603195202_change_read_me_scope_to_profile.rb new file mode 100644 index 0000000000..05e5984c48 --- /dev/null +++ b/db/post_migrate/20240603195202_change_read_me_scope_to_profile.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class ChangeReadMeScopeToProfile < ActiveRecord::Migration[7.1] + def up + replace_scopes('read:me', 'profile') + end + + def down + replace_scopes('profile', 'read:me') + end + + private + + def replace_scopes(old_scope, new_scope) + Doorkeeper::Application.where("scopes LIKE '%#{old_scope}%'").in_batches do |applications| + applications.update_all("scopes = replace(scopes, '#{old_scope}', '#{new_scope}')") + end + + Doorkeeper::AccessToken.where("scopes LIKE '%#{old_scope}%'").in_batches do |access_tokens| + access_tokens.update_all("scopes = replace(scopes, '#{old_scope}', '#{new_scope}')") + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 73f6b464e4..ce2951608b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_05_22_041528) do +ActiveRecord::Schema[7.1].define(version: 2024_06_03_195202) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/tasks/tests.rake b/lib/tasks/tests.rake index c8e0312bbd..c8e4dc31cd 100644 --- a/lib/tasks/tests.rake +++ b/lib/tasks/tests.rake @@ -130,11 +130,20 @@ namespace :tests do # This is checking the attribute rather than the method, to avoid the legacy fallback # and ensure the data has been migrated unless Account.find_local('qcuser').user[:otp_secret] == 'anotpsecretthatshouldbeencrypted' - puts "DEBUG: #{Account.find_local('qcuser').user.inspect}" puts 'OTP secret for user not preserved as expected' exit(1) end + unless Doorkeeper::Application.find(2)[:scopes] == 'write:accounts profile' + puts 'Application OAuth scopes not rewritten as expected' + exit(1) + end + + unless Doorkeeper::Application.find(2).access_tokens.first[:scopes] == 'write:accounts profile' + puts 'OAuth access token scopes not rewritten as expected' + exit(1) + end + puts 'No errors found. Database state is consistent with a successful migration process.' end @@ -152,6 +161,23 @@ namespace :tests do VALUES (1, 'https://example.com/users/foobar', 'foobar@example.com', now(), now()), (1, 'https://example.com/users/foobar', 'foobar@example.com', now(), now()); + + /* Doorkeeper records + While the `read:me` scope was technically not valid in 3.3.0, + it is still useful for the purposes of testing the `ChangeReadMeScopeToProfile` + migration. + */ + + INSERT INTO "oauth_applications" + (id, name, uid, secret, redirect_uri, scopes, created_at, updated_at) + VALUES + (2, 'foo', 'foo', 'foo', 'https://example.com/#foo', 'write:accounts read:me', now(), now()), + (3, 'bar', 'bar', 'bar', 'https://example.com/#bar', 'read:me', now(), now()); + + INSERT INTO "oauth_access_tokens" + (token, application_id, scopes, resource_owner_id, created_at) + VALUES + ('secret', 2, 'write:accounts read:me', 4, now()); SQL end diff --git a/spec/lib/scope_transformer_spec.rb b/spec/lib/scope_transformer_spec.rb index 8a9c7cf967..7bc226e94f 100644 --- a/spec/lib/scope_transformer_spec.rb +++ b/spec/lib/scope_transformer_spec.rb @@ -20,6 +20,12 @@ describe ScopeTransformer do end end + context 'with scope "profile"' do + let(:input) { 'profile' } + + it_behaves_like 'a scope', nil, 'profile', 'read' + end + context 'with scope "read"' do let(:input) { 'read' } diff --git a/spec/requests/api/v1/accounts/credentials_spec.rb b/spec/requests/api/v1/accounts/credentials_spec.rb index 8ae9c78a0e..a3f552cada 100644 --- a/spec/requests/api/v1/accounts/credentials_spec.rb +++ b/spec/requests/api/v1/accounts/credentials_spec.rb @@ -29,8 +29,8 @@ RSpec.describe 'credentials API' do }) end - describe 'allows the read:me scope' do - let(:scopes) { 'read:me' } + describe 'allows the profile scope' do + let(:scopes) { 'profile' } it 'returns the response successfully' do subject