Added a new PermissionSession model that session entities reference
rather than referencing the session_id of the real session. The session
has a reference to the permission_session_id -- this keeps permission
behavior from depending on the request session_id string value, since
with upstream gems/rails 3.2, the request session_id value is not
reset early enough to make use of it in the warden after_authentication
hook.
---
src/app/controllers/application_controller.rb | 2 +-
src/app/controllers/users_controller.rb | 2 +-
src/app/models/deployment.rb | 18 +++++----
.../{session_entity.rb => permission_session.rb} | 21 +++++-----
src/app/models/permissioned_object.rb | 22 +++++-----
src/app/models/pool.rb | 4 +-
src/app/models/session_entity.rb | 24 +----------
src/config/initializers/warden.rb | 23 ++++++++++-
.../20120816131500_create_permission_sessions.rb | 41 ++++++++++++++++++++
src/features/support/custom.rb | 8 ----
src/spec/models/deployment_spec.rb | 23 +++++++----
src/spec/models/permission_spec.rb | 32 ++++++++-------
src/spec/services/registration_service_spec.rb | 7 ++-
src/spec/spec_helper.rb | 7 +++-
14 files changed, 143 insertions(+), 91 deletions(-)
copy src/app/models/{session_entity.rb => permission_session.rb} (66%)
create mode 100644 src/db/migrate/20120816131500_create_permission_sessions.rb
diff --git a/src/app/controllers/application_controller.rb
b/src/app/controllers/application_controller.rb
index 8808b7e..2ce15ab 100644
--- a/src/app/controllers/application_controller.rb
+++ b/src/app/controllers/application_controller.rb
@@ -199,7 +199,7 @@ class ApplicationController < ActionController::Base
end
def current_session
- @current_session ||= request.session_options[:id]
+ @current_session ||=
PermissionSession.find_by_id(session[:permission_session_id])
end
def require_user
diff --git a/src/app/controllers/users_controller.rb
b/src/app/controllers/users_controller.rb
index 0c9f564..5179ed9 100644
--- a/src/app/controllers/users_controller.rb
+++ b/src/app/controllers/users_controller.rb
@@ -71,7 +71,7 @@ class UsersController < ApplicationController
@title = @user.name
@quota_resources = @user.quota.quota_resources
if current_user == user
- SessionEntity.update_session(current_session, current_user)
+ current_session.update_session_entities(current_user)
end
@user_groups = @user.all_groups
@groups_header = [
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 523c6cd..8fb9077 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -209,14 +209,14 @@ class Deployment < ActiveRecord::Base
@launch_parameters = launch_parameters
end
- def create_and_launch(session, user)
+ def create_and_launch(permission_session, user)
begin
# this method doesn't restore record state if this transaction
# fails, wrapping transaction in rollback_active_record_state!
# is not sufficient because restore then doesn't work if
# you have some nested save operations inside the transaction
transaction do
- create_instances_with_params!(session, user)
+ create_instances_with_params!(permission_session, user)
launch!(user)
end
return true
@@ -351,11 +351,12 @@ class Deployment < ActiveRecord::Base
# we try to create an instance for each assembly and check
# if a match is found
- def check_assemblies_matches(session, user)
+ def check_assemblies_matches(permission_session, user)
errs = []
begin
deployable_xml.assemblies.each do |assembly|
- hw_profile = permissioned_frontend_hwprofile(session, user,
assembly.hwp)
+ hw_profile = permissioned_frontend_hwprofile(permission_session,
+ user, assembly.hwp)
raise I18n.t('deployments.flash.error.no_hwp_permission', :hwp =>
assembly.hwp) unless hw_profile
instance = Instance.new(
:deployment => self,
@@ -537,8 +538,8 @@ class Deployment < ActiveRecord::Base
end
end
- def permissioned_frontend_hwprofile(session, user, hwp_name)
- HardwareProfile.list_for_user(session, user,
Privilege::VIEW).where('hardware_profiles.name = :name AND provider_id IS NULL',
{:name => hwp_name}).first
+ def permissioned_frontend_hwprofile(permission_session, user, hwp_name)
+ HardwareProfile.list_for_user(permission_session, user,
Privilege::VIEW).where('hardware_profiles.name = :name AND provider_id IS NULL',
{:name => hwp_name}).first
end
def inject_launch_parameters
@@ -606,10 +607,11 @@ class Deployment < ActiveRecord::Base
self[:pool_family_id] = pool.pool_family_id
end
- def create_instances_with_params!(session, user)
+ def create_instances_with_params!(permission_session, user)
errors = {}
deployable_xml.assemblies.each do |assembly|
- hw_profile = permissioned_frontend_hwprofile(session, user, assembly.hwp)
+ hw_profile = permissioned_frontend_hwprofile(permission_session,
+ user, assembly.hwp)
raise I18n.t('deployments.flash.error.no_hwp_permission', :hwp =>
assembly.hwp) unless hw_profile
Instance.transaction do
instance = Instance.create!(
diff --git a/src/app/models/session_entity.rb
b/src/app/models/permission_session.rb
similarity index 66%
copy from src/app/models/session_entity.rb
copy to src/app/models/permission_session.rb
index ea3f547..29a039a 100644
--- a/src/app/models/session_entity.rb
+++ b/src/app/models/permission_session.rb
@@ -14,31 +14,30 @@
# limitations under the License.
#
-class SessionEntity < ActiveRecord::Base
+class PermissionSession < ActiveRecord::Base
belongs_to :user
- belongs_to :entity
validates_presence_of :user_id
validates_presence_of :session_id
- validates_presence_of :entity_id
- validates_uniqueness_of :entity_id, :scope => [:user_id, :session_id]
- def self.update_session(session_id, user)
- self.transaction do
+ def update_session_entities(user)
+ SessionEntity.transaction do
# skips callbacks, which should be fine here
- self.delete_all(:session_id => session_id)
- self.add_to_session(session_id, user)
+ SessionEntity.delete_all(:permission_session_id => self.id)
+ add_to_session(user)
end
end
- def self.add_to_session(session_id, user)
+ def add_to_session(user)
return unless user
# create mapping for user-level permissions
- SessionEntity.create!(:session_id => session_id, :user => user,
+ SessionEntity.create!(:permission_session_id => self.id,
+ :user => user,
:entity => user.entity)
# create mappings for local groups
user.all_groups.each do |ug|
- SessionEntity.create!(:session_id => session_id, :user => user,
+ SessionEntity.create!(:permission_session_id => self.id,
+ :user => user,
:entity => ug.entity)
end
end
diff --git a/src/app/models/permissioned_object.rb
b/src/app/models/permissioned_object.rb
index b657054..bdf2ded 100644
--- a/src/app/models/permissioned_object.rb
+++ b/src/app/models/permissioned_object.rb
@@ -16,17 +16,17 @@
module PermissionedObject
- def has_privilege(session, user, action, target_type=nil)
- return false if session.nil? or user.nil? or action.nil?
+ def has_privilege(permission_session, user, action, target_type=nil)
+ return false if permission_session.nil? or user.nil? or action.nil?
target_type = self.class.default_privilege_target_type if target_type.nil?
if derived_permissions.includes(:role => :privileges,
:entity => :session_entities).where(
["session_entities.user_id=:user and
- session_entities.session_id=:session and
+ session_entities.permission_session_id=:permission_session_id and
privileges.target_type=:target_type and
privileges.action=:action",
{ :user => user.id,
- :session => session,
+ :permission_session_id => permission_session.id,
:target_type => target_type.name,
:action => action}]).any?
return true
@@ -35,11 +35,11 @@ module PermissionedObject
includes(:role => :privileges,
:entity => :session_entities).where(
["session_entities.user_id=:user and
- session_entities.session_id=:session and
+ session_entities.permission_session_id=:permission_session_id and
privileges.target_type=:target_type and
privileges.action=:action",
{ :user => user.id,
- :session => session,
+ :permission_session_id => permission_session,
:target_type => target_type.name,
:action => action}]).any?
end
@@ -113,23 +113,23 @@ module PermissionedObject
def self.default_privilege_target_type
self
end
- def self.list_for_user(session, user, action,
+ def self.list_for_user(permission_session, user, action,
target_type=self.default_privilege_target_type)
- if session.nil? or user.nil? or action.nil? or target_type.nil?
+ if permission_session.nil? or user.nil? or action.nil? or
target_type.nil?
return where("1=0")
end
if BasePermissionObject.general_permission_scope.
- has_privilege(session, user, action, target_type)
+ has_privilege(permission_session, user, action, target_type)
scoped
else
includes([:derived_permissions => {:role => :privileges,
:entity => :session_entities}]).
where("session_entities.user_id=:user and
- session_entities.session_id=:session and
+
session_entities.permission_session_id=:permission_session_id and
privileges.target_type=:target_type and
privileges.action=:action",
{:user => user.id,
- :session => session,
+ :permission_session_id => permission_session.id,
:target_type => target_type.name,
:action => action})
end
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 18ba564..0879262 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -101,7 +101,7 @@ class Pool < ActiveRecord::Base
end
# TODO: Implement Alerts and Updates
- def statistics(session=nil, user = nil)
+ def statistics(permission_session=nil, user = nil)
# TODO - Need to set up cache invalidation before this is safe
#Rails.cache.fetch("pool-#{id}-statistics") do
max = quota.maximum_running_instances
@@ -109,7 +109,7 @@ class Pool < ActiveRecord::Base
avail = max - total unless max.nil?
all_failed = instances.failed
failed = (user.nil? || all_failed.empty? ? all_failed :
- all_failed.list_for_user(session, user, Privilege::VIEW))
+ all_failed.list_for_user(permission_session, user,
Privilege::VIEW))
pool_family_quota_percent = pool_family.quota.percentage_used
quota.running_instances
statistics = {
:cloud_providers => instances.includes(:provider_account).collect{|i|
i.provider_account}.uniq.count,
diff --git a/src/app/models/session_entity.rb b/src/app/models/session_entity.rb
index ea3f547..98ac974 100644
--- a/src/app/models/session_entity.rb
+++ b/src/app/models/session_entity.rb
@@ -17,29 +17,11 @@
class SessionEntity < ActiveRecord::Base
belongs_to :user
belongs_to :entity
+ belongs_to :permission_session
validates_presence_of :user_id
- validates_presence_of :session_id
+ validates_presence_of :permission_session_id
validates_presence_of :entity_id
- validates_uniqueness_of :entity_id, :scope => [:user_id, :session_id]
+ validates_uniqueness_of :entity_id, :scope => [:user_id,
:permission_session_id]
- def self.update_session(session_id, user)
- self.transaction do
- # skips callbacks, which should be fine here
- self.delete_all(:session_id => session_id)
- self.add_to_session(session_id, user)
- end
- end
-
- def self.add_to_session(session_id, user)
- return unless user
- # create mapping for user-level permissions
- SessionEntity.create!(:session_id => session_id, :user => user,
- :entity => user.entity)
- # create mappings for local groups
- user.all_groups.each do |ug|
- SessionEntity.create!(:session_id => session_id, :user => user,
- :entity => ug.entity)
- end
- end
end
diff --git a/src/config/initializers/warden.rb
b/src/config/initializers/warden.rb
index e879b8f..9495881 100644
--- a/src/config/initializers/warden.rb
+++ b/src/config/initializers/warden.rb
@@ -82,5 +82,26 @@ Warden::Strategies.add(:ldap) do
end
end
Warden::Manager.after_authentication do |user,auth,opts|
- SessionEntity.update_session(auth.request.session_options[:id], user)
+ current_session_id = auth.request.session_options[:id]
+ session = auth.env['rack.session']
+ perm_session = PermissionSession.create!(:user => user,
+ :session_id => current_session_id)
+ session[:permission_session_id] = perm_session.id
+ perm_session.update_session_entities(user)
+end
+Warden::Manager.after_set_user do |user,auth,opts|
+ current_session_id = auth.request.session_options[:id]
+ session = auth.env['rack.session']
+ perm_session_id = session[:permission_session_id]
+ if perm_session_id
+ perm_session = PermissionSession.find(perm_session_id)
+ #if session_id doesn't match what we originally set, update the value
+ # This isn't needed for perm checks (that's why we store
+ # permission_session_id), but matching the correct session_id will
facilitate
+ # permission_session cleanup
+ if perm_session && (perm_session.session_id != current_session_id)
+ perm_session.session_id = current_session_id
+ perm_session.save!
+ end
+ end
end
diff --git a/src/db/migrate/20120816131500_create_permission_sessions.rb
b/src/db/migrate/20120816131500_create_permission_sessions.rb
new file mode 100644
index 0000000..a938242
--- /dev/null
+++ b/src/db/migrate/20120816131500_create_permission_sessions.rb
@@ -0,0 +1,41 @@
+#
+# Copyright 2012 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+class CreatePermissionSessions < ActiveRecord::Migration
+ def self.up
+ create_table :permission_sessions do |t|
+ t.references :user, :null => false
+ t.string :session_id, :null => false
+
+ t.integer :lock_version, :default => 0
+
+ t.timestamps
+ end
+ #delete any existing session entities, as they'll no longer be valid
+ SessionEntity.destroy_all
+ remove_column :session_entities, :session_id
+ add_column :session_entities, :permission_session_id, :integer
+ change_column :session_entities, :permission_session_id, :integer, :null
=> false
+ end
+
+ def self.down
+ SessionEntity.destroy_all
+ remove_column :session_entities, :permission_session_id
+ add_column :session_entities, :session_id, :string
+ change_column :session_entities, :session_id, :string, :null => false
+
+ drop_table :permission_sessions
+ end
+end
diff --git a/src/features/support/custom.rb b/src/features/support/custom.rb
index 6a2299b..02a60a4 100644
--- a/src/features/support/custom.rb
+++ b/src/features/support/custom.rb
@@ -67,11 +67,3 @@ User.class_eval do
alias authenticate_using_ldap authenticate
end
end
-
-# FIXME: For some reason, the session doesn't stick around properly in
-# cucumber without the after_set_user method, so for now, enable it
-# in the test suite.
-#
-Warden::Manager.after_set_user do |user,auth,opts|
- SessionEntity.update_session(auth.request.session_options[:id], user)
-end
diff --git a/src/spec/models/deployment_spec.rb
b/src/spec/models/deployment_spec.rb
index ab4e93e..aa78f50 100644
--- a/src/spec/models/deployment_spec.rb
+++ b/src/spec/models/deployment_spec.rb
@@ -221,14 +221,17 @@ describe Deployment do
@user_for_launch.quota.maximum_running_instances = 1
@session = FactoryGirl.create :session
@session_id = @session.session_id
- SessionEntity.update_session(@session_id, @user_for_launch)
+ @permission_session = PermissionSession.create!(:user =>
@user_for_launch,
+ :session_id =>
@session_id)
+ @permission_session.update_session_entities(@user_for_launch)
@deployment.stub(:common_provider_accounts_for).and_return(["test","test"])
end
it "return error when user quota was reached" do
Instance.any_instance.stub(:matches).and_return(["test","test"])
@deployment.stub!(:find_match_with_common_account).and_return([[], true,
[]])
- errors = @deployment.check_assemblies_matches(@session_id,
@user_for_launch)
+ errors = @deployment.check_assemblies_matches(@permission_session,
+ @user_for_launch)
errors.should have(1).items
errors.last.should include I18n.t('instances.errors.user_quota_reached')
end
@@ -248,20 +251,22 @@ describe Deployment do
@user_for_launch = admin_perms.user
@session = FactoryGirl.create :session
@session_id = @session.session_id
- SessionEntity.update_session(@session_id, @user_for_launch)
+ @permission_session = PermissionSession.create!(:user =>
@user_for_launch,
+ :session_id =>
@session_id)
+ @permission_session.update_session_entities(@user_for_launch)
end
it "should return errors when checking assemblies matches which are not
launchable" do
- @deployment.check_assemblies_matches(@session_id,
@user_for_launch).should be_empty
+ @deployment.check_assemblies_matches(@permission_session,
@user_for_launch).should be_empty
@deployment.pool.pool_family.provider_accounts.destroy_all
- @deployment.check_assemblies_matches(@session_id,
@user_for_launch).should_not be_empty
+ @deployment.check_assemblies_matches(@permission_session,
@user_for_launch).should_not be_empty
end
it "should launch instances when launching deployment" do
@deployment.instances.should be_empty
Taskomatic.stub!(:create_instance!).and_return(true)
- @deployment.create_and_launch(@session_id, @user_for_launch)
+ @deployment.create_and_launch(@permission_session, @user_for_launch)
@deployment.errors.should be_empty
@deployment.instances.count.should == 2
end
@@ -277,7 +282,7 @@ describe Deployment do
Taskomatic.stub!(:create_dcloud_instance).and_return(true)
Taskomatic.stub!(:handle_dcloud_error).and_return(true)
Taskomatic.stub!(:handle_instance_state).and_return(true)
- @deployment.create_and_launch(@session_id, @user_for_launch)
+ @deployment.create_and_launch(@permission_session, @user_for_launch)
@deployment.errors.should be_empty
@deployment.reload
@deployment.instances.count.should == 2
@@ -287,7 +292,7 @@ describe Deployment do
@deployment.instances.should be_empty
@deployment.pool.pool_family.provider_accounts.destroy_all
Taskomatic.stub!(:create_instance!).and_return(true)
- @deployment.create_and_launch(@session_id, @user_for_launch)
+ @deployment.create_and_launch(@permission_session, @user_for_launch)
@deployment.errors.should_not be_empty
lambda { Deployment.find(@deployment.id) }.should
raise_error(ActiveRecord::RecordNotFound)
end
@@ -300,7 +305,7 @@ describe Deployment do
it "should set create_failed status for instances if instance's launch raises
an exception" do
@deployment.instances.should be_empty
Taskomatic.stub!(:create_dcloud_instance).and_raise("an exception")
- @deployment.create_and_launch(@session_id, @user_for_launch)
+ @deployment.create_and_launch(@permission_session, @user_for_launch)
@deployment.reload
@deployment.instances.should_not be_empty
@deployment.instances.each {|i| i.state.should ==
Instance::STATE_CREATE_FAILED}
diff --git a/src/spec/models/permission_spec.rb
b/src/spec/models/permission_spec.rb
index ea9a20d..5073d58 100644
--- a/src/spec/models/permission_spec.rb
+++ b/src/spec/models/permission_spec.rb
@@ -33,53 +33,55 @@ describe Permission do
@pool = @pool_user_permission.pool
@session = FactoryGirl.create :session
@session_id = @session.session_id
- SessionEntity.update_session(@session_id, @admin)
- SessionEntity.add_to_session(@session_id, @provider_admin)
- SessionEntity.add_to_session(@session_id, @pool_user)
+ @permission_session = PermissionSession.create!(:user => @admin,
+ :session_id => @session_id)
+ @permission_session.update_session_entities(@admin)
+ @permission_session.add_to_session(@provider_admin)
+ @permission_session.add_to_session(@pool_user)
end
it "Admin should be able to create users" do
- BasePermissionObject.general_permission_scope.has_privilege(@session_id,
+
BasePermissionObject.general_permission_scope.has_privilege(@permission_session,
@admin,
Privilege::CREATE,
User).should
be_true
end
it "Provider Admin should NOT be able to create users" do
- BasePermissionObject.general_permission_scope.has_privilege(@session_id,
+
BasePermissionObject.general_permission_scope.has_privilege(@permission_session,
@provider_admin,
Privilege::CREATE,
User).should
be_false
end
it "Pool User should NOT be able to create users" do
- BasePermissionObject.general_permission_scope.has_privilege(@session_id,
+
BasePermissionObject.general_permission_scope.has_privilege(@permission_session,
@pool_user,
Privilege::CREATE,
User).should
be_false
end
it "Provider Admin should be able to edit provider" do
- @provider.has_privilege(@session_id, @provider_admin,
+ @provider.has_privilege(@permission_session, @provider_admin,
Privilege::MODIFY).should be_true
end
it "Admin should be able to edit provider" do
- @provider.has_privilege(@session_id, @admin, Privilege::MODIFY).should
be_true
+ @provider.has_privilege(@permission_session, @admin,
Privilege::MODIFY).should be_true
end
it "Pool User should NOT be able to edit provider" do
- @provider.has_privilege(@session_id, @pool_user,
+ @provider.has_privilege(@permission_session, @pool_user,
Privilege::MODIFY).should be_false
end
it "Pool User should be able to create instances in @pool" do
- @pool.has_privilege(@session_id, @pool_user,
+ @pool.has_privilege(@permission_session, @pool_user,
Privilege::CREATE, Instance).should be_true
end
it "Pool User should NOT be able to create instances in another pool" do
- FactoryGirl.create(:tpool).has_privilege(@session_id, @pool_user,
+ FactoryGirl.create(:tpool).has_privilege(@permission_session, @pool_user,
Privilege::CREATE, Instance).
should be_false
end
@@ -88,15 +90,15 @@ describe Permission do
newuser = FactoryGirl.create(:user)
group_admin_permission = FactoryGirl.create(:group_admin_permission)
user_group = group_admin_permission.user_group
- SessionEntity.update_session(@session_id, newuser)
- BasePermissionObject.general_permission_scope.has_privilege(@session_id,
+ @permission_session.update_session_entities(newuser)
+
BasePermissionObject.general_permission_scope.has_privilege(@permission_session,
newuser,
Privilege::CREATE,
User).should
be_false
user_group.members << newuser
newuser.reload
- SessionEntity.update_session(@session_id, newuser)
- BasePermissionObject.general_permission_scope.has_privilege(@session_id,
+ @permission_session.update_session_entities(newuser)
+
BasePermissionObject.general_permission_scope.has_privilege(@permission_session,
newuser,
Privilege::CREATE,
User).should
be_true
diff --git a/src/spec/services/registration_service_spec.rb
b/src/spec/services/registration_service_spec.rb
index 76c2544..2ae936a 100644
--- a/src/spec/services/registration_service_spec.rb
+++ b/src/spec/services/registration_service_spec.rb
@@ -39,7 +39,9 @@ describe RegistrationService do
@user = FactoryGirl.create :user
@session = FactoryGirl.create :session
@session_id = @session.session_id
- SessionEntity.update_session(@session_id, @user)
+ @permission_session = PermissionSession.create!(:user => @user,
+ :session_id =>
@session_id)
+ @permission_session.update_session_entities(@user)
@pool = MetadataObject.lookup("self_service_default_pool")
@role = MetadataObject.lookup("self_service_default_role")
@quota = FactoryGirl.create :quota
@@ -48,7 +50,8 @@ describe RegistrationService do
@registration_service = RegistrationService.new(@user)
@registration_service.save
- @pools = Pool.list_for_user(@session_id, @user, Privilege::CREATE,
Instance)
+ @pools = Pool.list_for_user(@permission_session, @user,
+ Privilege::CREATE, Instance)
@pools.length.should == 1
@pools[0].name.should == "Default"
diff --git a/src/spec/spec_helper.rb b/src/spec/spec_helper.rb
index ae89927..c0a2b9b 100644
--- a/src/spec/spec_helper.rb
+++ b/src/spec/spec_helper.rb
@@ -63,7 +63,12 @@ def mock_warden(user)
request.session_options[:id] = @session_id
@session = ActiveRecord::SessionStore::Session.find_by_session_id(@session_id)
@session = FactoryGirl.create :session unless @session
- SessionEntity.update_session(@session_id, user) if user
+ if user
+ @permission_session = PermissionSession.create!(:user => user,
+ :session_id => @session_id)
+ request.session[:permission_session_id] = @permission_session.id
+ @permission_session.update_session_entities(user)
+ end
end
# Without these here, controller specs fail. These 2 class_evals can
--
1.7.6.5
