From f70898be8363e19ab2c70779fd472b4c00cdc3b3 Mon Sep 17 00:00:00 2001 From: Markus Koller Date: Tue, 4 Oct 2016 20:11:01 +0200 Subject: [PATCH] feature: Add webfinger and keys endpoints for discovery --- .../openid_connect/discovery_controller.rb | 46 ++++++++++++++--- config/locales/en.yml | 9 ++-- doorkeeper-openid_connect.gemspec | 2 +- lib/doorkeeper/openid_connect.rb | 8 +++ lib/doorkeeper/openid_connect/config.rb | 24 ++------- .../openid_connect/models/id_token.rb | 8 +-- lib/doorkeeper/openid_connect/rails/routes.rb | 32 ++++++------ .../discovery_controller_spec.rb | 50 +++++++++++++++++-- .../initializers/doorkeeper_openid_connect.rb | 42 ++++++++++++++++ spec/dummy/config/routes.rb | 2 + .../doorkeeper/openid_connect/config_spec.rb | 20 +++++--- .../doorkeeper/openid_connect/routes_spec.rb | 38 ++++++++++---- spec/lib/doorkeeper/openid_connect_spec.rb | 16 ++++++ 13 files changed, 221 insertions(+), 76 deletions(-) create mode 100644 spec/lib/doorkeeper/openid_connect_spec.rb diff --git a/app/controllers/doorkeeper/openid_connect/discovery_controller.rb b/app/controllers/doorkeeper/openid_connect/discovery_controller.rb index 8711660..60091ff 100644 --- a/app/controllers/doorkeeper/openid_connect/discovery_controller.rb +++ b/app/controllers/doorkeeper/openid_connect/discovery_controller.rb @@ -3,24 +3,31 @@ module OpenidConnect class DiscoveryController < ::Doorkeeper::ApplicationController include Doorkeeper::Helpers::Controller - def show - render json: provider_configuration + WEBFINGER_RELATION = 'http://openid.net/specs/connect/1.0/issuer' + + def provider + render json: provider_response + end + + def webfinger + render json: webfinger_response + end + + def keys + render json: keys_response end private - def provider_configuration + def provider_response doorkeeper = ::Doorkeeper.configuration openid_connect = ::Doorkeeper::OpenidConnect.configuration - { issuer: openid_connect.issuer, authorization_endpoint: oauth_authorization_url(protocol: :https), token_endpoint: oauth_token_url(protocol: :https), userinfo_endpoint: oauth_userinfo_url(protocol: :https), - - # TODO: implement controller - #jwks_uri: oauth_keys_url(protocol: :https), + jwks_uri: oauth_discovery_keys_url(protocol: :https), scopes_supported: doorkeeper.scopes, @@ -48,6 +55,31 @@ def provider_configuration ], } end + + def webfinger_response + { + subject: params.require(:resource), + links: [ + { + rel: WEBFINGER_RELATION, + href: root_url(protocol: :https), + } + ] + } + end + + def keys_response + signing_key = Doorkeeper::OpenidConnect.signing_key + + { + keys: [ + signing_key.slice(:kty, :kid, :e, :n).merge( + use: 'sig', + alg: Doorkeeper::OpenidConnect::SIGNING_ALGORITHM + ) + ] + } + end end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 007a299..997e0b1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -14,9 +14,6 @@ en: server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.' temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.' - #configuration error messages - jwt_private_key_configured: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.jwt_private_key missing configuration.' - jwt_public_key_configured: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.jwt_public_key missing configuration.' - issuer: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.issuer missing configuration.' - resource_owner_from_access_token_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.resource_owner_from_access_token missing configuration.' - subject_configured: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.subject missing configuration.' + # Configuration error messages + resource_owner_from_access_token_not_configured: 'Failure due to Doorkeeper::OpenidConnect.configure.resource_owner_from_access_token missing configuration.' + subject_not_configured: 'ID Token generation failed due to Doorkeeper::OpenidConnect.configure.subject missing configuration.' diff --git a/doorkeeper-openid_connect.gemspec b/doorkeeper-openid_connect.gemspec index a3dc8a4..ebd60f9 100644 --- a/doorkeeper-openid_connect.gemspec +++ b/doorkeeper-openid_connect.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.add_runtime_dependency 'doorkeeper', '~> 4.0' - spec.add_runtime_dependency 'sandal', '~> 0.6' + spec.add_runtime_dependency 'json-jwt', '~> 1.6.5' spec.add_development_dependency 'rspec-rails' spec.add_development_dependency 'factory_girl' diff --git a/lib/doorkeeper/openid_connect.rb b/lib/doorkeeper/openid_connect.rb index 514b08e..b36dc54 100644 --- a/lib/doorkeeper/openid_connect.rb +++ b/lib/doorkeeper/openid_connect.rb @@ -12,6 +12,7 @@ require 'doorkeeper/openid_connect/rails/routes' require 'doorkeeper' +require 'json/jwt' module Doorkeeper class << self @@ -19,6 +20,9 @@ class << self end module OpenidConnect + # TODO: make this configurable + SIGNING_ALGORITHM = 'RS256' + def self.configured? @config.present? end @@ -26,6 +30,10 @@ def self.configured? def self.installed? configured? end + + def self.signing_key + JSON::JWK.new(OpenSSL::PKey.read(configuration.jws_private_key)) + end end end diff --git a/lib/doorkeeper/openid_connect/config.rb b/lib/doorkeeper/openid_connect/config.rb index c21c80e..3c8bfd2 100644 --- a/lib/doorkeeper/openid_connect/config.rb +++ b/lib/doorkeeper/openid_connect/config.rb @@ -96,33 +96,19 @@ def extended(base) extend Option - option :jws_private_key, - default: (lambda do - logger.warn(I18n.translate('doorkeeper.openid_connect.errors.messages.jws_private_key_configured')) - nil - end) - - option :jws_public_key, - default: (lambda do - logger.warn(I18n.translate('doorkeeper.openid_connect.errors.messages.jws_public_key_configured')) - nil - end) - - option :issuer, - default: (lambda do - logger.warn(I18n.translate('doorkeeper.openid_connect.errors.messages.issuer_configured')) - nil - end) + option :jws_private_key + option :jws_public_key + option :issuer option :resource_owner_from_access_token, default: (lambda do - logger.warn(I18n.translate('doorkeeper.openid_connect.errors.messages.resource_owner_from_access_token_configured')) + logger.warn(I18n.translate('doorkeeper.openid_connect.errors.messages.resource_owner_from_access_token_not_configured')) nil end) option :subject, default: (lambda do - logger.warn(I18n.translate('doorkeeper.openid_connect.errors.messages.subject_configured')) + logger.warn(I18n.translate('doorkeeper.openid_connect.errors.messages.subject_not_configured')) nil end) diff --git a/lib/doorkeeper/openid_connect/models/id_token.rb b/lib/doorkeeper/openid_connect/models/id_token.rb index dace436..566fc16 100644 --- a/lib/doorkeeper/openid_connect/models/id_token.rb +++ b/lib/doorkeeper/openid_connect/models/id_token.rb @@ -1,5 +1,3 @@ -require 'sandal' - module Doorkeeper module OpenidConnect module Models @@ -10,8 +8,6 @@ def initialize(access_token) @access_token = access_token @resource_owner = access_token.instance_eval(&Doorkeeper::OpenidConnect.configuration.resource_owner_from_access_token) @issued_at = Time.now - @signer = Sandal::Sig::RS256.new(Doorkeeper::OpenidConnect.configuration.jws_private_key) - @public_key = Doorkeeper::OpenidConnect.configuration.jws_public_key end def claims @@ -28,10 +24,8 @@ def as_json(options = {}) claims end - # TODO make signature strategy configurable with keys? - # TODO move this out of the model def as_jws_token - Sandal.encode_token(claims, @signer, typ: 'JWT') + JSON::JWT.new(claims).sign(Doorkeeper::OpenidConnect.signing_key).to_s end private diff --git a/lib/doorkeeper/openid_connect/rails/routes.rb b/lib/doorkeeper/openid_connect/rails/routes.rb index 3eb5d0e..37fa459 100644 --- a/lib/doorkeeper/openid_connect/rails/routes.rb +++ b/lib/doorkeeper/openid_connect/rails/routes.rb @@ -25,10 +25,11 @@ def generate_routes!(options) @mapping = Mapper.new.map(&@block) routes.scope options[:scope] || 'oauth', as: 'oauth' do map_route(:userinfo, :userinfo_routes) + map_route(:discovery, :discovery_routes) end routes.scope as: 'oauth' do - map_route(:discovery, :discovery_routes) + map_route(:discovery, :discovery_well_known_routes) end end @@ -36,26 +37,29 @@ def generate_routes!(options) def map_route(name, method) unless @mapping.skipped?(name) - send method, @mapping[name] + mapping = @mapping[name] + + routes.scope controller: mapping[:controllers], as: mapping[:as] do + send method, mapping + end end end def userinfo_routes(mapping) - routes.resource( - :userinfo, - path: 'userinfo', - only: [:show], as: mapping[:as], - controller: mapping[:controllers] - ) + routes.get :show, path: 'userinfo', as: '' end def discovery_routes(mapping) - routes.resource( - :discovery, - path: '.well-known/openid-configuration', - only: [:show], as: mapping[:as], - controller: mapping[:controllers] - ) + routes.scope path: 'discovery' do + routes.get :keys + end + end + + def discovery_well_known_routes(mapping) + routes.scope path: '.well-known' do + routes.get :provider, path: 'openid-configuration' + routes.get :webfinger + end end end end diff --git a/spec/controllers/doorkeeper/openid_connect/discovery_controller_spec.rb b/spec/controllers/doorkeeper/openid_connect/discovery_controller_spec.rb index 271ff61..6231322 100644 --- a/spec/controllers/doorkeeper/openid_connect/discovery_controller_spec.rb +++ b/spec/controllers/doorkeeper/openid_connect/discovery_controller_spec.rb @@ -1,16 +1,17 @@ require 'rails_helper' describe Doorkeeper::OpenidConnect::DiscoveryController, type: :controller do - describe '#show' do + describe '#provider' do it 'returns the provider configuration' do - get :show - configuration = JSON.parse(response.body) + get :provider + data = JSON.parse(response.body) - expect(configuration.sort).to eq({ + expect(data.sort).to eq({ 'issuer' => 'dummy', 'authorization_endpoint' => 'https://test.host/oauth/authorize', 'token_endpoint' => 'https://test.host/oauth/token', 'userinfo_endpoint' => 'https://test.host/oauth/userinfo', + 'jwks_uri' => 'https://test.host/oauth/discovery/keys', 'scopes_supported' => ['openid'], @@ -32,4 +33,45 @@ }.sort) end end + + describe '#webfinger' do + it 'requires the resource parameter' do + expect do + get :webfinger + end.to raise_error ActionController::ParameterMissing + end + + it 'returns the OpenID Connect relation' do + get :webfinger, resource: 'user@example.com' + data = JSON.parse(response.body) + + expect(data.sort).to eq({ + 'subject' => 'user@example.com', + 'links' => [ + 'rel' => 'http://openid.net/specs/connect/1.0/issuer', + 'href' => 'https://test.host/', + ], + }.sort) + end + end + + describe '#keys' do + it 'returns the key parameters' do + get :keys + data = JSON.parse(response.body) + + expect(data.sort).to eq({ + 'keys' => [ + { + 'kty' => 'RSA', + 'kid' => 'IqYwZo2cE6hsyhs48cU8QHH4GanKIx0S4Dc99kgTIMA', + 'e' => 'AQAB', + 'n' => 'sjdnSA6UWUQQHf6BLIkIEUhMRNBJC1NN_pFt1EJmEiI88GS0ceROO5B5Ooo9Y3QOWJ_n-u1uwTHBz0HCTN4wgArWd1TcqB5GQzQRP4eYnWyPfi4CfeqAHzQp-v4VwbcK0LW4FqtW5D0dtrFtI281FDxLhARzkhU2y7fuYhL8fVw5rUhE8uwvHRZ5CEZyxf7BSHxIvOZAAymhuzNLATt2DGkDInU1BmF75tEtBJAVLzWG_j4LPZh1EpSdfezqaXQlcy9PJi916UzTl0P7Yy-ulOdUsMlB6yo8qKTY1-AbZ5jzneHbGDU_O8QjYvii1WDmJ60t0jXicmOkGrOhruOptw', + 'use' => 'sig', + 'alg' => 'RS256', + } + ], + }.sort) + end + end end diff --git a/spec/dummy/config/initializers/doorkeeper_openid_connect.rb b/spec/dummy/config/initializers/doorkeeper_openid_connect.rb index 4980c2a..c43bd0b 100644 --- a/spec/dummy/config/initializers/doorkeeper_openid_connect.rb +++ b/spec/dummy/config/initializers/doorkeeper_openid_connect.rb @@ -1,6 +1,48 @@ Doorkeeper::OpenidConnect.configure do issuer 'dummy' + jws_private_key <<-EOL +-----BEGIN RSA PRIVATE KEY----- +MIIEpgIBAAKCAQEAsjdnSA6UWUQQHf6BLIkIEUhMRNBJC1NN/pFt1EJmEiI88GS0 +ceROO5B5Ooo9Y3QOWJ/n+u1uwTHBz0HCTN4wgArWd1TcqB5GQzQRP4eYnWyPfi4C +feqAHzQp+v4VwbcK0LW4FqtW5D0dtrFtI281FDxLhARzkhU2y7fuYhL8fVw5rUhE +8uwvHRZ5CEZyxf7BSHxIvOZAAymhuzNLATt2DGkDInU1BmF75tEtBJAVLzWG/j4L +PZh1EpSdfezqaXQlcy9PJi916UzTl0P7Yy+ulOdUsMlB6yo8qKTY1+AbZ5jzneHb +GDU/O8QjYvii1WDmJ60t0jXicmOkGrOhruOptwIDAQABAoIBAQChYNwMeu9IugJi +NsEf4+JDTBWMRpOuRrwcpfIvQAUPrKNEB90COPvCoju0j9OxCDmpdPtq1K/zD6xx +khlw485FVAsKufSp4+g6GJ75yT6gZtq1JtKo1L06BFFzb7uh069eeP7+wB6JxPHw +KlAqwxvsfADhxeolQUKCTMb3Vjv/Aw2cO/nn6RAOeftw2aDmFy8Xl+oTUtSxyib0 +YCdU9cK8MxsxDdmowwHp04xRTm/wfG5hLEn7HMz1PP86iP9BiFsCqTId9dxEUTS1 +K+VAt9FbxRAq5JlBocxUMHNxLigb94Ca2FOMR7F6l/tronLfHD801YoObF0fN9qW +Cgw4aTO5AoGBAOR79hiZVM7/l1cBid7hKSeMWKUZ/nrwJsVfNpu1H9xt9uDu+79U +mcGfM7pm7L2qCNGg7eeWBHq2CVg/XQacRNtcTlomFrw4tDXUkFN1hE56t1iaTs9m +dN9IDr6jFgf6UaoOxxoPT9Q1ZtO46l043Nzrkoz8cBEBaBY20bUDwCYjAoGBAMet +tt1ImGF1cx153KbOfjl8v54VYUVkmRNZTa1E821nL/EMpoONSqJmRVsX7grLyPL1 +QyZe245NOvn63YM0ng0rn2osoKsMVJwYBEYjHL61iF6dPtW5p8FIs7auRnC3NrG0 +XxHATZ4xhHD0iIn14iXh0XIhUVk+nGktHU1gbmVdAoGBANniwKdqqS6RHKBTDkgm +Dhnxw6MGa+CO3VpA1xGboxuRHeoY3KfzpIC5MhojBsZDvQ8zWUwMio7+w2CNZEfm +g99wYiOjyPCLXocrAssj+Rzh97AdzuQHf5Jh4/W2Dk9jTbdPSl02ltj2Z+2lnJFz +pWNjnqimHrSI09rDQi5NulJjAoGBAImquujVpDmNQFCSNA7NTzlTSMk09FtjgCZW +67cKUsqa2fLXRfZs84gD+s1TMks/NMxNTH6n57e0h3TSAOb04AM0kDQjkKJdXfhA +lrHEg4z4m4yf3TJ9Tat09HJ+tRIBPzRFp0YVz23Btg4qifiUDdcQWdbWIb/l6vCY +qhsu4O4BAoGBANbceYSDYRdT7a5QjJGibkC90Z3vFe4rDTBgZWg7xG0cpSU4JNg7 +SFR3PjWQyCg7aGGXiooCM38YQruACTj0IFub24MFRA4ZTXvrACvpsVokJlQiG0Z4 +tuQKYki41JvYqPobcq/rLE/AM7PKJftW35nqFuj0MrsUwPacaVwKBf5J +-----END RSA PRIVATE KEY----- + EOL + + jws_public_key <<-EOL +-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsjdnSA6UWUQQHf6BLIkI +EUhMRNBJC1NN/pFt1EJmEiI88GS0ceROO5B5Ooo9Y3QOWJ/n+u1uwTHBz0HCTN4w +gArWd1TcqB5GQzQRP4eYnWyPfi4CfeqAHzQp+v4VwbcK0LW4FqtW5D0dtrFtI281 +FDxLhARzkhU2y7fuYhL8fVw5rUhE8uwvHRZ5CEZyxf7BSHxIvOZAAymhuzNLATt2 +DGkDInU1BmF75tEtBJAVLzWG/j4LPZh1EpSdfezqaXQlcy9PJi916UzTl0P7Yy+u +lOdUsMlB6yo8qKTY1+AbZ5jzneHbGDU/O8QjYvii1WDmJ60t0jXicmOkGrOhruOp +twIDAQAB +-----END PUBLIC KEY----- + EOL + resource_owner_from_access_token do |access_token| User.find_by(id: access_token.resource_owner_id) end diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index 85185be..e182346 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -1,4 +1,6 @@ Rails.application.routes.draw do use_doorkeeper use_doorkeeper_openid_connect + + root 'dummy#index' end diff --git a/spec/lib/doorkeeper/openid_connect/config_spec.rb b/spec/lib/doorkeeper/openid_connect/config_spec.rb index f38af75..75aba8a 100644 --- a/spec/lib/doorkeeper/openid_connect/config_spec.rb +++ b/spec/lib/doorkeeper/openid_connect/config_spec.rb @@ -3,6 +3,10 @@ describe Doorkeeper::OpenidConnect, 'configuration' do subject { Doorkeeper::OpenidConnect.configuration } + after :all do + load "#{Rails.root}/config/initializers/doorkeeper_openid_connect.rb" + end + describe 'scopes' do it' adds the openid scope to the Doorkeeper configuration' do expect(Doorkeeper.configuration.scopes).to include 'openid' @@ -11,7 +15,7 @@ describe 'jws_private_key' do it 'sets the value that is accessible via jws_private_key' do - value = '' + value = 'private_key' Doorkeeper::OpenidConnect.configure do jws_private_key value end @@ -21,7 +25,7 @@ describe 'jws_public_key' do it 'sets the value that is accessible via jws_public_key' do - value = '' + value = 'public_key' Doorkeeper::OpenidConnect.configure do jws_public_key value end @@ -31,11 +35,11 @@ describe 'issuer' do it 'sets the value that is accessible via issuer' do - value = '' + value = 'issuer' Doorkeeper::OpenidConnect.configure do - jws_public_key value + issuer value end - expect(subject.jws_public_key).to eq(value) + expect(subject.issuer).to eq(value) end end @@ -43,7 +47,7 @@ it 'sets the block that is accessible via resource_owner_from_access_token' do block = proc {} Doorkeeper::OpenidConnect.configure do - resource_owner_from_access_token &block + resource_owner_from_access_token(&block) end expect(subject.resource_owner_from_access_token).to eq(block) end @@ -53,7 +57,7 @@ it 'sets the block that is accessible via subject' do block = proc {} Doorkeeper::OpenidConnect.configure do - subject &block + subject(&block) end expect(subject.subject).to eq(block) end @@ -61,7 +65,7 @@ describe 'expiration' do it 'sets the value that is accessible via expiration' do - value = '' + value = 'expiration' Doorkeeper::OpenidConnect.configure do expiration value end diff --git a/spec/lib/doorkeeper/openid_connect/routes_spec.rb b/spec/lib/doorkeeper/openid_connect/routes_spec.rb index 6bcc4a5..69d702b 100644 --- a/spec/lib/doorkeeper/openid_connect/routes_spec.rb +++ b/spec/lib/doorkeeper/openid_connect/routes_spec.rb @@ -1,17 +1,35 @@ require 'rails_helper' describe Doorkeeper::OpenidConnect::Rails::Routes, type: :routing do - it 'maps userinfo#show' do - expect(get: 'oauth/userinfo').to route_to( - controller: 'doorkeeper/openid_connect/userinfo', - action: 'show' - ) + describe 'userinfo' do + it 'maps #show' do + expect(get: 'oauth/userinfo').to route_to( + controller: 'doorkeeper/openid_connect/userinfo', + action: 'show' + ) + end end - it 'maps discovery#show' do - expect(get: '.well-known/openid-configuration').to route_to( - controller: 'doorkeeper/openid_connect/discovery', - action: 'show' - ) + describe 'discovery' do + it 'maps #provider' do + expect(get: '.well-known/openid-configuration').to route_to( + controller: 'doorkeeper/openid_connect/discovery', + action: 'provider' + ) + end + + it 'maps #webfinger' do + expect(get: '.well-known/webfinger').to route_to( + controller: 'doorkeeper/openid_connect/discovery', + action: 'webfinger' + ) + end + + it 'maps #keys' do + expect(get: 'oauth/discovery/keys').to route_to( + controller: 'doorkeeper/openid_connect/discovery', + action: 'keys' + ) + end end end diff --git a/spec/lib/doorkeeper/openid_connect_spec.rb b/spec/lib/doorkeeper/openid_connect_spec.rb new file mode 100644 index 0000000..610902a --- /dev/null +++ b/spec/lib/doorkeeper/openid_connect_spec.rb @@ -0,0 +1,16 @@ +require 'rails_helper' + +describe Doorkeeper::OpenidConnect do + describe 'SIGNING_ALGORITHM' do + it 'is hard-coded to RS256' do + expect(subject::SIGNING_ALGORITHM).to eq 'RS256' + end + end + + describe '.signing_key' do + it 'returns the private key as JWK instance' do + expect(subject.signing_key).to be_instance_of JSON::JWK + expect(subject.signing_key[:kid]).to eq 'IqYwZo2cE6hsyhs48cU8QHH4GanKIx0S4Dc99kgTIMA' + end + end +end