Adds PKCE Support

This commit adds support for PKCE to improve the security posture of OIDC Authentication. It also includes
small improvements to make the Authentication Handler and Authenticator Repository more generic and
durable.

This feature is behind a feature flag as the UI requires changes to support this improved workflow.

CHANGELOG.md

@@ -16,6 +16,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
  the OIDC provider endpoint would include duplicate OIDC authenticators. This
  change resolves ONYX-25530.
 
+### Added
+- Provides support for PKCE in the OIDC Authenticator code redirect workflow.
+ This is disabled by default, but is available under the
+ `CONJUR_FEATURE_PKCE_SUPPORT_ENABLED` feature flag.
+ [cyberark/conjur#2678](https://github.com/cyberark/conjur/pull/2678)
+
 ## [1.19.1] - 2022-12-08
 
 ### Security

app/db/repository/authenticator_repository.rb

@@ -1,10 +1,16 @@
 module DB
  module Repository
  class AuthenticatorRepository
- def initialize(data_object:, resource_repository: ::Resource, logger: Rails.logger)
+ def initialize(
+ data_object:,
+ resource_repository: ::Resource,
+ logger: Rails.logger,
+ pkce_support_enabled: Rails.configuration.feature_flags.enabled?(:pkce_support)
+ )
  @resource_repository = resource_repository
  @data_object = data_object
  @logger = logger
+ @pkce_support_enabled = pkce_support_enabled
  end
 
  def find_all(type:, account:)
@@ -65,7 +71,14 @@ def load_authenticator(type:, account:, service_id:)
  args[variable.resource_id.split('/')[-1].underscore.to_sym] = variable.secret.value
  end
  end
+
  begin
+ if @pkce_support_enabled
+ allowed_args = %i[account service_id] +
+ @data_object.const_get(:REQUIRED_VARIABLES) +
+ @data_object.const_get(:OPTIONAL_VARIABLES)
+ args_list = args_list.select{ |key, _| allowed_args.include?(key) }
+ end
  @data_object.new(**args_list)
  rescue ArgumentError => e
  @logger.debug("DB::Repository::AuthenticatorRepository.load_authenticator - exception: #{e}")

app/domain/authentication/authn_oidc/pkce_support_feature/client.rb

@@ -0,0 +1,117 @@
+module Authentication
+ module AuthnOidc
+ module PkceSupportFeature
+ class Client
+ def initialize(
+ authenticator:,
+ client: ::OpenIDConnect::Client,
+ oidc_id_token: ::OpenIDConnect::ResponseObject::IdToken,
+ discovery_configuration: ::OpenIDConnect::Discovery::Provider::Config,
+ cache: Rails.cache,
+ logger: Rails.logger
+ )
+ @authenticator = authenticator
+ @client = client
+ @oidc_id_token = oidc_id_token
+ @discovery_configuration = discovery_configuration
+ @cache = cache
+ @logger = logger
+ end
+
+ def oidc_client
+ @oidc_client ||= begin
+ issuer_uri = URI(@authenticator.provider_uri)
+ @client.new(
+ identifier: @authenticator.client_id,
+ secret: @authenticator.client_secret,
+ redirect_uri: @authenticator.redirect_uri,
+ scheme: issuer_uri.scheme,
+ host: issuer_uri.host,
+ port: issuer_uri.port,
+ authorization_endpoint: URI(discovery_information.authorization_endpoint).path,
+ token_endpoint: URI(discovery_information.token_endpoint).path,
+ userinfo_endpoint: URI(discovery_information.userinfo_endpoint).path,
+ jwks_uri: URI(discovery_information.jwks_uri).path,
+ end_session_endpoint: URI(discovery_information.end_session_endpoint).path
+ )
+ end
+ end
+
+ def callback(code:, nonce:, code_verifier:)
+ oidc_client.authorization_code = code
+ begin
+ bearer_token = oidc_client.access_token!(
+ scope: true,
+ client_auth_method: :basic,
+ code_verifier: code_verifier
+ )
+ rescue Rack::OAuth2::Client::Error => e
+ # Only handle the expected errors related to access token retrieval.
+ case e.message
+ when /PKCE verification failed/
+ raise Errors::Authentication::AuthnOidc::TokenRetrievalFailed,
+ 'PKCE verification failed'
+ when /The authorization code is invalid or has expired/
+ raise Errors::Authentication::AuthnOidc::TokenRetrievalFailed,
+ 'Authorization code is invalid or has expired'
+ when /Code not valid/
+ raise Errors::Authentication::AuthnOidc::TokenRetrievalFailed,
+ 'Authorization code is invalid'
+ end
+ raise e
+ end
+ id_token = bearer_token.id_token || bearer_token.access_token
+
+ begin
+ attempts ||= 0
+ decoded_id_token = @oidc_id_token.decode(
+ id_token,
+ discovery_information.jwks
+ )
+ rescue StandardError => e
+ attempts += 1
+ raise e if attempts > 1
+
+ # If the JWKS verification fails, blow away the existing cache and
+ # try again. This is intended to handle the case where the OIDC certificate
+ # changes, and we want to cache the new certificate without decode failing.
+ discovery_information(invalidate: true)
+ retry
+ end
+
+ begin
+ decoded_id_token.verify!(
+ issuer: @authenticator.provider_uri,
+ client_id: @authenticator.client_id,
+ nonce: nonce
+ )
+ rescue OpenIDConnect::ResponseObject::IdToken::InvalidNonce
+ raise Errors::Authentication::AuthnOidc::TokenVerificationFailed,
+ 'Provided nonce does not match the nonce in the JWT'
+ rescue OpenIDConnect::ResponseObject::IdToken::ExpiredToken
+ raise Errors::Authentication::AuthnOidc::TokenVerificationFailed,
+ 'JWT has expired'
+ rescue OpenIDConnect::ValidationFailed => e
+ raise Errors::Authentication::AuthnOidc::TokenVerificationFailed,
+ e.message
+ end
+ decoded_id_token
+ end
+
+ def discovery_information(invalidate: false)
+ @cache.fetch(
+ "#{@authenticator.account}/#{@authenticator.service_id}/#{URI::Parser.new.escape(@authenticator.provider_uri)}",
+ force: invalidate,
+ skip_nil: true
+ ) do
+ @discovery_configuration.discover!(@authenticator.provider_uri)
+ rescue HTTPClient::ConnectTimeoutError, Errno::ETIMEDOUT => e
+ raise Errors::Authentication::OAuth::ProviderDiscoveryTimeout.new(@authenticator.provider_uri, e.message)
+ rescue => e
+ raise Errors::Authentication::OAuth::ProviderDiscoveryFailed.new(@authenticator.provider_uri, e.message)
+ end
+ end
+ end
+ end
+ end
+end

app/domain/authentication/authn_oidc/pkce_support_feature/data_objects/authenticator.rb

@@ -0,0 +1,60 @@
+module Authentication
+ module AuthnOidc
+ module PkceSupportFeature
+ module DataObjects
+ class Authenticator
+
+ REQUIRED_VARIABLES = %i[provider_uri client_id client_secret claim_mapping].freeze
+ OPTIONAL_VARIABLES = %i[redirect_uri response_type provider_scope name].freeze
+
+ attr_reader(
+ :provider_uri,
+ :client_id,
+ :client_secret,
+ :claim_mapping,
+ :account,
+ :service_id,
+ :redirect_uri,
+ :response_type
+ )
+
+ def initialize(
+ provider_uri:,
+ client_id:,
+ client_secret:,
+ claim_mapping:,
+ account:,
+ service_id:,
+ redirect_uri: nil,
+ name: nil,
+ response_type: 'code',
+ provider_scope: nil
+ )
+ @account = account
+ @provider_uri = provider_uri
+ @client_id = client_id
+ @client_secret = client_secret
+ @claim_mapping = claim_mapping
+ @response_type = response_type
+ @service_id = service_id
+ @name = name
+ @provider_scope = provider_scope
+ @redirect_uri = redirect_uri
+ end
+
+ def scope
+ (%w[openid email profile] + [*@provider_scope.to_s.split(' ')]).uniq.join(' ')
+ end
+
+ def name
+ @name || @service_id.titleize
+ end
+
+ def resource_id
+ "#{account}:webservice:conjur/authn-oidc/#{service_id}"
+ end
+ end
+ end
+ end
+ end
+end

app/domain/authentication/authn_oidc/pkce_support_feature/resolve_identity.rb

@@ -0,0 +1,19 @@
+module Authentication
+ module AuthnOidc
+ module PkceSupportFeature
+ class ResolveIdentity
+ def call(identity:, account:, allowed_roles:)
+ # make sure role has a resource (ex. user, host)
+ roles = allowed_roles.select(&:resource?)
+
+ roles.each do |role|
+ role_account, _, role_id = role.id.split(':')
+ return role if role_account == account && identity == role_id
+ end
+
+ raise(Errors::Authentication::Security::RoleNotFound, identity)
+ end
+ end
+ end
+ end
+end

app/domain/authentication/authn_oidc/pkce_support_feature/strategy.rb

@@ -0,0 +1,42 @@
+module Authentication
+ module AuthnOidc
+ module PkceSupportFeature
+ class Strategy
+ def initialize(
+ authenticator:,
+ client: Authentication::AuthnOidc::PkceSupportFeature::Client,
+ logger: Rails.logger
+ )
+ @authenticator = authenticator
+ @client = client.new(authenticator: authenticator)
+ @logger = logger
+ end
+
+ def callback(args)
+ %i[code nonce code_verifier].each do |param|
+ unless args[param].present?
+ raise Errors::Authentication::RequestBody::MissingRequestParam, param.to_s
+ end
+ end
+
+ identity = resolve_identity(
+ jwt: @client.callback(
+ code: args[:code],
+ nonce: args[:nonce],
+ code_verifier: args[:code_verifier]
+ )
+ )
+ unless identity.present?
+ raise Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty,
+ @authenticator.claim_mapping
+ end
+ identity
+ end
+
+ def resolve_identity(jwt:)
+ jwt.raw_attributes.with_indifferent_access[@authenticator.claim_mapping]
+ end
+ end
+ end
+ end
+end

app/domain/authentication/authn_oidc/pkce_support_feature/views/provider_context.rb

@@ -0,0 +1,60 @@
+require 'securerandom'
+require 'digest'
+
+module Authentication
+ module AuthnOidc
+ module PkceSupportFeature
+ module Views
+ class ProviderContext
+ def initialize(
+ client: Authentication::AuthnOidc::V2::Client,
+ digest: Digest::SHA256,
+ random: SecureRandom,
+ logger: Rails.logger
+ )
+ @client = client
+ @logger = logger
+ @digest = digest
+ @random = random
+ end
+
+ def call(authenticators:)
+ authenticators.map do |authenticator|
+ nonce = @random.hex(25)
+ code_verifier = @random.hex(25)
+ code_challenge = @digest.base64digest(code_verifier).tr("+/", "-_").tr("=", "")
+ {
+ service_id: authenticator.service_id,
+ type: 'authn-oidc',
+ name: authenticator.name,
+ nonce: nonce,
+ code_verifier: code_verifier,
+ redirect_uri: generate_redirect_url(
+ client: @client.new(authenticator: authenticator),
+ authenticator: authenticator,
+ nonce: nonce,
+ code_challenge: code_challenge
+ )
+ }
+ end
+ end
+
+ def generate_redirect_url(client:, authenticator:, nonce:, code_challenge:)
+ params = {
+ client_id: authenticator.client_id,
+ response_type: authenticator.response_type,
+ scope: ERB::Util.url_encode(authenticator.scope),
+ nonce: nonce,
+ code_challenge: code_challenge,
+ code_challenge_method: 'S256',
+ redirect_uri: ERB::Util.url_encode(authenticator.redirect_uri)
+ }
+ formatted_params = params.map { |key, value| "#{key}=#{value}" }.join("&")
+
+ "#{client.discovery_information.authorization_endpoint}?#{formatted_params}"
+ end
+ end
+ end
+ end
+ end
+end

app/domain/authentication/authn_oidc/v2/client.rb

@@ -56,7 +56,7 @@ def callback(code:)
  id_token,
  discovery_information.jwks
  )
- rescue Exception => e
+ rescue StandardError => e
  attempts += 1
  raise e if attempts > 1
 

app/domain/authentication/authn_oidc/v2/status.rb

@@ -0,0 +1,13 @@
+module Authentication
+ module AuthnOidc
+ module V2
+ class Status
+ def call(authenticator:)
+ {
+
+ }
+ end
+ end
+ end
+ end
+end

app/domain/authentication/authn_oidc/v2/strategy.rb

@@ -13,7 +13,6 @@ def initialize(
  @logger = logger
  end
 
- # Don't love this name...
  def callback(args)
  # TODO: Check that `code` and `state` attributes are present
  raise Errors::Authentication::AuthnOidc::StateMismatch unless args[:state] == @authenticator.state