From 2262b2a9aaa75c23ad6230aac9aafd4cd646068e Mon Sep 17 00:00:00 2001
From: Tomoya Usami <tonchi.systemv@gmail.com>
Date: Thu, 28 Aug 2014 19:10:27 +0900
Subject: [PATCH 1/3] add more features support. - id_token verification -
 discovery.

---
 lib/omniauth/strategies/openid_connect.rb     | 198 ++++++++++++++----
 .../strategies/openid_connect_test.rb         |  37 +++-
 2 files changed, 188 insertions(+), 47 deletions(-)

diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb
index d350aad0..e01ee01a 100644
--- a/lib/omniauth/strategies/openid_connect.rb
+++ b/lib/omniauth/strategies/openid_connect.rb
@@ -1,7 +1,9 @@
 require 'addressable/uri'
-require "net/http"
+require 'timeout'
+require 'net/http'
+require 'open-uri'
 require 'omniauth'
-require "openid_connect"
+require 'openid_connect'
 
 module OmniAuth
   module Strategies
@@ -9,22 +11,28 @@ class OpenIDConnect
       include OmniAuth::Strategy
 
       option :client_options, {
-        identifier: nil,
-        secret: nil,
-        redirect_uri: nil,
-        scheme: "https",
-        host: nil,
-        port: 443,
-        authorization_endpoint: "/authorize",
-        token_endpoint: "/token",
-        userinfo_endpoint: "/userinfo"
+          identifier: nil,
+          secret: nil,
+          redirect_uri: nil,
+          scheme: "https",
+          host: nil,
+          port: 443,
+          authorization_endpoint: "/authorize",
+          token_endpoint: "/token",
+          userinfo_endpoint: "/userinfo",
+          jwks_uri: '/jwk'
       }
+      option :issuer
+      option :discover, false
+      option :client_signing_alg
+      option :client_jwk_signing_key
+      option :client_x509_signing_key
       option :scope, [:openid]
       option :response_type, "code"
       option :state
       option :response_mode
-      option :display, nil#, [:page, :popup, :touch, :wap]
-      option :prompt, nil#, [:none, :login, :consent, :select_account]
+      option :display, nil #, [:page, :popup, :touch, :wap]
+      option :prompt, nil #, [:none, :login, :consent, :select_account]
       option :max_age
       option :ui_locales
       option :id_token_hint
@@ -35,28 +43,28 @@ class OpenIDConnect
 
       info do
         {
-          name: user_info.name,
-          email: user_info.email,
-          nickname: user_info.preferred_username,
-          first_name: user_info.given_name,
-          last_name: user_info.family_name,
-          gender: user_info.gender,
-          image: user_info.picture,
-          phone: user_info.phone_number,
-          urls: { website: user_info.website }
+            name: user_info.name,
+            email: user_info.email,
+            nickname: user_info.preferred_username,
+            first_name: user_info.given_name,
+            last_name: user_info.family_name,
+            gender: user_info.gender,
+            image: user_info.picture,
+            phone: user_info.phone_number,
+            urls: {website: user_info.website}
         }
       end
 
       extra do
-        { raw_info: user_info.raw_attributes }
+        {raw_info: user_info.raw_attributes}
       end
 
       credentials do
         {
-          token: access_token.access_token,
-          refresh_token: access_token.refresh_token,
-          expires_in: access_token.expires_in,
-          scope: access_token.scope
+            token: access_token.access_token,
+            refresh_token: access_token.refresh_token,
+            expires_in: access_token.expires_in,
+            scope: access_token.scope
         }
       end
 
@@ -64,17 +72,39 @@ def client
         @client ||= ::OpenIDConnect::Client.new(client_options)
       end
 
+      def config
+        @config ||= ::OpenIDConnect::Discovery::Provider::Config.discover!(options.issuer)
+      end
+
       def request_phase
+        options.issuer = issuer if options.issuer.blank?
+        discover! if options.discover
         redirect authorize_uri
       end
 
       def callback_phase
-        client.redirect_uri = client_options.redirect_uri
-        client.authorization_code = authorization_code
-        access_token
-        super
+        error = request.params['error_reason'] || request.params['error']
+        if error
+          raise CallbackError.new(request.params['error'], request.params['error_description'] || request.params['error_reason'], request.params['error_uri'])
+        elsif request.params['state'].to_s.empty? || request.params['state'] != stored_state
+          raise CallbackError.new(:csrf_detected, 'CSRF detected')
+        else
+          options.issuer = issuer if options.issuer.blank?
+          discover! if options.discover
+          client.redirect_uri = client_options.redirect_uri
+          client.authorization_code = authorization_code
+          access_token
+          super
+        end
+      rescue CallbackError => e
+        fail!(:invalid_credentials, e)
+      rescue ::Timeout::Error, ::Errno::ETIMEDOUT => e
+        fail!(:timeout, e)
+      rescue ::SocketError => e
+        fail!(:failed_to_connect, e)
       end
 
+
       def authorization_code
         request.params["code"]
       end
@@ -82,33 +112,127 @@ def authorization_code
       def authorize_uri
         client.redirect_uri = client_options.redirect_uri
         client.authorization_uri(
-          response_type: options.response_type,
-          scope: options.scope,
-          nonce: nonce,
+            response_type: options.response_type,
+            scope: options.scope,
+            state: new_state,
+            nonce: new_nonce,
         )
       end
 
       private
 
+      def issuer
+        resource = "#{client_options.scheme}://#{client_options.host}" + ((client_options.port) ? ":#{client_options.port.to_s}" : '')
+        ::OpenIDConnect::Discovery::Provider.discover!(resource).issuer
+      end
+
+      def discover!
+        client_options.authorization_endpoint = config.authorization_endpoint
+        client_options.token_endpoint = config.token_endpoint
+        client_options.userinfo_endpoint = config.userinfo_endpoint
+        client_options.jwks_uri = config.jwks_uri
+      end
+
       def user_info
         @user_info ||= access_token.userinfo!
       end
 
       def access_token
-        @access_token ||= client.access_token!(scope: options.scope)
+        @access_token ||= lambda {
+          _access_token = client.access_token!
+          _id_token = decode_id_token _access_token.id_token
+          _id_token.verify!(
+              issuer: options.issuer,
+              client_id: client_options.identifier,
+              nonce: stored_nonce
+          )
+          _access_token
+        }.call()
       end
 
+      def decode_id_token(id_token)
+        ::OpenIDConnect::ResponseObject::IdToken.decode(id_token, public_key)
+      end
+
+
       def client_options
         options.client_options
       end
 
-      def nonce
-        session[:nonce] = SecureRandom.hex(16)
+      def new_state
+        session['omniauth.state'] = SecureRandom.hex(16)
+      end
+
+      def stored_state
+        session.delete('omniauth.state')
+      end
+
+      def new_nonce
+        session['omniauth.nonce'] = SecureRandom.hex(16)
+      end
+
+      def stored_nonce
+        session.delete('omniauth.nonce')
       end
 
       def session
         @env.nil? ? {} : super
       end
+
+      def public_key
+        if options.discover
+          config.public_keys.first
+        else
+          key_or_secret
+        end
+      end
+
+      def key_or_secret
+        case options.client_signing_alg
+          when :HS256, :HS384, :HS512
+            return options.client_secret
+          when :RS256, :RS384, :RS512
+            if options.client_jwk_signing_key
+              return parse_jwk_key(options.client_jwk_signing_key)
+            elsif options.client_x509_signing_key
+              return parse_x509_key(options.client_x509_signing_key)
+            end
+          else
+        end
+      end
+
+      def parse_x509_key(key)
+        OpenSSL::X509::Certificate.new key
+      end
+
+      def parse_jwk_key(key)
+        json = JSON.parse(key)
+        jwk = json['keys'].first
+        create_rsa_key(jwk['n'], jwk['e'])
+      end
+
+      def create_rsa_key(mod, exp)
+        key = OpenSSL::PKey::RSA.new
+        exponent = OpenSSL::BN.new decode(exp)
+        modulus = OpenSSL::BN.new decode(mod)
+        key.e = exponent
+        key.n = modulus
+        key
+      end
+
+      def decode(str)
+        UrlSafeBase64.decode64(str).unpack('B*').first.to_i(2).to_s
+      end
+
+      class CallbackError < StandardError
+        attr_accessor :error, :error_reason, :error_uri
+
+        def initialize(error, error_reason=nil, error_uri=nil)
+          self.error = error
+          self.error_reason = error_reason
+          self.error_uri = error_uri
+        end
+      end
     end
   end
 end
diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb
index 2c851792..4378164e 100644
--- a/test/lib/omniauth/strategies/openid_connect_test.rb
+++ b/test/lib/omniauth/strategies/openid_connect_test.rb
@@ -2,15 +2,16 @@
 
 class OmniAuth::Strategies::OpenIDConnectTest < StrategyTestCase
   def test_client_options_defaults
-    assert_equal "https", strategy.options.client_options.scheme
+    assert_equal 'https', strategy.options.client_options.scheme
     assert_equal 443, strategy.options.client_options.port
-    assert_equal "/authorize", strategy.options.client_options.authorization_endpoint
-    assert_equal "/token", strategy.options.client_options.token_endpoint
+    assert_equal '/authorize', strategy.options.client_options.authorization_endpoint
+    assert_equal '/token', strategy.options.client_options.token_endpoint
   end
 
   def test_request_phase
-    expected_redirect = /^https:\/\/example\.com\/authorize\?client_id=1234&nonce=[\w\d]{32}&response_type=code&scope=openid$/
-    strategy.options.client_options.host = "example.com"
+    expected_redirect = /^https:\/\/example\.com\/authorize\?client_id=1234&nonce=[\w\d]{32}&response_type=code&scope=openid&state=[\w\d]{32}$/
+    strategy.options.issuer = 'example.com'
+    strategy.options.client_options.host = 'example.com'
     strategy.expects(:redirect).with(regexp_matches(expected_redirect))
     strategy.request_phase
   end
@@ -21,8 +22,16 @@ def test_uid
 
   def test_callback_phase
     code = SecureRandom.hex(16)
-    request.stubs(:params).returns({"code" => code})
-    request.stubs(:path_info).returns("")
+    state = SecureRandom.hex(16)
+    nonce = SecureRandom.hex(16)
+    request.stubs(:params).returns({'code' => code,'state' => state})
+    request.stubs(:path_info).returns('')
+
+    strategy.options.issuer = 'example.com'
+
+    id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+    id_token.stubs(:verify!).with({:issuer => strategy.options.issuer, :client_id => @identifier, :nonce => nonce}).returns(true)
+    ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
 
     strategy.unstub(:user_info)
     access_token = stub('OpenIDConnect::AccessToken')
@@ -30,10 +39,11 @@ def test_callback_phase
     access_token.stubs(:refresh_token)
     access_token.stubs(:expires_in)
     access_token.stubs(:scope)
-    client.expects(:access_token!).returns(access_token)
+    access_token.stubs(:id_token).returns('id_token')
+    client.expects(:access_token!).at_least_once.returns(access_token)
     access_token.expects(:userinfo!).returns(user_info)
 
-    strategy.call!({"rack.session" => {}})
+    strategy.call!({'rack.session' => {'omniauth.state' => state, 'omniauth.nonce' => nonce}})
     strategy.callback_phase
   end
 
@@ -55,11 +65,18 @@ def test_extra
   end
 
   def test_credentials
+    strategy.options.issuer = 'example.com'
+
+    id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+    id_token.stubs(:verify!).returns(true)
+    ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(id_token)
+
     access_token = stub('OpenIDConnect::AccessToken')
     access_token.stubs(:access_token).returns(SecureRandom.hex(16))
     access_token.stubs(:refresh_token).returns(SecureRandom.hex(16))
     access_token.stubs(:expires_in).returns(Time.now)
-    access_token.stubs(:scope).returns("openidconnect")
+    access_token.stubs(:scope).returns('openidconnect')
+    access_token.stubs(:id_token).returns(id_token)
 
     client.expects(:access_token!).returns(access_token)
     access_token.expects(:refresh_token).returns(access_token.refresh_token)

From e14336eae7db17d117c70d97a7069b49f5808855 Mon Sep 17 00:00:00 2001
From: Tomoya Usami <tonchi.systemv@gmail.com>
Date: Fri, 29 Aug 2014 13:40:05 +0900
Subject: [PATCH 2/3] add test case.

---
 lib/omniauth/strategies/openid_connect.rb     | 25 +++---
 .../strategies/openid_connect_test.rb         | 89 ++++++++++++++++++-
 2 files changed, 101 insertions(+), 13 deletions(-)

