diff --git a/CHANGELOG.md b/CHANGELOG.md index 0049465b9..f7b0feb27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ * [#2837](https://github.com/ruby-grape/grape/pull/2837): Document the definition-time rejection of an `Array`/`Set` of an uncoercible element type introduced by #2817, and pin it with specs - [@ericproulx](https://github.com/ericproulx). * [#2836](https://github.com/ruby-grape/grape/pull/2836): Define `#hash` alongside the `eql?`/`==` pairs on `Grape::Endpoint`, `Grape::Util::InheritableSetting`, `Grape::Middleware::Stack::Middleware`, `Grape::ServeStream::StreamResponse` and `Grape::ServeStream::FileBody`, so equal objects hash alike in a `Hash`, `Set` or `uniq` - [@ericproulx](https://github.com/ericproulx). * [#2835](https://github.com/ruby-grape/grape/pull/2835): Return the compiled instance from `Grape::API::Instance.compile!` so `call` and `recognize_path` no longer re-read `@instance`, which a concurrent `change!` could nil between the two reads - [@ericproulx](https://github.com/ericproulx). +* [#2849](https://github.com/ruby-grape/grape/pull/2849): Remove `http_digest`, which has had no strategy behind it since 2.0.0 and raised on the first request rather than when the API was defined (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/UPGRADING.md b/UPGRADING.md index ab4838229..4b106185f 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -304,16 +304,16 @@ rescue_from MyError, with: :other_handler Calls that only use one meta selector or only use exception classes (the documented forms) are unaffected. -#### `auth`, `http_basic` and `http_digest` now take keyword arguments +#### `auth` and `http_basic` now take keyword arguments -`Grape::Middleware::Auth::DSL#auth`, `#http_basic` and `#http_digest` now accept their options as keyword arguments instead of a positional `Hash`. Calls using bare keyword syntax or a block are unaffected: +`Grape::Middleware::Auth::DSL#auth` and `#http_basic` now accept their options as keyword arguments instead of a positional `Hash`. Calls using bare keyword syntax or a block are unaffected: ```ruby http_basic realm: 'API' do |u, p| # ... end -auth :http_digest, realm: 'API', opaque: 'secret', &proc +auth :my_strategy, realm: 'API', &proc ``` Passing a positional options `Hash` still works but is deprecated and will be removed in a future release: @@ -321,13 +321,33 @@ Passing a positional options `Hash` still works but is deprecated and will be re ```ruby # deprecated http_basic({ realm: 'API' }) -auth :http_digest, { realm: 'API', opaque: 'secret' } +auth :my_strategy, { realm: 'API' } # preferred http_basic(realm: 'API') -auth :http_digest, realm: 'API', opaque: 'secret' +auth :my_strategy, realm: 'API' ``` +#### `http_digest` is removed + +`Grape::Middleware::Auth::DSL#http_digest` is gone. Calling it now raises `NoMethodError` while the API class is being defined. + +Nothing it could reach has existed since **2.0.0**, which removed `Rack::Auth::Digest` along with Grape's `:http_digest` strategy ([#2361](https://github.com/ruby-grape/grape/pull/2361)) after Rack 3 dropped digest authentication. The method survived that removal and kept recording its settings happily, so an API declaring `http_digest` still booted — and then raised `Grape::Exceptions::UnknownAuthStrategy` on the *first request*, from inside the middleware build, as an uncaught exception rather than a response. Failing while the class is defined is the point of removing it. + +**If you registered your own `:http_digest` strategy**, it still works; call `auth` directly: + +```ruby +Grape::Middleware::Auth::Strategies.add(:http_digest, MyDigestStrategy, ->(settings) { [settings[:realm]] }) + +class API < Grape::API + auth :http_digest, realm: 'API Authorization', opaque: 'secret' do |username| + # ... + end +end +``` + +The removed method supplied two defaults that `auth` does not, so pass them explicitly if you were relying on them: `realm` defaulted to `'API Authorization'`, and `opaque` to `'secret'` (nested inside `realm` when `realm` was itself a Hash). + #### Middleware options now route through per-class `Options` `Data` value objects `Grape::Middleware::Error`, `Grape::Middleware::Formatter`, and `Grape::Middleware::Versioner::Base` each declare an `Options` `Data.define` and route their `**options` kwargs through it on `initialize`. This means **unknown kwargs now raise `ArgumentError`** instead of being silently swallowed: diff --git a/lib/grape/middleware/auth/dsl.rb b/lib/grape/middleware/auth/dsl.rb index 52829cd22..5c62673c0 100644 --- a/lib/grape/middleware/auth/dsl.rb +++ b/lib/grape/middleware/auth/dsl.rb @@ -22,19 +22,6 @@ def http_basic(*legacy_options, **options, &) auth(:http_basic, **options, &) end - def http_digest(*legacy_options, **options, &) - options = merge_legacy_auth_options(:http_digest, legacy_options, options) - options[:realm] ||= 'API Authorization' - - if options[:realm].respond_to?(:values_at) - options[:realm][:opaque] ||= 'secret' - else - options[:opaque] ||= 'secret' - end - - auth(:http_digest, **options, &) - end - private # @deprecated Passing a positional options Hash is deprecated; pass diff --git a/spec/grape/middleware/auth/dsl_spec.rb b/spec/grape/middleware/auth/dsl_spec.rb index 612c511c8..29d9e2775 100644 --- a/spec/grape/middleware/auth/dsl_spec.rb +++ b/spec/grape/middleware/auth/dsl_spec.rb @@ -4,12 +4,15 @@ subject { Class.new(Grape::API) } let(:block) { -> {} } + # #auth records whatever it is given; the strategy behind the label is looked + # up when the middleware is built. A label with no built-in sugar method keeps + # that distinction visible. let(:settings) do { opaque: 'secret', proc: block, realm: 'API Authorization', - type: :http_digest + type: :custom } end @@ -17,7 +20,7 @@ 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] + subject.auth :custom, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] expect(subject.auth).to eq(settings) end @@ -25,10 +28,10 @@ expect(subject.base_instance).to receive(:use).with(Grape::Middleware::Auth::Base, settings) expect(subject.base_instance).to receive(:use).with(Grape::Middleware::Auth::Base, settings.merge(realm: 'super_secret')) - subject.auth :http_digest, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] + subject.auth :custom, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] first_settings = subject.auth - subject.auth :http_digest, realm: 'super_secret', opaque: settings[:opaque], &settings[:proc] + subject.auth :custom, realm: 'super_secret', opaque: settings[:opaque], &settings[:proc] expect(subject.auth).to eq(settings.merge(realm: 'super_secret')) expect(subject.auth.object_id).not_to eq(first_settings.object_id) @@ -42,29 +45,13 @@ end end - describe '.http_digest' do - 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 - describe 'deprecated positional options Hash' do it 'deprecates a positional Hash for `auth` but still works when silenced' do - expect { subject.auth :http_digest, { realm: 'r', opaque: 'o' }, &block } + expect { subject.auth :custom, { realm: 'r', opaque: 'o' }, &block } .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `auth`/) - Grape.deprecator.silence { subject.auth :http_digest, { realm: 'r', opaque: 'o' }, &block } - expect(subject.auth).to eq(realm: 'r', opaque: 'o', type: :http_digest, proc: block) + Grape.deprecator.silence { subject.auth :custom, { realm: 'r', opaque: 'o' }, &block } + expect(subject.auth).to eq(realm: 'r', opaque: 'o', type: :custom, proc: block) end it 'deprecates a positional Hash for `http_basic` but still works when silenced' do @@ -74,13 +61,5 @@ Grape.deprecator.silence { subject.http_basic({ realm: 'my_realm' }, &block) } expect(subject.auth).to eq(realm: 'my_realm', type: :http_basic, proc: block) end - - it 'deprecates a positional Hash for `http_digest` but still works when silenced' do - expect { subject.http_digest({ realm: 'my_realm' }, &block) } - .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `http_digest`/) - - Grape.deprecator.silence { subject.http_digest({ realm: 'my_realm' }, &block) } - expect(subject.auth).to eq(realm: 'my_realm', opaque: 'secret', type: :http_digest, proc: block) - end end end