diff --git a/.rubocop.yml b/.rubocop.yml index d5ce27c..985a27e 100644 --- a/.rubocop.yml +++ b/.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 - \ No newline at end of file diff --git a/Rakefile b/Rakefile index 8452dcd..fd5db1e 100755 --- a/Rakefile +++ b/Rakefile @@ -1,4 +1,5 @@ #!/usr/bin/env rake + require "bundler/gem_tasks" require "rspec/core/rake_task" diff --git a/lib/omniauth-oauth2.rb b/lib/omniauth-oauth2.rb index 894d81c..9986eca 100644 --- a/lib/omniauth-oauth2.rb +++ b/lib/omniauth-oauth2.rb @@ -1,2 +1,2 @@ -require "omniauth-oauth2/version" # rubocop:disable FileName +require "omniauth-oauth2/version" require "omniauth/strategies/oauth2" diff --git a/lib/omniauth/strategies/oauth2.rb b/lib/omniauth/strategies/oauth2.rb index 318f699..0c50bd6 100644 --- a/lib/omniauth/strategies/oauth2.rb +++ b/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 diff --git a/spec/omniauth/strategies/oauth2_spec.rb b/spec/omniauth/strategies/oauth2_spec.rb index 4765ad5..3b988f8 100644 --- a/spec/omniauth/strategies/oauth2_spec.rb +++ b/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