Permalink
Browse files

Fix: Fixes digest authentication with Cookies (#235)

Fix: Fixes digest authentication with Cookies

Bug description
-----

In issue #234 a bug regarding digest authentication has been reported.
In order to reproduce it there are two steps necessary:
1. Make sure the app responds with 401 and a digest challange
2. Make sure a Set-Cookie header is set.
If these two conditions are fulfilled, the reported error will
occur:

```
NoMethodError:
  undefined method `host' for "/digest":String
```
The stack trace already indicates that the issue is taking effect in
the cookie jar class.

```
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test/cookie_jar.rb:25:in `initialize'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test/cookie_jar.rb:149:in `new'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test/cookie_jar.rb:149:in `block in merge'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test/cookie_jar.rb:148:in `each'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test/cookie_jar.rb:148:in `merge'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/mock_session.rb:36:in `request'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test.rb:261:in `process_request'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test.rb:267:in `process_request'
 .../ruby-2.5.1/gems/rack-test-1.0.0/lib/rack/test.rb:119:in `request'
 ./spec/requests/api_spec.rb:16:in `block (3 levels) in <top (required)>'
```

Problem (root cause) description
-----
When a cookie is set and digest authentication is requested, we
invalidly pass `uri.path` to `process_request` instead of just `uri`.

Detailled explanation
----
After some research I came to the following result.
When a digest request is issued the following if-condition is executed:
* https://github.com/rack-test/rack-test/blob/v1.0.0/lib/rack/test.rb#L261
There `process_request` calls itself with `uri.path` instead of `uri`.

Next `@rack_mock_session.request` is called, which in turn calls
[CookieJar#merge](https://github.com/rack-test/rack-test/blob/v1.0.0/lib/rack/mock_session.rb#L34)
with `last_response.headers['Set-Cookie']`.
Merge has a early return if nil is passed as the argument [see code](https://github.com/rack-test/rack-test/blob/v1.0.0/lib/rack/test/cookie_jar.rb#L139).
However if Set-Cookie is provided (a cookie is sent), then the cookie
is split and cookie objects are created per line. When this happends
the uri is passed (which in case of the .path invocation is just a string),
and then the initializer calls uri.host [here](https://github.com/rack-test/rack-test/blob/v1.0.0/lib/rack/test/cookie_jar.rb#L23).
This will then trigger the exception above.

This description also makes clear that this bugs only occurs, when the
user is testing auth-digest and also setting a header during the
authentication. However according to the sample rails app provided,
this might be a normal case for rails apps.
(depending for what they use cookies)
  • Loading branch information...
scepticulous committed Sep 24, 2018
1 parent 7a94af8 commit 6c07bf53adf4a4082e206561f5b9b43142fdc821
Showing with 68 additions and 1 deletion.
  1. +1 −1 lib/rack/test.rb
  2. +67 −0 spec/rack/test_spec.rb
@@ -269,7 +269,7 @@ def process_request(uri, env)
auth_env = env.merge('HTTP_AUTHORIZATION' => digest_auth_header,
'rack-test.digest_auth_retry' => true)
auth_env.delete('rack.request')
process_request(uri.path, auth_env)
process_request(uri, auth_env)
else
yield last_response if block_given?
@@ -356,6 +356,73 @@ def close
end
end
describe '#digest_authorize' do
let(:challenge_data) do
'realm="test-realm", qop="auth", nonce="nonsensenonce", opaque="morenonsense"'
end
let(:digest_app) do
basic_headers = { 'Content-Type' => 'text/html', 'Content-Length' => '13' }
digest_challenge = "Digest #{challenge_data}"
auth_challenge_headers = { 'WWW-Authenticate' => digest_challenge }
cookie_headers = { 'Set-Cookie' => 'digest_auth_session=OZEnmjeekUSW%3D%3D; path=/; HttpOnly' }
lambda do |_env|
[401, basic_headers.merge(auth_challenge_headers).merge(cookie_headers), '']
end
end
let(:digest_session) do
session = Rack::Test::Session.new(Rack::MockSession.new(digest_app))
session.digest_authorize('test-name', 'test-password')
session
end
it 'retries digest requests' do
session = digest_session
session.request('/')
expect(session.last_request.env['rack-test.digest_auth_retry']).to be_truthy
end
it 'sends a digest auth header' do
session = digest_session
session.request('/')
auth_headers = session.last_request.env['HTTP_AUTHORIZATION']
expect(auth_headers).to include('Digest realm')
end
it 'includes the response based on the username,password and nonce' do
session = digest_session
session.request('/')
auth_headers = session.last_request.env['HTTP_AUTHORIZATION']
expect(auth_headers).to include('response="d773034bdc162b31c50c62764016bd31"')
end
it 'includes the challenge headers' do
session = digest_session
session.request('/')
auth_headers = session.last_request.env['HTTP_AUTHORIZATION']
expect(auth_headers).to include(challenge_data)
end
it 'includes the username' do
session = digest_session
session.request('/')
auth_headers = session.last_request.env['HTTP_AUTHORIZATION']
expect(auth_headers).to include('username="test-name"')
end
end
describe 'follow_redirect!' do
it 'follows redirects' do
get '/redirect'

0 comments on commit 6c07bf5

Please sign in to comment.