Merge PR #131 from jessedoyle/pkce

Author: BobbyMcWho

.rubocop.yml

@@ -1,15 +1,34 @@
+AllCops:
+  NewCops: enable
+
+Gemspec/RequiredRubyVersion:
+  Enabled: false
+
 Layout/AccessModifierIndentation:
   EnforcedStyle: outdent
 
+Layout/LineLength:
+  AllowURI: true
+  Enabled: false
+
 Layout/SpaceInsideHashLiteralBraces:
   EnforcedStyle: no_space
 
+Lint/MissingSuper:
+  Enabled: false
+
+Metrics/AbcSize:
+  Max: 18
+
+Metrics/BlockLength:
+  Exclude:
+    - spec/omniauth/strategies/oauth2_spec.rb
+
 Metrics/BlockNesting:
   Max: 2
 
-Metrics/LineLength:
-  AllowURI: true
-  Enabled: false
+Metrics/ClassLength:
+  Max: 110
 
 Metrics/MethodLength:
   CountComments: false
@@ -19,6 +38,10 @@ Metrics/ParameterLists:
   Max: 4
   CountKeywordArgs: true
 
+Naming/FileName:
+  Exclude:
+    - lib/omniauth-oauth2.rb
+
 Style/CollectionMethods:
   PreferredMethods:
     map:      'collect'
@@ -35,6 +58,9 @@ Style/DoubleNegation:
 Style/ExpandPathArguments:
   Enabled: false
 
+Style/FrozenStringLiteralComment:
+  Enabled: false
+
 Style/HashSyntax:
   EnforcedStyle: hash_rockets
 
@@ -52,4 +78,3 @@ Style/TrailingCommaInHashLiteral:
 
 Style/TrailingCommaInArrayLiteral:
   EnforcedStyleForMultiline: comma
-  

Rakefile

@@ -1,4 +1,5 @@
 #!/usr/bin/env rake
+
 require "bundler/gem_tasks"
 require "rspec/core/rake_task"
 

lib/omniauth-oauth2.rb

@@ -1,2 +1,2 @@
-require "omniauth-oauth2/version" # rubocop:disable FileName
+require "omniauth-oauth2/version"
 require "omniauth/strategies/oauth2"

lib/omniauth/strategies/oauth2.rb

@@ -24,11 +24,22 @@ def self.inherited(subclass)
       option :client_secret, nil
       option :client_options, {}
       option :authorize_params, {}
-      option :authorize_options, [:scope, :state]
+      option :authorize_options, %i[scope state]
       option :token_params, {}
       option :token_options, []
       option :auth_token_params, {}
       option :provider_ignores_state, false
+      option :pkce, false
+      option :pkce_verifier, nil
+      option :pkce_options, {
+        :code_challenge => proc { |verifier|
+          Base64.urlsafe_encode64(
+            Digest::SHA2.digest(verifier),
+            :padding => false,
+          )
+        },
+        :code_challenge_method => "S256",
+      }
 
       attr_accessor :access_token
 
@@ -48,22 +59,29 @@ def request_phase
         redirect client.auth_code.authorize_url({:redirect_uri => callback_url}.merge(authorize_params))
       end
 
-      def authorize_params
+      def authorize_params# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
         options.authorize_params[:state] = SecureRandom.hex(24)
-        params = options.authorize_params.merge(options_for("authorize"))
+
         if OmniAuth.config.test_mode
           @env ||= {}
           @env["rack.session"] ||= {}
         end
+
+        params = options.authorize_params
+                        .merge(options_for("authorize"))
+                        .merge(pkce_authorize_params)
+
+        session["omniauth.pkce.verifier"] = options.pkce_verifier if options.pkce
         session["omniauth.state"] = params[:state]
+
         params
       end
 
       def token_params
-        options.token_params.merge(options_for("token"))
+        options.token_params.merge(options_for("token")).merge(pkce_token_params)
       end
 
-      def callback_phase # rubocop:disable AbcSize, CyclomaticComplexity, MethodLength, PerceivedComplexity
+      def callback_phase # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
         error = request.params["error_reason"] || request.params["error"]
         if error
           fail!(error, CallbackError.new(request.params["error"], request.params["error_description"] || request.params["error_reason"], request.params["error_uri"]))
@@ -84,27 +102,44 @@ def callback_phase # rubocop:disable AbcSize, CyclomaticComplexity, MethodLength
 
     protected
 
+      def pkce_authorize_params
+        return {} unless options.pkce
+
+        options.pkce_verifier = SecureRandom.hex(64)
+
+        # NOTE: see https://tools.ietf.org/html/rfc7636#appendix-A
+        {
+          :code_challenge => options.pkce_options[:code_challenge]
+                                    .call(options.pkce_verifier),
+          :code_challenge_method => options.pkce_options[:code_challenge_method],
+        }
+      end
+
+      def pkce_token_params
+        return {} unless options.pkce
+
+        {:code_verifier => session.delete("omniauth.pkce.verifier")}
+      end
+
       def build_access_token
         verifier = request.params["code"]
         client.auth_code.get_token(verifier, {:redirect_uri => callback_url}.merge(token_params.to_hash(:symbolize_keys => true)), deep_symbolize(options.auth_token_params))
       end
 
       def deep_symbolize(options)
-        hash = {}
-        options.each do |key, value|
+        options.each_with_object({}) do |(key, value), hash|
           hash[key.to_sym] = value.is_a?(Hash) ? deep_symbolize(value) : value
         end
-        hash
       end
 
       def options_for(option)
         hash = {}
         options.send(:"#{option}_options").select { |key| options[key] }.each do |key|
           hash[key.to_sym] = if options[key].respond_to?(:call)
-            options[key].call(env)
-          else
-            options[key]
-          end
+                               options[key].call(env)
+                             else
+                               options[key]
+                             end
         end
         hash
       end

spec/omniauth/strategies/oauth2_spec.rb

@@ -1,6 +1,6 @@
 require "helper"
 
-describe OmniAuth::Strategies::OAuth2 do # rubocop:disable Metrics/BlockLength
+describe OmniAuth::Strategies::OAuth2 do
   def app
     lambda do |_env|
       [200, {}, ["Hello."]]
@@ -62,10 +62,17 @@ def app
     end
 
     it "includes custom state in the authorize params" do
-      instance = subject.new("abc", "def", state: Proc.new { "qux" } )
+      instance = subject.new("abc", "def", :state => proc { "qux" })
       expect(instance.authorize_params.keys).to eq(["state"])
       expect(instance.session["omniauth.state"]).to eq("qux")
     end
+
+    it "includes PKCE parameters if enabled" do
+      instance = subject.new("abc", "def", :pkce => true)
+      expect(instance.authorize_params[:code_challenge]).to be_a(String)
+      expect(instance.authorize_params[:code_challenge_method]).to eq("S256")
+      expect(instance.session["omniauth.pkce.verifier"]).to be_a(String)
+    end
   end
 
   describe "#token_params" do
@@ -80,6 +87,13 @@ def app
       instance = subject.new("abc", "def", :token_options => %i[scope foo], :scope => "bar", :foo => "baz")
       expect(instance.token_params).to eq("scope" => "bar", "foo" => "baz")
     end
+
+    it "includes the PKCE code_verifier if enabled" do
+      instance = subject.new("abc", "def", :pkce => true)
+      # setup session
+      instance.authorize_params
+      expect(instance.token_params[:code_verifier]).to be_a(String)
+    end
   end
 
   describe "#callback_phase" do