Add more granular OAuth scopes (#7929)

* Add more granular OAuth scopes * Add human-readable descriptions of the new scopes * Ensure new scopes look good on the app UI * Add tests * Group scopes in screen and color-code dangerous ones * Fix wrong extra scope
2018-07-05 18:31:35 +02:00 · 2018-07-05 18:31:35 +02:00 · 1f6ed4f86a
commit 1f6ed4f86a
parent ca2cc556f1
69 changed files with 295 additions and 105 deletions
--- a/app/controllers/api/base_controller.rb
+++ b/app/controllers/api/base_controller.rb
@ -78,4 +78,8 @@ class Api::BaseController < ApplicationController
  def render_empty
    render json: {}, status: 200
  end
+
+  def authorize_if_got_token!(*scopes)
+    doorkeeper_authorize!(*scopes) if doorkeeper_token
+  end
 end
--- a/app/controllers/api/v1/accounts/credentials_controller.rb
+++ b/app/controllers/api/v1/accounts/credentials_controller.rb
@ -1,8 +1,8 @@
 # frozen_string_literal: true

 class Api::V1::Accounts::CredentialsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }, except: [:update]
-  before_action -> { doorkeeper_authorize! :write }, only: [:update]
+  before_action -> { doorkeeper_authorize! :read, :'read:accounts' }, except: [:update]
+  before_action -> { doorkeeper_authorize! :write, :'write:accounts' }, only: [:update]
  before_action :require_user!

  def show
--- a/app/controllers/api/v1/accounts/follower_accounts_controller.rb
+++ b/app/controllers/api/v1/accounts/follower_accounts_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Accounts::FollowerAccountsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:accounts' }
  before_action :set_account
  after_action :insert_pagination_headers

--- a/app/controllers/api/v1/accounts/following_accounts_controller.rb
+++ b/app/controllers/api/v1/accounts/following_accounts_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Accounts::FollowingAccountsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:accounts' }
  before_action :set_account
  after_action :insert_pagination_headers

--- a/app/controllers/api/v1/accounts/lists_controller.rb
+++ b/app/controllers/api/v1/accounts/lists_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Accounts::ListsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:lists' }
  before_action :require_user!
  before_action :set_account

--- a/app/controllers/api/v1/accounts/relationships_controller.rb
+++ b/app/controllers/api/v1/accounts/relationships_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Accounts::RelationshipsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:follows' }
  before_action :require_user!

  respond_to :json
--- a/app/controllers/api/v1/accounts/search_controller.rb
+++ b/app/controllers/api/v1/accounts/search_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Accounts::SearchController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:accounts' }
  before_action :require_user!

  respond_to :json
--- a/app/controllers/api/v1/accounts/statuses_controller.rb
+++ b/app/controllers/api/v1/accounts/statuses_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Accounts::StatusesController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:statuses' }
  before_action :set_account
  after_action :insert_pagination_headers

--- a/app/controllers/api/v1/accounts_controller.rb
+++ b/app/controllers/api/v1/accounts_controller.rb
@ -1,8 +1,11 @@
 # frozen_string_literal: true

 class Api::V1::AccountsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }, except: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
-  before_action -> { doorkeeper_authorize! :follow }, only: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
+  before_action -> { authorize_if_got_token! :read, :'read:accounts' }, except: [:follow, :unfollow, :block, :unblock, :mute, :unmute]
+  before_action -> { doorkeeper_authorize! :follow, :'write:follows' }, only: [:follow, :unfollow]
+  before_action -> { doorkeeper_authorize! :follow, :'write:mutes' }, only: [:mute, :unmute]
+  before_action -> { doorkeeper_authorize! :follow, :'write:blocks' }, only: [:block, :unblock]
+
  before_action :require_user!, except: [:show]
  before_action :set_account
  before_action :check_account_suspension, only: [:show]
--- a/app/controllers/api/v1/blocks_controller.rb
+++ b/app/controllers/api/v1/blocks_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::BlocksController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :follow }
+  before_action -> { doorkeeper_authorize! :follow, :'read:blocks' }
  before_action :require_user!
  after_action :insert_pagination_headers

--- a/app/controllers/api/v1/domain_blocks_controller.rb
+++ b/app/controllers/api/v1/domain_blocks_controller.rb
@ -3,7 +3,8 @@
 class Api::V1::DomainBlocksController < Api::BaseController
  BLOCK_LIMIT = 100

