wardencommunity / warden

General Rack Authentication Framework
MIT License
2.48k stars 204 forks source link

`winning_strategy` not set when using `throw :warden` #171

Closed f1sherman closed 5 years ago

f1sherman commented 5 years ago

I noticed as of version 1.2.8 that if throw :abort is used from within a strategy, winning_strategy is no longer set. This appears to be due to the change in #144.

Should this new behavior be considered expected or a bug?

jsmestad commented 5 years ago

@f1sherman mind spiking on a spec to test this against? It appears we overlooked the spec in #144 and we should get a spec together before patching it.

f1sherman commented 5 years ago

@jsmestad sure thing! How does this look? https://github.com/wardencommunity/warden/compare/master...f1sherman:add-spec-for-winning-strategy-not-set-on-throw?expand=1

$ bundle exec rspec spec/warden/proxy_spec.rb:646
Run options: include {:locations=>{"./spec/warden/proxy_spec.rb"=>[646]}}

Warden::Proxy
  messages
    should allow access to the failure message (FAILED - 1)

Failures:

  1) Warden::Proxy messages should allow access to the failure message
     Failure/Error: expect(result.last).to eq(["The Fails Strategy Has Failed You"])

       expected: ["The Fails Strategy Has Failed You"]
            got: [nil]

       (compared using ==)

       Diff:
       @@ -1,2 +1,2 @@
       -["The Fails Strategy Has Failed You"]
       +[nil]

     # ./spec/warden/proxy_spec.rb:654:in `block (3 levels) in <top (required)>'

Finished in 0.1097 seconds (files took 0.23948 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/warden/proxy_spec.rb:646 # Warden::Proxy messages should allow access to the failure message
landongrindheim commented 5 years ago

This issue can be closed now that #175 has been merged