PKCE support for rails 3.2 (#1)

* PKCE support for rails 3.2

* Add Gemfile for Ruby 2.7 and Rails 3.2

* fix code challenge and code challenge method not getting saved issue and update specs

* find client using client_id if code verifier is present

* fix failing test case

* Add CircleCI

Co-authored-by: Ian Lesperance <[email protected]>

app/views/doorkeeper/authorizations/new.html.erb

@@ -26,6 +26,8 @@
       <%= hidden_field_tag :state, @pre_auth.state %>
       <%= hidden_field_tag :response_type, @pre_auth.response_type %>
       <%= hidden_field_tag :scope, @pre_auth.scope %>
+      <%= hidden_field_tag :code_challenge, @pre_auth.code_challenge %>
+      <%= hidden_field_tag :code_challenge_method, @pre_auth.code_challenge_method %>
       <%= submit_tag t('doorkeeper.authorizations.buttons.authorize'), class: "btn btn-success btn-lg btn-block" %>
     <% end %>
     <%= form_tag oauth_authorization_path, method: :delete do %>
@@ -34,6 +36,8 @@
       <%= hidden_field_tag :state, @pre_auth.state %>
       <%= hidden_field_tag :response_type, @pre_auth.response_type %>
       <%= hidden_field_tag :scope, @pre_auth.scope %>
+      <%= hidden_field_tag :code_challenge, @pre_auth.code_challenge %>
+      <%= hidden_field_tag :code_challenge_method, @pre_auth.code_challenge_method %>
       <%= submit_tag t('doorkeeper.authorizations.buttons.deny'), class: "btn btn-danger btn-lg btn-block" %>
     <% end %>
   </div>

config/locales/en.yml

@@ -79,6 +79,7 @@ en:
         unauthorized_client: 'The client is not authorized to perform this request using this method.'
         access_denied: 'The resource owner or authorization server denied the request.'
         invalid_scope: 'The requested scope is invalid, unknown, or malformed.'
+        invalid_code_challenge_method: 'The code challenge method must be plain or S256.'
         server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.'
         temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.'
 

doorkeeper.gemspec

@@ -1,27 +1,28 @@
-$:.push File.expand_path("../lib", __FILE__)
+$:.push File.expand_path('lib', __dir__)
 
-require "doorkeeper/version"
+require 'doorkeeper/version'
 
 Gem::Specification.new do |s|
-  s.name        = "doorkeeper"
+  s.name        = 'doorkeeper'
   s.version     = Doorkeeper::VERSION
-  s.authors     = ["Felipe Elias Philipp", "Tute Costa"]
-  s.email       = %w([email protected])
-  s.homepage    = "https://github.com/doorkeeper-gem/doorkeeper"
-  s.summary     = "OAuth 2 provider for Rails and Grape"
-  s.description = "Doorkeeper is an OAuth 2 provider for Rails and Grape."
+  s.authors     = ['Felipe Elias Philipp', 'Tute Costa']
+  s.email       = %w[[email protected]]
+  s.homepage    = 'https://github.com/doorkeeper-gem/doorkeeper'
+  s.summary     = 'OAuth 2 provider for Rails and Grape'
+  s.description = 'Doorkeeper is an OAuth 2 provider for Rails and Grape.'
   s.license     = 'MIT'
 
   s.files         = `git ls-files`.split("\n")
   s.test_files    = `git ls-files -- spec/*`.split("\n")
-  s.require_paths = ["lib"]
+  s.require_paths = ['lib']
 
-  s.add_dependency "railties", ">= 3.2"
+  s.add_dependency 'railties', '>= 3.2'
 
-  s.add_development_dependency "rspec-rails", "~> 3.4.0"
-  s.add_development_dependency "capybara", "~> 2.3.0"
-  s.add_development_dependency "generator_spec", "~> 0.9.0"
-  s.add_development_dependency "factory_girl", "~> 4.5.0"
-  s.add_development_dependency "timecop", "~> 0.7.0"
-  s.add_development_dependency "database_cleaner", "~> 1.3.0"
+  s.add_development_dependency 'rspec-rails', '~> 3.4.0'
+  s.add_development_dependency 'capybara', '~> 2.3.0'
+  s.add_development_dependency 'generator_spec', '~> 0.9.0'
+  s.add_development_dependency 'factory_girl', '~> 4.5.0'
+  s.add_development_dependency 'timecop', '~> 0.7.0'
+  s.add_development_dependency 'database_cleaner', '~> 1.3.0'
+  s.add_development_dependency 'bigdecimal', '1.3.5'
 end

lib/doorkeeper/models/access_grant_mixin.rb

@@ -13,7 +13,8 @@ module AccessGrantMixin
       belongs_to :application, class_name: 'Doorkeeper::Application', inverse_of: :access_grants
 
       if respond_to?(:attr_accessible)
-        attr_accessible :resource_owner_id, :application_id, :expires_in, :redirect_uri, :scopes
+        attr_accessible :resource_owner_id, :application_id, :expires_in, :redirect_uri, :scopes,
+                        :code_challenge, :code_challenge_method
       end
 
       validates :resource_owner_id, :application_id, :token, :expires_in, :redirect_uri, presence: true
@@ -22,10 +23,66 @@ module AccessGrantMixin
       before_validation :generate_token, on: :create
     end
 
+    # never uses pkce, if pkce migrations were not generated
+    def uses_pkce?
+      pkce_supported? && code_challenge.present?
+    end
+
+    def pkce_supported?
+      respond_to? :code_challenge
+    end
+
     module ClassMethods
       def by_token(token)
         where(token: token.to_s).limit(1).to_a.first
       end
+
+      # Implements PKCE code_challenge encoding without base64 padding as described in the spec.
+      # https://tools.ietf.org/html/rfc7636#appendix-A
+      #   Appendix A.  Notes on Implementing Base64url Encoding without Padding
+      #
+      #   This appendix describes how to implement a base64url-encoding
+      #   function without padding, based upon the standard base64-encoding
+      #   function that uses padding.
+      #
+      #       To be concrete, example C# code implementing these functions is shown
+      #   below.  Similar code could be used in other languages.
+      #
+      #   static string base64urlencode(byte [] arg)
+      #   {
+      #       string s = Convert.ToBase64String(arg); // Regular base64 encoder
+      #       s = s.Split('=')[0]; // Remove any trailing '='s
+      #       s = s.Replace('+', '-'); // 62nd char of encoding
+      #       s = s.Replace('/', '_'); // 63rd char of encoding
+      #       return s;
+      #   }
+      #
+      #   An example correspondence between unencoded and encoded values
+      #   follows.  The octet sequence below encodes into the string below,
+      #   which when decoded, reproduces the octet sequence.
+      #
+      #   3 236 255 224 193
+      #
+      #   A-z_4ME
+      #
+      # https://ruby-doc.org/stdlib-2.1.3/libdoc/base64/rdoc/Base64.html#method-i-urlsafe_encode64
+      #
+      # urlsafe_encode64(bin)
+      # Returns the Base64-encoded version of bin. This method complies with
+      # “Base 64 Encoding with URL and Filename Safe Alphabet” in RFC 4648.
+      # The alphabet uses ‘-’ instead of ‘+’ and ‘_’ instead of ‘/’.
+
+      # @param code_verifier [#to_s] a one time use value (any object that responds to `#to_s`)
+      #
+      # @return [#to_s] An encoded code challenge based on the provided verifier suitable for PKCE validation
+      def generate_code_challenge(code_verifier)
+        padded_result = Base64.urlsafe_encode64(Digest::SHA256.digest(code_verifier))
+        padded_result.split('=')[0] # Remove any trailing '='
+      end
+
+      def pkce_supported?
+        new.pkce_supported?
+      end
     end
 
     private

lib/doorkeeper/oauth/authorization/code.rb

@@ -5,18 +5,12 @@ class Code
         attr_accessor :pre_auth, :resource_owner, :token
 
         def initialize(pre_auth, resource_owner)
-          @pre_auth       = pre_auth
+          @pre_auth = pre_auth
           @resource_owner = resource_owner
         end
 
         def issue_token
-          @token ||= AccessGrant.create!(
-            application_id: pre_auth.client.id,
-            resource_owner_id: resource_owner.id,
-            expires_in: configuration.authorization_code_expires_in,
-            redirect_uri: pre_auth.redirect_uri,
-            scopes: pre_auth.scopes.to_s
-          )
+          @token ||= AccessGrant.create! access_grant_attributes
         end
 
         def native_redirect
@@ -26,6 +20,37 @@ def native_redirect
         def configuration
           Doorkeeper.configuration
         end
+
+        private
+
+        def authorization_code_expires_in
+          configuration.authorization_code_expires_in
+        end
+
+        def access_grant_attributes
+          pkce_attributes.merge application_id: pre_auth.client.id,
+                                resource_owner_id: resource_owner.id,
+                                expires_in: authorization_code_expires_in,
+                                redirect_uri: pre_auth.redirect_uri,
+                                scopes: pre_auth.scopes.to_s
+        end
+
+        def pkce_attributes
+          if pkce_supported?
+            {
+              code_challenge: pre_auth.code_challenge,
+              code_challenge_method: pre_auth.code_challenge_method
+            }
+          else
+            {}
+          end
+        end
+
+        # ensures firstly, if migration with additional pcke columns was
+        # generated and migrated
+        def pkce_supported?
+          Doorkeeper::AccessGrant.pkce_supported?
+        end
       end
     end
   end

lib/doorkeeper/oauth/authorization_code_request.rb

@@ -8,14 +8,17 @@ class AuthorizationCodeRequest
       validate :client,       error: :invalid_client
       validate :grant,        error: :invalid_grant
       validate :redirect_uri, error: :invalid_grant
+      validate :code_verifier, error: :invalid_grant
 
-      attr_accessor :server, :grant, :client, :redirect_uri, :access_token
+      attr_accessor :server, :grant, :client, :redirect_uri, :access_token,
+                    :code_verifier
 
       def initialize(server, grant, client, parameters = {})
         @server = server
         @client = client
         @grant  = grant
         @redirect_uri = parameters[:redirect_uri]
+        @code_verifier = parameters[:code_verifier]
       end
 
       private
@@ -34,6 +37,9 @@ def before_successful_response
       end
 
       def validate_attributes
+        return false if grant && grant.uses_pkce? && code_verifier.blank?
+        return false if grant && !grant.pkce_supported? && !code_verifier.blank?
+
         redirect_uri.present?
       end
 
@@ -43,12 +49,28 @@ def validate_client
 
       def validate_grant
         return false unless grant && grant.application_id == client.id
+
         grant.accessible?
       end
 
       def validate_redirect_uri
         grant.redirect_uri == redirect_uri
       end
+
+      # if either side (server or client) request pkce, check the verifier
+      # against the DB - if pkce is supported
+      def validate_code_verifier
+        return true unless grant.uses_pkce? || code_verifier
+        return false unless grant.pkce_supported?
+
+        if grant.code_challenge_method == 'S256'
+          grant.code_challenge == AccessGrant.generate_code_challenge(code_verifier)
+        elsif grant.code_challenge_method == 'plain'
+          grant.code_challenge == code_verifier
+        else
+          false
+        end
+      end
     end
   end
 end

lib/doorkeeper/oauth/pre_authorization.rb

@@ -7,8 +7,10 @@ class PreAuthorization
       validate :client, error: :invalid_client
       validate :scopes, error: :invalid_scope
       validate :redirect_uri, error: :invalid_redirect_uri
+      validate :code_challenge_method, error: :invalid_code_challenge_method
 
-      attr_accessor :server, :client, :response_type, :redirect_uri, :state
+      attr_accessor :server, :client, :response_type, :redirect_uri, :state,
+                    :code_challenge, :code_challenge_method
       attr_writer   :scope
 
       def initialize(server, client, attrs = {})
@@ -18,6 +20,8 @@ def initialize(server, client, attrs = {})
         @redirect_uri  = attrs[:redirect_uri]
         @scope         = attrs[:scope]
         @state         = attrs[:state]
+        @code_challenge = attrs[:code_challenge]
+        @code_challenge_method = attrs[:code_challenge_method]
       end
 
       def authorizable?
@@ -48,6 +52,7 @@ def validate_client
 
       def validate_scopes
         return true unless scope.present?
+
         Helpers::ScopeChecker.valid?(
           scope,
           server.scopes,
@@ -58,9 +63,14 @@ def validate_scopes
       # TODO: test uri should be matched against the client's one
       def validate_redirect_uri
         return false unless redirect_uri.present?
+
         Helpers::URIChecker.native_uri?(redirect_uri) ||
           Helpers::URIChecker.valid_for_authorization?(redirect_uri, client.redirect_uri)
       end
+
+      def validate_code_challenge_method
+        !code_challenge.present? || (code_challenge_method.present? && code_challenge_method =~ /^plain$|^S256$/)
+      end
     end
   end
 end

lib/doorkeeper/request/authorization_code.rb

@@ -4,15 +4,24 @@ module Doorkeeper
   module Request
     class AuthorizationCode < Strategy
       delegate :grant, :client, :parameters, to: :server
+      delegate :client_via_uid, :parameters, to: :server
 
       def request
         @request ||= OAuth::AuthorizationCodeRequest.new(
           Doorkeeper.configuration,
           grant,
-          client,
+          client_for_request,
           parameters
         )
       end
+
+      def client_for_request
+        if parameters.include?(:code_verifier) && parameters[:code_verifier].present?
+          client_via_uid
+        else
+          client
+        end
+      end
     end
   end
 end

spec/dummy/db/schema.rb

@@ -22,6 +22,8 @@
     t.datetime "created_at",                     null: false
     t.datetime "revoked_at"
     t.string   "scopes"
+    t.string   "code_challenge"
+    t.string   "code_challenge_method"
   end
 
   add_index "oauth_access_grants", ["token"], name: "index_oauth_access_grants_on_token", unique: true

spec/lib/oauth/code_request_spec.rb

@@ -10,7 +10,9 @@ module Doorkeeper::OAuth
         scopes: nil,
         state: nil,
         error: nil,
-        authorizable?: true
+        authorizable?: true,
+        code_challenge: nil,
+        code_challenge_method: nil,
       )
     end