Disentangle the api abilities from the web abilities

This will allow us to rename api actions without causing permissions headaches. The choice of
abilities files is made by inheriting from either api_controller or application_controller.

Also rename capabilities to api_capabilites, for consistency.

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index 7e8e921a29f4d0c35b69c4a2d1a45331d41147cf..d2864e452ab975c06ba98c4d2b6ac221f67ee693 100644 (file)
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -6,28 +6,21 @@ class Ability
   def initialize(user)
     can [:relation, :relation_history, :way, :way_history, :node, :node_history,
          :changeset, :note, :new_note, :query], :browse
   def initialize(user)
     can [:relation, :relation_history, :way, :way_history, :node, :node_history,
          :changeset, :note, :new_note, :query], :browse

-    can :show, :capability
-    can :index, :change

     can :search, :direction
     can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :preview, :copyright, :key, :id], :site
     can [:finish, :embed], :export
     can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim,
          :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder
     can :search, :direction
     can [:index, :permalink, :edit, :help, :fixthemap, :offline, :export, :about, :preview, :copyright, :key, :id], :site
     can [:finish, :embed], :export
     can [:search, :search_latlon, :search_ca_postcode, :search_osm_nominatim,
          :search_geonames, :search_osm_nominatim_reverse, :search_geonames_reverse], :geocoder

-    can :index, :map

     can [:token, :request_token, :access_token, :test_request], :oauth
     can [:token, :request_token, :access_token, :test_request], :oauth

-    can :show, :permission
-    can [:search_all, :search_nodes, :search_ways, :search_relations], :search
-    can [:trackpoints], :swf

 
     if Settings.status != "database_offline"
 
     if Settings.status != "database_offline"

-      can [:index, :feed, :show, :download, :query], Changeset
+      can [:index, :feed], Changeset

       can :index, ChangesetComment
       can [:index, :rss, :show, :comments], DiaryEntry
       can :index, ChangesetComment
       can [:index, :rss, :show, :comments], DiaryEntry

-      can [:index, :create, :comment, :feed, :show, :search, :mine], Note
+      can [:mine], Note

       can [:index, :show], Redaction
       can [:index, :show, :data, :georss, :picture, :icon], Trace
       can [:index, :show], Redaction
       can [:index, :show, :data, :georss, :picture, :icon], Trace

-      can :index, Tracepoint
-      can [:terms, :api_users, :login, :logout, :new, :create, :save, :confirm, :confirm_resend, :confirm_email, :lost_password, :reset_password, :show, :api_read, :auth_success, :auth_failure], User
+      can [:terms, :login, :logout, :new, :create, :save, :confirm, :confirm_resend, :confirm_email, :lost_password, :reset_password, :show, :auth_success, :auth_failure], User

       can [:index, :show, :blocks_on, :blocks_by], UserBlock
       can [:index, :show], Node
       can [:index, :show, :full, :ways_for_node], Way
       can [:index, :show, :blocks_on, :blocks_by], UserBlock
       can [:index, :show], Node
       can [:index, :show, :full, :ways_for_node], Way


@@ -47,31 +40,14 @@ class Ability
         can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message
         can [:close, :reopen], Note
         can [:new, :create], Report
         can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message
         can [:close, :reopen], Note
         can [:new, :create], Report

-        can [:mine, :new, :create, :edit, :update, :delete, :api_create, :api_read, :api_update, :api_delete, :api_data], Trace
-        can [:account, :go_public, :make_friend, :remove_friend, :api_details, :api_gpx_files], User
-        can [:read, :read_one, :update, :update_one, :delete_one], UserPreference
-
-        if user.terms_agreed?
-          can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset
-          can :create, ChangesetComment
-          can [:create, :update, :delete], Node
-          can [:create, :update, :delete], Way
-          can [:create, :update, :delete], Relation
-        end
+        can [:mine, :new, :create, :edit, :update, :delete], Trace
+        can [:account, :go_public, :make_friend, :remove_friend], User

 
         if user.moderator?
 
         if user.moderator?

-          can [:destroy, :restore], ChangesetComment

           can [:index, :show, :resolve, :ignore, :reopen], Issue
           can :create, IssueComment
           can [:index, :show, :resolve, :ignore, :reopen], Issue
           can :create, IssueComment

-          can :destroy, Note

           can [:new, :create, :edit, :update, :destroy], Redaction
           can [:new, :edit, :create, :update, :revoke], UserBlock
           can [:new, :create, :edit, :update, :destroy], Redaction
           can [:new, :edit, :create, :update, :revoke], UserBlock

-
-          if user.terms_agreed?
-            can :redact, OldNode
-            can :redact, OldWay
-            can :redact, OldRelation
-          end

         end
 
         if user.administrator?
         end
 
         if user.administrator?

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
new file mode 100644 (file)
index 0000000..54bc8fb
--- /dev/null
+++ b/app/abilities/api_ability.rb
@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+class ApiAbility
+  include CanCan::Ability
+
+  def initialize(user)
+    can :show, :capability
+    can :index, :change
+    can :index, :map
+    can :show, :permission
+    can [:search_all, :search_nodes, :search_ways, :search_relations], :search
+    can [:trackpoints], :swf
+
+    if Settings.status != "database_offline"
+      can [:show, :download, :query], Changeset
+      can [:index, :create, :comment, :feed, :show, :search], Note
+      can :index, Tracepoint
+      can [:api_users, :api_read], User
+      can [:index, :show], Node
+      can [:index, :show, :full, :ways_for_node], Way
+      can [:index, :show, :full, :relations_for_node, :relations_for_way, :relations_for_relation], Relation
+      can [:history, :version], OldNode
+      can [:history, :version], OldWay
+      can [:history, :version], OldRelation
+    end
+
+    if user
+      can :welcome, :site
+      can [:revoke, :authorize], :oauth
+
+      if Settings.status != "database_offline"
+        can [:index, :new, :create, :show, :edit, :update, :destroy], ClientApplication
+        can [:new, :create, :reply, :show, :inbox, :outbox, :mark, :destroy], Message
+        can [:close, :reopen], Note
+        can [:new, :create], Report
+        can [:api_create, :api_read, :api_update, :api_delete, :api_data], Trace
+        can [:api_details, :api_gpx_files], User
+        can [:read, :read_one, :update, :update_one, :delete_one], UserPreference
+
+        if user.terms_agreed?
+          can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset
+          can :create, ChangesetComment
+          can [:create, :update, :delete], Node
+          can [:create, :update, :delete], Way
+          can [:create, :update, :delete], Relation
+        end
+
+        if user.moderator?
+          can [:destroy, :restore], ChangesetComment
+          can :destroy, Note
+
+          if user.terms_agreed?
+            can :redact, OldNode
+            can :redact, OldWay
+            can :redact, OldRelation
+          end
+        end
+      end
+    end
+
+    # Define abilities for the passed in user here. For example:
+    #
+    #   user ||= User.new # guest user (not logged in)
+    #   if user.admin?
+    #     can :manage, :all
+    #   else
+    #     can :read, :all
+    #   end
+    #
+    # The first argument to `can` is the action you are giving the user
+    # permission to do.
+    # If you pass :manage it will apply to every action. Other common actions
+    # here are :read, :create, :update and :destroy.
+    #
+    # The second argument is the resource the user can perform the action on.
+    # If you pass :all it will apply to every resource. Otherwise pass a Ruby
+    # class of the resource.
+    #
+    # The third argument is an optional hash of conditions to further filter the
+    # objects.
+    # For example, here the user can only update published articles.
+    #
+    #   can :update, Article, :published => true
+    #
+    # See the wiki for details:
+    # https://github.com/CanCanCommunity/cancancan/wiki/Defining-Abilities
+  end
+end

diff --git a/app/abilities/capability.rb b/app/abilities/api_capability.rb
similarity index 98%
rename from app/abilities/capability.rb
rename to app/abilities/api_capability.rb
index f4c24e97d7ffd24122888ee1d069294f567c7a4c..9f59b1f2442f3568f67395e629b393563dc8e34b 100644 (file)
--- a/app/abilities/capability.rb
+++ b/app/abilities/api_capability.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
 # frozen_string_literal: true
 