-  before_action -> { doorkeeper_authorize! :follow }
+  before_action -> { doorkeeper_authorize! :follow, :'read:blocks' }, only: :show
+  before_action -> { doorkeeper_authorize! :follow, :'write:blocks' }, except: :show
  before_action :require_user!
  after_action :insert_pagination_headers, only: :show

--- a/app/controllers/api/v1/favourites_controller.rb
+++ b/app/controllers/api/v1/favourites_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::FavouritesController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:favourites' }
  before_action :require_user!
  after_action :insert_pagination_headers

--- a/app/controllers/api/v1/filters_controller.rb
+++ b/app/controllers/api/v1/filters_controller.rb
@ -1,8 +1,8 @@
 # frozen_string_literal: true

 class Api::V1::FiltersController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }, only: [:index, :show]
-  before_action -> { doorkeeper_authorize! :write }, except: [:index, :show]
+  before_action -> { doorkeeper_authorize! :read, :'read:filters' }, only: [:index, :show]
+  before_action -> { doorkeeper_authorize! :write, :'write:filters' }, except: [:index, :show]
  before_action :require_user!
  before_action :set_filters, only: :index
  before_action :set_filter, only: [:show, :update, :destroy]
--- a/app/controllers/api/v1/follow_requests_controller.rb
+++ b/app/controllers/api/v1/follow_requests_controller.rb
@ -1,7 +1,8 @@
 # frozen_string_literal: true

 class Api::V1::FollowRequestsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :follow }
+  before_action -> { doorkeeper_authorize! :follow, :'read:follows' }, only: :index
+  before_action -> { doorkeeper_authorize! :follow, :'write:follows' }, except: :index
  before_action :require_user!
  after_action :insert_pagination_headers, only: :index

--- a/app/controllers/api/v1/follows_controller.rb
+++ b/app/controllers/api/v1/follows_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::FollowsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :follow }
+  before_action -> { doorkeeper_authorize! :follow, :'write:follows' }
  before_action :require_user!

  respond_to :json
--- a/app/controllers/api/v1/lists/accounts_controller.rb
+++ b/app/controllers/api/v1/lists/accounts_controller.rb
@ -1,8 +1,8 @@
 # frozen_string_literal: true

 class Api::V1::Lists::AccountsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read },    only: [:show]
-  before_action -> { doorkeeper_authorize! :write }, except: [:show]
+  before_action -> { doorkeeper_authorize! :read, :'read:lists' },    only:  [:show]
+  before_action -> { doorkeeper_authorize! :write, :'write:lists' }, except: [:show]

  before_action :require_user!
  before_action :set_list
--- a/app/controllers/api/v1/lists_controller.rb
+++ b/app/controllers/api/v1/lists_controller.rb
@ -1,8 +1,8 @@
 # frozen_string_literal: true

 class Api::V1::ListsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read },    only: [:index, :show]
-  before_action -> { doorkeeper_authorize! :write }, except: [:index, :show]
+  before_action -> { doorkeeper_authorize! :read, :'read:lists' },    only:  [:index, :show]
+  before_action -> { doorkeeper_authorize! :write, :'write:lists' }, except: [:index, :show]

  before_action :require_user!
  before_action :set_list, except: [:index, :create]
--- a/app/controllers/api/v1/media_controller.rb
+++ b/app/controllers/api/v1/media_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::MediaController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :write }
+  before_action -> { doorkeeper_authorize! :write, :'write:media' }
  before_action :require_user!

  include ObfuscateFilename
--- a/app/controllers/api/v1/mutes_controller.rb
+++ b/app/controllers/api/v1/mutes_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::MutesController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :follow }
+  before_action -> { doorkeeper_authorize! :follow, :'read:mutes' }
  before_action :require_user!
  after_action :insert_pagination_headers

--- a/app/controllers/api/v1/notifications_controller.rb
+++ b/app/controllers/api/v1/notifications_controller.rb
@ -1,7 +1,8 @@
 # frozen_string_literal: true

 class Api::V1::NotificationsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:notifications' }, except: [:clear, :dismiss]
+  before_action -> { doorkeeper_authorize! :write, :'write:notifications' }, only: [:clear, :dismiss]
  before_action :require_user!
  after_action :insert_pagination_headers, only: :index

--- a/app/controllers/api/v1/reports_controller.rb
+++ b/app/controllers/api/v1/reports_controller.rb
@ -1,8 +1,8 @@
 # frozen_string_literal: true

 class Api::V1::ReportsController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }, except: [:create]
