From ffda2dd29a15c83674d3b3f300d9564a9cd53ab5 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 21 Nov 2022 23:47:05 -0500 Subject: [PATCH] Add PKCE support to OpenID discovery endpoint RFC 8414 Section 2 (https://www.rfc-editor.org/rfc/rfc8414#section-2) says `code_challenge_methods_supported` must be listed to indicate that the authorization server supports PKCE. Doorkeeper only supports `plain` and `S256`, so return that if PKCE is enabled. --- CHANGELOG.md | 2 +- .../doorkeeper/openid_connect/discovery_controller.rb | 8 ++++++++ spec/controllers/discovery_controller_spec.rb | 5 +++++ spec/dummy/db/migrate/20221122044143_enable_pkce.rb | 8 ++++++++ spec/dummy/db/schema.rb | 4 +++- spec/lib/oauth/authorization/code_spec.rb | 2 ++ 6 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 spec/dummy/db/migrate/20221122044143_enable_pkce.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c331701..f1fdd80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## Unreleased -- [#] Add here +- [#180] Add PKCE support to OpenID discovery endpoint ## v1.8.2 (2022-07-13) diff --git a/app/controllers/doorkeeper/openid_connect/discovery_controller.rb b/app/controllers/doorkeeper/openid_connect/discovery_controller.rb index 5f4cc63..f834139 100644 --- a/app/controllers/doorkeeper/openid_connect/discovery_controller.rb +++ b/app/controllers/doorkeeper/openid_connect/discovery_controller.rb @@ -68,6 +68,8 @@ def provider_response exp iat ] | openid_connect.claims.to_h.keys, + + code_challenge_methods_supported: code_challenge_methods_supported(doorkeeper), }.compact end @@ -81,6 +83,12 @@ def response_modes_supported(doorkeeper) doorkeeper.authorization_response_flows.flat_map(&:response_mode_matches).uniq end + def code_challenge_methods_supported(doorkeeper) + return unless doorkeeper.access_grant_model.pkce_supported? + + %w[plain S256] + end + def webfinger_response { subject: params.require(:resource), diff --git a/spec/controllers/discovery_controller_spec.rb b/spec/controllers/discovery_controller_spec.rb index e09d4ab..68a0341 100644 --- a/spec/controllers/discovery_controller_spec.rb +++ b/spec/controllers/discovery_controller_spec.rb @@ -51,6 +51,11 @@ id_token_response user_info_response ], + + 'code_challenge_methods_supported' => %w[ + plain + S256 + ], }.sort) end diff --git a/spec/dummy/db/migrate/20221122044143_enable_pkce.rb b/spec/dummy/db/migrate/20221122044143_enable_pkce.rb new file mode 100644 index 0000000..96b4e1f --- /dev/null +++ b/spec/dummy/db/migrate/20221122044143_enable_pkce.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class EnablePkce < ActiveRecord::Migration[6.0] + def change + add_column :oauth_access_grants, :code_challenge, :string, null: true + add_column :oauth_access_grants, :code_challenge_method, :string, null: true + end +end diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index c1f944d..8c4e100 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_05_19_091115) do +ActiveRecord::Schema.define(version: 2022_11_22_044143) do create_table "oauth_access_grants", force: :cascade do |t| t.integer "resource_owner_id", null: false @@ -21,6 +21,8 @@ t.datetime "created_at", null: false t.datetime "revoked_at" t.string "scopes" + t.string "code_challenge" + t.string "code_challenge_method" t.index ["token"], name: "index_oauth_access_grants_on_token", unique: true end diff --git a/spec/lib/oauth/authorization/code_spec.rb b/spec/lib/oauth/authorization/code_spec.rb index 69c8cdf..727fc94 100644 --- a/spec/lib/oauth/authorization/code_spec.rb +++ b/spec/lib/oauth/authorization/code_spec.rb @@ -16,6 +16,8 @@ allow(pre_auth).to receive(:redirect_uri).and_return('redirect_uri') allow(pre_auth).to receive(:scopes).and_return('scopes') allow(pre_auth).to receive(:nonce).and_return('123456') + allow(pre_auth).to receive(:code_challenge).and_return('987654') + allow(pre_auth).to receive(:code_challenge_method).and_return('plain') allow(client).to receive(:id).and_return('client_id') allow(Doorkeeper::AccessGrant).to receive(:create!) { access_grant }