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

@@ -9,6 +9,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Nothing should go in this section, please add to the latest unreleased version
  (and update the corresponding date), or add a new version.
 
+## [1.19.1] - 2022-11-23
+### Added
+- Provides support for PKCE in the OIDC Authenticator code redirect workflow.
+ This is disabled by default, but is available under the
+ `CONJUR_FEATURE_POLICY_LOAD_EXTENSIONS` feature flag.
+ [cyberark/conjur#2678](https://github.com/cyberark/conjur/pull/2678)
+
 ## [1.19.0] - 2022-10-11
 
 ### Added

app/db/repository/authenticator_repository.rb

@@ -54,8 +54,17 @@ def load_authenticator(type:, account:, id:)
  args[variable.resource_id.split('/')[-1].underscore.to_sym] = variable.secret.value
  end
  end
+
  begin
- @data_object.new(**args_list)
+ if Rails.configuration.feature_flags.enabled?(:pkce_support)
+ allowed_args = %i[account service_id] +
+ @data_object.const_get(:REQUIRED_VARIABLES) +
+ @data_object.const_get(:OPTIONAL_VARIABLES)
+ allowed_arg_list = args_list.select{ |key, _| allowed_args.include?(key) }
+ @data_object.new(**allowed_arg_list)
+ else
+ @data_object.new(**args_list)
+ end
  rescue ArgumentError => e
  @logger.debug("DB::Repository::AuthenticatorRepository.load_authenticator - exception: #{e}")
  nil

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 Exception => 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,52 @@
+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
+ attr_reader :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,43 @@
+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
+
+ # Don't love this name...
+ 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/handler/authentication_handler.rb

@@ -31,10 +31,12 @@ def initialize(
  end
 
  def call(parameters:, request_ip:)
- required_parameters = %i[state code]
- required_parameters.each do |parameter|
- if !parameters.key?(parameter) || parameters[parameter].strip.empty?
- raise Errors::Authentication::RequestBody::MissingRequestParam, parameter
+ unless Rails.configuration.feature_flags.enabled?(:pkce_support)
+ required_parameters = %i[state code]
+ required_parameters.each do |parameter|
+ if !parameters.key?(parameter) || parameters[parameter].strip.empty?
+ raise Errors::Authentication::RequestBody::MissingRequestParam, parameter
+ end
  end
  end
 

app/domain/authentication/util/namespace_selector.rb

@@ -9,7 +9,11 @@ def self.select(authenticator_type:)
  # 'V2' is a bit of a hack to handle the fact that
  # the original OIDC authenticator is really a
  # glorified JWT authenticator.
- 'Authentication::AuthnOidc::V2'
+ if Rails.configuration.feature_flags.enabled?(:pkce_support)
+ 'Authentication::AuthnOidc::PkceSupportFeature'
+ else
+ 'Authentication::AuthnOidc::V2'
+ end
  else
  raise "'#{authenticator_type}' is not a supported authenticator type"
  # TODO: make this dynamic based on authenticator type.