diff --git a/lib/omniauth/strategies/openid_connect.rb b/lib/omniauth/strategies/openid_connect.rb
index e01ee01a..d79904df 100644
--- a/lib/omniauth/strategies/openid_connect.rb
+++ b/lib/omniauth/strategies/openid_connect.rb
@@ -23,7 +23,7 @@ class OpenIDConnect
           jwks_uri: '/jwk'
       }
       option :issuer
-      option :discover, false
+      option :discovery, false
       option :client_signing_alg
       option :client_jwk_signing_key
       option :client_x509_signing_key
@@ -61,6 +61,7 @@ class OpenIDConnect
 
       credentials do
         {
+            id_token: access_token.id_token,
             token: access_token.access_token,
             refresh_token: access_token.refresh_token,
             expires_in: access_token.expires_in,
@@ -78,7 +79,7 @@ def config
 
       def request_phase
         options.issuer = issuer if options.issuer.blank?
-        discover! if options.discover
+        discover! if options.discovery
         redirect authorize_uri
       end
 
@@ -90,7 +91,7 @@ def callback_phase
           raise CallbackError.new(:csrf_detected, 'CSRF detected')
         else
           options.issuer = issuer if options.issuer.blank?
-          discover! if options.discover
+          discover! if options.discovery
           client.redirect_uri = client_options.redirect_uri
           client.authorization_code = authorization_code
           access_token
@@ -119,6 +120,14 @@ def authorize_uri
         )
       end
 
+      def public_key
+        if options.discover
+          config.public_keys.first
+        else
+          key_or_secret
+        end
+      end
+
       private
 
       def issuer
@@ -179,18 +188,10 @@ def session
         @env.nil? ? {} : super
       end
 
-      def public_key
-        if options.discover
-          config.public_keys.first
-        else
-          key_or_secret
-        end
-      end
-
       def key_or_secret
         case options.client_signing_alg
           when :HS256, :HS384, :HS512
-            return options.client_secret
+            return client_options.secret
           when :RS256, :RS384, :RS512
             if options.client_jwk_signing_key
               return parse_jwk_key(options.client_jwk_signing_key)
diff --git a/test/lib/omniauth/strategies/openid_connect_test.rb b/test/lib/omniauth/strategies/openid_connect_test.rb
index 4378164e..78166dda 100644
--- a/test/lib/omniauth/strategies/openid_connect_test.rb
+++ b/test/lib/omniauth/strategies/openid_connect_test.rb
@@ -16,6 +16,32 @@ def test_request_phase
     strategy.request_phase
   end
 
+  def test_request_phase_with_discovery
+    expected_redirect = /^https:\/\/example\.com\/authorization\?client_id=1234&nonce=[\w\d]{32}&response_type=code&scope=openid&state=[\w\d]{32}$/
+    strategy.options.client_options.host = 'example.com'
+    strategy.options.discovery = true
+
+    issuer = stub('OpenIDConnect::Discovery::Issuer')
+    issuer.stubs(:issuer).returns('https://example.com/')
+    ::OpenIDConnect::Discovery::Provider.stubs(:discover!).returns(issuer)
+
+    config = stub('OpenIDConnect::Discovery::Provder::Config')
+    config.stubs(:authorization_endpoint).returns('https://example.com/authorization')
+    config.stubs(:token_endpoint).returns('https://example.com/token')
+    config.stubs(:userinfo_endpoint).returns('https://example.com/userinfo')
+    config.stubs(:jwks_uri).returns('https://example.com/jwks')
+    OpenIDConnect::Discovery::Provider::Config.stubs(:discover!).with('https://example.com/').returns(config)
+
+    strategy.expects(:redirect).with(regexp_matches(expected_redirect))
+    strategy.request_phase
+
+    assert_equal strategy.options.issuer, 'https://example.com/'
+    assert_equal strategy.options.client_options.authorization_endpoint, 'https://example.com/authorization'
+    assert_equal strategy.options.client_options.token_endpoint, 'https://example.com/token'
+    assert_equal strategy.options.client_options.userinfo_endpoint, 'https://example.com/userinfo'
+    assert_equal strategy.options.client_options.jwks_uri, 'https://example.com/jwks'
+  end
+
   def test_uid
     assert_equal user_info.sub, strategy.uid
   end
