Merge pull request #2563 from ericproulx/revisit_auth_middleware

Update Grape::Middleware::Auth::Base

Author: dblock

CHANGELOG.md

@@ -14,6 +14,7 @@
 * [#2558](https://github.com/ruby-grape/grape/pull/2558): Add Ruby's option `enable_frozen_string_literal` in CI - [@ericproulx](https://github.com/ericproulx).
 * [#2557](https://github.com/ruby-grape/grape/pull/2557): Add `lint!` - [@ericproulx](https://github.com/ericproulx).
 * [#2561](https://github.com/ruby-grape/grape/pull/2561): Optimize hash alloc for middleware's default options - [@ericproulx](https://github.com/ericproulx).
+* [#2563](https://github.com/ruby-grape/grape/pull/2563): Update `Grape::Middleware::Auth::Base` - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

UPGRADING.md

@@ -3,6 +3,9 @@ Upgrading Grape
 
 ### Upgrading to >= 2.4.0
 
+#### Grape::Middleware::Auth::Base
+`type` is now validated at compile time and will raise a `Grape::Exceptions::UnknownAuthStrategy` if unknown.
+
 #### Grape::Middleware::Base
 
 - Second argument `options` is now a double splat (**) instead of single splat (*). If you're redefining `initialize` in your middleware and/or calling `super` in it, you might have to adapt the signature and the `super` call. Also, you might have to remove `{}` if you're pass `options` as a literal `Hash` or add `**` if you're using a variable.
@@ -18,7 +21,7 @@ Here are the notable changes:
 - `HTTP_HEADERS` has been moved to `Grape::Request` and renamed `KNOWN_HEADERS`. The last has been refreshed with new headers, and it's not lazy anymore.
 - `SUPPORTED_METHODS_WITHOUT_OPTIONS` and `find_supported_method` have been removed.
 
-### Grape::Middleware::Base
+#### Grape::Middleware::Base
 
 - Constant `TEXT_HTML` has been removed in favor of using literal string 'text/html'.
 - `rack_request` and `query_params` have been added. Feel free to call these in your middlewares.
@@ -34,7 +37,7 @@ Here are the notable changes:
 
 Your API might act differently since it will strictly follow the [RFC2616 14.1](https://datatracker.ietf.org/doc/html/rfc2616#section-14.1) when interpreting the `Accept` header. Here are the differences:
 
-###### Invalid or missing quality ranking
+##### Invalid or missing quality ranking
 The following used to yield `application/xml` and now will yield `application/json` as the preferred media type:
 - `application/json;q=invalid,application/xml;q=0.5`
 - `application/json,application/xml;q=1.0`
@@ -43,7 +46,7 @@ For the invalid case, the value `invalid` was automatically `to_f` and `invalid.
 
 For the non provided case, 1.0 was automatically assigned and in a case of multiple best matches, the first was returned based on Ruby's sort_by `quality`. Now, 1.0 is still assigned and the last is returned in case of multiple best matches. See [Rack's implementation](https://github.com/rack/rack/blob/e8f47608668d507e0f231a932fa37c9ca551c0a5/lib/rack/utils.rb#L167) of the RFC.
 
-###### Considering the closest generic when vendor tree
+##### Considering the closest generic when vendor tree
 Excluding the [header versioning strategy](https://github.com/ruby-grape/grape?tab=readme-ov-file#header), whenever a media type with the [vendor tree](https://datatracker.ietf.org/doc/html/rfc6838#section-3.2) leading facet `vnd.` like `application/vnd.api+json` was provided, Grape would also consider its closest generic when negotiating. In that case, `application/json` was added to the negotiation. Now, it will just consider the provided media types without considering any closest generics, and you'll need to [register](https://github.com/ruby-grape/grape?tab=readme-ov-file#api-formats) it.
 You can find the official vendor tree registrations on [IANA](https://www.iana.org/assignments/media-types/media-types.xhtml)
 

lib/grape/api/instance.rb

@@ -5,6 +5,7 @@ class API
     # The API Instance class, is the engine behind Grape::API. Each class that inherits
     # from this will represent a different API instance
     class Instance
+      extend Grape::Middleware::Auth::DSL
       include Grape::DSL::API
 
       class << self

lib/grape/dsl/api.rb

@@ -5,8 +5,6 @@ module DSL
     module API
       extend ActiveSupport::Concern
 
-      include Grape::Middleware::Auth::DSL
-
       include Grape::DSL::Validations
       include Grape::DSL::Callbacks
       include Grape::DSL::Configuration

lib/grape/exceptions/unknown_auth_strategy.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Grape
+  module Exceptions
+    class UnknownAuthStrategy < Base
+      def initialize(strategy:)
+        super(message: compose_message(:unknown_auth_strategy, strategy: strategy))
+      end
+    end
+  end
+end

lib/grape/locale/en.yml

@@ -1,63 +1,59 @@
+---
 en:
   grape:
     errors:
-      format: ! '%{attributes} %{message}'
+      format: '%{attributes} %{message}'
       messages:
-        coerce: 'is invalid'
-        presence: 'is missing'
-        regexp: 'is invalid'
+        all_or_none: 'provide all or none of parameters'
+        at_least_one: 'are missing, at least one parameter must be provided'
         blank: 'is empty'
-        values: 'does not have a valid value'
+        coerce: 'is invalid'
+        conflicting_types: 'query params contains conflicting types'
+        empty_message_body: 'empty message body supplied with %{body_format} content-type'
+        exactly_one: 'are missing, exactly one parameter must be provided'
         except_values: 'has a value not allowed'
-        same_as: 'is not the same as %{parameter}'
+        incompatible_option_values: '%{option1}: %{value1} is incompatible with %{option2}: %{value2}'
+        invalid_accept_header:
+          problem: 'invalid accept header'
+          resolution: '%{message}'
+        invalid_formatter: 'cannot convert %{klass} to %{to_format}'
+        invalid_message_body:
+          problem: 'message body does not match declared format'
+          resolution: 'when specifying %{body_format} as content-type, you must pass valid %{body_format} in the request''s ''body'' '
+        invalid_parameters: 'query params contains invalid format or byte sequence'
+        invalid_response: 'Invalid response'
+        invalid_version_header:
+          problem: 'invalid version header'
+          resolution: '%{message}'
+        invalid_versioner_option:
+          problem: 'unknown :using for versioner: %{strategy}'
+          resolution: 'available strategy for :using is :path, :header, :accept_version_header, :param'
+        invalid_with_option_for_represent:
+          problem: 'you must specify an entity class in the :with option'
+          resolution: 'eg: represent User, :with => Entity::User'
         length: 'is expected to have length within %{min} and %{max}'
         length_is: 'is expected to have length exactly equal to %{is}'
-        length_min: 'is expected to have length greater than or equal to %{min}'
         length_max: 'is expected to have length less than or equal to %{max}'
-        missing_vendor_option:
-          problem: 'missing :vendor option'
-          summary: 'when version using header, you must specify :vendor option'
-          resolution: "eg: version 'v1', using: :header, vendor: 'twitter'"
+        length_min: 'is expected to have length greater than or equal to %{min}'
+        missing_group_type: 'group type is required'
         missing_mime_type:
           problem: 'missing mime type for %{new_format}'
-          resolution:
-            "you can choose existing mime type from Grape::ContentTypes::CONTENT_TYPES
-            or add your own with content_type :%{new_format}, 'application/%{new_format}'
-            "
-        invalid_with_option_for_represent:
-          problem: 'you must specify an entity class in the :with option'
-          resolution: 'eg: represent User, :with => Entity::User'
+          resolution: 'you can choose existing mime type from Grape::ContentTypes::CONTENT_TYPES or add your own with content_type :%{new_format}, ''application/%{new_format}'' '
         missing_option: 'you must specify :%{option} options'
-        invalid_formatter: 'cannot convert %{klass} to %{to_format}'
-        invalid_versioner_option:
-          problem: 'unknown :using for versioner: %{strategy}'
-          resolution: 'available strategy for :using is :path, :header, :accept_version_header, :param'
-        unknown_validator: 'unknown validator: %{validator_type}'
-        unknown_params_builder: 'unknown params_builder: %{params_builder_type}'
+        missing_vendor_option:
+          problem: 'missing :vendor option'
+          resolution: 'eg: version ''v1'', using: :header, vendor: ''twitter'''
+          summary: 'when version using header, you must specify :vendor option'
+        mutual_exclusion: 'are mutually exclusive'
+        presence: 'is missing'
+        regexp: 'is invalid'
+        same_as: 'is not the same as %{parameter}'
+        too_deep_parameters: 'query params are recursively nested over the specified limit (%{limit})'
+        too_many_multipart_files: 'the number of uploaded files exceeded the system''s configured limit (%{limit})'
+        unknown_auth_strategy: 'unknown auth strategy: %{strategy}'
         unknown_options: 'unknown options: %{options}'
         unknown_parameter: 'unknown parameter: %{param}'
-        incompatible_option_values: '%{option1}: %{value1} is incompatible with %{option2}: %{value2}'
-        mutual_exclusion: 'are mutually exclusive'
-        at_least_one: 'are missing, at least one parameter must be provided'
-        exactly_one: 'are missing, exactly one parameter must be provided'
-        all_or_none: 'provide all or none of parameters'
-        missing_group_type: 'group type is required'
+        unknown_params_builder: 'unknown params_builder: %{params_builder_type}'
+        unknown_validator: 'unknown validator: %{validator_type}'
         unsupported_group_type: 'group type must be Array, Hash, JSON or Array[JSON]'
-        invalid_message_body:
-          problem: "message body does not match declared format"
-          resolution:
-            "when specifying %{body_format} as content-type, you must pass valid
-            %{body_format} in the request's 'body'
-            "
-        empty_message_body: 'empty message body supplied with %{body_format} content-type'
-        too_many_multipart_files: "the number of uploaded files exceeded the system's configured limit (%{limit})"
-        invalid_accept_header:
-          problem: 'invalid accept header'
-          resolution: '%{message}'
-        invalid_version_header:
-          problem: 'invalid version header'
-          resolution: '%{message}'
-        invalid_response: 'Invalid response'
-        conflicting_types: 'query params contains conflicting types'
-        invalid_parameters: 'query params contains invalid format or byte sequence'
-        too_deep_parameters: 'query params are recursively nested over the specified limit (%{limit})'
+        values: 'does not have a valid value'

lib/grape/middleware/auth/base.rb

@@ -3,27 +3,18 @@
 module Grape
   module Middleware
     module Auth
-      class Base
-        attr_accessor :options, :app, :env
-
+      class Base < Grape::Middleware::Base
         def initialize(app, **options)
-          @app = app
-          @options = options
-        end
-
-        def call(env)
-          dup._call(env)
+          super
+          @auth_strategy = Grape::Middleware::Auth::Strategies[options[:type]].tap do |auth_strategy|
+            raise Grape::Exceptions::UnknownAuthStrategy.new(strategy: options[:type]) unless auth_strategy
+          end
         end
 
-        def _call(env)
-          self.env = env
-          return app.call(env) unless options.key?(:type)
-
-          strategy_info = Grape::Middleware::Auth::Strategies[options[:type]]
-          throw :error, status: 401, message: 'API Authorization Failed.' if strategy_info.blank?
-
-          strategy_info.create(@app, options) do |*args|
-            env[Grape::Env::API_ENDPOINT].instance_exec(*args, &options[:proc])
+        def call!(env)
+          @env = env
+          @auth_strategy.create(app, options) do |*args|
+            context.instance_exec(*args, &options[:proc])
           end.call(env)
         end
       end

lib/grape/middleware/auth/dsl.rb

@@ -4,40 +4,34 @@ module Grape
   module Middleware
     module Auth
       module DSL
-        extend ActiveSupport::Concern
-
-        module ClassMethods
-          # Add an authentication type to the API. Currently
-          # only `:http_basic`, `:http_digest` are supported.
-          def auth(type = nil, options = {}, &block)
-            if type
-              namespace_inheritable(:auth, options.reverse_merge(type: type.to_sym, proc: block))
-              use Grape::Middleware::Auth::Base, namespace_inheritable(:auth)
-            else
-              namespace_inheritable(:auth)
-            end
-          end
-
-          # Add HTTP Basic authorization to the API.
-          #
-          # @param [Hash] options A hash of options.
-          # @option options [String] :realm "API Authorization" The HTTP Basic realm.
-          def http_basic(options = {}, &block)
-            options[:realm] ||= 'API Authorization'
-            auth :http_basic, options, &block
+        def auth(type = nil, options = {}, &block)
+          if type
+            namespace_inheritable(:auth, options.reverse_merge(type: type.to_sym, proc: block))
+            use Grape::Middleware::Auth::Base, namespace_inheritable(:auth)
+          else
+            namespace_inheritable(:auth)
           end
+        end
 
-          def http_digest(options = {}, &block)
-            options[:realm] ||= 'API Authorization'
+        # Add HTTP Basic authorization to the API.
+        #
+        # @param [Hash] options A hash of options.
+        # @option options [String] :realm "API Authorization" The HTTP Basic realm.
+        def http_basic(options = {}, &block)
+          options[:realm] ||= 'API Authorization'
+          auth :http_basic, options, &block
+        end
 
-            if options[:realm].respond_to?(:values_at)
-              options[:realm][:opaque] ||= 'secret'
-            else
-              options[:opaque] ||= 'secret'
-            end
+        def http_digest(options = {}, &block)
+          options[:realm] ||= 'API Authorization'
 
-            auth :http_digest, options, &block
+          if options[:realm].respond_to?(:values_at)
+            options[:realm][:opaque] ||= 'secret'
+          else
+            options[:opaque] ||= 'secret'
           end
+
+          auth :http_digest, options, &block
         end
       end
     end

spec/grape/middleware/auth/strategies_spec.rb

@@ -27,4 +27,19 @@
       expect(last_response).to be_unauthorized
     end
   end
+
+  describe 'Unknown Auth' do
+    context 'when type is not register' do
+      let(:app) do
+        Class.new(Grape::API) do
+          use Grape::Middleware::Auth::Base, type: :unknown
+          get('/whatever') { 'Hello there.' }
+        end
+      end
+
+      it 'throws a 401' do
+        expect { get '/whatever' }.to raise_error(Grape::Exceptions::UnknownAuthStrategy, 'unknown auth strategy: unknown')
+      end
+    end
+  end
 end