-  before_action -> { doorkeeper_authorize! :write }, only:  [:create]
+  before_action -> { doorkeeper_authorize! :read, :'read:reports' }, except: [:create]
+  before_action -> { doorkeeper_authorize! :write, :'write:reports' }, only: [:create]
  before_action :require_user!

  respond_to :json
--- a/app/controllers/api/v1/search_controller.rb
+++ b/app/controllers/api/v1/search_controller.rb
@ -5,7 +5,7 @@ class Api::V1::SearchController < Api::BaseController

  RESULTS_LIMIT = 5

-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:search' }
  before_action :require_user!

  respond_to :json
--- a/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb
+++ b/app/controllers/api/v1/statuses/favourited_by_accounts_controller.rb
@ -3,7 +3,7 @@
 class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController
  include Authorization

-  before_action :authorize_if_got_token
+  before_action -> { authorize_if_got_token! :read, :'read:accounts' }
  before_action :set_status
  after_action :insert_pagination_headers

@ -71,11 +71,6 @@ class Api::V1::Statuses::FavouritedByAccountsController < Api::BaseController
    raise ActiveRecord::RecordNotFound
  end

-  def authorize_if_got_token
-    request_token = Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods)
-    doorkeeper_authorize! :read if request_token
-  end
-
  def pagination_params(core_params)
    params.slice(:limit).permit(:limit).merge(core_params)
  end
--- a/app/controllers/api/v1/statuses/favourites_controller.rb
+++ b/app/controllers/api/v1/statuses/favourites_controller.rb
@ -3,7 +3,7 @@
 class Api::V1::Statuses::FavouritesController < Api::BaseController
  include Authorization

-  before_action -> { doorkeeper_authorize! :write }
+  before_action -> { doorkeeper_authorize! :write, :'write:favourites' }
  before_action :require_user!

  respond_to :json
--- a/app/controllers/api/v1/statuses/mutes_controller.rb
+++ b/app/controllers/api/v1/statuses/mutes_controller.rb
@ -3,7 +3,7 @@
 class Api::V1::Statuses::MutesController < Api::BaseController
  include Authorization

-  before_action -> { doorkeeper_authorize! :write }
+  before_action -> { doorkeeper_authorize! :write, :'write:mutes' }
  before_action :require_user!
  before_action :set_status
  before_action :set_conversation
--- a/app/controllers/api/v1/statuses/pins_controller.rb
+++ b/app/controllers/api/v1/statuses/pins_controller.rb
@ -3,7 +3,7 @@
 class Api::V1::Statuses::PinsController < Api::BaseController
  include Authorization

-  before_action -> { doorkeeper_authorize! :write }
+  before_action -> { doorkeeper_authorize! :write, :'write:accounts' }
  before_action :require_user!
  before_action :set_status

--- a/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb
+++ b/app/controllers/api/v1/statuses/reblogged_by_accounts_controller.rb
@ -3,7 +3,7 @@
 class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController
  include Authorization

-  before_action :authorize_if_got_token
+  before_action -> { authorize_if_got_token! :read, :'read:accounts' }
  before_action :set_status
  after_action :insert_pagination_headers

@ -68,11 +68,6 @@ class Api::V1::Statuses::RebloggedByAccountsController < Api::BaseController
    raise ActiveRecord::RecordNotFound
  end

-  def authorize_if_got_token
-    request_token = Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods)
-    doorkeeper_authorize! :read if request_token
-  end
-
  def pagination_params(core_params)
    params.slice(:limit).permit(:limit).merge(core_params)
  end
--- a/app/controllers/api/v1/statuses/reblogs_controller.rb
+++ b/app/controllers/api/v1/statuses/reblogs_controller.rb
@ -3,7 +3,7 @@
 class Api::V1::Statuses::ReblogsController < Api::BaseController
  include Authorization

-  before_action -> { doorkeeper_authorize! :write }
+  before_action -> { doorkeeper_authorize! :write, :'write:statuses' }
  before_action :require_user!

  respond_to :json
--- a/app/controllers/api/v1/statuses_controller.rb
+++ b/app/controllers/api/v1/statuses_controller.rb
@ -3,8 +3,8 @@
 class Api::V1::StatusesController < Api::BaseController
  include Authorization

