Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spec is failing on rails 5 (devise 4.2.0) #132

Open
AjayBarot opened this issue Oct 18, 2016 · 7 comments
Open

Spec is failing on rails 5 (devise 4.2.0) #132

AjayBarot opened this issue Oct 18, 2016 · 7 comments

Comments

@AjayBarot
Copy link

AjayBarot commented Oct 18, 2016

Hi ,

I am getting these two issue while running test suite.

  Devise::Strategies::CasAuthenticatable should fail CAS login if user is unregistered and cas_create_user is false
  Failure/Error: visit destroy_user_session_url

  ActionController::RoutingError:
  No route matches [GET] "/users/sign_out" 

   /home/abarot/.rvm/gems/ruby-2.3.0@rails5.0/gems/actionpack-5.0.0.1/lib/action_dispatch/middleware/debug_exceptions.rb:53:in `call'
   /home/abarot/.rvm/gems/ruby-2.3.0@rails5.0/gems/actionpack-5.0.0.1/lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
  ./spec/strategy_spec.rb:18:in `block (2 levels) in <top (required)>'

 Devise::Strategies::CasAuthenticatable should work correctly with Devise trackable
 Failure/Error: @app.call(env)

  NoMethodError:
    undefined method `ago' for 7200:Fixnum
    /home/abarot/.rvm/gems/ruby-2.3.0@rails5.0/gems/devise-4.2.0/lib/devise/models/timeoutable.rb:29:in `timedout?'
   ./lib/devise_cas_authenticatable/single_sign_out/rack.rb:14:in
   ./spec/strategy_spec.rb:38:in `sign_into_cas'
   ./spec/strategy_spec.rb:136:in `block (2 levels) in <top (required)>'

rails version = 5.0.0.1
ruby = 2.3.0p0
devise = 4.2.0
devise_cas_authenticatable = 1.9.2

Let me know If I miss something in code.

Thanks in Advance

@nbudin : Any idea about this ?

@MarcReniu
Copy link

Spec is still failing? Still, is it fully compatible with Rails 5 / Ruby 2.5.0?

@nbudin
Copy link
Owner

nbudin commented Mar 20, 2018

@MarcReniu thanks for the bump. The test suite hasn't really been working right since Rails 4, as you know. I think this has something to do with the really awful shenanigans we're using to fake out a CAS server in the dummy app, which suggests that the gem itself is fine even though the test suite fails (although I'm not actually positive that's true).

The truth is, I haven't had a lot of energy to put into this gem recently. If you have time to look into this and figure out a way to make the dummy app work with Rails 4 and up, I would be very happy to review and merge a PR.

@sauy7
Copy link

sauy7 commented Jun 6, 2018

I've had a stab at getting the specs running under Rails 5.2 (most likely with breaking changes for earlier versions) and Devise 4.4.3. I have all but one spec passing.

Relevant forks:
https://github.com/sauy7/devise_cas_authenticatable
https://github.com/sauy7/castronaut

The one failing spec:
https://github.com/sauy7/devise_cas_authenticatable/blob/cd8bf0d2b647886524e4e5aa5bbe934fa7547201/spec/single_sign_out_spec.rb#L39

It appears to go into an infinite loop of redirection.

With a local Gemfile containing:

source 'https://rubygems.org'

# Specify your gem's dependencies in devise_cas_authenticatable.gemspec
gemspec

gem 'rails', '~> 5.2.0'
gem 'devise', '~> 4.4.3'
gem 'activerecord-session_store'
gem 'sinatra', '~> 2.0.0.beta2'

group :test do
  gem 'castronaut', git: 'git@github.com:sauy7/castronaut', branch: 'dam5s-merge'
end

@nbudin
Copy link
Owner

nbudin commented Jun 7, 2018

Cool, thank you @sauy7! That's a whole bunch of work you put in and I really appreciate it.

I think you're probably right that these are breaking changes for earlier versions of Rails. At this point I'm more than OK dropping support for earlier versions and releasing a major version update to signal that it's a breaking change.

Would you like me to merge in your changes now, or wait and see about the one failing spec?

@sauy7
Copy link

sauy7 commented Jun 7, 2018

@nbudin I'm a little worried about the failing spec as an infinite loop is nasty. That said, I don't use a Custom Warden Failure App, so I've no motivation to look into that part. I may also not use CAS at all for my SSO needs, I'm just evaluating different options right now.

Perhaps others following this issue could chime in on fixing the last failing spec?

@nbudin
Copy link
Owner

nbudin commented Jun 7, 2018

@sauy7 In the case of this particular use case, I don't think it matters whether you use a custom Warden failure app, because devise_cas_authenticatable's single sign-out feature will insert one of its own. So it definitely needs to be tested. That said, I think this is pretty likely to be a bug in the test suite (or, possibly, in our weird fork of castronaut that we use just for the test suite) rather than in the gem.

Re choosing whether to use CAS or not: to be totally frank, I would recommend not using CAS for a new SSO implementation today. There are better options out there now, and I myself am trying to migrate off it in favor of OAuth2, since it has a much better ecosystem in Ruby. (I realize OAuth2 isn't an authentication solution per se, but it's good enough for what I need it to do.)

@sauy7
Copy link

sauy7 commented Jun 7, 2018

@nbudin I think I've come to the same conclusion (OAuth2) :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants