Don't allow any abilities for inactive users

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index c03cf665c5089dd8af0530f42e1fc47665b6a822..bb0d7750ade40e572ab2ee51c3235aef8c23dd99 100644 (file)
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -65,7 +65,7 @@ Metrics/ClassLength:
 # Offense count: 58
 # Configuration parameters: AllowedMethods, AllowedPatterns, IgnoredMethods.
 Metrics/CyclomaticComplexity:
-  Max: 25
+  Max: 26
 
 # Offense count: 751
 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, AllowedMethods, AllowedPatterns, IgnoredMethods.
@@ -80,7 +80,7 @@ Metrics/ParameterLists:
 # Offense count: 57
 # Configuration parameters: AllowedMethods, AllowedPatterns, IgnoredMethods.
 Metrics/PerceivedComplexity:
-  Max: 26
+  Max: 27
 
 # Offense count: 2495
 # This cop supports safe autocorrection (--autocorrect).

diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb
index e022f7993e79adcfd8d7047fd4cabc94269077d9..bb9cd6300526aec6afa84e4c3549e7ac710c2040 100644 (file)
--- a/app/abilities/ability.rb
+++ b/app/abilities/ability.rb
@@ -33,7 +33,7 @@ class Ability
       can [:history, :version], OldRelation
     end
 
-    if user
+    if user&.active?
       can :welcome, :site
       can [:revoke, :authorize], :oauth
       can [:show], :deletion

diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb
index 62cd2b17ebbd41ad9408ebcfc5730134991b3506..9b274ec845d299f2ee6af894e80cb0f3ac0dac3a 100644 (file)
--- a/app/abilities/api_ability.rb
+++ b/app/abilities/api_ability.rb
@@ -23,7 +23,7 @@ class ApiAbility
       can [:history, :version], OldRelation
     end
 
-    if user
+    if user&.active?
       can :welcome, :site
       can [:revoke, :authorize], :oauth
 

diff --git a/app/abilities/api_capability.rb b/app/abilities/api_capability.rb
index 04d7fe10ab0953928a89c455b87761ae2c0ed173..8c52327cfe488cc0d4e0710d85c48625dd7a166d 100644 (file)
--- a/app/abilities/api_capability.rb
+++ b/app/abilities/api_capability.rb
@@ -11,29 +11,31 @@ class ApiCapability
                token.user
              end
 
-      can [:create, :comment, :close, :reopen], Note if scope?(token, :write_notes)
-      can [:show, :data], Trace if scope?(token, :read_gpx)
-      can [:create, :update, :destroy], Trace if scope?(token, :write_gpx)
-      can [:details], User if scope?(token, :read_prefs)
-      can [:gpx_files], User if scope?(token, :read_gpx)
-      can [:index, :show], UserPreference if scope?(token, :read_prefs)
-      can [:update, :update_all, :destroy], UserPreference if scope?(token, :write_prefs)
+      if user&.active?
+        can [:create, :comment, :close, :reopen], Note if scope?(token, :write_notes)
+        can [:show, :data], Trace if scope?(token, :read_gpx)
+        can [:create, :update, :destroy], Trace if scope?(token, :write_gpx)
+        can [:details], User if scope?(token, :read_prefs)
+        can [:gpx_files], User if scope?(token, :read_gpx)
+        can [:index, :show], UserPreference if scope?(token, :read_prefs)
+        can [:update, :update_all, :destroy], UserPreference if scope?(token, :write_prefs)
 
-      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, :delete], Node if scope?(token, :write_api)
-        can [:create, :update, :delete], Way if scope?(token, :write_api)
-        can [:create, :update, :delete], Relation if scope?(token, :write_api)
-      end
+        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, :delete], Node if scope?(token, :write_api)
+          can [:create, :update, :delete], Way if scope?(token, :write_api)
+          can [:create, :update, :delete], Relation if scope?(token, :write_api)
+        end
 
-      if user&.moderator?
-        can [:destroy, :restore], ChangesetComment if scope?(token, :write_api)
-        can :destroy, Note if scope?(token, :write_notes)
-        if user&.terms_agreed?
-          can :redact, OldNode if scope?(token, :write_api)
-          can :redact, OldWay if scope?(token, :write_api)
-          can :redact, OldRelation if scope?(token, :write_api)
+        if user.moderator?
+          can [:destroy, :restore], ChangesetComment if scope?(token, :write_api)
+          can :destroy, Note if scope?(token, :write_notes)
+          if user&.terms_agreed?
+            can :redact, OldNode if scope?(token, :write_api)
+            can :redact, OldWay if scope?(token, :write_api)
+            can :redact, OldRelation if scope?(token, :write_api)
+          end
         end
       end
     end

diff --git a/test/abilities/api_capability_test.rb b/test/abilities/api_capability_test.rb
index dccde57580db834fb2a7d81b3ecb34e073ffae0b..10419c0f814b2e8b240cb80b3263b2f85857a7dd 100644 (file)
--- a/test/abilities/api_capability_test.rb
+++ b/test/abilities/api_capability_test.rb
@@ -2,19 +2,7 @@
 
 require "test_helper"
 
-class ApiCapabilityTest < ActiveSupport::TestCase
-  private
-
-  def tokens(*toks)
-    AccessToken.new do |token|
-      toks.each do |t|
-        token.public_send("#{t}=", true)
-      end
-    end
-  end
-end
-
-class ChangesetCommentApiCapabilityTest < ApiCapabilityTest
+class ChangesetCommentApiCapabilityTest < ActiveSupport::TestCase
   test "as a normal user with permissionless token" do
     token = create(:access_token)
     capability = ApiCapability.new token
@@ -56,7 +44,7 @@ class ChangesetCommentApiCapabilityTest < ApiCapabilityTest
   end
 end
 
-class NoteApiCapabilityTest < ApiCapabilityTest
+class NoteApiCapabilityTest < ActiveSupport::TestCase
   test "as a normal user with permissionless token" do
     token = create(:access_token)
     capability = ApiCapability.new token
@@ -98,7 +86,7 @@ class NoteApiCapabilityTest < ApiCapabilityTest
   end
 end
 
-class UserApiCapabilityTest < ApiCapabilityTest
+class UserApiCapabilityTest < ActiveSupport::TestCase
   test "user preferences" do
     # a user with no tokens
     capability = ApiCapability.new nil
@@ -107,13 +95,15 @@ class UserApiCapabilityTest < ApiCapabilityTest
     end
 
     # A user with empty tokens
-    capability = ApiCapability.new tokens
+    token = create(:access_token)
+    capability = ApiCapability.new token
 
     [:index, :show, :update_all, :update, :destroy].each do |act|
       assert capability.cannot? act, UserPreference
     end
 
-    capability = ApiCapability.new tokens(:allow_read_prefs)
+    token = create(:access_token, :allow_read_prefs => true)
+    capability = ApiCapability.new token
 
     [:update_all, :update, :destroy].each do |act|
       assert capability.cannot? act, UserPreference
@@ -123,7 +113,9 @@ class UserApiCapabilityTest < ApiCapabilityTest
       assert capability.can? act, UserPreference
     end
 
-    capability = ApiCapability.new tokens(:allow_write_prefs)
+    token = create(:access_token, :allow_write_prefs => true)
+    capability = ApiCapability.new token
+
     [:index, :show].each do |act|
       assert capability.cannot? act, UserPreference
     end