ref: refs/heads/jreidinger_errorhandling
commit 3c449f42d0fbdb1969270d7c0711b8431066d294
Author: Josef Reidinger
Date: Thu Sep 3 09:52:48 2009 +0200
update modules to new behavior or permission_check
---
.../app/controllers/language_controller.rb | 11 ++---
.../patches/app/controllers/packages_controller.rb | 4 +-
.../patches/app/controllers/patches_controller.rb | 8 +--
.../app/controllers/sambashares_controller.rb | 40 +++++---------
.../app/controllers/securities_controller.rb | 16 ++----
.../app/controllers/services_controller.rb | 9 +---
.../status/app/controllers/status_controller.rb | 56 +++++++++-----------
.../system/app/controllers/system_controller.rb | 8 +--
.../time/app/controllers/systemtimes_controller.rb | 9 +---
plugins/users/app/controllers/users_controller.rb | 20 ++-----
webservice/test/plugin_basic_tests.rb | 8 ++--
11 files changed, 68 insertions(+), 121 deletions(-)
diff --git a/plugins/language/app/controllers/language_controller.rb b/plugins/language/app/controllers/language_controller.rb
index 60a4bac..f6a1a5a 100644
--- a/plugins/language/app/controllers/language_controller.rb
+++ b/plugins/language/app/controllers/language_controller.rb
@@ -13,9 +13,8 @@ class LanguageController < ApplicationController
# Actualizes language settings. Requires write permissions for language YaPI.
def update
if params.has_key?(:language)
- unless permission_check("org.opensuse.yast.modules.yapi.language.write")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "language.write"
+
@language = Language.new
@language.language = params[:language][:current]
@language.utf8 = params[:language][:utf8]
@@ -36,11 +35,9 @@ class LanguageController < ApplicationController
# Shows language settings. Requires read permission for language YaPI.
def show
- unless permission_check("org.opensuse.yast.modules.yapi.language.read")
- render ErrorResult.error(403, 1, "no permissions") and return
- end
- @language = Language.find
+ yapi_perm_check "language.read"
+ @language = Language.find
end
end
diff --git a/plugins/patches/app/controllers/packages_controller.rb b/plugins/patches/app/controllers/packages_controller.rb
index e3e9718..eee77a3 100644
--- a/plugins/patches/app/controllers/packages_controller.rb
+++ b/plugins/patches/app/controllers/packages_controller.rb
@@ -15,9 +15,7 @@ class PackagesController < ApplicationController
private
def check_read_permissions
- unless permission_check( "org.opensuse.yast.system.patches.read")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ permission_check "org.opensuse.yast.system.patches.read"
end
# check whether the cached result is still valid
diff --git a/plugins/patches/app/controllers/patches_controller.rb b/plugins/patches/app/controllers/patches_controller.rb
index 0c61832..7b6f30f 100644
--- a/plugins/patches/app/controllers/patches_controller.rb
+++ b/plugins/patches/app/controllers/patches_controller.rb
@@ -15,9 +15,7 @@ class PatchesController < ApplicationController
private
def check_read_permissions
- unless permission_check( "org.opensuse.yast.system.patches.read")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ permission_check "org.opensuse.yast.system.patches.read"
end
# check whether the cached result is still valid
@@ -65,9 +63,7 @@ class PatchesController < ApplicationController
# PUT /patch_updates/1
# PUT /patch_updates/1.xml
def update
- unless permission_check( "org.opensuse.yast.system.patches.install")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ permission_check "org.opensuse.yast.system.patches.install"
@patch_update = Patch.find(params[:id])
if @patch_update.nil?
logger.error "Patch: #{params[:id]} not found."
diff --git a/plugins/samba_server/app/controllers/sambashares_controller.rb b/plugins/samba_server/app/controllers/sambashares_controller.rb
index ceed299..d25b70d 100644
--- a/plugins/samba_server/app/controllers/sambashares_controller.rb
+++ b/plugins/samba_server/app/controllers/sambashares_controller.rb
@@ -10,9 +10,7 @@ class SambasharesController < ApplicationController
# GET /sambashares.json
def index
# read all Samba shares
- if !permission_check("org.opensuse.yast.modules.yapi.samba.getalldirectories")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "samba.getalldirectories"
@shares = SambaShare.find_all
@@ -28,9 +26,7 @@ class SambasharesController < ApplicationController
# GET /sambashares/share_name.json
# return properties of a samba share, use YaPI::Samba Yast module
def show
- if !permission_check("org.opensuse.yast.modules.yapi.samba.getshare")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "samba.getshare"
@share = SambaShare.new
@share.id = params[:id]
@@ -52,20 +48,18 @@ class SambasharesController < ApplicationController
def create
@share = SambaShare.new
- if !permission_check("org.opensuse.yast.modules.yapi.samba.addshare")
- render ErrorResult.error(403, 1, "no permission") and return
- else
- if !@share.update_attributes(params[:sambashares][:parameters], true)
- render ErrorResult.error(404, 2, "input error") and return
- end
+ yapi_perm_check "samba.addshare"
- if !@share.id.blank?
- if !@share.add
- render ErrorResult.error(404, 3, "adding share failed") and return
- end
- else
- render ErrorResult.error(404, 4, "empty share name") and return
- end
+ if !@share.update_attributes(params[:sambashares][:parameters], true)
+ render ErrorResult.error(404, 2, "input error") and return
+ end
+
+ if !@share.id.blank?
+ if !@share.add
+ render ErrorResult.error(404, 3, "adding share failed") and return
+ end
+ else
+ render ErrorResult.error(404, 4, "empty share name") and return
end
respond_to do |format|
@@ -79,9 +73,7 @@ class SambasharesController < ApplicationController
# PUT /sambashares/share.xml
# PUT /sambashares/share.json
def update
- if !permission_check("org.opensuse.yast.modules.yapi.samba.editshare")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "samba.editshare"
@share = SambaShare.new
@share.id = params[:id]
@@ -113,9 +105,7 @@ class SambasharesController < ApplicationController
# DELETE /sambashares/share.xml
# DELETE /sambashares/share.json
def destroy
- if !permission_check("org.opensuse.yast.modules.yapi.samba.deleteshare")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "samba.deleteshare"
@share = SambaShare.new
@share.id = params[:id]
diff --git a/plugins/securities/app/controllers/securities_controller.rb b/plugins/securities/app/controllers/securities_controller.rb
index 92cde11..da32bf0 100644
--- a/plugins/securities/app/controllers/securities_controller.rb
+++ b/plugins/securities/app/controllers/securities_controller.rb
@@ -13,9 +13,7 @@ class SecuritiesController < ApplicationController
# PUT /security/1
# PUT /security/1.xml
def update
- unless permission_check( "org.opensuse.yast.system.security.write")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ permission_check( "org.opensuse.yast.system.security.write")
# get security object and set values
@security = Security.new
@@ -37,12 +35,10 @@ class SecuritiesController < ApplicationController
# GET /security/1
# GET /security/1.xml
def show
- unless permission_check( "org.opensuse.yast.system.security.read")
- render ErrorResult.error(403, 1, "no permission") and return
- else
- @security = Security.new
- @security.update
- logger.debug "SHOW: #{@security.inspect}"
- end
+ permission_check( "org.opensuse.yast.system.security.read")
+
+ @security = Security.new
+ @security.update
+ logger.debug "SHOW: #{@security.inspect}"
end
end
diff --git a/plugins/services/app/controllers/services_controller.rb b/plugins/services/app/controllers/services_controller.rb
index c3d5b51..9b4007f 100644
--- a/plugins/services/app/controllers/services_controller.rb
+++ b/plugins/services/app/controllers/services_controller.rb
@@ -4,9 +4,7 @@ class ServicesController < ApplicationController
before_filter :login_required
def index
- unless permission_check("org.opensuse.yast.modules.yapi.services.read")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "services.read"
@services = Service.find_all params
end
@@ -14,10 +12,7 @@ class ServicesController < ApplicationController
# PUT /services/1.xml
# Shows service status. Requires execute permission for services YaPI.
def update
-
- unless permission_check( "org.opensuse.yast.modules.yapi.services.execute")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "services.execute"
begin
@service = Service.find params[:id]
diff --git a/plugins/status/app/controllers/status_controller.rb b/plugins/status/app/controllers/status_controller.rb
index ee80bd5..2615c06 100644
--- a/plugins/status/app/controllers/status_controller.rb
+++ b/plugins/status/app/controllers/status_controller.rb
@@ -33,35 +33,33 @@ class StatusController < ApplicationController
# POST /status
# POST /status.xml
def create
- unless permission_check("org.opensuse.yast.system.status.writelimits")
- render ErrorResult.error(403, 1, "no permission") and return
- else
- #find the correct plugin path for the config file
- plugin_config_dir = "#{RAILS_ROOT}/config" #default
- Rails.configuration.plugin_paths.each do |plugin_path|
- if File.directory?(File.join(plugin_path, "status"))
- plugin_config_dir = plugin_path+"/status/config"
- Dir.mkdir(plugin_config_dir) unless File.directory?(plugin_config_dir)
- break
- end
- end
- limits = Hash.new
- limits = create_limit(params["status"])
- f = File.open(File.join(plugin_config_dir, "status_limits.yaml"), "w")
- f.write(limits.to_yaml)
- f.close
+ permission_check("org.opensuse.yast.system.status.writelimits")
- @status = Status.new
- # use now if time is not valid
- begin
- stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i)
- start = params[:start].blank? ? stop - 300 : Time.at(params[:start].to_i)
- @status.collect_data(start, stop, params[:data])
- render :show
- rescue Exception => e
- render :text => e.to_s, :status => 400 # bad request
+ #find the correct plugin path for the config file
+ plugin_config_dir = "#{RAILS_ROOT}/config" #default
+ Rails.configuration.plugin_paths.each do |plugin_path|
+ if File.directory?(File.join(plugin_path, "status"))
+ plugin_config_dir = plugin_path+"/status/config"
+ Dir.mkdir(plugin_config_dir) unless File.directory?(plugin_config_dir)
+ break
end
end
+ limits = Hash.new
+ limits = create_limit(params["status"])
+ f = File.open(File.join(plugin_config_dir, "status_limits.yaml"), "w")
+ f.write(limits.to_yaml)
+ f.close
+
+ @status = Status.new
+ # use now if time is not valid
+ begin
+ stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i)
+ start = params[:start].blank? ? stop - 300 : Time.at(params[:start].to_i)
+ @status.collect_data(start, stop, params[:data])
+ render :show
+ rescue Exception => e
+ render :text => e.to_s, :status => 400 # bad request
+ end
end
# GET /status
@@ -73,9 +71,7 @@ class StatusController < ApplicationController
# GET /status/1
# GET /status/1.xml
def show
- unless permission_check("org.opensuse.yast.system.status.read")
- render ErrorResult.error(403, 1, "no permission") and return
- else
+ permission_check("org.opensuse.yast.system.status.read")
begin
@status = Status.new
stop = params[:stop].blank? ? Time.now : Time.at(params[:stop].to_i)
@@ -84,7 +80,5 @@ class StatusController < ApplicationController
rescue Exception => e
render :text => e.to_s, :status => 400 # bad request
end
- end
end
-
end
diff --git a/plugins/system/app/controllers/system_controller.rb b/plugins/system/app/controllers/system_controller.rb
index 542865b..062c9ad 100644
--- a/plugins/system/app/controllers/system_controller.rb
+++ b/plugins/system/app/controllers/system_controller.rb
@@ -41,17 +41,13 @@ class SystemController < ApplicationController
case k
when 'reboot'
- unless permission_check( 'org.freedesktop.hal.power-management.reboot')
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ permission_check( 'org.freedesktop.hal.power-management.reboot')
if v['active'] == true and @system.actions[k.to_sym][:active] == false
do_reboot = true
end
when 'shutdown'
- unless permission_check( 'org.freedesktop.hal.power-management.shutdown')
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ permission_check( 'org.freedesktop.hal.power-management.shutdown')
if v['active'] == true and @system.actions[k.to_sym][:active] == false
do_shutdown = true
diff --git a/plugins/time/app/controllers/systemtimes_controller.rb b/plugins/time/app/controllers/systemtimes_controller.rb
index 16c556e..7a2433d 100644
--- a/plugins/time/app/controllers/systemtimes_controller.rb
+++ b/plugins/time/app/controllers/systemtimes_controller.rb
@@ -15,9 +15,7 @@ class SystemtimesController < ApplicationController
# Sets time settings. Requires write permissions for time YaPI.
def update
- unless permission_check( "org.opensuse.yast.modules.yapi.time.write")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "time.write"
root = params[:time]
if root == nil
@@ -36,11 +34,8 @@ class SystemtimesController < ApplicationController
# Shows time settings. Requires read permission for time YaPI.
def show
+ yapi_perm_check "time.read"
- unless permission_check( "org.opensuse.yast.modules.yapi.time.read")
- render ErrorResult.error( 403, 1, "no permission" ) and return
- end
-
systemtime = Systemtime.find
respond_to do |format|
diff --git a/plugins/users/app/controllers/users_controller.rb b/plugins/users/app/controllers/users_controller.rb
index 8d01db1..055164a 100644
--- a/plugins/users/app/controllers/users_controller.rb
+++ b/plugins/users/app/controllers/users_controller.rb
@@ -11,18 +11,14 @@ class UsersController < ApplicationController
# GET /users.xml
# GET /users.json
def index
- unless permission_check("org.opensuse.yast.modules.yapi.users.usersget")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "users.usersget"
@users = User.find_all
end
# GET /users/1
# GET /users/1.xml
def show
- unless permission_check("org.opensuse.yast.modules.yapi.users.userget")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "users.userget"
if params[:id].blank?
render ErrorResult.error(404, 2, "empty parameter") and return
end
@@ -44,9 +40,7 @@ class UsersController < ApplicationController
# POST /users.xml
# POST /users.json
def create
- unless permission_check("org.opensuse.yast.modules.yapi.users.useradd")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "users.useradd"
begin
@user = User.create(params[:users])
@@ -60,9 +54,7 @@ class UsersController < ApplicationController
# PUT /users/1
# PUT /users/1.xml
def update
- unless permission_check("org.opensuse.yast.modules.yapi.users.usermodify")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "users.usermodify"
if params[:users] && params[:users][:uid]
params[:id] = params[:users][:uid] #for sync only
@@ -87,9 +79,7 @@ class UsersController < ApplicationController
# DELETE /users/1.xml
# DELETE /users/1.json
def destroy
- unless permission_check("org.opensuse.yast.modules.yapi.users.userdelete")
- render ErrorResult.error(403, 1, "no permission") and return
- end
+ yapi_perm_check "users.userdelete"
begin
@user = User.find(params[:id])
diff --git a/webservice/test/plugin_basic_tests.rb b/webservice/test/plugin_basic_tests.rb
index 68409c5..f77ffb7 100644
--- a/webservice/test/plugin_basic_tests.rb
+++ b/webservice/test/plugin_basic_tests.rb
@@ -46,9 +46,9 @@ module PluginBasicTests
def test_access_denied
#mock model to test only controller
@model_class.stubs(:find)
- @controller.stubs(:permission_check).returns(false);
+ @controller.stubs(:permission_check).raises(NoPermissionException.new("action", "test"));
get :show
- assert_response :forbidden
+ assert_response 503
end
def test_access_show_xml
@@ -73,11 +73,11 @@ module PluginBasicTests
#ensure that nothing is saved
@model_class.expects(:save).never
- @controller.stubs(:permission_check).returns(false);
+ @controller.stubs(:permission_check).raises(NoPermissionException.new("action", "test"));
put :update, @data
- assert_response :forbidden
+ assert_response 503
end
end
--
To unsubscribe, e-mail: yast-commit+unsubscribe@opensuse.org
For additional commands, e-mail: yast-commit+help@opensuse.org
