The Implicit Flow: Part 1

.gitignore

@@ -18,3 +18,6 @@ tmp
 .ruby-version
 .ruby-gemset
 Gemfile.lock
+
+# Backup files
+*~

README.md

@@ -53,7 +53,7 @@ config.omniauth :openid_connect, {
 | discovery                    | Should OpenID discovery be used. This is recommended if the IDP provides a discovery endpoint. See client config for how to manually enter discovered values. | no       | false                      | one of: true, false                                 |
 | client_auth_method           | Which authentication method to use to authenticate your app with the authorization server                                                                     | no       | Sym: basic                 | "basic", "jwks"                                     |
 | scope                        | Which OpenID scopes to include (:openid is always required)                                                                                                   | no       | Array<sym> [:openid]       | [:openid, :profile, :email]                         |
-| response_type                | Which OAuth2 response type to use with the authorization request                                                                                              | no       | String: code               | one of: 'code', 'id_token'                          |
+| `response_type`                | Which OAuth2 response type to use with the authorization request                                                                                              | no       | String: code               | one of: 'code', 'id_token token' <br />**Security note:** Do not use 'token' or 'id_token'. The 'token' (raw OAuth 2.0) MUST NOT be used for the authentication purpose. The 'id_token' is used only for the Self-Issued OpenID Providers. Instead, use 'id_token token' (the Implicit Flow).                          |
 | state                        | A value to be used for the OAuth2 state parameter on the authorization request. Can be a proc that generates a string.                                        | no       | Random 16 character string | Proc.new { SecureRandom.hex(32) }                   |
 | response_mode                | The response mode per [spec](https://openid.net/specs/oauth-v2-form-post-response-mode-1_0.html)                                                              | no       | nil                        | one of: :query, :fragment, :form_post, :web_message |
 | display                      | An optional parameter to the authorization request to determine how the authorization and consent page                                                        | no       | nil                        | one of: :page, :popup, :touch, :wap                 |

lib/omniauth/openid_connect.rb

@@ -2,4 +2,4 @@
 
 require 'omniauth/openid_connect/errors'
 require 'omniauth/openid_connect/version'
-require 'omniauth/strategies/openid_connect'
+require 'omniauth/openid_connect/util'

lib/omniauth/openid_connect/util.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module OmniAuth
+  module OpenIDConnect
+    module Util
+      # @param [String or Hash] key_or_hash  Certificate data in PEM format
+      # @raise [OpenSSL::X509::CertificateError] Certificate format is incorrect
+      def self.parse_x509_key(key_or_hash, kid)
+        raise TypeError if !key_or_hash.is_a?(String) && !key_or_hash.is_a?(Hash)
+
+        if key_or_hash.is_a?(Hash)
+          # https://www.googleapis.com/oauth2/v1/certs format
+          raise TypeError unless kid
+
+          key_or_hash.each do |key, pem|
+            return OpenSSL::X509::Certificate.new(pem).public_key if key == kid
+          end
+          raise ArgumentError, "missing kid: #{kid}"
+        else
+          OpenSSL::X509::Certificate.new(key_or_hash).public_key
+        end
+      end
+
+      # Decode JSON Web Key (JWK) or JWK Set format.
+      # See RFC 7517
+      # @param [String or Hash] key_or_hash   JSON-formatted string, or a hash.
+      #        Sample: https://www.googleapis.com/oauth2/v3/certs
+      def self.parse_jwk_key(key_or_hash, _kid)
+        case key_or_hash
+        when String
+          json = JSON.parse(key_or_hash)
+        when Hash
+          json = key_or_hash
+        else
+          raise TypeError, "key was #{key_or_hash.class}, #{key_or_hash.inspect}"
+        end
+
+        if json.key?('keys')
+          JSON::JWK::Set.new json['keys']
+        else
+          JSON::JWK.new json
+        end
+      end
+    end
+  end
+end

lib/omniauth/strategies/openid_connect.rb

@@ -37,10 +37,13 @@ class OpenIDConnect
       option :issuer
       option :discovery, false
       option :client_signing_alg
+      # Required if you set 'discovery:false'.
+      # IdP's public keys. NOT client's.
       option :client_jwk_signing_key
       option :client_x509_signing_key
+
       option :scope, [:openid]
-      option :response_type, 'code' # ['code', 'id_token']
+      option :response_type, 'code' # 'code', ['id_token', 'token']
       option :state
       option :response_mode # [:query, :fragment, :form_post, :web_message]
       option :display, nil # [:page, :popup, :touch, :wap]
@@ -98,6 +101,23 @@ def config
         @config ||= ::OpenIDConnect::Discovery::Provider::Config.discover!(options.issuer)
       end
 
+      # @override
+      # Called before both of request_phase and callback_phase
+      def setup_phase
+        super
+
+        if configured_response_type != 'code' &&
+            configured_response_type != 'id_token token'
+          raise ArgumentError, 'Not supported response_type'
+        end
+
+        if configured_response_type == 'id_token token' &&
+            client_options.secret
+          raise ArgumentError, 'MUST NOT set client_secret on the Implicit Flow'
+        end
+      end
+
+      # @override
       def request_phase
         options.issuer = issuer if options.issuer.to_s.empty?
         discover!
@@ -112,15 +132,15 @@ def callback_phase
         raise CallbackError, error: params['error'], reason: error_description, uri: params['error_uri'] if error
         raise CallbackError, error: :csrf_detected, reason: "Invalid 'state' parameter" if invalid_state
 
-        return unless valid_response_type?
+        return if configured_response_type == 'code' && !valid_response_type?
 
         options.issuer = issuer if options.issuer.nil? || options.issuer.empty?
 
-        verify_id_token!(params['id_token']) if configured_response_type == 'id_token'
+        verify_id_token!(params['id_token']) if configured_response_type == 'id_token token'
         discover!
         client.redirect_uri = redirect_uri
 
-        return id_token_callback_phase if configured_response_type == 'id_token'
+        return implicit_flow_callback_phase if configured_response_type == 'id_token token'
 
         client.authorization_code = authorization_code
         access_token
@@ -181,10 +201,19 @@ def authorize_uri
         client.authorization_uri(opts.reject { |_k, v| v.nil? })
       end
 
-      def public_key
+      # @return [JSON::JWK::Set or JSON::JWK] IdP's RSA public keys. NOT client's.
+      def public_key(kid = nil)
+        # [Security issue] Do not call key_or_secret() here.
+
         return config.jwks if options.discovery
 
-        key_or_secret || config.jwks
+        return OmniAuth::OpenIDConnect::Util.parse_jwk_key(options.client_jwk_signing_key, kid) if options.client_jwk_signing_key
+
+        if options.client_x509_signing_key
+          return OmniAuth::OpenIDConnect::Util.parse_x509_key(options.client_x509_signing_key, kid)
+        end
+
+        raise 'internal error: missing RSA public key'
       end
 
       private
@@ -273,34 +302,22 @@ def session
         super
       end
 
-      def key_or_secret
-        case options.client_signing_alg
+      # For HMAC-SHA256, return client_secret as the common key.
+      # For RSA, return the public key of the authentication server
+      def key_or_secret(header = nil)
+        raise TypeError if header && !header.respond_to?(:[])
+
+        case header ? header['alg'].to_sym : options.client_signing_alg
         when :HS256, :HS384, :HS512
           client_options.secret
         when :RS256, :RS384, :RS512
-          if options.client_jwk_signing_key
-            parse_jwk_key(options.client_jwk_signing_key)
-          elsif options.client_x509_signing_key
-            parse_x509_key(options.client_x509_signing_key)
-          end
+          public_key(header['kid'])
+        else
+          # ES256 : ECDSA using P-256 curve and SHA-256 hash
+          raise ArgumentError, "unsupported alg: #{header['alg']}"
         end
       end
 
-      def parse_x509_key(key)
-        OpenSSL::X509::Certificate.new(key).public_key
-      end
-
-      def parse_jwk_key(key)
-        json = JSON.parse(key)
-        return JSON::JWK::Set.new(json['keys']) if json.key?('keys')
-
-        JSON::JWK.new(json)
-      end
-
-      def decode(str)
-        UrlSafeBase64.decode64(str).unpack1('B*').to_i(2).to_s
-      end
-
       def redirect_uri
         return client_options.redirect_uri unless params['redirect_uri']
 
@@ -324,7 +341,20 @@ def logout_path_pattern
         @logout_path_pattern ||= %r{\A#{Regexp.quote(request_path)}(/logout)}
       end
 
-      def id_token_callback_phase
+      # The Implicit Flow:
+      # Get an access token at the same time as id_token. There is a risk that
+      # one of them has been tampered with. (Token Hijacking)
+      # For that reason,
+      # (1) You MUST verify the signature of the id_token by the public key of
+      #     IdP. Instead of choosing the key with the response header, you have
+      #     to use always the public key.
+      # (2) The access token must be validated by the id_token.
+      def implicit_flow_callback_phase
+        if !params['access_token'] || !params['id_token']
+          fail! :missing_id_token,
+                OmniAuth::OpenIDConnect::MissingIdTokenError.new(params['error'])
+        end
+
         user_data = decode_id_token(params['id_token']).raw_attributes
         env['omniauth.auth'] = AuthHash.new(
           provider: name,
@@ -335,6 +365,7 @@ def id_token_callback_phase
         call_app!
       end
 
+      # Called only from callback_phase()
       def valid_response_type?
         return true if params.key?(configured_response_type)
 
@@ -344,8 +375,18 @@ def valid_response_type?
         false
       end
 
+      # Normalize options.response_type.
+      # @return [String] 'code' or 'id_token token'
       def configured_response_type
-        @configured_response_type ||= options.response_type.to_s
+        unless @configured_response_type
+          ary = case options.response_type
+                when Array then options.response_type
+                when Symbol then [options.response_type]
+                else options.response_type.split(/[ \t]+/)
+                end
+          @configured_response_type = ary.sort.join(' ')
+        end
+        @configured_response_type
       end
 
       def verify_id_token!(id_token)

lib/omniauth_openid_connect.rb

@@ -1,3 +1,4 @@
 # frozen_string_literal: true
 
 require 'omniauth/openid_connect'
+require 'omniauth/strategies/openid_connect'

omniauth_openid_connect.gemspec

@@ -30,7 +30,6 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'addressable', '~> 2.5'
   spec.add_dependency 'omniauth', '>= 1.9', '< 3'
   spec.add_dependency 'openid_connect', '~> 1.1'
-  spec.add_development_dependency 'coveralls', '~> 0.8'
   spec.add_development_dependency 'faker', '~> 2.0'
   spec.add_development_dependency 'guard', '~> 2.14'
   spec.add_development_dependency 'guard-bundler', '~> 2.2'
@@ -39,5 +38,6 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'mocha', '~> 1.7'
   spec.add_development_dependency 'rake', '~> 12.0'
   spec.add_development_dependency 'rubocop', '~> 1.12'
-  spec.add_development_dependency 'simplecov', '~> 0.12'
+  spec.add_development_dependency 'simplecov', '~> 0.21'
+  spec.add_development_dependency 'simplecov-lcov', '~> 0.8'
 end

test/lib/omniauth/strategies/implicit_flow_test.rb

@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+require_relative '../../../test_helper'
+
+module OmniAuth
+  module Strategies
+    class NotYet
+      # Implicit Flow
+      def test_request_phase_with_response_mode
+        expected_redirect = %r{^https://example\.com/authorize\?client_id=1234&nonce=\w{32}&response_mode=form_post&response_type=id_token&scope=openid&state=\w{32}$}
+        strategy.options.issuer = 'example.com'
+        strategy.options.response_mode = 'form_post'
+        strategy.options.response_type = 'id_token'
+        strategy.options.client_options.host = 'example.com'
+
+        strategy.expects(:redirect).with(regexp_matches(expected_redirect))
+        strategy.request_phase
+      end
+
+      def test_request_phase_with_response_mode_symbol
+        expected_redirect = %r{^https://example\.com/authorize\?client_id=1234&nonce=\w{32}&response_mode=form_post&response_type=id_token&scope=openid&state=\w{32}$}
+        strategy.options.issuer = 'example.com'
+        strategy.options.response_mode = 'form_post'
+        strategy.options.response_type = :id_token
+        strategy.options.client_options.host = 'example.com'
+
+        strategy.expects(:redirect).with(regexp_matches(expected_redirect))
+        strategy.request_phase
+      end
+
+      def test_callback_phase_with_id_token
+        code = SecureRandom.hex(16)
+        state = SecureRandom.hex(16)
+        nonce = SecureRandom.hex(16)
+        request.stubs(:params).returns('id_token' => code, 'state' => state)
+        request.stubs(:path).returns('')
+
+        strategy.options.issuer = 'example.com'
+        strategy.options.client_signing_alg = :RS256
+        strategy.options.client_jwk_signing_key = File.read('test/fixtures/jwks.json')
+        strategy.options.response_type = 'id_token'
+
+        strategy.unstub(:user_info)
+        access_token = stub('OpenIDConnect::AccessToken')
+        access_token.stubs(:access_token)
+        access_token.stubs(:refresh_token)
+        access_token.stubs(:expires_in)
+        access_token.stubs(:scope)
+        access_token.stubs(:id_token).returns(File.read('test/fixtures/id_token.txt'))
+
+        id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+        id_token.stubs(:raw_attributes).returns('sub' => 'sub', 'name' => 'name', 'email' => 'email')
+        id_token.stubs(:verify!).with(issuer: strategy.options.issuer, client_id: @identifier, nonce: nonce).returns(true)
+        ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+        id_token.expects(:verify!)
+
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+        strategy.callback_phase
+      end
+
+      # Implicit Flow
+      def test_callback_phase_without_id_token
+        state = SecureRandom.hex(16)
+        nonce = SecureRandom.hex(16)
+        request.stubs(:params).returns('state' => state)
+        request.stubs(:path).returns('')
+        strategy.options.response_type = 'id_token'
+
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+
+        strategy.expects(:fail!).with(:missing_id_token, is_a(OmniAuth::OpenIDConnect::MissingIdTokenError))
+        strategy.callback_phase
+      end
+
+      # Implicit Flow
+      def test_callback_phase_without_id_token_symbol
+        state = SecureRandom.hex(16)
+        nonce = SecureRandom.hex(16)
+        request.stubs(:params).returns('state' => state)
+        request.stubs(:path).returns('')
+        strategy.options.response_type = :id_token
+
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+
+        strategy.expects(:fail!).with(:missing_id_token, is_a(OmniAuth::OpenIDConnect::MissingIdTokenError))
+        strategy.callback_phase
+      end
+
+      # Implicit Flow
+      def test_id_token_auth_hash
+        state = SecureRandom.hex(16)
+        nonce = SecureRandom.hex(16)
+        strategy.options.response_type = 'token id_token'
+        strategy.options.issuer = 'example.com'
+
+        id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+        id_token.stubs(:verify!).returns(true)
+        id_token.stubs(:raw_attributes, :to_h).returns(
+          {
+            "iss": 'http://server.example.com',
+            "sub": '248289761001',
+            "aud": 's6BhdRkqt3',
+            "nonce": 'n-0S6_WzA2Mj',
+            "exp": 1_311_281_970,
+            "iat": 1_311_280_970,
+          }
+        )
+
+        request.stubs(:params).returns('state' => state, 'nounce' => nonce, 'id_token' => id_token)
+        request.stubs(:path).returns('')
+
+        strategy.stubs(:decode_id_token).returns(id_token)
+        strategy.stubs(:stored_state).returns(state)
+
+        strategy.call!('rack.session' => { 'omniauth.state' => state, 'omniauth.nonce' => nonce })
+        strategy.callback_phase
+
+        auth_hash = strategy.send(:env)['omniauth.auth']
+        assert auth_hash.key?('provider')
+        assert auth_hash.key?('uid')
+        assert auth_hash.key?('info')
+        assert auth_hash.key?('extra')
+        assert auth_hash['extra'].key?('raw_info')
+      end
+    end
+  end
+end