On Thu, 1 Sep 2011 17:21:15 +0200
tsedovic(a)redhat.com wrote:
From: Tomas Sedovic <tsedovic(a)redhat.com>
https://www.aeolusproject.org/redmine/issues/2205
---
.../controllers/hardware_profiles_controller.rb | 20 ++++--
src/app/models/hardware_profile.rb | 2 +
...d_view_hardware_profile_privileges_to_admins.rb | 19 +++++
src/db/seeds.rb | 4 +-
src/features/hardware_profile.feature | 2 +-
.../hardware_profiles_controller_spec.rb | 79
++++++++++++++++++++ 6 files changed, 117 insertions(+), 9
deletions(-) create mode 100644
src/db/migrate/20110901150708_add_view_hardware_profile_privileges_to_admins.rb
diff --git a/src/app/controllers/hardware_profiles_controller.rb
b/src/app/controllers/hardware_profiles_controller.rb index
5f4f9d2..3d66fd2 100644 ---
a/src/app/controllers/hardware_profiles_controller.rb +++
b/src/app/controllers/hardware_profiles_controller.rb @@ -26,6 +26,8
@@ class HardwareProfilesController < ApplicationController
def show
@hardware_profile = HardwareProfile.find(params[:id].to_a.first)
+ require_privilege(Privilege::VIEW, @hardware_profile)
+
@tab_captions = ['Properties', 'History', 'Matching Provider
Hardware Profiles'] @details_tab = params[:details_tab].blank? ?
'properties' : params[:details_tab] properties
@@ -44,6 +46,8 @@ class HardwareProfilesController <
ApplicationController end
def new
+ require_privilege(Privilege::CREATE, HardwareProfile)
+
respond_to do |format|
format.html { render :action => 'new'}
format.js { render :partial =>
'matching_provider_hardware_profiles' } @@ -51,7 +55,10 @@ class
HardwareProfilesController < ApplicationController end
def create
+ require_privilege(Privilege::CREATE, HardwareProfile)
+
build_hardware_profile(params[:hardware_profile])
+ require_privilege(Privilege::CREATE, HardwareProfile)
this require is probably not needed
if params[:commit] == 'Save'
if @hardware_profile.save
redirect_to hardware_profiles_path
@@ -66,7 +73,9 @@ class HardwareProfilesController <
ApplicationController end
def destroy
- if HardwareProfile.destroy(params[:id])
+ hardware_profile = HardwareProfile.find(params[:id])
+ require_privilege(Privilege::MODIFY, hardware_profile)
+ if hardware_profile.destroy
flash[:notice] = "Hardware profile was deleted!"
else
flash[:error] = "Hardware profile was not deleted!"
@@ -78,20 +87,19 @@ class HardwareProfilesController <
ApplicationController unless @hardware_profile
@hardware_profile = HardwareProfile.find(params[:id])
end
+ require_privilege(Privilege::MODIFY, @hardware_profile)
matching_provider_hardware_profiles
end
def update
- if params[:commit] == "Reset"
- redirect_to edit_hardware_profile_url(@hardware_profile) and
return
- end
-
if params[:id]
@hardware_profile = HardwareProfile.find(params[:id])
+ require_privilege(Privilege::MODIFY, @hardware_profile)
build_hardware_profile(params[:hardware_profile])
end
if params[:commit] == "Check Matches"
+ require_privilege(Privilege::VIEW, HardwareProfile)
matching_provider_hardware_profiles
render :edit and return
end
@@ -109,7 +117,7 @@ class HardwareProfilesController <
ApplicationController not_deleted=[]
HardwareProfile.find(params[:hardware_profile_selected]).each do
|hwp|
- if hwp.destroy
+ if check_privilege(Privilege::MODIFY, hwp) && hwp.destroy
deleted << hwp.name
else
not_deleted << hwp.name
diff --git a/src/app/models/hardware_profile.rb
b/src/app/models/hardware_profile.rb index 57d5fd8..a1fda4c 100644
--- a/src/app/models/hardware_profile.rb
+++ b/src/app/models/hardware_profile.rb
@@ -38,6 +38,8 @@
# Likewise, all the methods added will be available for all
controllers.
class HardwareProfile < ActiveRecord::Base
+ include PermissionedObject
+ has_many :permissions, :as => :permission_object, :dependent
=> :destroy has_many :instances
scope :frontend, :conditions => { :provider_id => nil }
has_many :provider_instances, :class_name => "Instance",
diff --git
a/src/db/migrate/20110901150708_add_view_hardware_profile_privileges_to_admins.rb
b/src/db/migrate/20110901150708_add_view_hardware_profile_privileges_to_admins.rb
new file mode 100644 index 0000000..ab2bfcc --- /dev/null
+++
b/src/db/migrate/20110901150708_add_view_hardware_profile_privileges_to_admins.rb
@@ -0,0 +1,19 @@ +class AddViewHardwareProfilePrivilegesToAdmins <
ActiveRecord::Migration
+ def self.up
+ return if Role.all.empty?
+
+ Role.transaction do
+ ["HWP Administrator", "Administrator"].each do |role_name|
+ role = Role.find_or_initialize_by_name(role_name)
+
+ priv_type = HardwareProfile
+ priv_action = 'view'
+ Privilege.create!(:role => role, :target_type =>
'HardwareProfile',
+ :action => 'view')
+ end
+ end
+ end
+
+ def self.down
+ end
+end
diff --git a/src/db/seeds.rb b/src/db/seeds.rb
index 170375c..12d185c 100644
--- a/src/db/seeds.rb
+++ b/src/db/seeds.rb
@@ -51,7 +51,7 @@ roles =
{"Provider Creator" => [false, {Provider =>
[ CRE]}], "Provider Administrator" => [false,
{Provider => [VIEW, MOD,CRE,VPRM,GPRM], ProviderAccount =>
[VIEW,USE,MOD,CRE,VPRM,GPRM]}],
- "HWP Administrator" => [false, {HardwareProfile =>
[ MOD,CRE,VPRM,GPRM]}],
+ "HWP Administrator" => [false, {HardwareProfile =>
[VIEW, MOD,CRE,VPRM,GPRM]}], "Realm Administrator" => [false,
{Realm => [ USE,MOD,CRE,VPRM,GPRM]}], "Pool
Creator" => [false, {Pool => [ CRE]}],
"Pool Administrator" => [false, {Pool => [VIEW,
MOD,CRE,VPRM,GPRM], @@ -67,7 +67,7 @@ roles = Quota =>
[VIEW]}], "Administrator" => [false, {Provider =>
[VIEW, MOD,CRE,VPRM,GPRM], ProviderAccount =>
[VIEW,USE,MOD,CRE,VPRM,GPRM],
- HardwareProfile =>
[ MOD,CRE,VPRM,GPRM],
+ HardwareProfile =>
[VIEW, MOD,CRE,VPRM,GPRM], Realm =>
[ USE,MOD,CRE,VPRM,GPRM], User => [VIEW, MOD,CRE],
Pool => [VIEW,
MOD,CRE,VPRM,GPRM], diff --git
a/src/features/hardware_profile.feature
b/src/features/hardware_profile.feature index 7c0587e..1733716 100644
--- a/src/features/hardware_profile.feature +++
b/src/features/hardware_profile.feature @@ -1,4 +1,4 @@
-Feature: Manage Pools
+Feature: Manage Hardware Profiles
In order to manage my cloud infrastructure
As an admin
I want to manage Hardware Profiles
diff --git
a/src/spec/controllers/hardware_profiles_controller_spec.rb
b/src/spec/controllers/hardware_profiles_controller_spec.rb index
099dac7..392e51c 100644 ---
a/src/spec/controllers/hardware_profiles_controller_spec.rb +++
b/src/spec/controllers/hardware_profiles_controller_spec.rb @@ -6,6
+6,8 @@ describe HardwareProfilesController do before(:each) do
@admin_permission = FactoryGirl.create :admin_permission @admin =
@admin_permission.user
+ @user_permission = FactoryGirl.create :pool_user_permission
+ @user = @user_permission.user
activate_authlogic
end
@@ -18,4 +20,81 @@ describe HardwareProfilesController do
response.should render_template("index")
end
+ it "should allow authorized users to create hardware profiles" do
+ UserSession.create(@admin)
+ lambda do
+ post :create, :commit => 'Save', :hardware_profile => {
+ :name => 'test',
+ :memory_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_memory),
+ :cpu_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_cpu),
+ :storage_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_storage),
+ :architecture_attributes =>
FactoryGirl.attributes_for(:mock_hwp2_memory),
+ }
+ end.should change(HardwareProfile, :count).by(1)
+ HardwareProfile.find_by_name('test').should_not be_nil
+ response.should redirect_to(hardware_profiles_path)
+ end
+
+ it "should prevent unauthorized users from creating hardware
profiles" do
+ UserSession.create(@user)
+ lambda do
+ post :create, :commit => 'Save', :hardware_profile => {
+ :name => 'test',
+ :memory_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_memory),
+ :cpu_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_cpu),
+ :storage_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_storage),
+ :architecture_attributes =>
FactoryGirl.attributes_for(:mock_hwp2_memory),
+ }
+ end.should_not change(HardwareProfile, :count)
+ HardwareProfile.find_by_name('test').should be_nil
+ response.should render_template('layouts/error')
+ end
+
+ it "should allow authorized users to edit hardware profiles" do
+ hardware_profile = Factory.create :hardware_profile
+ UserSession.create(@admin)
+ put :update, :id => hardware_profile.id, :hardware_profile => {
+ :name => 'updated hwp',
+ :memory_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_memory),
+ :cpu_attributes => FactoryGirl.attributes_for(:mock_hwp1_cpu),
+ :storage_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_storage),
+ :architecture_attributes =>
FactoryGirl.attributes_for(:mock_hwp2_memory),
+ }
+ HardwareProfile.find_by_name('updated hwp').should_not be_nil
+ response.should redirect_to(hardware_profiles_path)
+ end
+
+ it "should prevent unauthorized users from editing hardware
profiles" do
+ hardware_profile = Factory.create :hardware_profile
+ UserSession.create(@user)
+ put :update, :id => hardware_profile.id, :hardware_profile => {
+ :name => 'updated hwp',
+ :memory_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_memory),
+ :cpu_attributes => FactoryGirl.attributes_for(:mock_hwp1_cpu),
+ :storage_attributes =>
FactoryGirl.attributes_for(:mock_hwp1_storage),
+ :architecture_attributes =>
FactoryGirl.attributes_for(:mock_hwp2_memory),
+ }
+ HardwareProfile.find_by_name('updated hwp').should be_nil
+ response.should render_template('layouts/error')
+ end
+
+ it "should allow authorized users to delete hardware profiles" do
+ hardware_profile = Factory.create :hardware_profile
+ UserSession.create(@admin)
+
+ HardwareProfile.exists?(hardware_profile.id).should be_true
+ delete :destroy, :id => hardware_profile.id
+ HardwareProfile.exists?(hardware_profile.id).should be_false
+ response.should redirect_to(hardware_profiles_path)
+ end
+
+ it "should prevent unauthorized users from deleting hardware
profiles" do
+ hardware_profile = Factory.create :hardware_profile
+ UserSession.create(@user)
+
+ HardwareProfile.exists?(hardware_profile.id).should be_true
+ delete :destroy, :id => hardware_profile.id
+ HardwareProfile.exists?(hardware_profile.id).should be_true
+ response.should render_template('layouts/error')
+ end
end
Rspec allow us to group examples with describe and context some hints
I find useful.
http://eggsonbread.com/2010/03/28/my-rspec-best-practices-and-tips/
http://napkin.highgroove.com/articles/2011/06/21/rspec-tip-let-and-nested...
ACK. notes inline