Merge branch 'private-permission-active2' into develop

app/assets/javascripts/app/models/user.coffee

@@ -188,6 +188,11 @@ class App.User extends App.Model
     # if any permission exists
     return true if _.contains(keys, '*')
 
+    # verify direct permissions
+    for key in keys
+      permission = App.Permission.findByAttribute('name', key)
+      return false if permission && permission.active is false
+
     # get all permissions of user
     permissions = {}
     for role_id in @role_ids
@@ -195,6 +200,9 @@ class App.User extends App.Model
       if role.active is true
         for permission_id in role.permission_ids
           permission = App.Permission.findNative(permission_id)
+          if !permission
+            throw "No such permission for id #{permission_id}"
+          if permission.active is true
             permissions[permission.name] = true
 
     for localKey in keys

app/controllers/user_access_token_controller.rb

@@ -26,7 +26,7 @@ class UserAccessTokenController < ApplicationController
       }
     }
     permissions = []
-    Permission.all.order(:name).each { |permission|
+    Permission.all.where(active: true).order(:name).each { |permission|
       next if !local_permissions_new.key?(permission.name) && !current_user.permissions?(permission.name)
       permission_attributes = permission.attributes
       if local_permissions_new[permission.name] == false

app/models/role.rb

@@ -107,7 +107,7 @@ returns
         permission_ids.push permission.id
       }
       next if permission_ids.empty?
-      Role.joins(:roles_permissions).where('permissions_roles.permission_id IN (?) AND roles.active = ?', permission_ids, true).uniq().each { |role|
+      Role.joins(:roles_permissions).joins(:permissions).where('permissions_roles.permission_id IN (?) AND roles.active = ? AND permissions.active = ?', permission_ids, true, true).uniq().each { |role|
         roles.push role
       }
     }

app/models/user.rb

@@ -334,7 +334,7 @@ returns
 
   def permissions
     list = {}
-    Object.const_get('Permission').select('permissions.name, permissions.preferences').joins(:roles).where('roles.id IN (?)', role_ids).pluck(:name, :preferences).each { |permission|
+    Object.const_get('Permission').select('permissions.name, permissions.preferences').joins(:roles).where('roles.id IN (?) AND permissions.active = ?', role_ids, true).pluck(:name, :preferences).each { |permission|
       next if permission[1]['selectable'] == false
       list[permission[0]] = true
     }
@@ -375,10 +375,12 @@ returns
       if local_key =~ /\.\*$/
         local_key.sub!('.*', '.%')
         permissions = Object.const_get('Permission').with_parents(local_key)
-        list = Object.const_get('Permission').select('preferences').joins(:roles).where('roles.id IN (?) AND roles.active = ? AND (permissions.name IN (?) OR permissions.name LIKE ?)', role_ids, true, permissions, local_key).pluck(:preferences)
+        list = Object.const_get('Permission').select('preferences').joins(:roles).where('roles.id IN (?) AND roles.active = ? AND (permissions.name IN (?) OR permissions.name LIKE ?) AND permissions.active = ?', role_ids, true, permissions, local_key, true).pluck(:preferences)
       else
+        permission = Object.const_get('Permission').lookup(name: local_key)
+        break if permission && permission.active == false
         permissions = Object.const_get('Permission').with_parents(local_key)
-        list = Object.const_get('Permission').select('preferences').joins(:roles).where('roles.id IN (?) AND roles.active = ? AND permissions.name IN (?)', role_ids, true, permissions).pluck(:preferences)
+        list = Object.const_get('Permission').select('preferences').joins(:roles).where('roles.id IN (?) AND roles.active = ? AND permissions.name IN (?) AND permissions.active = ?', role_ids, true, permissions, true).pluck(:preferences)
       end
       list.each { |preferences|
         next if preferences[:selectable] == false
@@ -420,7 +422,7 @@ returns
         permission_ids.push permission.id
       }
       next if permission_ids.empty?
-      Role.joins(:roles_permissions).where('permissions_roles.permission_id IN (?) AND roles.active = ?', permission_ids, true).uniq().pluck(:id).each { |role_id|
+      Role.joins(:roles_permissions).joins(:permissions).where('permissions_roles.permission_id IN (?) AND roles.active = ? AND permissions.active = ?', permission_ids, true, true).uniq().pluck(:id).each { |role_id|
         role_ids.push role_id
       }
       total_role_ids.push role_ids

db/migrate/20120101000001_create_base.rb

@@ -109,6 +109,7 @@ class CreateBase < ActiveRecord::Migration
       t.string :name,                   limit: 255, null: false
       t.string :note,                   limit: 500, null: true
       t.string :preferences,            limit: 10_000, null: true
+      t.boolean :active,                               null: false, default: true
       t.timestamps limit: 3, null: false
     end
     add_index :permissions, [:name], unique: true

db/migrate/20160921000001_permission_active.rb.rb

@@ -0,0 +1,10 @@
+class PermissionActive < ActiveRecord::Migration
+  def up
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    add_column :permissions, :active, :boolean, null: false, default: true
+
+    Cache.clear
+  end
+end

test/unit/permission_test.rb

@@ -12,10 +12,17 @@ class PermissionTest < ActiveSupport::TestCase
 
   test 'user permission' do
 
-    Permission.create_if_not_exists(
+    permission1 = Permission.create_or_update(
       name: 'admin.permission1',
       note: 'Admin Interface',
       preferences: {},
+      active: true,
+    )
+    permission2 = Permission.create_or_update(
+      name: 'admin.permission2',
+      note: 'Admin Interface',
+      preferences: {},
+      active: true,
     )
     role_permission1 = Role.create_or_update(
       name: 'AdminPermission1',
@@ -27,6 +34,7 @@ class PermissionTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
+    role_permission1.permission_revoke('admin')
     role_permission1.permission_grand('admin.permission1')
     user_with_permission1 = User.create_or_update(
       login: 'setting-permission1',
@@ -39,24 +47,45 @@ class PermissionTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
+
     assert_equal(true, user_with_permission1.permissions?('admin.permission1'))
     assert_equal(true, user_with_permission1.permissions?('admin.*'))
     assert_equal(false, user_with_permission1.permissions?('admi.*'))
     assert_equal(false, user_with_permission1.permissions?('admin.permission2'))
     assert_equal(false, user_with_permission1.permissions?('admin'))
 
+    permission1.active = false
+    permission1.save!
+
+    assert_equal(false, user_with_permission1.permissions?('admin.permission1'))
+    assert_equal(false, user_with_permission1.permissions?('admin.*'))
+    assert_equal(false, user_with_permission1.permissions?('admi.*'))
+    assert_equal(false, user_with_permission1.permissions?('admin.permission2'))
+    assert_equal(false, user_with_permission1.permissions?('admin'))
+
+    role_permission1.permission_grand('admin')
+
+    assert_equal(false, user_with_permission1.permissions?('admin.permission1'))
+    assert_equal(true, user_with_permission1.permissions?('admin.*'))
+    assert_equal(false, user_with_permission1.permissions?('admi.*'))
+    assert_equal(true, user_with_permission1.permissions?('admin.permission2'))
+    assert_equal(true, user_with_permission1.permissions?('admin'))
+
+    role_permission1.permission_revoke('admin')
+
   end
 
   test 'user permission with invalid role' do
 
-    Permission.create_if_not_exists(
+    permission3 = Permission.create_or_update(
-      name: 'admin.permission2',
+      name: 'admin.permission3',
       note: 'Admin Interface',
       preferences: {},
+      active: true,
     )
-    role_permission2 = Role.create_or_update(
+    role_permission3 = Role.create_or_update(
       name: 'AdminPermission2',
-      note: 'To configure your permission2.',
+      note: 'To configure your permission3.',
       preferences: {
         not: ['Customer'],
       },
@@ -65,32 +94,32 @@ class PermissionTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
-    role_permission2.permission_grand('admin.permission2')
+    role_permission3.permission_grand('admin.permission3')
-    user_with_permission2 = User.create_or_update(
+    user_with_permission3 = User.create_or_update(
-      login: 'setting-permission2',
+      login: 'setting-permission3',
       firstname: 'Setting',
       lastname: 'Admin Permission2',
-      email: 'setting-admin-permission2@example.com',
+      email: 'setting-admin-permission3@example.com',
       password: 'some_pw',
       active: true,
-      roles: [role_permission2],
+      roles: [role_permission3],
       updated_by_id: 1,
       created_by_id: 1,
     )
-    assert_equal(true, user_with_permission2.permissions?('admin.permission2'))
+    assert_equal(true, user_with_permission3.permissions?('admin.permission3'))
-    assert_equal(true, user_with_permission2.permissions?('admin.*'))
+    assert_equal(true, user_with_permission3.permissions?('admin.*'))
-    assert_equal(false, user_with_permission2.permissions?('admi.*'))
+    assert_equal(false, user_with_permission3.permissions?('admi.*'))
-    assert_equal(false, user_with_permission2.permissions?('admin.permission3'))
+    assert_equal(false, user_with_permission3.permissions?('admin.permission4'))
-    assert_equal(false, user_with_permission2.permissions?('admin'))
+    assert_equal(false, user_with_permission3.permissions?('admin'))
 
-    role_permission2.active = false
+    role_permission3.active = false
-    role_permission2.save
+    role_permission3.save
-    user_with_permission2.reload
+    user_with_permission3.reload
-    assert_equal(false, user_with_permission2.permissions?('admin.permission2'))
+    assert_equal(false, user_with_permission3.permissions?('admin.permission3'))
-    assert_equal(false, user_with_permission2.permissions?('admin.*'))
+    assert_equal(false, user_with_permission3.permissions?('admin.*'))
-    assert_equal(false, user_with_permission2.permissions?('admi.*'))
+    assert_equal(false, user_with_permission3.permissions?('admi.*'))
-    assert_equal(false, user_with_permission2.permissions?('admin.permission3'))
+    assert_equal(false, user_with_permission3.permissions?('admin.permission4'))
-    assert_equal(false, user_with_permission2.permissions?('admin'))
+    assert_equal(false, user_with_permission3.permissions?('admin'))
 
   end