search

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 using ActiveRecord/Support 3.1 and 3.2 #118

Closed tpickett66 closed 11 years ago

tpickett66 commented 12 years ago

This update adjusts the dependencies to allow for the use of ActiveRecord/Support versions beyond the 3.0.x series all the way up to the current 3.2.8. Also included is the removal of the magic type column on the casserver_st table since STI isn't being used and the fix for R18n's API changes.

tpickett66 commented 12 years ago

Please don't merge this yet, I've broken the STI for Service and Proxy. I'll have it fixed by the end of the day.

larryzhao commented 12 years ago

Good job @tpickett66!

tpickett66 commented 12 years ago

Ok, I'd say this is ready. I went ahead and took the liberty to refactor some items in the models to shrink down the inheritance chain and the model.rb file.

tpickett66 commented 12 years ago

@zuk is there an objection to this patch or have you just not gotten a chance to review it. I'd love to continue contributing additional features.

zuk commented 11 years ago

No objection, only that I'm at the job where rubycas-server stuff gets done one day every two weeks, and haven't had any time to look in to this the last couple of times I was here. It would make things a lot easier if the different changes came in separately so that I could pull them in piece by piece. But I've got a bit of time now... will try to go through it and cherry-pick as much as I can for now.

tpickett66 commented 11 years ago

Copy, thanks for letting me know about your schedule. It makes me much less nervous about the lack of communication. I'll be sending along an early stage pull request for Remember Me knowing that most of it will break as soon as any of this is merged but it hopefully will spark some dialog/interest with other community members.

On Wed, Oct 17, 2012 at 11:38 AM, Matt Zukowski notifications@github.comwrote:

No objection, only that I'm at the job where rubycas-server stuff gets done one day every two weeks, and haven't had any time to look in to it. It would make things a lot easier if the different changes came in separately so that I could pull them in piece by piece. But I've got a bit of time now... will try to go through it and cherry-pick as much as I can for now.

— Reply to this email directly or view it on GitHubhttps://github.com/rubycas/rubycas-server/pull/118#issuecomment-9534211.

Tyler Pickett Mobile: (417) 827-6416 Skype: t.pickett66

zuk commented 11 years ago

Thanks Tyler. Appreciate your patience on getting this merged in. Judging from your interest and commitment, you should probably just have proper access to the main repo, but need to hold off on that until the first round of changes gets merged.

zuk commented 11 years ago

Looks great... one hopefully minor problem though. The last spec -- CASServer proxyValidate should have extra attributes in proper format -- is failing under ruby 1.8.7 (but not 1.9.2). Looks like some sort UTF-8 problem.

<test_utf_string>Ютф</test_utf_string> is getting returned as <test_utf_string>&#1070;&#1090;&#1092;</test_utf_string>

Probably something to do with 954e2e96db224eec33949fce3431603be050f498. I'll look into it some more.

zuk commented 11 years ago

Okay looks like it's just a matter of adding xml.instruct! :xml, :version=>"1.0", :encoding=>"UTF-8" to the .builder template files.

tpickett66 commented 11 years ago

I was actually a bit confused about the handling of UTF-8 strings and this particular test. Specifically why we were expecting them to get encoded to byte strings for representation in ascii. Are you making that change or would you like me to?

zuk commented 11 years ago

I've made the changes and pushed to master. Looking good so I'm going to close this. Thanks!

tpickett66 commented 11 years ago

Hmm, I just sat down and ran the specs against all versions of ActiveSupport/Record and am getting a failure around the UTF-8 string test again.

Running the specs via rake appraisal:rails2 and rake appraisal:rails30 on 1.9.3 yields:

  1) CASServer proxyValidate should have extra attributes in proper format
     Failure/Error: page.body.should match("<test_utf_string>Ютф</test_utf_string>")
       expected "
<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.0 Transitional//EN\" \"http://www.w3.org/TR/REC-html40/loose.dtd\">
<?xml version=\"1.0\" encoding=\"UTF-8\"?>
<html>
    <body>
        <serviceresponse xmlns:cas=\"http://www.yale.edu/tp/cas\">
            <authenticationsuccess>
                <user>spec_user</user>
                <test_utf_string>&#1070;&#1090;&#1092;</test_utf_string>
                <test_numeric>123.45</test_numeric>
                <test_serialized></test_serialized>
            </authenticationsuccess>
        </serviceresponse>
    </body>
</html>" to match "<test_utf_string>Ютф</test_utf_string>"
     # ./spec/casserver_spec.rb:150:in `block (3 levels) in <top (required)>'

But on 1.8.7 we are all green.

zuk commented 11 years ago

Can you go into your lib/casserver/views/proxy_validate.builder and make sure that it looks like this:

https://github.com/rubycas/rubycas-server/blob/master/lib/casserver/views/proxy_validate.builder#L2

You should have the xml.instruct! call at the top. Looking at the output you pasted, I'm willing to bet that you haven't pulled in the changes from @rubycas . If you had, it would start with <?xml instead of <!DOCTYPE html

tpickett66 commented 11 years ago

That is what I've got. I cleaned up the formatting of response I posted and realized that it is giving both HTML and XML headers... odd. I will be working on getting to the bottom of it today.

zuk commented 11 years ago

Yeah this is kind of odd... it was working fine for me under both 1.8.7 and 1.9.2. I'll give it another go when I have a chance.

On Thu, Oct 18, 2012 at 9:47 AM, Tyler Pickett notifications@github.comwrote:

That is what I've got. I cleaned up the formatting of response I posted and realized that it is giving both HTML and XML headers... odd. I will be working on getting to the bottom of it today.

— Reply to this email directly or view it on GitHubhttps://github.com/rubycas/rubycas-server/pull/118#issuecomment-9564841.

zuk commented 11 years ago

P.S. we could always just dump Builder there and just generate the XML text manually. Unless I'm forgetting something, I think this is the only place where we're using Builder, so as an added bonus we could dump that extra dependency. The only concern would be making sure that we can get all of the encoding/escaping right—something Builder is supposed to take care of for us.

On Thu, Oct 18, 2012 at 10:38 AM, Matt Zukowski matt@zukowski.ca wrote:

Yeah this is kind of odd... it was working fine for me under both 1.8.7 and 1.9.2. I'll give it another go when I have a chance.

On Thu, Oct 18, 2012 at 9:47 AM, Tyler Pickett notifications@github.comwrote:

That is what I've got. I cleaned up the formatting of response I posted and realized that it is giving both HTML and XML headers... odd. I will be working on getting to the bottom of it today.

— Reply to this email directly or view it on GitHubhttps://github.com/rubycas/rubycas-server/pull/118#issuecomment-9564841.

tpickett66 commented 11 years ago

Well I've managed to get the HTML headers and decoration to go away by telling adding content_type 'text/xml', :charset => 'utf-8' near the top of the xml generating actions and dropping down to the Rack::Test dsl from Capybara's. Capybara isn't really intended to work with anything other than HTML and web pages as opposed to the API we're testing here (the author of Capybara gets a bit nasty when pointing this out to people on the mailing list). No dice on fixing the utf string though, still mangled into the ASCII representation of them.

tpickett66 commented 11 years ago

I'm opening another pull request with what I've changed so it'll get tracked as we work through it.