-class Capability
+class ApiCapability

   include CanCan::Ability
 
   def initialize(token)
   include CanCan::Ability
 
   def initialize(token)

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
index 579af27cf4d8fcac9683cab09feaf640d7df5f64..df7cfe93be97d9ffa704db88d759be75523ffc32 100644 (file)
--- a/app/controllers/api_controller.rb
+++ b/app/controllers/api_controller.rb
@@ -16,6 +16,15 @@ class ApiController < ApplicationController
     end
   end
 
     end
   end
 

+  def current_ability
+    # Use capabilities from the oauth token if it exists and is a valid access token
+    if Authenticator.new(self, [:token]).allow?
+      ApiAbility.new(nil).merge(ApiCapability.new(current_token))
+    else
+      ApiAbility.new(current_user)
+    end
+  end
+

   def deny_access(_exception)
     if current_token
       set_locale
   def deny_access(_exception)
     if current_token
       set_locale

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 3ab09b63d805bc67899c6e54dabeac86f7d5fa69..c880e6add00d756ec0b0d6c061a76d00bfa4e40c 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -329,12 +329,7 @@ class ApplicationController < ActionController::Base
   end
 
   def current_ability
   end
 
   def current_ability

-    # Use capabilities from the oauth token if it exists and is a valid access token
-    if Authenticator.new(self, [:token]).allow?
-      Ability.new(nil).merge(Capability.new(current_token))
-    else
-      Ability.new(current_user)
-    end
+    Ability.new(current_user)

   end
 
   def deny_access(_exception)
   end
 
   def deny_access(_exception)

diff --git a/test/abilities/abilities_test.rb b/test/abilities/abilities_test.rb
index b951e23e5b68b0b5fcfcb3e4f22a6dd04c0ec09b..f43b6bf50de811b2f5b5fcc66fdca7d82e99b299 100644 (file)
--- a/test/abilities/abilities_test.rb
+++ b/test/abilities/abilities_test.rb
@@ -30,13 +30,9 @@ class GuestAbilityTest < AbilityTest
   test "note permissions for a guest" do
     ability = Ability.new nil
 
   test "note permissions for a guest" do
     ability = Ability.new nil
 

-    [:index, :create, :comment, :feed, :show, :search, :mine].each do |action|
+    [:mine].each do |action|

       assert ability.can?(action, Note), "should be able to #{action} Notes"
     end
       assert ability.can?(action, Note), "should be able to #{action} Notes"
     end

-
-    [:close, :reopen, :destroy].each do |action|
-      assert ability.cannot?(action, Note), "should not be able to #{action} Notes"
-    end

   end
 
   test "user roles permissions for a guest" do
   end
 
   test "user roles permissions for a guest" do


@@ -65,18 +61,6 @@ class UserAbilityTest < AbilityTest
       assert ability.cannot?(action, Issue), "should not be able to #{action} Issues"
     end
   end
       assert ability.cannot?(action, Issue), "should not be able to #{action} Issues"
     end
   end

-
-  test "Note permissions" do
-    ability = Ability.new create(:user)
-
-    [:index, :create, :comment, :feed, :show, :search, :mine, :close, :reopen].each do |action|
-      assert ability.can?(action, Note), "should be able to #{action} Notes"
-    end
-
-    [:destroy].each do |action|
-      assert ability.cannot?(action, Note), "should not be able to #{action} Notes"
-    end
-  end

 end
 
 class ModeratorAbilityTest < AbilityTest
 end
 
 class ModeratorAbilityTest < AbilityTest


@@ -88,14 +72,6 @@ class ModeratorAbilityTest < AbilityTest
     end
   end
 
     end
   end
 

-  test "Note permissions" do
-    ability = Ability.new create(:moderator_user)
-
-    [:index, :create, :comment, :feed, :show, :search, :mine, :close, :reopen, :destroy].each do |action|
-      assert ability.can?(action, Note), "should be able to #{action} Notes"
-    end
-  end
-

   test "User Roles permissions" do
     ability = Ability.new create(:moderator_user)
 
   test "User Roles permissions" do
     ability = Ability.new create(:moderator_user)
 

