From a7b3b73599579d09ffab588156b7003e1b07cab3 Mon Sep 17 00:00:00 2001 From: Jesse Doyle Date: Sun, 28 Jun 2020 00:47:11 -0600 Subject: [PATCH] OAuth2 - PKCE | CI * Resolve all current rubocop violations for rubocop v0.86.0. --- .rubocop.yml | 24 ++++++++++--- Gemfile | 2 ++ Rakefile | 2 ++ lib/omniauth-oauth2.rb | 4 ++- lib/omniauth-oauth2/version.rb | 4 ++- lib/omniauth/strategies/oauth2.rb | 46 +++++++++++++++---------- omniauth-oauth2.gemspec | 2 ++ spec/helper.rb | 2 ++ spec/omniauth/strategies/oauth2_spec.rb | 10 +++--- 9 files changed, 68 insertions(+), 28 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d5ce27c..3930505 100644 --- 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 - \ No newline at end of file diff --git a/Gemfile b/Gemfile index 75f41f6..94bd336 100644 --- 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 index 8452dcd..7097e8d 100755 --- 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 index 894d81c..48d302a 100644 --- 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 index c2c7281..b8d6e73 100644 --- 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 index 2fa6a48..b69e6eb 100644 --- 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,8 +64,7 @@ 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 @@ -79,7 +72,7 @@ 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 index 20abb26..f443431 100644 --- 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 index 280be5f..561e680 100644 --- 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 index 1832cc0..f3c7d51 100644 --- 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)