Poor Authenticator Design

[mdeiters]

It looks as though when CAS Server boots up it instantiates all of the
authenticators and then during validation each authenticator in $AUTH
is validated against the user's credentials. Later in the code we look
at each authenticator for an error message that is set which seems
completely flawed since these Authenticators are not threadsafe since
the error is stored in a instance variable that could be modified by
multiple users attempting to authenticate at the same time. Am I
understanding the code correctly?


credentials_are_valid = false
extra_attributes = {}
successful_authenticator = nil
begin
$AUTH.each do |auth|
credentials_are_valid = auth.validate(
:username => @username,
:password => @password,
:service => @service,
:request => @env
)
if credentials_are_valid
extra_attributes.merge!(auth.extra_attributes) unless
auth.extra_attributes.blank?
successful_authenticator = auth
break
end
end
rescue CASServer::AuthenticatorError => e
$LOG.error(e)
@message = {:type => 'mistake', :message => e.to_s}
return back_to_login
end


#later on in the post method of the LoginController

message = authenticator_with_error &&
authenticator_with_error.error
message ||= "Sorry the username and/or password you entered is
invalid. Please note after 5 attempts your account will be locked."
$LOG.warn("Error message from the authenticator - #
{message}")
@message = {:type => 'mistake', :message => _(message)}

def authenticator_with_error
@failed_authenticator ||= $AUTH.select{|auth|auth.respond_to?
(:error)}.first
end

[Matt Zukowski]

http://github.com/gunark/rubycas-server/issues/#issue/6

[Matt Zukowski]

This has now been fixed -- Authenticators are instantiated at the time
of authentication rather than globally at server startup.

Thanks for bringing this to my attention.

[Dan Coutu]

Ooooooo, nice. Now if the same trick could be done with themes it would
be possible to use a single CAS instance with multiple authenticators
and multiple themed login screens. Powerful!

Dan

[Matt Zukowski]

This change is not sitting well... I had to roll back to the previous version on our production machine. It looks like the additional authenticator instances are using up the ActiveRecord connection pool and sooner or later the server comes to a crawling halt. I don't have the time and patience right now to figure out how to take care of databse connection management for authenticators. I'd appreciate it though if anyone using the latest code from github let me know if they encounter similar issues.

[Lin Jen-Shin]

I am not sure what's going on here,
but taking a glimpse at this commit:
http://github.com/gunark/rubycas-server/commit/653be6d08421c8d7e87f027a1d4a3bca473831e5

MRI is not thread-safe in many places,
especially in `require', constants setup, etc.
I don't think instantiating any constants
while serving service is a good idea generally.
That is, just instantiate "all" constants at
boot-up is the safest way.

Kernel#autoload has the same problem.
Avoid them in thread critical process.
Explicitly requiring and instantiating is
tedious and annoying but a lot safer in
multi-threaded environments.

I am not sure if rubycas-server is running
in multi-threaded environments. But if
multi-threaded is considered, we should
explicitly require and instantiate any
shared value, e.g. constants, mutable data, etc.
Or get a mutex which instantiated in boot-up,
for a quick and dirty fix I guess...

Sorry that I don't have enough time to
take a deep look into it. I was doing too
many projects parallelly... /sigh/

[Lin Jen-Shin]

FYI:
I don't have enough time to provide a good patch,
but I am throwing commits to my own fork. Here's
what I do to my fork to try to solve this problem:

http://github.com/godfat/rubycas-server/commit/aa7f50fa71f7232094dbff76a870fb3db78babd2

And I was start refatoring to some of internals.
I would try to keep these simple enough to
merge back to mainstream, feel free to use them,
or ask me what is the reason behind any commit,
or provide a patch to my fork.

Sorry I won't do many testing to them since I am
lacking of time to concentrate on this project.

On Sep 25, 12:30 am, Lin Jen-Shin <god...@gmail.com> wrote:
> I am not sure what's going on here,

> but taking a glimpse at this commit:http://github.com/gunark/rubycas-server/commit/653be6d08421c8d7e87f02...