diff --git a/test/abilities/api_abilities_test.rb b/test/abilities/api_abilities_test.rb
new file mode 100644 (file)
index 0000000..7734ce9
--- /dev/null
+++ b/test/abilities/api_abilities_test.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class ApiAbilityTest < ActiveSupport::TestCase
+end
+
+class GuestApiAbilityTest < ApiAbilityTest
+  test "note permissions for a guest" do
+    ability = ApiAbility.new nil
+
+    [:index, :create, :comment, :feed, :show, :search].each do |action|
+      assert ability.can?(action, Note), "should be able to #{action} Notes"
+    end
+
+    [:close, :reopen, :destroy].each do |action|
+      assert ability.cannot?(action, Note), "should not be able to #{action} Notes"
+    end
+  end
+end
+
+class UserApiAbilityTest < ApiAbilityTest
+  test "Note permissions" do
+    ability = ApiAbility.new create(:user)
+
+    [:index, :create, :comment, :feed, :show, :search, :close, :reopen].each do |action|
+      assert ability.can?(action, Note), "should be able to #{action} Notes"
+    end
+
+    [:destroy].each do |action|
+      assert ability.cannot?(action, Note), "should not be able to #{action} Notes"
+    end
+  end
+end
+
+class ModeratorApiAbilityTest < ApiAbilityTest
+  test "Note permissions" do
+    ability = ApiAbility.new create(:moderator_user)
+
+    [:index, :create, :comment, :feed, :show, :search, :close, :reopen, :destroy].each do |action|
+      assert ability.can?(action, Note), "should be able to #{action} Notes"
+    end
+  end
+end

diff --git a/test/abilities/capability_test.rb b/test/abilities/api_capability_test.rb
similarity index 80%
rename from test/abilities/capability_test.rb
rename to test/abilities/api_capability_test.rb
index ed42ef01a029937339a0042b24c8d69c2f03bee8..8d0e682f62b4a3ac91b5b875e43c8d311f82c483 100644 (file)
--- a/test/abilities/capability_test.rb
+++ b/test/abilities/api_capability_test.rb
@@ -2,7 +2,7 @@
 
 require "test_helper"
 
 
 require "test_helper"
 

-class CapabilityTest < ActiveSupport::TestCase
+class ApiCapabilityTest < ActiveSupport::TestCase

   def tokens(*toks)
     AccessToken.new do |token|
       toks.each do |t|
   def tokens(*toks)
     AccessToken.new do |token|
       toks.each do |t|


@@ -12,10 +12,10 @@ class CapabilityTest < ActiveSupport::TestCase
   end
 end
 
   end
 end
 

-class ChangesetCommentCapabilityTest < CapabilityTest
+class ChangesetCommentApiCapabilityTest < ApiCapabilityTest

   test "as a normal user with permissionless token" do
     token = create(:access_token)
   test "as a normal user with permissionless token" do
     token = create(:access_token)

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:create, :destroy, :restore].each do |action|
       assert capability.cannot? action, ChangesetComment
 
     [:create, :destroy, :restore].each do |action|
       assert capability.cannot? action, ChangesetComment


@@ -24,7 +24,7 @@ class ChangesetCommentCapabilityTest < CapabilityTest
 
   test "as a normal user with allow_write_api token" do
     token = create(:access_token, :allow_write_api => true)
 
   test "as a normal user with allow_write_api token" do
     token = create(:access_token, :allow_write_api => true)

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:destroy, :restore].each do |action|
       assert capability.cannot? action, ChangesetComment
 
     [:destroy, :restore].each do |action|
       assert capability.cannot? action, ChangesetComment


@@ -37,7 +37,7 @@ class ChangesetCommentCapabilityTest < CapabilityTest
 
   test "as a moderator with permissionless token" do
     token = create(:access_token, :user => create(:moderator_user))
 
   test "as a moderator with permissionless token" do
     token = create(:access_token, :user => create(:moderator_user))

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:create, :destroy, :restore].each do |action|
       assert capability.cannot? action, ChangesetComment
 
     [:create, :destroy, :restore].each do |action|
       assert capability.cannot? action, ChangesetComment


