Fail save
While working with the excellent writertopia authorization plugin I made some small changes to let it fail more secure in certain situations.
With
this plugin you are able to query and set permissions for users and
optional additionally in relation to certain models. To quote their
website: "This plugin provides a flexible way to add authorization to
Rails." This is true.
You can write code like
do_something if current_user.is_moderator_of? forum_obj
or
do_something if current_user.is_registered?
There is no need to write a specific method called 'is_moderator_of?'. The plugin does its magic and queries the database to get the needed information.
In the first example it would ask if the current_user has a role called 'moderator' and if this role is connected to the forum object. In the second example, the plugin would just check if the current_user has a role called 'registered'.
The problem comes now:
If I call
current_user.is_ROLENAME_of? authorizable_obj
it evaluates to true in case authorizable_obj equals nil and the current_user has at least one role named ROLENAME.
In
my opinion it should evaluate to false as this is more restrictive and
prevents unwanted authorization in case the programmer did not make
sure to test if authorizable_obj.nil?.
This all goes down to the
problem that by the way 'has_role?' is called, it can not differentiate
between no authorizable_obj parameter at all and an authorizable_obj
which equals nil.
Going from the problem backwards to the use of the api, I would change the the code like this:
The 'has_role?' method inside object_roles_table.rb from
def has_role?( role_name, authorizable_obj = nil )
if authorizable_obj.nil?
# If we ask a general role question, return true if any role is defined.
self.roles.find_by_name( role_name ) ? true : false
else
role = get_role( role_name, authorizable_obj )
role ? self.roles.exists?( role.id ) : false
end
end
to:
def has_role?( role_name, authorizable_obj = -1 )
return false if authorizable_obj.nil?
if authorizable_obj == -1
# If we ask a general role question, return true if any role is defined.
self.roles.find_by_name( role_name ) ? true : false
else
role = get_role( role_name, authorizable_obj )
role ? self.roles.exists?( role.id ) : false
end
end
In identity.rb change
def is_role?( role_name, authorizable_object )
if authorizable_object.nil?
return self.has_role?(role_name)
elsif authorizable_object.respond_to?(:accepts_role?)
return self.has_role?(role_name, authorizable_object)
end
false
end
to:
def is_role?( role_name, authorizable_object )
if authorizable_object.nil?
return self.has_role?(role_name,nil)
elsif authorizable_object == -1
return self.has_role?(role_name)
elsif authorizable_object.respond_to?(:accepts_role?)
return self.has_role?(role_name, authorizable_object)
end
false
end
And in 'method_missing' the initialisation of authorizable_obj needs to be changed from
def method_missing( method_sym, *args )
method_name = method_sym.to_s
authorizable_object = args.empty? ? nil : args[0]
...
...
to
def method_missing( method_sym, *args )
method_name = method_sym.to_s
authorizable_object = args.empty? ? -1 : args[0]
...
...
In
case it is not obvious, I use -1 to show has_role? that there was no
parameter supplied, as it seems more likely that the parameter is nil
by accident. I sent my suggestions to writertopia and expect to hear
from them in a few days.
If you have any comments or spotted an error, or unwanted side effect, please use the comment system.
ruby ruby on rails security fail save
