diff --git a/CHANGELOG.md b/CHANGELOG.md index b6998446bd..1448c755c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +* [#2196](https://github.com/ruby-grape/grape/pull/2196): Add support for `passwords_hashed` param for `digest_auth` - [@lHydra](https://github.com/lhydra). * Your contribution here. #### Fixes diff --git a/README.md b/README.md index a2b7f31363..4716597438 100644 --- a/README.md +++ b/README.md @@ -3297,12 +3297,20 @@ http_basic do |username, password| end ``` +Digest auth supports clear-text passwords and password hashes. + ```ruby http_digest({ realm: 'Test Api', opaque: 'app secret' }) do |username| # lookup the user's password here end ``` +```ruby +http_digest(realm: { realm: 'Test Api', opaque: 'app secret', passwords_hashed: true }) do |username| + # lookup the user's password hash here +end +``` + ### Register custom middleware for authentication Grape can use custom Middleware for authentication. How to implement these diff --git a/lib/grape/middleware/auth/dsl.rb b/lib/grape/middleware/auth/dsl.rb index 1b2e8f4568..d85171fc59 100644 --- a/lib/grape/middleware/auth/dsl.rb +++ b/lib/grape/middleware/auth/dsl.rb @@ -32,7 +32,13 @@ def http_basic(options = {}, &block) def http_digest(options = {}, &block) options[:realm] ||= 'API Authorization' - options[:opaque] ||= 'secret' + + if options[:realm].respond_to?(:values_at) + options[:realm][:opaque] ||= 'secret' + else + options[:opaque] ||= 'secret' + end + auth :http_digest, options, &block end end diff --git a/spec/grape/middleware/auth/dsl_spec.rb b/spec/grape/middleware/auth/dsl_spec.rb index 41169a3bf6..5e694d08eb 100644 --- a/spec/grape/middleware/auth/dsl_spec.rb +++ b/spec/grape/middleware/auth/dsl_spec.rb @@ -16,7 +16,7 @@ end describe '.auth' do - it 'stets auth parameters' do + it 'sets auth parameters' do expect(subject.base_instance).to receive(:use).with(Grape::Middleware::Auth::Base, settings) subject.auth :http_digest, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] @@ -38,16 +38,25 @@ end describe '.http_basic' do - it 'stets auth parameters' do + it 'sets auth parameters' do subject.http_basic realm: 'my_realm', &settings[:proc] expect(subject.auth).to eq(realm: 'my_realm', type: :http_basic, proc: block) end end describe '.http_digest' do - it 'stets auth parameters' do - subject.http_digest realm: 'my_realm', opaque: 'my_opaque', &settings[:proc] - expect(subject.auth).to eq(realm: 'my_realm', type: :http_digest, proc: block, opaque: 'my_opaque') + context 'when realm is a hash' do + it 'sets auth parameters' do + subject.http_digest realm: { realm: 'my_realm', opaque: 'my_opaque' }, &settings[:proc] + expect(subject.auth).to eq(realm: { realm: 'my_realm', opaque: 'my_opaque' }, type: :http_digest, proc: block) + end + end + + context 'when realm is not hash' do + it 'sets auth parameters' do + subject.http_digest realm: 'my_realm', opaque: 'my_opaque', &settings[:proc] + expect(subject.auth).to eq(realm: 'my_realm', type: :http_digest, proc: block, opaque: 'my_opaque') + end end end end diff --git a/spec/grape/middleware/auth/strategies_spec.rb b/spec/grape/middleware/auth/strategies_spec.rb index 954c996315..fcbe9e7bda 100644 --- a/spec/grape/middleware/auth/strategies_spec.rb +++ b/spec/grape/middleware/auth/strategies_spec.rb @@ -42,7 +42,17 @@ def app end module StrategiesSpec - class Test < Grape::API + class PasswordHashed < Grape::API + http_digest(realm: { realm: 'Test Api', opaque: 'secret', passwords_hashed: true }) do |username| + { 'foo' => Digest::MD5.hexdigest(['foo', 'Test Api', 'bar'].join(':')) }[username] + end + + get '/test' do + [{ hey: 'you' }, { there: 'bar' }, { foo: 'baz' }] + end + end + + class PasswordIsNotHashed < Grape::API http_digest(realm: 'Test Api', opaque: 'secret') do |username| { 'foo' => 'bar' }[username] end @@ -53,30 +63,60 @@ class Test < Grape::API end end - def app - StrategiesSpec::Test - end + context 'when password is hashed' do + def app + StrategiesSpec::PasswordHashed + end - it 'is a digest authentication challenge' do - get '/test' - expect(last_response).to be_challenge - end + it 'is a digest authentication challenge' do + get '/test' + expect(last_response).to be_challenge + end - it 'throws a 401 if no auth is given' do - get '/test' - expect(last_response.status).to eq(401) - end + it 'throws a 401 if no auth is given' do + get '/test' + expect(last_response.status).to eq(401) + end - it 'authenticates if given valid creds' do - digest_authorize 'foo', 'bar' - get '/test' - expect(last_response.status).to eq(200) + it 'authenticates if given valid creds' do + digest_authorize 'foo', 'bar' + get '/test' + expect(last_response.status).to eq(200) + end + + it 'throws a 401 if given invalid creds' do + digest_authorize 'bar', 'foo' + get '/test' + expect(last_response.status).to eq(401) + end end - it 'throws a 401 if given invalid creds' do - digest_authorize 'bar', 'foo' - get '/test' - expect(last_response.status).to eq(401) + context 'when password is not hashed' do + def app + StrategiesSpec::PasswordIsNotHashed + end + + it 'is a digest authentication challenge' do + get '/test' + expect(last_response).to be_challenge + end + + it 'throws a 401 if no auth is given' do + get '/test' + expect(last_response.status).to eq(401) + end + + it 'authenticates if given valid creds' do + digest_authorize 'foo', 'bar' + get '/test' + expect(last_response.status).to eq(200) + end + + it 'throws a 401 if given invalid creds' do + digest_authorize 'bar', 'foo' + get '/test' + expect(last_response.status).to eq(401) + end end end end