@@ -46,7 +46,7 @@ class ChangesetCommentCapabilityTest < CapabilityTest
 
   test "as a moderator with allow_write_api token" do
     token = create(:access_token, :user => create(:moderator_user), :allow_write_api => true)
 
   test "as a moderator with allow_write_api token" do
     token = create(:access_token, :user => create(:moderator_user), :allow_write_api => true)

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:create, :destroy, :restore].each do |action|
       assert capability.can? action, ChangesetComment
 
     [:create, :destroy, :restore].each do |action|
       assert capability.can? action, ChangesetComment


@@ -54,10 +54,10 @@ class ChangesetCommentCapabilityTest < CapabilityTest
   end
 end
 
   end
 end
 

-class NoteCapabilityTest < CapabilityTest
+class NoteApiCapabilityTest < ApiCapabilityTest

   test "as a normal user with permissionless token" do
     token = create(:access_token)
   test "as a normal user with permissionless token" do
     token = create(:access_token)

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:create, :comment, :close, :reopen, :destroy].each do |action|
       assert capability.cannot? action, Note
 
     [:create, :comment, :close, :reopen, :destroy].each do |action|
       assert capability.cannot? action, Note


@@ -66,7 +66,7 @@ class NoteCapabilityTest < CapabilityTest
 
   test "as a normal user with allow_write_notes token" do
     token = create(:access_token, :allow_write_notes => true)
 
   test "as a normal user with allow_write_notes token" do
     token = create(:access_token, :allow_write_notes => true)

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:destroy].each do |action|
       assert capability.cannot? action, Note
 
     [:destroy].each do |action|
       assert capability.cannot? action, Note


@@ -79,7 +79,7 @@ class NoteCapabilityTest < CapabilityTest
 
   test "as a moderator with permissionless token" do
     token = create(:access_token, :user => create(:moderator_user))
 
   test "as a moderator with permissionless token" do
     token = create(:access_token, :user => create(:moderator_user))

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:destroy].each do |action|
       assert capability.cannot? action, Note
 
     [:destroy].each do |action|
       assert capability.cannot? action, Note


@@ -88,7 +88,7 @@ class NoteCapabilityTest < CapabilityTest
 
   test "as a moderator with allow_write_notes token" do
     token = create(:access_token, :user => create(:moderator_user), :allow_write_notes => true)
 
   test "as a moderator with allow_write_notes token" do
     token = create(:access_token, :user => create(:moderator_user), :allow_write_notes => true)

-    capability = Capability.new token
+    capability = ApiCapability.new token

 
     [:destroy].each do |action|
       assert capability.can? action, Note
 
     [:destroy].each do |action|
       assert capability.can? action, Note


@@ -96,22 +96,22 @@ class NoteCapabilityTest < CapabilityTest
   end
 end
 
   end
 end
 

-class UserCapabilityTest < CapabilityTest
+class UserApiCapabilityTest < ApiCapabilityTest

   test "user preferences" do
     # a user with no tokens
   test "user preferences" do
     # a user with no tokens

-    capability = Capability.new nil
+    capability = ApiCapability.new nil

     [:read, :read_one, :update, :update_one, :delete_one].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
     # A user with empty tokens
     [:read, :read_one, :update, :update_one, :delete_one].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
     # A user with empty tokens

-    capability = Capability.new tokens
+    capability = ApiCapability.new tokens

 
     [:read, :read_one, :update, :update_one, :delete_one].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
 
     [:read, :read_one, :update, :update_one, :delete_one].each do |act|
       assert capability.cannot? act, UserPreference
     end
 

-    capability = Capability.new tokens(:allow_read_prefs)
+    capability = ApiCapability.new tokens(:allow_read_prefs)

 
     [:update, :update_one, :delete_one].each do |act|
       assert capability.cannot? act, UserPreference
 
     [:update, :update_one, :delete_one].each do |act|
       assert capability.cannot? act, UserPreference


@@ -121,7 +121,7 @@ class UserCapabilityTest < CapabilityTest
       assert capability.can? act, UserPreference
     end
 
       assert capability.can? act, UserPreference
     end
 

-    capability = Capability.new tokens(:allow_write_prefs)
+    capability = ApiCapability.new tokens(:allow_write_prefs)

     [:read, :read_one].each do |act|
       assert capability.cannot? act, UserPreference
     end
     [:read, :read_one].each do |act|
       assert capability.cannot? act, UserPreference
     end