From c92e69fa4a39d77abe5d8312357bc2b82d427e84 Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Sun, 9 Nov 2025 11:54:45 +1100 Subject: [PATCH] Add PCKE --- VERSION | 1 + app/controllers/oidc_controller.rb | 88 ++++++++++++++++++++++++++- app/models/oidc_authorization_code.rb | 13 ++++ config/environments/test.rb | 2 +- db/schema.rb | 5 +- 5 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 VERSION diff --git a/VERSION b/VERSION new file mode 100644 index 0000000..5c27e65 --- /dev/null +++ b/VERSION @@ -0,0 +1 @@ +2025.02 diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb index 1f8dd5c..340afd3 100644 --- a/app/controllers/oidc_controller.rb +++ b/app/controllers/oidc_controller.rb @@ -15,11 +15,13 @@ class OidcController < ApplicationController jwks_uri: "#{base_url}/.well-known/jwks.json", end_session_endpoint: "#{base_url}/logout", response_types_supported: ["code"], + response_modes_supported: ["query"], subject_types_supported: ["public"], id_token_signing_alg_values_supported: ["RS256"], scopes_supported: ["openid", "profile", "email", "groups"], token_endpoint_auth_methods_supported: ["client_secret_post", "client_secret_basic"], - claims_supported: ["sub", "email", "email_verified", "name", "preferred_username", "groups", "admin"] + claims_supported: ["sub", "email", "email_verified", "name", "preferred_username", "groups", "admin"], + code_challenge_methods_supported: ["plain", "S256"] } render json: config @@ -39,6 +41,8 @@ class OidcController < ApplicationController nonce = params[:nonce] scope = params[:scope] || "openid" response_type = params[:response_type] + code_challenge = params[:code_challenge] + code_challenge_method = params[:code_challenge_method] || "plain" # Validate required parameters unless client_id.present? && redirect_uri.present? && response_type == "code" @@ -46,6 +50,20 @@ class OidcController < ApplicationController return end + # Validate PKCE parameters if present + if code_challenge.present? + unless %w[plain S256].include?(code_challenge_method) + render plain: "Invalid code_challenge_method. Supported: plain, S256", status: :bad_request + return + end + + # Validate code challenge format (base64url-encoded, 43-128 characters) + unless code_challenge.match?(/\A[A-Za-z0-9\-_]{43,128}\z/) + render plain: "Invalid code_challenge format. Must be 43-128 characters of base64url encoding", status: :bad_request + return + end + end + # Find the application @application = Application.find_by(client_id: client_id, app_type: "oidc") unless @application @@ -67,7 +85,9 @@ class OidcController < ApplicationController redirect_uri: redirect_uri, state: state, nonce: nonce, - scope: scope + scope: scope, + code_challenge: code_challenge, + code_challenge_method: code_challenge_method } redirect_to signin_path, alert: "Please sign in to continue" return @@ -96,6 +116,8 @@ class OidcController < ApplicationController redirect_uri: redirect_uri, scope: scope, nonce: nonce, + code_challenge: code_challenge, + code_challenge_method: code_challenge_method, expires_at: 10.minutes.from_now ) @@ -112,7 +134,9 @@ class OidcController < ApplicationController redirect_uri: redirect_uri, state: state, nonce: nonce, - scope: scope + scope: scope, + code_challenge: code_challenge, + code_challenge_method: code_challenge_method } # Render consent page @@ -165,6 +189,8 @@ class OidcController < ApplicationController redirect_uri: oauth_params['redirect_uri'], scope: oauth_params['scope'], nonce: oauth_params['nonce'], + code_challenge: oauth_params['code_challenge'], + code_challenge_method: oauth_params['code_challenge_method'], expires_at: 10.minutes.from_now ) @@ -205,6 +231,7 @@ class OidcController < ApplicationController # Get the authorization code code = params[:code] redirect_uri = params[:redirect_uri] + code_verifier = params[:code_verifier] auth_code = OidcAuthorizationCode.find_by( application: application, @@ -229,6 +256,11 @@ class OidcController < ApplicationController return end + # Validate PKCE if code challenge is present + unless validate_pkce(auth_code, code_verifier) + return + end + # Mark code as used auth_code.update!(used: true) @@ -342,6 +374,56 @@ class OidcController < ApplicationController private + def validate_pkce(auth_code, code_verifier) + # Skip PKCE validation if no code challenge was stored (legacy clients) + return true unless auth_code.code_challenge.present? + + # PKCE is required but no verifier provided + unless code_verifier.present? + render json: { + error: "invalid_request", + error_description: "code_verifier is required when code_challenge was provided" + }, status: :bad_request + return false + end + + # Validate code verifier format (base64url-encoded, 43-128 characters) + unless code_verifier.match?(/\A[A-Za-z0-9\-_]{43,128}\z/) + render json: { + error: "invalid_request", + error_description: "Invalid code_verifier format. Must be 43-128 characters of base64url encoding" + }, status: :bad_request + return false + end + + # Recreate code challenge based on method + expected_challenge = case auth_code.code_challenge_method + when "plain" + code_verifier + when "S256" + Digest::SHA256.base64digest(code_verifier) + .tr("+/", "-_") + .tr("=", "") + else + render json: { + error: "server_error", + error_description: "Unsupported code challenge method" + }, status: :internal_server_error + return false + end + + # Validate the code challenge + unless auth_code.code_challenge == expected_challenge + render json: { + error: "invalid_grant", + error_description: "Invalid code verifier" + }, status: :bad_request + return false + end + + true + end + def extract_client_credentials # Try Authorization header first (Basic auth) if request.headers["Authorization"]&.start_with?("Basic ") diff --git a/app/models/oidc_authorization_code.rb b/app/models/oidc_authorization_code.rb index dd9c598..bca0784 100644 --- a/app/models/oidc_authorization_code.rb +++ b/app/models/oidc_authorization_code.rb @@ -7,6 +7,8 @@ class OidcAuthorizationCode < ApplicationRecord validates :code, presence: true, uniqueness: true validates :redirect_uri, presence: true + validates :code_challenge_method, inclusion: { in: %w[plain S256], allow_nil: true } + validate :validate_code_challenge_format, if: -> { code_challenge.present? } scope :valid, -> { where(used: false).where("expires_at > ?", Time.current) } scope :expired, -> { where("expires_at <= ?", Time.current) } @@ -23,6 +25,10 @@ class OidcAuthorizationCode < ApplicationRecord update!(used: true) end + def uses_pkce? + code_challenge.present? + end + private def generate_code @@ -32,4 +38,11 @@ class OidcAuthorizationCode < ApplicationRecord def set_expiry self.expires_at ||= 10.minutes.from_now end + + def validate_code_challenge_format + # PKCE code challenge should be base64url-encoded, 43-128 characters + unless code_challenge.match?(/\A[A-Za-z0-9\-_]{43,128}\z/) + errors.add(:code_challenge, "must be 43-128 characters of base64url encoding") + end + end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 74a1d2d..6d15427 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -53,5 +53,5 @@ Rails.application.configure do # Disable Sentry in test environment to avoid interference with tests # Sentry can be explicitly enabled for integration testing if needed - config.sentry.enabled = false + ENV["SENTRY_ENABLED_IN_DEVELOPMENT"] = "false" end diff --git a/db/schema.rb b/db/schema.rb index 8500aba..ddaba2d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2025_11_04_064114) do +ActiveRecord::Schema[8.1].define(version: 2025_11_08_090123) do create_table "application_groups", force: :cascade do |t| t.integer "application_id", null: false t.datetime "created_at", null: false @@ -69,6 +69,8 @@ ActiveRecord::Schema[8.1].define(version: 2025_11_04_064114) do create_table "oidc_authorization_codes", force: :cascade do |t| t.integer "application_id", null: false t.string "code", null: false + t.string "code_challenge" + t.string "code_challenge_method" t.datetime "created_at", null: false t.datetime "expires_at", null: false t.string "nonce" @@ -80,6 +82,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_11_04_064114) do t.index ["application_id", "user_id"], name: "index_oidc_authorization_codes_on_application_id_and_user_id" t.index ["application_id"], name: "index_oidc_authorization_codes_on_application_id" t.index ["code"], name: "index_oidc_authorization_codes_on_code", unique: true + t.index ["code_challenge"], name: "index_oidc_authorization_codes_on_code_challenge" t.index ["expires_at"], name: "index_oidc_authorization_codes_on_expires_at" t.index ["user_id"], name: "index_oidc_authorization_codes_on_user_id" end