dlindahl / omniauth-cas

A CAS OmniAuth Strategy
MIT License
88 stars 79 forks source link

Service Ticket Validation Failure #3

Closed iterateNZ closed 12 years ago

iterateNZ commented 12 years ago

Service ticket is failing validation as the service_url is changing from unescaped to escaped.

https://github.com/dlindahl/omniauth-cas/blob/master/lib/omniauth/strategies/cas.rb Line 108 { :service => service_url.to_s, :ticket => ticket } Line 117 { :service => Rack::Utils.unescape(service) }

Rails Log:

Started GET "/users/auth/cas" for 127.0.0.1 at Mon Jan 16 10:12:25 +1300 2012

Started GET "/users/auth/cas/callback?url=http://127.0.0.10/&ticket=ST-1-LI2RfXBXynZ3wbFcv9Vl-sso.myacg.org" for 127.0.0.1 at Mon Jan 16 10:12:25 +1300 2012 Dalli::Server#connect 127.0.0.1:11211 Processing by Users::OmniauthCallbacksController#failure as HTML Parameters: {"ticket"=>"ST-1-LI2RfXBXynZ3wbFcv9Vl-sso.myacg.org", "url"=>"http://127.0.0.10/"} Redirected to http://127.0.0.10/users/sign_in Completed 302 Found in 1ms

Cas Log:

2012-01-16 10:12:25,445 INFO [org.jasig.cas.CentralAuthenticationServiceImpl] - Granted service ticket [ST-1-LI2RfXBXynZ3wbFcv9Vl-sso.myacg.org] for service [http://127.0.0.10/users/auth/cas/callback?url=http://127.0.0.10/] for user [052463] 2012-01-16 10:12:26,127 ERROR [org.jasig.cas.CentralAuthenticationServiceImpl] - ServiceTicket [ST-1-LI2RfXBXynZ3wbFcv9Vl-sso.myacg.org] with service [http://127.0.0.10/users/auth/cas/callback?url=http://127.0.0.10/ does not match supplied service [http://127.0.0.10/users/auth/cas/callback?url=http%3A%2F%2F127.0.0.10%2F]

It does work if there is no url param

Rails.log

Started GET "/users/auth/cas" for 127.0.0.1 at Mon Jan 16 10:27:14 +1300 2012

Started GET "/users/auth/cas/callback?url=&ticket=ST-4-osxtwCW30aIfMf5auBej-sso.myacg.org" for 127.0.0.1 at Mon Jan 16 10:27:15 +1300 2012 Processing by Users::OmniauthCallbacksController#cas as HTML Parameters: {"ticket"=>"ST-4-osxtwCW30aIfMf5auBej-sso.myacg.org", "url"=>""}

Cas.log

2012-01-16 10:27:15,068 INFO [org.jasig.cas.CentralAuthenticationServiceImpl] - Granted service ticket [ST-4-osxtwCW30aIfMf5auBej-sso.myacg.org] for service [http://127.0.0.10/users/auth/cas/callback?url=] for user [052463]

Gems:

devise 1.5.3 omniauth 1.0.1 omniauth-cas 0.0.3

dlindahl commented 12 years ago

Thanks, taking a look right now.

dlindahl commented 12 years ago

Sorry its taken so long to get back on this, the day job was in the way.

I'm having a hard time validating a fix for this. I think WebMock is altering the URL so its difficult to assert the correct value.

Do you mind checking out the issue_3 branch and manually testing it in your app?

gem 'omniauth-cas', :git => 'git://github.com/dlindahl/omniauth-cas.git', :branch => 'issue_3'

If this doesn't work, please post your log files again. Those were very useful.

jbbarth commented 12 years ago

Hi,

I think I ran into a similar issue. I didn't find these posts at first so I didn't try your branch for now. I use "rubycas-server" for these tests, I cannot run them at work before monday on a jasig implementation..

Using PRY, I was able to see what get_service_response_body returns, it turned out to be that kind of thing:

> get_service_response_body
=> "<cas:serviceResponse xmlns:cas=\"http://www.yale.edu/tp/cas\">\n  <cas:authenticationFailure code=\"INVALID_SERVICE\">The ticket 'ST-1327127903rB12C213912794FA12D' be longing to user 'jean-baptiste.barth@xxxxx' is valid, but the requested service 'http://localhost:3000/auth/cas/callback?url=http%3A%2F%2Flocalhost%3A3000%2Fauth%2Ffailure%3Fmessage%3Dinvalid_ticket' does not match the service 'http://localhost:3000/auth/cas/callback?url=http://localhost:3000/auth/failure?message=invalid_ticket' associated with this ticket.</cas:authenticationFailure>\n</cas:serviceResponse>\n"

After various tries, I found that escaping the "url" parameter at first works :

> get_service_response_body
=> "<cas:serviceResponse xmlns:cas=\"http://www.yale.edu/tp/cas\">\n  <cas:authenticationSuccess>\n   <cas:user>jean-baptiste.barth@xxxxx</cas:user>\n  </cas:authenticationSuccess>\n</cas:serviceResponse>\n"

The change consists in modifying line 148 of lib/omniauth/strategies/cas.rb : { :url => request.referer } became { :url => Rack::Utils.escape(request.referer) }

Your fix in the "issue_3" branch doesn't seem to work for me at first sight, but I should have a better look at it, I did a lot of tests and don't know if my repo+gemset is still clean... It might be a better idea to escape in append_params though.

Let me know what you think, and anyway, thanks for this gem !

dlindahl commented 12 years ago

@jbbarth The fix in the issue_3 branch is doing just that, escaping in append_params.

Please double check that it is not working for you when you can and get back to me.

Thanks for your feedback!

jbbarth commented 12 years ago

I confirm it doesn't work as is, I keep getting a difference between urls:

> get_service_response_body
=> "<cas:serviceResponse xmlns:cas=\"http://www.yale.edu/tp/cas\">\n  <cas:authenticationFailure code=\"INVALID_SERVICE\">The ticket 'ST-1327186730r93199DA6DA260006A0' be longing to user 'jean-baptiste.barth@xxxxx' is valid, but the requested service 'http://localhost:3000/auth/cas/callback?url=http%3A%2F%2Flocalhost%3A3000%2Fauth%2Ffailure%3Fmessage%3Dinvalid_ticket' does not match the service 'http://localhost:3000/auth/cas/callback?url=http://localhost:3000/auth/failure?message=invalid_ticket' associated with this ticket.</cas:authenticationFailure>\n</cas:serviceResponse>\n"

But it works if I remove the Rack::Utils.unescape from login_url method.

I'll try to have a deeper look at it and provide a sample app to reproduce and/or a pull request with a test.

iterateNZ commented 12 years ago

issue_3 branch still isn't working

Escaping everthing in append_params is fine, but you are then overriding it in login_url by explicitly unescaping it.

removing the Rack::Utils.unescape resolves the issue. def login_url(service)

cas_host + append_params( @options.login_url, { :service => Rack::Utils.unescape(service) })

cas_host + append_params( @options.login_url, { :service => service }) end

dlindahl commented 12 years ago

@iterateNZ -- I think that was the missing piece. I made the change you suggested and a couple small changes to the tests and everything seems to be passing again.

Update your copy of the issue_3 branch and let me know how it goes.

`bundle update omniauth-cas`
iterateNZ commented 12 years ago

Yes that appears to be working correctly.

dlindahl commented 12 years ago

I've release 0.0.4 which includes this fix, please update accordingly.

Thanks for the help guys.