Pass scopes instead of token to ApiAbility

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 3bc82eab290b714670f41288ba47fccc03eda980..c62f65368a32a2243c73994afd0b58a4531ef417 100644 (file)
--- 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 27f262d00341aa8cabfcf7e5d54e6c11f0541c95..86924d55d0e5a3a64cec9c385ab292604679d200 100644 (file)
--- 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 c32300c60997297aabc7e85240f2e95dba0bf811..38154174c4f8a8527eb15f67d5a005a70c15107c 100644 (file)
--- 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 12bbc49658075d27b8e1c5ff5e3fbb154260bea5..0f69ddba9583a86eb33e5b2a19d448736b857862 100644 (file)
--- 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