rubycas / rubycas-server

Provides single sign-on authentication for web applications, implementing the server-end of Jasig's CAS protocol.
http://rubycas.github.com
Other
628 stars 270 forks source link

Allow multiple SQL authenticators to be used without conflict. #219

Closed davexunit closed 10 years ago

davexunit commented 10 years ago

Rather than clobber the user_model class variable every time a SQL authenticator is used, use a hash to map auth_index to the associated user model. This allows for using a configuration that authenticates with multiple SQL databases. Without this patch, the server will only query against the last database specified in the authenicator array in your config.yml.

There are no unit tests at all for the SQL authenticators so I did not include tests, but I could try to add some if required.

Thoughts?

As as aside, is it important that the generated classes have a name? The code can further cleaned up by removing the calls to class_eval and const_get and instead use Class.new(ActiveRecord::Base) to create the user model class.

mitfik commented 10 years ago

Hi @davexunit ,

Thanks a lot for your contribution. I believe that without proper tests we should not merge it as in first place we need to make sure that it won't break anything for people who are not using multiple SQL authenticators.

So if you will be so kind to prepare those test, that will be great. Otherwise we will need to wait a bit until someone else will have time for that.

Regarding the named class, I am not sure about it, seems that it does not matter. As we are anyway setting there everything (like table_name, connection etc.) But again hard to say without proper tests.

davexunit commented 10 years ago

Thanks for the feedback, I will try to write some basic test cases.

davexunit commented 10 years ago

Some tests have been added, @mitfik. I demonstrated that a single SQL authenticator works alone, and that 2 SQL authenticators work together without clobbering each other.

What do you think?

davexunit commented 10 years ago

@mitfik: ping! It's been over a month, just wondering if you've had a chance to review this patch again.

Thanks!

mitfik commented 10 years ago

Thanks a lot for contribution, seems to be ok.

korun commented 10 years ago

Hmm, I have error on this version O_o

$ ruby -v
ruby 2.0.0p481 (2014-05-08 revision 45883) [i686-linux]

$ rubycas-server -c config/config.yml

=> Using custom config file "config/config.yml"
   (100.7ms)  SELECT "schema_migrations"."version" FROM "schema_migrations" 
Migrating to CreateInitialStructure (1)
Migrating to AddIndexesForPerformance (2)
/home/username/rubycas_server/lib/casserver/authenticators/sql_encrypted.rb:43:in `setup': undefined local variable or method `user_model' for CASServer::Authenticators::SQLEncrypted:Class (NameError)
    from /home/username/rubycas_server/lib/casserver/server.rb:227:in `block in init_authenticators!'
    from /home/username/rubycas_server/lib/casserver/server.rb:224:in `each'
    from /home/username/rubycas_server/lib/casserver/server.rb:224:in `each_with_index'
    from /home/username/rubycas_server/lib/casserver/server.rb:224:in `init_authenticators!'
    from /home/username/rubycas_server/lib/casserver/server.rb:274:in `block in <class:Server>'

I'll check config file, maybe problem in my side...

korun commented 10 years ago

Seems like problem in this line. user_model method has been changed to user_models. We have instance method user_model, but we need class usage. I'll try make quick fix now...