Browse Source

Improved permission check of personal tokens.

Martin Edenhofer 8 years ago
parent
commit
731c237d0c

+ 9 - 1
app/controllers/application_controller.rb

@@ -265,9 +265,17 @@ class ApplicationController < ActionController::Base
       user = Token.check(
         action: 'api',
         name: token,
-        permission: auth_param[:permission],
         inactive_user: true,
       )
+      if user && auth_param[:permission]
+        user = Token.check(
+          action: 'api',
+          name: token,
+          permission: auth_param[:permission],
+          inactive_user: true,
+        )
+        raise Exceptions::NotAuthorized, 'No permission!' if !user
+      end
       @_token_auth = token # remember for permission_check
       return authentication_check_prerequesits(user, 'token_auth', auth_param) if user
     end

+ 1 - 4
app/controllers/roles_controller.rb

@@ -1,7 +1,7 @@
 # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/
 
 class RolesController < ApplicationController
-  before_action :authentication_check
+  before_action { authentication_check(permission: 'admin.role') }
 
 =begin
 
@@ -95,7 +95,6 @@ curl http://localhost/api/v1/roles.json -v -u #{login}:#{password} -H "Content-T
 =end
 
   def create
-    permission_check('admin.role')
     model_create_render(Role, params)
   end
 
@@ -124,7 +123,6 @@ curl http://localhost/api/v1/roles.json -v -u #{login}:#{password} -H "Content-T
 =end
 
   def update
-    permission_check('admin.role')
     model_update_render(Role, params)
   end
 
@@ -139,7 +137,6 @@ Test:
 =end
 
   def destroy
-    permission_check('admin.role')
     model_destory_render(Role, params)
   end
 end

+ 10 - 2
app/controllers/user_access_token_controller.rb

@@ -17,14 +17,22 @@ class UserAccessTokenController < ApplicationController
     local_permissions.each { |key, _value|
       keys = Object.const_get('Permission').with_parents(key)
       keys.each { |local_key|
-        next if local_permissions_new[local_key]
+        next if local_permissions_new.key?([local_key])
+        if local_permissions[local_key] == true
+          local_permissions_new[local_key] = true
+          next
+        end
         local_permissions_new[local_key] = false
       }
     }
     permissions = []
     Permission.all.order(:name).each { |permission|
       next if !local_permissions_new.key?(permission.name)
-      permissions.push permission
+      permission_attributes = permission.attributes
+      if local_permissions_new[permission.name] == false
+        permission_attributes['preferences']['disabled'] = true
+      end
+      permissions.push permission_attributes
     }
 
     render json: {

+ 1 - 1
app/models/token.rb

@@ -79,7 +79,7 @@ returns
     if data[:permission]
       return if !user.permissions?(data[:permission])
       return if !token.preferences[:permission]
-      return if token.preferences[:permission][data[:permission]] != true
+      return if !token.preferences[:permission].include?(data[:permission])
     end
 
     # return token user

+ 20 - 6
test/controllers/api_auth_controller_test.rb

@@ -114,9 +114,7 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
       persistent:  true,
       user_id:     @admin.id,
       preferences: {
-        permission: {
-          'admin.session' => true,
-        }
+        permission: ['admin.session'],
       },
     )
     admin_credentials = "Token token=#{admin_token.name}"
@@ -135,7 +133,7 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
     assert_equal(Hash, result.class)
     assert(result)
 
-    admin_token.preferences[:permission]['admin.session'] = false
+    admin_token.preferences[:permission] = ['admin.session_not_existing']
     admin_token.save!
 
     get '/api/v1/sessions', {}, @headers.merge('Authorization' => admin_credentials)
@@ -144,7 +142,7 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
     assert_equal(Hash, result.class)
     assert_equal('No permission!', result['error'])
 
-    admin_token.preferences[:permission] = {}
+    admin_token.preferences[:permission] = []
     admin_token.save!
 
     get '/api/v1/sessions', {}, @headers.merge('Authorization' => admin_credentials)
@@ -162,7 +160,7 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
     assert_equal(Hash, result.class)
     assert_equal('User is inactive!', result['error'])
 
-    admin_token.preferences[:permission]['admin.session'] = true
+    admin_token.preferences[:permission] = ['admin.session']
     admin_token.save!
 
     get '/api/v1/sessions', {}, @headers.merge('Authorization' => admin_credentials)
@@ -179,6 +177,22 @@ class ApiAuthControllerTest < ActionDispatch::IntegrationTest
     result = JSON.parse(@response.body)
     assert_equal(Hash, result.class)
     assert(result)
+
+    get '/api/v1/roles', {}, @headers.merge('Authorization' => admin_credentials)
+    assert_response(401)
+    result = JSON.parse(@response.body)
+    assert_equal(Hash, result.class)
+    assert_equal('No permission!', result['error'])
+
+    admin_token.preferences[:permission] = ['admin.session_not_existing', 'admin.role']
+    admin_token.save!
+
+    get '/api/v1/roles', {}, @headers.merge('Authorization' => admin_credentials)
+    assert_response(200)
+    result = JSON.parse(@response.body)
+    assert_equal(Array, result.class)
+    assert(result)
+
   end
 
   test 'token auth - agent' do

+ 1 - 4
test/unit/token_test.rb

@@ -83,10 +83,7 @@ class TokenTest < ActiveSupport::TestCase
       persistent:  true,
       user_id:     agent1.id,
       preferences: {
-        permission: {
-          'admin' => true, # agent has no access to admin.*
-          'ticket.agent' => true,
-        }
+        permission: ['admin', 'ticket.agent'], # agent has no access to admin.*
       }
     )
     user = Token.check(