@@ -47,6 +73,48 @@ def test_callback_phase
     strategy.callback_phase
   end
 
+  def test_callback_phase_with_discovery
+    code = SecureRandom.hex(16)
+    state = SecureRandom.hex(16)
+    nonce = SecureRandom.hex(16)
+    public_key = OpenSSL::PKey::RSA.generate(2048).public_key
+    request.stubs(:params).returns({'code' => code,'state' => state})
+    request.stubs(:path_info).returns('')
+
+    strategy.options.client_options.host = 'example.com'
+    strategy.options.discovery = true
+
+    issuer = stub('OpenIDConnect::Discovery::Issuer')
+    issuer.stubs(:issuer).returns('https://example.com/')
+    ::OpenIDConnect::Discovery::Provider.stubs(:discover!).returns(issuer)
+
+    config = stub('OpenIDConnect::Discovery::Provder::Config')
+    config.stubs(:authorization_endpoint).returns('https://example.com/authorization')
+    config.stubs(:token_endpoint).returns('https://example.com/token')
+    config.stubs(:userinfo_endpoint).returns('https://example.com/userinfo')
+    config.stubs(:jwks_uri).returns('https://example.com/jwks')
+    config.stubs(:public_keys).returns([public_key])
+    OpenIDConnect::Discovery::Provider::Config.stubs(:discover!).with('https://example.com/').returns(config)
+
+    id_token = stub('OpenIDConnect::ResponseObject::IdToken')
+    id_token.stubs(:verify!).with({:issuer => 'https://example.com/', :client_id => @identifier, :nonce => nonce}).returns(true)
+    ::OpenIDConnect::ResponseObject::IdToken.stubs(:decode).returns(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('id_token')
+    client.expects(:access_token!).at_least_once.returns(access_token)
+    access_token.expects(:userinfo!).returns(user_info)
+
+    strategy.call!({'rack.session' => {'omniauth.state' => state, 'omniauth.nonce' => nonce}})
+    strategy.callback_phase
+
+  end
+
   def test_info
     info = strategy.info
     assert_equal user_info.name, info[:name]
@@ -82,10 +150,29 @@ def test_credentials
     access_token.expects(:refresh_token).returns(access_token.refresh_token)
     access_token.expects(:expires_in).returns(access_token.expires_in)
 
-    assert_equal({ token: access_token.access_token,
+    assert_equal({ id_token: access_token.id_token,
+                   token: access_token.access_token,
                    refresh_token: access_token.refresh_token,
                    expires_in: access_token.expires_in,
                    scope: access_token.scope
                  }, strategy.credentials)
   end
+
+  def test_public_key_with_jwk
+    strategy.options.client_signing_alg = :RS256
+    strategy.options.client_jwk_signing_key = File.read('./test/fixtures/jwks.json')
+    assert_equal OpenSSL::PKey::RSA, strategy.public_key.class
+  end
+
+  def test_public_key_with_x509
+    strategy.options.client_signing_alg = :RS256
+    strategy.options.client_x509_signing_key = File.read('./test/fixtures/test.crt')
+    assert_equal OpenSSL::X509::Certificate, strategy.public_key.class
+  end
+
+  def test_public_key_with_hmac
+    strategy.options.client_options.secret = 'secret'
+    strategy.options.client_signing_alg = :HS256
+    assert_equal strategy.options.client_options.secret, strategy.public_key
+  end
 end

From 08969a2baeaf42be6a73cb467fa6fe88e5def7a4 Mon Sep 17 00:00:00 2001
From: Tomoya Usami <tonchi.systemv@gmail.com>
Date: Fri, 29 Aug 2014 13:40:18 +0900
Subject: [PATCH 3/3] bump up version.

---
 lib/omniauth/openid_connect/version.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/omniauth/openid_connect/version.rb b/lib/omniauth/openid_connect/version.rb
index d58f616d..d25ae3ad 100644
--- a/lib/omniauth/openid_connect/version.rb
+++ b/lib/omniauth/openid_connect/version.rb
@@ -1,5 +1,5 @@
 module OmniAuth
   module OpenIDConnect
-    VERSION = "0.1.0"
+    VERSION = "0.1.1"
   end
 end