-  before_action :authorize_if_got_token, except:            [:create, :destroy]
-  before_action -> { doorkeeper_authorize! :write }, only:  [:create, :destroy]
+  before_action -> { authorize_if_got_token! :read, :'read:statuses' }, except: [:create, :destroy]
+  before_action -> { doorkeeper_authorize! :write, :'write:statuses' }, only:   [:create, :destroy]
  before_action :require_user!, except:  [:show, :context, :card]
  before_action :set_status, only:       [:show, :context, :card]

@ -84,9 +84,4 @@ class Api::V1::StatusesController < Api::BaseController
  def pagination_params(core_params)
    params.slice(:limit).permit(:limit).merge(core_params)
  end
-
-  def authorize_if_got_token
-    request_token = Doorkeeper::OAuth::Token.from_request(request, *Doorkeeper.configuration.access_token_methods)
-    doorkeeper_authorize! :read if request_token
-  end
 end
--- a/app/controllers/api/v1/timelines/direct_controller.rb
+++ b/app/controllers/api/v1/timelines/direct_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Timelines::DirectController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }, only: [:show]
+  before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, only: [:show]
  before_action :require_user!, only: [:show]
  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }

--- a/app/controllers/api/v1/timelines/home_controller.rb
+++ b/app/controllers/api/v1/timelines/home_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Timelines::HomeController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }, only: [:show]
+  before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, only: [:show]
  before_action :require_user!, only: [:show]
  after_action :insert_pagination_headers, unless: -> { @statuses.empty? }

--- a/app/controllers/api/v1/timelines/list_controller.rb
+++ b/app/controllers/api/v1/timelines/list_controller.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 class Api::V1::Timelines::ListController < Api::BaseController
-  before_action -> { doorkeeper_authorize! :read }
+  before_action -> { doorkeeper_authorize! :read, :'read:lists' }
  before_action :require_user!
  before_action :set_list
  before_action :set_statuses
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@ -1,6 +1,12 @@
 # frozen_string_literal: true

 module ApplicationHelper
+  DANGEROUS_SCOPES = %w(
+    read
+    write
+    follow
+  ).freeze
+
  def active_nav_class(path)
    current_page?(path) ? 'active' : ''
  end
@ -43,6 +49,10 @@ module ApplicationHelper
    Rails.env.production? ? site_title : "#{site_title} (Dev)"
  end

+  def class_for_scope(scope)
+    'scope-danger' if DANGEROUS_SCOPES.include?(scope.to_s)
+  end
+
  def can?(action, record)
    return false if record.nil?
    policy(record).public_send("#{action}?")
--- a/app/javascript/styles/mastodon/forms.scss
+++ b/app/javascript/styles/mastodon/forms.scss
@ -612,3 +612,7 @@ code {
    display: block;
  }
 }
+
+.scope-danger {
+  color: $warning-red;
+}
--- a/app/views/settings/applications/_fields.html.haml
+++ b/app/views/settings/applications/_fields.html.haml
@ -8,14 +8,9 @@
  %p.hint= t('doorkeeper.applications.help.native_redirect_uri', native_redirect_uri: Doorkeeper.configuration.native_redirect_uri)

 .field-group
-  = f.input :scopes,
-    label: t('activerecord.attributes.doorkeeper/application.scopes'),
-    collection: Doorkeeper.configuration.scopes,
-    wrapper: :with_label,
-    include_blank: false,
-    label_method: lambda { |scope| safe_join([scope, content_tag(:span, t("doorkeeper.scopes.#{scope}"), class: 'hint')]) },
-    selected: f.object.scopes.all,
-    required: false,
-    as: :check_boxes,
-    collection_wrapper_tag: 'ul',
-    item_wrapper_tag: 'li'
+  .input.with_block_label
+    %label= t('activerecord.attributes.doorkeeper/application.scopes')
+    %span.hint= t('simple_form.hints.defaults.scopes')
+
+  - Doorkeeper.configuration.scopes.group_by { |s| s.split(':').first }.each do |k, v|
+    = f.input :scopes, label: false, hint: false, collection: v.sort, wrapper: :with_block_label, include_blank: false, label_method: lambda { |scope| safe_join([content_tag(:samp, scope, class: class_for_scope(scope)), content_tag(:span, t("doorkeeper.scopes.#{scope}"), class: 'hint')]) }, selected: f.object.scopes.all, required: false, as: :check_boxes, collection_wrapper_tag: 'ul', item_wrapper_tag: 'li'