OAuth2 - PKCE | CI

* Resolve all current rubocop violations for rubocop v0.86.0.
omniauth · Jun 28, 2020 · a7b3b73 · a7b3b73
1 parent e53f2cb
commit a7b3b73
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 28 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -1,15 +1,28 @@
+AllCops:
+  NewCops: enable
+
 Layout/AccessModifierIndentation:
   EnforcedStyle: outdent
 
+Layout/LineLength:
+  AllowURI: true
+  Enabled: false
+
 Layout/SpaceInsideHashLiteralBraces:
   EnforcedStyle: no_space
 
+Metrics/AbcSize:
+  Max: 17
+
+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 +32,10 @@ Metrics/ParameterLists:
   Max: 4
   CountKeywordArgs: true
 
+Naming/FileName:
+  Exclude:
+    - lib/omniauth-oauth2.rb
+
 Style/CollectionMethods:
   PreferredMethods:
     map:      'collect'
@@ -52,4 +69,3 @@ Style/TrailingCommaInHashLiteral:
 
 Style/TrailingCommaInArrayLiteral:
   EnforcedStyleForMultiline: comma
-
diff --git a/Gemfile b/Gemfile
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 source "https://rubygems.org"
 
 gem "rake", "~> 12.0"

diff --git a/Rakefile b/Rakefile
@@ -1,4 +1,6 @@
 #!/usr/bin/env rake
+# frozen_string_literal: true
+
 require "bundler/gem_tasks"
 require "rspec/core/rake_task"
 

diff --git a/lib/omniauth-oauth2.rb b/lib/omniauth-oauth2.rb
@@ -1,2 +1,4 @@
-require "omniauth-oauth2/version" # rubocop:disable FileName
+# frozen_string_literal: true
+
+require "omniauth-oauth2/version"
 require "omniauth/strategies/oauth2"
diff --git a/lib/omniauth-oauth2/version.rb b/lib/omniauth-oauth2/version.rb
@@ -1,5 +1,7 @@
+# frozen_string_literal: true
+
 module OmniAuth
   module OAuth2
-    VERSION = "1.6.0".freeze
+    VERSION = "1.6.0"
   end
 end
diff --git a/lib/omniauth/strategies/oauth2.rb b/lib/omniauth/strategies/oauth2.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "oauth2"
 require "omniauth"
 require "securerandom"
@@ -24,7 +26,7 @@ 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, {}
@@ -52,15 +54,7 @@ def request_phase
       def authorize_params
         verifier = SecureRandom.hex(64)
 
-        if options.pkce
-          # NOTE: see https://tools.ietf.org/html/rfc7636#appendix-A
-          challenge = Base64
-                 .urlsafe_encode64(Digest::SHA2.digest(verifier))
-                 .split("=")
-                 .first
-          options.authorize_params[:code_challenge] = challenge
-          options.authorize_params[:code_challenge_method] = "S256"
-        end
+        pkce_authorize_params!(verifier)
 
         options.authorize_params[:state] = SecureRandom.hex(24)
         params = options.authorize_params.merge(options_for("authorize"))
@@ -70,16 +64,15 @@ def authorize_params
           @env["rack.session"] ||= {}
         end
 
-        session["omniauth.pkce.verifier"] = verifier if options.pkce
-        session["omniauth.state"] = params[:state]
+        build_authorize_session!(params, verifier)
         params
       end
 
       def token_params
         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"]))
@@ -100,10 +93,27 @@ def callback_phase # rubocop:disable AbcSize, CyclomaticComplexity, MethodLength
 
     protected
 
+      def build_authorize_session!(params, verifier)
+        session["omniauth.pkce.verifier"] = verifier if options.pkce
+        session["omniauth.state"] = params[:state]
+      end
+
+      def pkce_authorize_params!(verifier)
+        return unless options.pkce
+
+        # NOTE: see https://tools.ietf.org/html/rfc7636#appendix-A
+        challenge = Base64
+                    .urlsafe_encode64(Digest::SHA2.digest(verifier))
+                    .split("=")
+                    .first
+        options.authorize_params[:code_challenge] = challenge
+        options.authorize_params[:code_challenge_method] = "S256"
+      end
+
       def pkce_token_params
         return {} unless options.pkce
 
-        { code_verifier: session.delete("omniauth.pkce.verifier") }
+        {:code_verifier => session.delete("omniauth.pkce.verifier")}
       end
 
       def build_access_token
@@ -121,10 +131,10 @@ 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

diff --git a/omniauth-oauth2.gemspec b/omniauth-oauth2.gemspec
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 lib = File.expand_path("../lib", __FILE__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 require "omniauth-oauth2/version"

diff --git a/spec/helper.rb b/spec/helper.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 $LOAD_PATH.unshift File.expand_path("..", __FILE__)
 $LOAD_PATH.unshift File.expand_path("../../lib", __FILE__)
 

diff --git a/spec/omniauth/strategies/oauth2_spec.rb b/spec/omniauth/strategies/oauth2_spec.rb
@@ -1,6 +1,8 @@
+# frozen_string_literal: true
+
 require "helper"
 
-describe OmniAuth::Strategies::OAuth2 do # rubocop:disable Metrics/BlockLength
+describe OmniAuth::Strategies::OAuth2 do
   def app
     lambda do |_env|
       [200, {}, ["Hello."]]
@@ -62,13 +64,13 @@ 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)
+      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)
@@ -89,7 +91,7 @@ def app
     end
 
     it "includes the PKCE code_verifier if enabled" do
-      instance = subject.new("abc", "def", pkce: true)
+      instance = subject.new("abc", "def", :pkce => true)
       # setup session
       instance.authorize_params
       expect(instance.token_params[:code_verifier]).to be_a(String)