From: Anton Khorev Date: Wed, 12 Feb 2025 15:13:56 +0000 (+0300) Subject: Pass scopes instead of token to ApiAbility X-Git-Tag: live~158^2~2 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/77a2657d33f0066dbdda5fce831113b6e165a264?hp=-c Pass scopes instead of token to ApiAbility --- 77a2657d33f0066dbdda5fce831113b6e165a264 diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 3bc82eab2..c62f65368 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -3,7 +3,7 @@ class ApiAbility include CanCan::Ability - def initialize(user, token) + def initialize(user, scopes) can :read, [:version, :capability, :permission, :map] if Settings.status != "database_offline" @@ -17,31 +17,31 @@ class ApiAbility can :read, UserBlock if user&.active? - can [:create, :comment, :close, :reopen], Note if scope?(token, :write_notes) - can [:create, :destroy], NoteSubscription if scope?(token, :write_notes) + can [:create, :comment, :close, :reopen], Note if scopes.include?("write_notes") + can [:create, :destroy], NoteSubscription if scopes.include?("write_notes") - can :read, Trace if scope?(token, :read_gpx) - can [:create, :update, :destroy], Trace if scope?(token, :write_gpx) + can :read, Trace if scopes.include?("read_gpx") + can [:create, :update, :destroy], Trace if scopes.include?("write_gpx") - can :details, User if scope?(token, :read_prefs) - can :read, UserPreference if scope?(token, :read_prefs) - can [:update, :update_all, :destroy], UserPreference if scope?(token, :write_prefs) + can :details, User if scopes.include?("read_prefs") + can :read, UserPreference if scopes.include?("read_prefs") + can [:update, :update_all, :destroy], UserPreference if scopes.include?("write_prefs") - can [:read, :update, :destroy], Message if scope?(token, :consume_messages) - can :create, Message if scope?(token, :send_messages) + can [:read, :update, :destroy], Message if scopes.include?("consume_messages") + can :create, Message if scopes.include?("send_messages") if user.terms_agreed? - can [:create, :update, :upload, :close, :subscribe, :unsubscribe], Changeset if scope?(token, :write_api) - can :create, ChangesetComment if scope?(token, :write_api) - can [:create, :update, :destroy], [Node, Way, Relation] if scope?(token, :write_api) + can [:create, :update, :upload, :close, :subscribe, :unsubscribe], Changeset if scopes.include?("write_api") + can :create, ChangesetComment if scopes.include?("write_api") + can [:create, :update, :destroy], [Node, Way, Relation] if scopes.include?("write_api") end if user.moderator? - can [:destroy, :restore], ChangesetComment if scope?(token, :write_api) + can [:destroy, :restore], ChangesetComment if scopes.include?("write_api") - can :destroy, Note if scope?(token, :write_notes) + can :destroy, Note if scopes.include?("write_notes") - can :redact, [OldNode, OldWay, OldRelation] if user&.terms_agreed? && scope?(token, :write_redactions) + can :redact, [OldNode, OldWay, OldRelation] if user&.terms_agreed? && scopes.include?("write_redactions") end end end @@ -73,10 +73,4 @@ class ApiAbility # See the wiki for details: # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities end - - private - - def scope?(token, scope) - token&.includes_scope?(scope) - end end diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 27f262d00..86924d55d 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -66,9 +66,10 @@ class ApiController < ApplicationController # Use capabilities from the oauth token if it exists and is a valid access token if doorkeeper_token&.accessible? user = User.find(doorkeeper_token.resource_owner_id) - ApiAbility.new(user, doorkeeper_token) + scopes = Set.new doorkeeper_token.scopes + ApiAbility.new(user, scopes) else - ApiAbility.new(nil, nil) + ApiAbility.new(nil, Set.new) end end diff --git a/test/abilities/api_abilities_test.rb b/test/abilities/api_abilities_test.rb index c32300c60..38154174c 100644 --- a/test/abilities/api_abilities_test.rb +++ b/test/abilities/api_abilities_test.rb @@ -7,7 +7,8 @@ end class GuestApiAbilityTest < ApiAbilityTest test "note permissions for a guest" do - ability = ApiAbility.new nil, nil + scopes = Set.new + ability = ApiAbility.new nil, scopes [:index, :create, :feed, :show, :search].each do |action| assert ability.can?(action, Note), "should be able to #{action} Notes" @@ -22,8 +23,8 @@ end class UserApiAbilityTest < ApiAbilityTest test "Note permissions" do user = create(:user) - token = create(:oauth_access_token, :user => user, :scopes => %w[write_notes]) - ability = ApiAbility.new user, token + scopes = Set.new %w[write_notes] + ability = ApiAbility.new user, scopes [:index, :create, :comment, :feed, :show, :search, :close, :reopen].each do |action| assert ability.can?(action, Note), "should be able to #{action} Notes" @@ -38,8 +39,8 @@ end class ModeratorApiAbilityTest < ApiAbilityTest test "Note permissions" do user = create(:moderator_user) - token = create(:oauth_access_token, :user => user, :scopes => %w[write_notes]) - ability = ApiAbility.new user, token + scopes = Set.new %w[write_notes] + ability = ApiAbility.new user, scopes [:index, :create, :comment, :feed, :show, :search, :close, :reopen, :destroy].each do |action| assert ability.can?(action, Note), "should be able to #{action} Notes" diff --git a/test/abilities/api_capability_test.rb b/test/abilities/api_capability_test.rb index 12bbc4965..0f69ddba9 100644 --- a/test/abilities/api_capability_test.rb +++ b/test/abilities/api_capability_test.rb @@ -3,20 +3,20 @@ require "test_helper" class ChangesetCommentApiCapabilityTest < ActiveSupport::TestCase - test "as a normal user with permissionless token" do + test "as a normal user without scopes" do user = create(:user) - token = create(:oauth_access_token, :user => user) - ability = ApiAbility.new user, token + scopes = Set.new + ability = ApiAbility.new user, scopes [:create, :destroy, :restore].each do |action| assert ability.cannot? action, ChangesetComment end end - test "as a normal user with write_api token" do + test "as a normal user with write_api scope" do user = create(:user) - token = create(:oauth_access_token, :user => user, :scopes => %w[write_api]) - ability = ApiAbility.new user, token + scopes = Set.new %w[write_api] + ability = ApiAbility.new user, scopes [:destroy, :restore].each do |action| assert ability.cannot? action, ChangesetComment @@ -27,20 +27,20 @@ class ChangesetCommentApiCapabilityTest < ActiveSupport::TestCase end end - test "as a moderator with permissionless token" do + test "as a moderator without scopes" do user = create(:moderator_user) - token = create(:oauth_access_token, :user => user) - ability = ApiAbility.new user, token + scopes = Set.new + ability = ApiAbility.new user, scopes [:create, :destroy, :restore].each do |action| assert ability.cannot? action, ChangesetComment end end - test "as a moderator with write_api token" do + test "as a moderator with write_api scope" do user = create(:moderator_user) - token = create(:oauth_access_token, :user => user, :scopes => %w[write_api]) - ability = ApiAbility.new user, token + scopes = Set.new %w[write_api] + ability = ApiAbility.new user, scopes [:create, :destroy, :restore].each do |action| assert ability.can? action, ChangesetComment @@ -49,20 +49,20 @@ class ChangesetCommentApiCapabilityTest < ActiveSupport::TestCase end class NoteApiCapabilityTest < ActiveSupport::TestCase - test "as a normal user with permissionless token" do + test "as a normal user without scopes" do user = create(:user) - token = create(:oauth_access_token, :user => user) - ability = ApiAbility.new user, token + scopes = Set.new + ability = ApiAbility.new user, scopes [:create, :comment, :close, :reopen, :destroy].each do |action| assert ability.cannot? action, Note end end - test "as a normal user with write_notes token" do + test "as a normal user with write_notes scope" do user = create(:user) - token = create(:oauth_access_token, :user => user, :scopes => %w[write_notes]) - ability = ApiAbility.new user, token + scopes = Set.new %w[write_notes] + ability = ApiAbility.new user, scopes [:destroy].each do |action| assert ability.cannot? action, Note @@ -73,20 +73,20 @@ class NoteApiCapabilityTest < ActiveSupport::TestCase end end - test "as a moderator with permissionless token" do + test "as a moderator without scopes" do user = create(:moderator_user) - token = create(:oauth_access_token, :user => user) - ability = ApiAbility.new user, token + scopes = Set.new + ability = ApiAbility.new user, scopes [:destroy].each do |action| assert ability.cannot? action, Note end end - test "as a moderator with write_notes token" do + test "as a moderator with write_notes scope" do user = create(:moderator_user) - token = create(:oauth_access_token, :user => user, :scopes => %w[write_notes]) - ability = ApiAbility.new user, token + scopes = Set.new %w[write_notes] + ability = ApiAbility.new user, scopes [:destroy].each do |action| assert ability.can? action, Note @@ -96,17 +96,16 @@ end class UserApiCapabilityTest < ActiveSupport::TestCase test "user preferences" do - # A user with empty tokens user = create(:user) - token = create(:oauth_access_token, :user => user) - ability = ApiAbility.new user, token + scopes = Set.new + ability = ApiAbility.new user, scopes [:index, :show, :update_all, :update, :destroy].each do |act| assert ability.cannot? act, UserPreference end - token = create(:oauth_access_token, :user => user, :scopes => %w[read_prefs]) - ability = ApiAbility.new user, token + scopes = Set.new %w[read_prefs] + ability = ApiAbility.new user, scopes [:update_all, :update, :destroy].each do |act| assert ability.cannot? act, UserPreference @@ -116,8 +115,8 @@ class UserApiCapabilityTest < ActiveSupport::TestCase assert ability.can? act, UserPreference end - token = create(:oauth_access_token, :user => user, :scopes => %w[write_prefs]) - ability = ApiAbility.new user, token + scopes = Set.new %w[write_prefs] + ability = ApiAbility.new user, scopes [:index, :show].each do |act| assert ability.cannot? act, UserPreference