From 5b58f2f1bf5ed1f91c41d15b87bee58caece7eea Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Thu, 19 Oct 2023 10:06:10 -0600 Subject: [PATCH 1/8] Initial rework & migrate API key authentication to the new format --- app/controllers/authenticate_controller.rb | 15 ++ .../authenticator_role_repository.rb | 134 +++++++++++ .../v2/data_objects/authenticator.rb | 36 +++ .../authn_api_key/v2/strategy.rb | 70 ++++++ app/domain/authentication/base/data_object.rb | 44 ++++ .../authentication/base/role_identifier.rb | 39 ++++ .../handler/authentication_handler.rb | 208 +++++++++++------- .../installed_authenticators.rb | 6 +- .../authentication/util/namespace_selector.rb | 2 + app/domain/responses.rb | 5 +- config/routes.rb | 4 + dev/start | 4 +- 12 files changed, 487 insertions(+), 80 deletions(-) create mode 100644 app/db/repository/authenticator_role_repository.rb create mode 100644 app/domain/authentication/authn_api_key/v2/data_objects/authenticator.rb create mode 100644 app/domain/authentication/authn_api_key/v2/strategy.rb create mode 100644 app/domain/authentication/base/data_object.rb create mode 100644 app/domain/authentication/base/role_identifier.rb diff --git a/app/controllers/authenticate_controller.rb b/app/controllers/authenticate_controller.rb index 12e8b73c2b..7827683963 100644 --- a/app/controllers/authenticate_controller.rb +++ b/app/controllers/authenticate_controller.rb @@ -4,6 +4,21 @@ class AuthenticateController < ApplicationController include BasicAuthenticator include AuthorizeResource + def authenticate_via_post + auth_token = Authentication::Handler::AuthenticationHandler.new( + authenticator_type: params[:authenticator] + ).call( + parameters: params, + request_body: request.body.read, + request_ip: request.ip + ) + + render_authn_token(auth_token) + rescue => e + log_backtrace(e) + raise e + end + def oidc_authenticate_code_redirect # TODO: need a mechanism for an authenticator strategy to define the required # params. This will likely need to be done via the Handler. diff --git a/app/db/repository/authenticator_role_repository.rb b/app/db/repository/authenticator_role_repository.rb new file mode 100644 index 0000000000..3f7d752914 --- /dev/null +++ b/app/db/repository/authenticator_role_repository.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +module DB + module Repository + # This class is responsible for loading the variables associated with a + # particular type of authenticator. Each authenticator requires a Data + # Object and Data Object Contract (for validation). Data Objects that + # fail validation are not returned. + # + # This class includes one public methods: + # - `find` - returns a single authenticator based on the provided type, + # account, and service identifier. + # + class AuthenticatorRoleRepository + def initialize(authenticator:, role: Role, logger: Rails.logger) + @authenticator = authenticator + @role = role + @logger = logger + end + + def find(role_identifier:, role_contract: nil) + role = @role[role_identifier.conjur_role] + unless role.present? + raise(Errors::Authentication::Security::RoleNotFound, role_identifier.role_for_error) + end + + return role unless role.resource? + + relevant_annotations = relevant_annotations( + annotations: {}.tap { |h| role.resource.annotations.each {|a| h[a.name] = a.value }}, + authenticator: @authenticator + ) + + validate_role_annotations_against_contract( + annotations: relevant_annotations, + role_contract: role_contract + ) + + annotations_match?( + role_annotations: relevant_annotations, + target_annotations: role_identifier.annotations + ) + + role + end + + private + + def validate_role_annotations_against_contract(annotations:, role_contract:) + # If authenticator requires annotations, verify some are present + if @authenticator.annotations_required && annotations.empty? + raise(Errors::Authentication::Constraints::RoleMissingAnyRestrictions) + end + + # Only run contract validations if they are present + return if role_contract.nil? + + annotations.each do |annotation, value| + annotation_valid = role_contract.new(authenticator: @authenticator, utils: ::Util::ContractUtils).call( + annotation: annotation, + annotation_value: value, + annotations: annotations + ) + next if annotation_valid.success? + + raise(annotation_valid.errors.first.meta[:exception]) + end + end + + # Need to account for the following two options: + # Annotations relevant to specific authenticator + # - !host + # id: myapp + # annotations: + # authn-jwt/raw/ref: valid + + # Annotations relevant to type of authenticator + # - !host + # id: myapp + # annotations: + # authn-jwt/project_id: myproject + # authn-jwt/aud: myaud + + def relevant_annotations(annotations:, authenticator:) + # Verify that at least one service specific auth token is present + if annotations.keys.any?{|k,_|k.include?(authenticator.type.to_s) } && + !annotations.keys.any?{|k,_|k.include?("#{authenticator.type}/#{authenticator.service_id}") } + raise(Errors::Authentication::Constraints::RoleMissingAnyRestrictions) + end + + generic = annotations + .select{|k, _| k.count('/') == 1 } + .select{|k, _| k.match?(%r{^authn-jwt/})} + .reject{|k, _| k.match?(%r{^authn-jwt/#{authenticator.service_id}})} + .transform_keys{|k| k.split('/').last} + + specific = annotations + .select{|k, _| k.count('/') > 1 } + .select{|k, _| k.match?(%r{^authn-jwt/#{authenticator.service_id}/})} + .transform_keys{|k| k.split('/').last} + + generic.merge(specific) + # relevant_annotations = generic.merge(specific) + + # validate_role_annotations(annotations: relevant_annotations, authenticator: authenticator, relevant_annotations: relevant_annotations) + # relevant_annotations + end + + def annotations_match?(role_annotations:, target_annotations:) + # If there are no annotations to match, just return + return if target_annotations.empty? + + role_annotations.each do |annotation, value| + next unless annotation.present? + + @logger.debug(LogMessages::Authentication::ResourceRestrictions::ValidatingResourceRestrictionOnRequest.new(annotation)) + if target_annotations.key?(annotation) && target_annotations[annotation] == value + @logger.debug(LogMessages::Authentication::ResourceRestrictions::ValidatedResourceRestrictionsValues.new(annotation)) + next + end + + unless target_annotations.key?(annotation) + raise(Errors::Authentication::AuthnJwt::JwtTokenClaimIsMissing, annotation) + end + + raise(Errors::Authentication::ResourceRestrictions::InvalidResourceRestrictions, annotation) + end + + @logger.debug(LogMessages::Authentication::ResourceRestrictions::ValidatedResourceRestrictions.new) + @logger.debug(LogMessages::Authentication::AuthnJwt::ValidateRestrictionsPassed.new) + end + end + end +end diff --git a/app/domain/authentication/authn_api_key/v2/data_objects/authenticator.rb b/app/domain/authentication/authn_api_key/v2/data_objects/authenticator.rb new file mode 100644 index 0000000000..481d6ffa26 --- /dev/null +++ b/app/domain/authentication/authn_api_key/v2/data_objects/authenticator.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Authentication + module AuthnApiKey + module V2 + module DataObjects + + # This DataObject encapsulates the data required for an Authn-IAM + # authenticator. + # + class Authenticator < Authentication::Base::DataObject + + REQUIRES_ROLE_ANNOTIONS = false + + attr_reader(:account) + + # Authn API Key has no variables. + # Service ID is ignored as + # + # rubocop:disable Lint/MissingSuper + def initialize(account:) + @account = account + end + # rubocop:enable Lint/MissingSuper + + # Override type as this class's name does not match the expected + # value of 'authn'. This is because API key is the default mechanism + # for logging into Conjur. + def type + 'authn' + end + end + end + end + end +end diff --git a/app/domain/authentication/authn_api_key/v2/strategy.rb b/app/domain/authentication/authn_api_key/v2/strategy.rb new file mode 100644 index 0000000000..f4c68a2226 --- /dev/null +++ b/app/domain/authentication/authn_api_key/v2/strategy.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +# Conjur API authenticator +module Authentication + module AuthnApiKey + module V2 + class Strategy + + # This authenticator is a bit different because it validates based on the + # information stored in the Conjur database. As such, Role and Credential + # are made available. Longer term, they should probably become part of this + # authenticator. + def initialize(authenticator:, logger: Rails.logger, credentials: ::Credentials, role: ::Role) + @authenticator = authenticator + @logger = logger + @credentials = credentials + @role = role + @success = ::SuccessResponse + @failure = ::FailureResponse + end + + def callback(request_body:, parameters:) + # TODO: Check that `id` is present in the parameters list + + role_id = parameters['id'] + api_key = request_body + + role_id = if role_id.match?(/^host\/./) + id = role_id.split('/')[1..-1].join + "#{@authenticator.account}:host:#{id}" + else + "#{@authenticator.account}:user:#{role_id}" + end + + role_identifier = Authentication::Base::RoleIdentifier.new( + role_identifier: role_id + ) + + if @role[role_id].nil? + return @failure.new( + role_identifier, + exception: Errors::Authentication::Security::RoleNotFound.new(role_id) + ) + end + + role_credentials = @credentials[role_id] + if role_credentials.nil? + return @failure.new( + role_identifier, + exception: Errors::Authentication::RoleHasNoCredentials.new(role_id) + ) + end + + return @success.new(role_identifier) if role_credentials.valid_api_key?(api_key) + + @failure.new( + role_identifier, + exception: Errors::Conjur::ApiKeyNotFound.new(role_id) + ) + end + + # Called by status handler. This handles checking as much of the strategy + # integrity as possible without performing an actual authentication. + def verify_status + true + end + end + end + end +end diff --git a/app/domain/authentication/base/data_object.rb b/app/domain/authentication/base/data_object.rb new file mode 100644 index 0000000000..3982c2a61a --- /dev/null +++ b/app/domain/authentication/base/data_object.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Authentication + module Base + class DataObject + + def type + # TODO: dynamically find this based on class name. + # 'authn-jwt' + @type ||= self.class.to_s.split('::')[1].underscore.dasherize + end + + def resource_id + [ + @account, + 'webservice', + [ + 'conjur', + type, + @service_id + ].compact.join('/') + ].join(':') + end + + + def token_ttl + ttl = @token_ttl.present? ? @token_ttl : 'PT8M' + ActiveSupport::Duration.parse(ttl.to_s) + rescue ActiveSupport::Duration::ISO8601Parser::ParsingError + raise Errors::Authentication::DataObjects::InvalidTokenTTL.new(resource_id, @token_ttl) + end + + + def annotations_required + # binding.pry + requires_role_annotations = self.class::REQUIRES_ROLE_ANNOTIONS + return requires_role_annotations unless requires_role_annotations.nil? + + raise "class constant 'REQUIRES_ROLED_ANNOTIONS' must be defined" + end + + end + end +end diff --git a/app/domain/authentication/base/role_identifier.rb b/app/domain/authentication/base/role_identifier.rb new file mode 100644 index 0000000000..c4d1096483 --- /dev/null +++ b/app/domain/authentication/base/role_identifier.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Authentication + module Base + class RoleIdentifier + attr_reader :role_identifier, :annotations #, :exception + + def initialize(role_identifier:, annotations: {}, exception: nil) + @role_identifier = role_identifier + @annotations = annotations + # @exception = exception + end + + def type + @role_identifier.split(':')[1] + end + + def account + @role_identifier.split(':')[0] + end + + # Role identifier within the account and type context: + # :: + def id + @role_identifier.split(':')[2] + end + + # Essentially just an alias, but doing it this way to + # avoid duplicate alias Rubocop warning. + def conjur_role + @role_identifier + end + + def role_for_error + type == 'host' ? "host/#{id}" : id + end + end + end +end diff --git a/app/domain/authentication/handler/authentication_handler.rb b/app/domain/authentication/handler/authentication_handler.rb index 8e2a36d22c..7488947ee6 100644 --- a/app/domain/authentication/handler/authentication_handler.rb +++ b/app/domain/authentication/handler/authentication_handler.rb @@ -5,142 +5,200 @@ module Handler class AuthenticationHandler def initialize( authenticator_type:, - role: ::Role, - resource: ::Resource, authn_repo: DB::Repository::AuthenticatorRepository, namespace_selector: Authentication::Util::NamespaceSelector, logger: Rails.logger, - authentication_error: LogMessages::Authentication::AuthenticationError + audit_logger: ::Audit.logger, + authentication_error: LogMessages::Authentication::AuthenticationError, + available_authenticators: Authentication::InstalledAuthenticators, + role_repository: DB::Repository::AuthenticatorRoleRepository ) - @role = role - @resource = resource @authenticator_type = authenticator_type @logger = logger + @audit_logger = audit_logger @authentication_error = authentication_error + @available_authenticators = available_authenticators + @role_repository = role_repository # Dynamically load authenticator specific classes - namespace = namespace_selector.select( + @namespace = namespace_selector.select( authenticator_type: authenticator_type ) - @identity_resolver = "#{namespace}::ResolveIdentity".constantize - @strategy = "#{namespace}::Strategy".constantize + @strategy = "#{@namespace}::Strategy".constantize @authn_repo = authn_repo.new( - data_object: "#{namespace}::DataObjects::Authenticator".constantize + data_object: "#{@namespace}::DataObjects::Authenticator".constantize ) + @contract = set_if_present { "#{@namespace}::Validations::AuthenticatorContract".constantize.new(utils: ::Util::ContractUtils) } + @role_contract = set_if_present { "#{@namespace}::Validations::RoleContract".constantize } end - def call(parameters:, request_ip:) - # Load Authenticator policy and values (validates data stored as variables) - authenticator = @authn_repo.find( - type: @authenticator_type, + def set_if_present(&block) + block.call + rescue NameError + nil + end + + def params_allowed + allowed = %i[authenticator service_id account] + allowed += @strategy::ALLOWED_PARAMS if @strategy.const_defined?('ALLOWED_PARAMS') + allowed + end + + def call(request_ip:, parameters:, request_body: nil, action: nil) + authenticator_identifier = [parameters[:authenticator], parameters[:service_id]].compact.join('/') + + authenticator = retrieve_authenticator( + identifier: authenticator_identifier, account: parameters[:account], service_id: parameters[:service_id] ) - if authenticator.nil? - raise( - Errors::Conjur::RequestedResourceNotFound, - "Unable to find authenticator with account: #{parameters[:account]} and service-id: #{parameters[:service_id]}" - ) + role_identifier_response = @strategy.new( + authenticator: authenticator + ).callback(parameters: parameters, request_body: request_body) + + if role_identifier_response.success? + role_identifier = role_identifier_response.result + else + role = role_identifier_response.message.role_identifier + raise role_identifier_response.exception end - role = @identity_resolver.new.call( - identity: @strategy.new( - authenticator: authenticator - ).callback(parameters), - account: parameters[:account], - allowed_roles: @role.that_can( - :authenticate, - @resource[authenticator.resource_id] - ).all + role = @role_repository.new( + authenticator: authenticator + ).find( + role_identifier: role_identifier, + role_contract: @role_contract ) - # TODO: Add an error message - raise 'failed to authenticate' unless role + # Add an error message (this may actually never be hit as we raise + # upstream if there is a problem with authentication & lookup) + raise Errors::Authorization::AuthenticationFailed unless role + + permitted?( + role: role, + authenticator_identifier: authenticator_identifier, + account: parameters[:account] + ) unless role.valid_origin?(request_ip) raise Errors::Authentication::InvalidOrigin end - log_audit_success(authenticator, role, request_ip, @authenticator_type) + log_audit_success(authenticator, role.role_id, request_ip, authenticator.type) TokenFactory.new.signed_token( account: parameters[:account], - username: role.role_id.split(':').last, + username: role.login, user_ttl: authenticator.token_ttl ) rescue => e - log_audit_failure(parameters[:account], parameters[:service_id], request_ip, @authenticator_type, e) + role_identifier = role.is_a?(String) ? role : role&.role_id + log_audit_failure(authenticator, role_identifier, request_ip, authenticator.type, e) handle_error(e) end + private + + def permitted?(role:, authenticator_identifier:, account:) + return true if @available_authenticators.native_authenticators.include?(authenticator_identifier) + + # Verify that the identified role is permitted to use this authenticator + RBAC::Permission.new.permitted?( + role_id: role.id, + resource_id: "#{account}:webservice:conjur/#{authenticator_identifier}", + privilege: :authenticate + ) + end + + + def retrieve_authenticator(identifier:, service_id:, account:) + # verify authenticator is whitelisted.... + unless @available_authenticators.enabled_authenticators.include?(identifier) + raise Errors::Authentication::Security::AuthenticatorNotWhitelisted, identifier + end + + authenticator = if @available_authenticators.native_authenticators.include?(identifier) + set_if_present do + "#{@namespace}::DataObjects::Authenticator".constantize.new( + account: account + ) + end + else + # Load Authenticator policy and values (validates data stored as variables) + @authn_repo.find( + type: @authenticator_type, + account: account, + service_id: service_id + ) + end + + return authenticator unless authenticator.nil? + + # TODO: this error should be in the authn repository + raise( + Errors::Conjur::RequestedResourceNotFound, + "#{parameters[:account]}:webservice:conjur/#{authenticator_identifier}" + ) + end + def handle_error(err) + # Log authentication errors (but don't raise...) + authentication_error = LogMessages::Authentication::AuthenticationError.new(err.inspect) + @logger.info(authentication_error) + @logger.info("#{err.class.name}: #{err.message}") + err.backtrace.each {|l| @logger.info(l) } case err - when Errors::Authentication::Security::RoleNotAuthorizedOnResource + when Errors::Authentication::Security::RoleNotAuthorizedOnResource, + Errors::Authentication::Security::MultipleRoleMatchesFound raise ApplicationController::Forbidden when Errors::Authentication::RequestBody::MissingRequestParam, Errors::Authentication::AuthnOidc::TokenVerificationFailed, - Errors::Authentication::AuthnOidc::TokenRetrievalFailed + Errors::Authentication::AuthnOidc::TokenRetrievalFailed, + Rack::OAuth2::Client::Error # Code value mismatch raise ApplicationController::BadRequest - when Errors::Conjur::RequestedResourceNotFound - raise ApplicationController::RecordNotFound.new(err.message) - - when Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty + when Errors::Conjur::RequestedResourceNotFound, + Errors::Authentication::Security::RoleNotFound, + Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty, + Errors::Authentication::Security::AuthenticatorNotWhitelisted raise ApplicationController::Unauthorized when Errors::Authentication::Jwt::TokenExpired raise ApplicationController::Unauthorized.new(err.message, true) - when Errors::Authentication::Security::RoleNotFound - raise ApplicationController::BadRequest - - when Errors::Authentication::Security::MultipleRoleMatchesFound - raise ApplicationController::Forbidden - # Code value mismatch - when Rack::OAuth2::Client::Error - raise ApplicationController::BadRequest - else raise ApplicationController::Unauthorized end end - def log_audit_success(authenticator, conjur_role, client_ip, type) - ::Authentication::LogAuditEvent.new.call( - authentication_params: - Authentication::AuthenticatorInput.new( - authenticator_name: "#{type}", - service_id: authenticator.service_id, - account: authenticator.account, - username: conjur_role.role_id, - client_ip: client_ip, - credentials: nil, - request: nil - ), - audit_event_class: Audit::Event::Authn::Authenticate, - error: nil + def log_audit_success(service, role_id, client_ip, type) + @audit_logger.log( + ::Audit::Event::Authn::Authenticate.new( + authenticator_name: type, + service: service, + role_id: role_id, + client_ip: client_ip, + success: true, + error_message: nil + ) ) end - def log_audit_failure(account, service_id, client_ip, type, error) - ::Authentication::LogAuditEvent.new.call( - authentication_params: - Authentication::AuthenticatorInput.new( - authenticator_name: "#{type}", - service_id: service_id, - account: account, - username: nil, - client_ip: client_ip, - credentials: nil, - request: nil - ), - audit_event_class: Audit::Event::Authn::Authenticate, - error: error + def log_audit_failure(service, role_id, client_ip, type, error) + @audit_logger.log( + ::Audit::Event::Authn::Authenticate.new( + authenticator_name: type, + service: service, + role_id: role_id, + client_ip: client_ip, + success: false, + error_message: error.message + ) ) end end diff --git a/app/domain/authentication/installed_authenticators.rb b/app/domain/authentication/installed_authenticators.rb index 295d5c19de..850e778e74 100644 --- a/app/domain/authentication/installed_authenticators.rb +++ b/app/domain/authentication/installed_authenticators.rb @@ -44,8 +44,12 @@ def enabled_authenticators_str enabled_authenticators.join(',') end + def native_authenticators + %w[authn] + end + private - + def db_enabled_authenticators # Always include 'authn' when enabling authenticators via CLI so that it # doesn't get disabled when another authenticator is enabled diff --git a/app/domain/authentication/util/namespace_selector.rb b/app/domain/authentication/util/namespace_selector.rb index d168505d43..9339560f94 100644 --- a/app/domain/authentication/util/namespace_selector.rb +++ b/app/domain/authentication/util/namespace_selector.rb @@ -5,6 +5,8 @@ module Util class NamespaceSelector def self.select(authenticator_type:) case authenticator_type + when 'authn' + 'Authentication::AuthnApiKey::V2' when 'authn-oidc' # 'V2' is a bit of a hack to handle the fact that # the original OIDC authenticator is really a diff --git a/app/domain/responses.rb b/app/domain/responses.rb index 4fc9948f17..014312d926 100644 --- a/app/domain/responses.rb +++ b/app/domain/responses.rb @@ -26,12 +26,13 @@ def bind(&_block) # Responsible for handling "failed" requests. # Log level and Response code are both option. class FailureResponse - attr_reader :message, :status + attr_reader :message, :status, :exception - def initialize(message, level: :warn, status: :unauthorized) + def initialize(message, level: :warn, status: :unauthorized, exception: nil) @message = message @level = level @status = status + @exception = exception end def success? diff --git a/config/routes.rb b/config/routes.rb index 702ed3277e..8369de483d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -29,6 +29,10 @@ def matches?(request) get '/:authenticator(/:service_id)/:account/login' => 'authenticate#login' + constraints authenticator: /authn/ do + post '/:authenticator/:account(/:id)/authenticate' => 'authenticate#authenticate_via_post' + end + constraints authenticator: /authn|authn-azure|authn-iam|authn-k8s|authn-ldap/ do post '/:authenticator(/:service_id)/:account/:id/authenticate' => 'authenticate#authenticate' end diff --git a/dev/start b/dev/start index d0817759ee..54c33a8954 100755 --- a/dev/start +++ b/dev/start @@ -13,9 +13,9 @@ if [ ! -f "../VERSION" ]; then fi # Minimal set of services. We add to this list based on cmd line flags. -services=(pg conjur client) +services=(pg conjur client cucumber) -# Authenticators to enable. +# Authenticators to enable. default_authenticators="authn,authn-k8s/test" enabled_authenticators="$default_authenticators" From 2189cc32fb442111778be228e494dba3870050bb Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Fri, 20 Oct 2023 12:01:10 -0600 Subject: [PATCH 2/8] Initial refactor work This commit removes the need for the looping the allowed roles, which reduces the scope of a new authenticator. It also introduces the ability to perform validations on an authenticator as well as validating annotations on the identified role. --- Gemfile | 1 + Gemfile.lock | 19 ++++ app/controllers/authenticate_controller.rb | 17 ++++ app/db/repository/authenticator_repository.rb | 96 ++++++++++++------- .../authenticator_role_repository.rb | 33 +++---- .../authentication/base/role_identifier.rb | 19 ++-- .../handler/authentication_handler.rb | 71 +++++++------- app/domain/errors.rb | 5 - app/domain/rbac/permission.rb | 25 +++++ app/domain/util/contract_utils.rb | 11 +++ 10 files changed, 191 insertions(+), 106 deletions(-) create mode 100644 app/domain/rbac/permission.rb create mode 100644 app/domain/util/contract_utils.rb diff --git a/Gemfile b/Gemfile index 78e6dc78cb..d4b2f323e1 100644 --- a/Gemfile +++ b/Gemfile @@ -55,6 +55,7 @@ gem 'rack-rewrite' gem 'dry-struct' gem 'dry-types' +gem 'dry-validation' gem 'net-ldap' # for AWS rotator diff --git a/Gemfile.lock b/Gemfile.lock index be6c74648c..48910f6d95 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -185,14 +185,26 @@ GEM multi_json domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) + dry-configurable (1.1.0) + dry-core (~> 1.0, < 2) + zeitwerk (~> 2.6) dry-core (1.0.0) concurrent-ruby (~> 1.0) zeitwerk (~> 2.6) dry-inflector (1.0.0) + dry-initializer (3.1.1) dry-logic (1.5.0) concurrent-ruby (~> 1.0) dry-core (~> 1.0, < 2) zeitwerk (~> 2.6) + dry-schema (1.13.3) + concurrent-ruby (~> 1.0) + dry-configurable (~> 1.0, >= 1.0.1) + dry-core (~> 1.0, < 2) + dry-initializer (~> 3.0) + dry-logic (>= 1.4, < 2) + dry-types (>= 1.7, < 2) + zeitwerk (~> 2.6) dry-struct (1.6.0) dry-core (~> 1.0, < 2) dry-types (>= 1.7, < 2) @@ -204,6 +216,12 @@ GEM dry-inflector (~> 1.0) dry-logic (~> 1.4) zeitwerk (~> 2.6) + dry-validation (1.10.0) + concurrent-ruby (~> 1.0) + dry-core (~> 1.0, < 2) + dry-initializer (~> 3.0) + dry-schema (>= 1.12, < 2) + zeitwerk (~> 2.6) erubi (1.12.0) event_emitter (0.2.6) eventmachine (1.2.7) @@ -542,6 +560,7 @@ DEPENDENCIES debase-ruby_core_source (~> 3.2.1) dry-struct dry-types + dry-validation event_emitter faye-websocket ffi (>= 1.9.24) diff --git a/app/controllers/authenticate_controller.rb b/app/controllers/authenticate_controller.rb index 7827683963..ace5f396b4 100644 --- a/app/controllers/authenticate_controller.rb +++ b/app/controllers/authenticate_controller.rb @@ -4,6 +4,23 @@ class AuthenticateController < ApplicationController include BasicAuthenticator include AuthorizeResource + def authenticate_via_get + handler = Authentication::Handler::AuthenticationHandler.new( + authenticator_type: params[:authenticator] + ) + + # Allow an authenticator to define the params it's expecting + auth_token = handler.call( + parameters: params.permit(handler.params_allowed).to_h.symbolize_keys, + request_ip: request.ip + ) + + render_authn_token(auth_token) + rescue => e + log_backtrace(e) + raise e + end + def authenticate_via_post auth_token = Authentication::Handler::AuthenticationHandler.new( authenticator_type: params[:authenticator] diff --git a/app/db/repository/authenticator_repository.rb b/app/db/repository/authenticator_repository.rb index d546c01474..a43e9a25bc 100644 --- a/app/db/repository/authenticator_repository.rb +++ b/app/db/repository/authenticator_repository.rb @@ -1,84 +1,112 @@ module DB module Repository + # This class is responsible for loading the variables associated with a + # particular type of authenticator. Each authenticator requires a Data + # Object and Data Object Contract (for validation). Data Objects that + # fail validation are not returned. + # + # This class includes two public methods: + # - `find_all` - returns all available authenticators of a specified type + # from an account + # - `find` - returns a single authenticator based on the provided type, + # account, and service identifier. + # class AuthenticatorRepository def initialize( data_object:, + validations: nil, resource_repository: ::Resource, logger: Rails.logger ) @resource_repository = resource_repository @data_object = data_object + @validations = validations @logger = logger end def find_all(type:, account:) - @resource_repository.where( - Sequel.like( - :resource_id, - "#{account}:webservice:conjur/#{type}/%" - ) - ).all.map do |webservice| + authenticator_webservices(type: type, account: account).map do |webservice| service_id = service_id_from_resource_id(webservice.id) - - # Querying for the authenticator webservice above includes the webservices - # for the authenticator status. The filter below removes webservices that - # don't match the authenticator policy. - next unless webservice.id.split(':').last == "conjur/#{type}/#{service_id}" - - load_authenticator(account: account, service_id: service_id, type: type) + begin + load_authenticator(account: account, service_id: service_id, type: type) + rescue => e + @logger.info("failed to load #{type} authenticator '#{service_id}' do to validation failure: #{e.message}") + nil + end end.compact end def find(type:, account:, service_id:) - webservice = @resource_repository.where( + webservice_identifier = [type, service_id].compact.join('/') + webservice = @resource_repository.where( Sequel.like( :resource_id, - "#{account}:webservice:conjur/#{type}/#{service_id}" + "#{account}:webservice:conjur/#{webservice_identifier}" ) ).first - return unless webservice + unless webservice + raise Errors::Authentication::Security::WebserviceNotFound.new(webservice_identifier, account) + end load_authenticator(account: account, service_id: service_id, type: type) end - def exists?(type:, account:, service_id:) - @resource_repository.with_pk("#{account}:webservice:conjur/#{type}/#{service_id}") != nil - end - private + def authenticator_webservices(type:, account:) + @resource_repository.where( + Sequel.like( + :resource_id, + "#{account}:webservice:conjur/#{type}/%" + ) + ).all.select do |webservice| + # Querying for the authenticator webservice above includes the webservices + # for the authenticator status. The filter below removes webservices that + # don't match the authenticator policy. + webservice.id.split(':').last.match?(%r{^conjur/#{type}/[\w\-_]+$}) + end + end + def service_id_from_resource_id(id) full_id = id.split(':').last full_id.split('/')[2] end def load_authenticator(type:, account:, service_id:) + identifier = [type, service_id].compact.join('/') variables = @resource_repository.where( Sequel.like( :resource_id, - "#{account}:variable:conjur/#{type}/#{service_id}/%" + "#{account}:variable:conjur/#{identifier}/%" ) ).eager(:secrets).all - args_list = {}.tap do |args| args[:account] = account args[:service_id] = service_id variables.each do |variable| - next unless variable.secret - - args[variable.resource_id.split('/')[-1].underscore.to_sym] = variable.secret.value + # If variable exists but does not have a secret, set the value to an empty string. + # This is used downstream for validating if a variable has been set or not, and thus, + # what error to raise. + value = variable.secret ? variable.secret.value : '' + args[variable.resource_id.split('/')[-1].underscore.to_sym] = value end end - begin - allowed_args = %i[account service_id] + - @data_object.const_get(:REQUIRED_VARIABLES) + - @data_object.const_get(:OPTIONAL_VARIABLES) - args_list = args_list.select { |key, value| allowed_args.include?(key) && value.present? } - @data_object.new(**args_list) - rescue ArgumentError => e - @logger.debug("DB::Repository::AuthenticatorRepository.load_authenticator - exception: #{e}") - nil + # Validate the variables against the authenticator contract + result = @validations.call(args_list) + if result.success? + @data_object.new(**result.to_h) + else + errors = result.errors + @logger.info(errors.to_h.inspect) + + # If contract fails, raise the first defined exception... + error = errors.first + raise(error.meta[:exception]) if error.meta[:exception].present? + + # Otherwise, it's a validation error so raise the appropriate exception + raise(Errors::Conjur::RequiredSecretMissing, + "#{account}:variable:conjur/#{type}/#{service_id}/#{error.path.first.to_s.dasherize}") end end end diff --git a/app/db/repository/authenticator_role_repository.rb b/app/db/repository/authenticator_role_repository.rb index 3f7d752914..da0b4f66c9 100644 --- a/app/db/repository/authenticator_role_repository.rb +++ b/app/db/repository/authenticator_role_repository.rb @@ -12,14 +12,15 @@ module Repository # account, and service identifier. # class AuthenticatorRoleRepository - def initialize(authenticator:, role: Role, logger: Rails.logger) + def initialize(authenticator:, role_contract: nil, role: Role, logger: Rails.logger) @authenticator = authenticator @role = role @logger = logger + @role_contract = role_contract end - def find(role_identifier:, role_contract: nil) - role = @role[role_identifier.conjur_role] + def find(role_identifier:) + role = @role[role_identifier.identifier] unless role.present? raise(Errors::Authentication::Security::RoleNotFound, role_identifier.role_for_error) end @@ -27,13 +28,11 @@ def find(role_identifier:, role_contract: nil) return role unless role.resource? relevant_annotations = relevant_annotations( - annotations: {}.tap { |h| role.resource.annotations.each {|a| h[a.name] = a.value }}, - authenticator: @authenticator + annotations: {}.tap { |h| role.resource.annotations.each {|a| h[a.name] = a.value }} ) validate_role_annotations_against_contract( - annotations: relevant_annotations, - role_contract: role_contract + annotations: relevant_annotations ) annotations_match?( @@ -46,17 +45,17 @@ def find(role_identifier:, role_contract: nil) private - def validate_role_annotations_against_contract(annotations:, role_contract:) + def validate_role_annotations_against_contract(annotations:) # If authenticator requires annotations, verify some are present if @authenticator.annotations_required && annotations.empty? raise(Errors::Authentication::Constraints::RoleMissingAnyRestrictions) end # Only run contract validations if they are present - return if role_contract.nil? + return if @role_contract.nil? annotations.each do |annotation, value| - annotation_valid = role_contract.new(authenticator: @authenticator, utils: ::Util::ContractUtils).call( + annotation_valid = @role_contract.new(authenticator: @authenticator, utils: ::Util::ContractUtils).call( annotation: annotation, annotation_value: value, annotations: annotations @@ -81,29 +80,25 @@ def validate_role_annotations_against_contract(annotations:, role_contract:) # authn-jwt/project_id: myproject # authn-jwt/aud: myaud - def relevant_annotations(annotations:, authenticator:) + def relevant_annotations(annotations:) # Verify that at least one service specific auth token is present - if annotations.keys.any?{|k,_|k.include?(authenticator.type.to_s) } && - !annotations.keys.any?{|k,_|k.include?("#{authenticator.type}/#{authenticator.service_id}") } + if annotations.keys.any?{|k,_|k.include?(@authenticator.type.to_s) } && + !annotations.keys.any?{|k,_|k.include?("#{@authenticator.type}/#{@authenticator.service_id}") } raise(Errors::Authentication::Constraints::RoleMissingAnyRestrictions) end generic = annotations .select{|k, _| k.count('/') == 1 } .select{|k, _| k.match?(%r{^authn-jwt/})} - .reject{|k, _| k.match?(%r{^authn-jwt/#{authenticator.service_id}})} + .reject{|k, _| k.match?(%r{^authn-jwt/#{@authenticator.service_id}})} .transform_keys{|k| k.split('/').last} specific = annotations .select{|k, _| k.count('/') > 1 } - .select{|k, _| k.match?(%r{^authn-jwt/#{authenticator.service_id}/})} + .select{|k, _| k.match?(%r{^authn-jwt/#{@authenticator.service_id}/})} .transform_keys{|k| k.split('/').last} generic.merge(specific) - # relevant_annotations = generic.merge(specific) - - # validate_role_annotations(annotations: relevant_annotations, authenticator: authenticator, relevant_annotations: relevant_annotations) - # relevant_annotations end def annotations_match?(role_annotations:, target_annotations:) diff --git a/app/domain/authentication/base/role_identifier.rb b/app/domain/authentication/base/role_identifier.rb index c4d1096483..867bffb00c 100644 --- a/app/domain/authentication/base/role_identifier.rb +++ b/app/domain/authentication/base/role_identifier.rb @@ -3,33 +3,32 @@ module Authentication module Base class RoleIdentifier - attr_reader :role_identifier, :annotations #, :exception + attr_reader :identifier, :annotations - def initialize(role_identifier:, annotations: {}, exception: nil) - @role_identifier = role_identifier + def initialize(identifier:, annotations: {}) + @identifier = identifier @annotations = annotations - # @exception = exception end def type - @role_identifier.split(':')[1] + @identifier.split(':')[1] end def account - @role_identifier.split(':')[0] + @identifier.split(':')[0] end # Role identifier within the account and type context: # :: def id - @role_identifier.split(':')[2] + @identifier.split(':')[2] end # Essentially just an alias, but doing it this way to # avoid duplicate alias Rubocop warning. - def conjur_role - @role_identifier - end + # def conjur_role + # @role_identifier + # end def role_for_error type == 'host' ? "host/#{id}" : id diff --git a/app/domain/authentication/handler/authentication_handler.rb b/app/domain/authentication/handler/authentication_handler.rb index 7488947ee6..d29b085b45 100644 --- a/app/domain/authentication/handler/authentication_handler.rb +++ b/app/domain/authentication/handler/authentication_handler.rb @@ -11,7 +11,9 @@ def initialize( audit_logger: ::Audit.logger, authentication_error: LogMessages::Authentication::AuthenticationError, available_authenticators: Authentication::InstalledAuthenticators, - role_repository: DB::Repository::AuthenticatorRoleRepository + role_repository: DB::Repository::AuthenticatorRoleRepository, + authorization: RBAC::Permission.new, + token_factory: TokenFactory.new ) @authenticator_type = authenticator_type @logger = logger @@ -19,6 +21,8 @@ def initialize( @authentication_error = authentication_error @available_authenticators = available_authenticators @role_repository = role_repository + @authorization = authorization + @token_factory = token_factory # Dynamically load authenticator specific classes @namespace = namespace_selector.select( @@ -27,27 +31,13 @@ def initialize( @strategy = "#{@namespace}::Strategy".constantize @authn_repo = authn_repo.new( - data_object: "#{@namespace}::DataObjects::Authenticator".constantize + data_object: "#{@namespace}::DataObjects::Authenticator".constantize, + validations: set_if_present { "#{@namespace}::Validations::AuthenticatorConfiguration".constantize.new(utils: ::Util::ContractUtils) } ) - @contract = set_if_present { "#{@namespace}::Validations::AuthenticatorContract".constantize.new(utils: ::Util::ContractUtils) } - @role_contract = set_if_present { "#{@namespace}::Validations::RoleContract".constantize } - end - - def set_if_present(&block) - block.call - rescue NameError - nil - end - - def params_allowed - allowed = %i[authenticator service_id account] - allowed += @strategy::ALLOWED_PARAMS if @strategy.const_defined?('ALLOWED_PARAMS') - allowed end def call(request_ip:, parameters:, request_body: nil, action: nil) authenticator_identifier = [parameters[:authenticator], parameters[:service_id]].compact.join('/') - authenticator = retrieve_authenticator( identifier: authenticator_identifier, account: parameters[:account], @@ -61,15 +51,17 @@ def call(request_ip:, parameters:, request_body: nil, action: nil) if role_identifier_response.success? role_identifier = role_identifier_response.result else - role = role_identifier_response.message.role_identifier + if role_identifier_response.message.is_a?(Authentication::Base::RoleIdentifier) + role = role_identifier_response.message.role_identifier + end raise role_identifier_response.exception end role = @role_repository.new( - authenticator: authenticator - ).find( - role_identifier: role_identifier, + authenticator: authenticator, role_contract: @role_contract + ).find( + role_identifier: role_identifier ) # Add an error message (this may actually never be hit as we raise @@ -88,24 +80,36 @@ def call(request_ip:, parameters:, request_body: nil, action: nil) log_audit_success(authenticator, role.role_id, request_ip, authenticator.type) - TokenFactory.new.signed_token( + @token_factory.signed_token( account: parameters[:account], username: role.login, user_ttl: authenticator.token_ttl ) rescue => e role_identifier = role.is_a?(String) ? role : role&.role_id - log_audit_failure(authenticator, role_identifier, request_ip, authenticator.type, e) + log_audit_failure(authenticator, role_identifier, request_ip, authenticator&.type, e) handle_error(e) end private + def params_allowed + allowed = %i[authenticator service_id account] + allowed += @strategy::ALLOWED_PARAMS if @strategy.const_defined?('ALLOWED_PARAMS') + allowed + end + + def set_if_present(&block) + block.call + rescue NameError + nil + end + def permitted?(role:, authenticator_identifier:, account:) return true if @available_authenticators.native_authenticators.include?(authenticator_identifier) # Verify that the identified role is permitted to use this authenticator - RBAC::Permission.new.permitted?( + @authorization.permitted?( role_id: role.id, resource_id: "#{account}:webservice:conjur/#{authenticator_identifier}", privilege: :authenticate @@ -119,7 +123,7 @@ def retrieve_authenticator(identifier:, service_id:, account:) raise Errors::Authentication::Security::AuthenticatorNotWhitelisted, identifier end - authenticator = if @available_authenticators.native_authenticators.include?(identifier) + if @available_authenticators.native_authenticators.include?(identifier) set_if_present do "#{@namespace}::DataObjects::Authenticator".constantize.new( account: account @@ -133,14 +137,6 @@ def retrieve_authenticator(identifier:, service_id:, account:) service_id: service_id ) end - - return authenticator unless authenticator.nil? - - # TODO: this error should be in the authn repository - raise( - Errors::Conjur::RequestedResourceNotFound, - "#{parameters[:account]}:webservice:conjur/#{authenticator_identifier}" - ) end def handle_error(err) @@ -152,20 +148,19 @@ def handle_error(err) err.backtrace.each {|l| @logger.info(l) } case err - when Errors::Authentication::Security::RoleNotAuthorizedOnResource, - Errors::Authentication::Security::MultipleRoleMatchesFound + when Errors::Authentication::Security::RoleNotAuthorizedOnResource raise ApplicationController::Forbidden when Errors::Authentication::RequestBody::MissingRequestParam, + Errors::Authentication::Security::RoleNotFound, + Errors::Authentication::Security::AuthenticatorNotWhitelisted, Errors::Authentication::AuthnOidc::TokenVerificationFailed, Errors::Authentication::AuthnOidc::TokenRetrievalFailed, Rack::OAuth2::Client::Error # Code value mismatch raise ApplicationController::BadRequest when Errors::Conjur::RequestedResourceNotFound, - Errors::Authentication::Security::RoleNotFound, - Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty, - Errors::Authentication::Security::AuthenticatorNotWhitelisted + Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty raise ApplicationController::Unauthorized when Errors::Authentication::Jwt::TokenExpired diff --git a/app/domain/errors.rb b/app/domain/errors.rb index 8f654f4025..9127fe025c 100644 --- a/app/domain/errors.rb +++ b/app/domain/errors.rb @@ -170,11 +170,6 @@ module Security msg: "Account '{0-account-name}' is not defined in Conjur", code: "CONJ00008E" ) - - MultipleRoleMatchesFound = ::Util::TrackableErrorClass.new( - msg: "'{0-role}' matched multiple roles", - code: "CONJ00009E" - ) end module RequestBody diff --git a/app/domain/rbac/permission.rb b/app/domain/rbac/permission.rb new file mode 100644 index 0000000000..f5eccbe255 --- /dev/null +++ b/app/domain/rbac/permission.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module RBAC + class Permission + def initialize(role_library: Role, resource_library: Resource) + @role_library = role_library + @resource_library = resource_library + end + + def permitted?(role_id:, resource_id:, privilege:) + role = @role_library[role_id] + resource = @resource_library[resource_id] + + return true if role.present? && role.allowed_to?(privilege, resource) + + raise( + Errors::Authentication::Security::RoleNotAuthorizedOnResource.new( + [role.kind, role.identifier].join('/'), + privilege, + resource.id + ) + ) + end + end +end diff --git a/app/domain/util/contract_utils.rb b/app/domain/util/contract_utils.rb new file mode 100644 index 0000000000..cc8139ba72 --- /dev/null +++ b/app/domain/util/contract_utils.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Util + class ContractUtils + class << self + def failed_response(error:, key:) + key.failure(exception: error, text: error.message) + end + end + end +end From 6fc43b443fcf587b739f59408d227939b1740e1d Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Fri, 20 Oct 2023 13:58:48 -0600 Subject: [PATCH 3/8] Refactors API Key authentication --- app/domain/authentication/authn_api_key/v2/strategy.rb | 2 +- config/routes.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/domain/authentication/authn_api_key/v2/strategy.rb b/app/domain/authentication/authn_api_key/v2/strategy.rb index f4c68a2226..18b37d81b7 100644 --- a/app/domain/authentication/authn_api_key/v2/strategy.rb +++ b/app/domain/authentication/authn_api_key/v2/strategy.rb @@ -33,7 +33,7 @@ def callback(request_body:, parameters:) end role_identifier = Authentication::Base::RoleIdentifier.new( - role_identifier: role_id + identifier: role_id ) if @role[role_id].nil? diff --git a/config/routes.rb b/config/routes.rb index 8369de483d..b994c458f3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,7 +30,7 @@ def matches?(request) get '/:authenticator(/:service_id)/:account/login' => 'authenticate#login' constraints authenticator: /authn/ do - post '/:authenticator/:account(/:id)/authenticate' => 'authenticate#authenticate_via_post' + post '/:authenticator/:account/:id/authenticate' => 'authenticate#authenticate_via_post' end constraints authenticator: /authn|authn-azure|authn-iam|authn-k8s|authn-ldap/ do From b174d5ca7cc632267798339df77deebca79b1e38 Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Fri, 27 Oct 2023 09:58:22 -0600 Subject: [PATCH 4/8] Initial work on the Monad-based Authenticator refactor --- app/controllers/authenticate_controller.rb | 27 +- app/controllers/providers_controller.rb | 26 +- app/db/repository/authenticator_repository.rb | 65 ++--- .../authenticator_role_repository.rb | 82 ++++-- app/db/validation.rb | 32 +++ .../authn_api_key/v2/strategy.rb | 13 +- .../authentication/authn_oidc/v2/client.rb | 52 ++-- .../v2/data_objects/authenticator.rb | 26 +- .../authentication/authn_oidc/v2/strategy.rb | 82 ++++-- .../authenticator_configuration.rb | 51 ++++ app/domain/authentication/base/data_object.rb | 9 +- .../handler/authentication_handler.rb | 243 ++++++++++-------- .../authentication/util/klass_loader.rb | 21 ++ app/domain/rbac/permission.rb | 36 ++- app/domain/responses.rb | 3 +- ci/oauth/keycloak/fetch_certificate | 2 + config/routes.rb | 2 +- .../features/authn_oidc_v2.feature | 4 +- dev/start | 2 +- 19 files changed, 515 insertions(+), 263 deletions(-) create mode 100644 app/db/validation.rb create mode 100644 app/domain/authentication/authn_oidc/v2/validations/authenticator_configuration.rb create mode 100644 app/domain/authentication/util/klass_loader.rb diff --git a/app/controllers/authenticate_controller.rb b/app/controllers/authenticate_controller.rb index ace5f396b4..7391c63ba2 100644 --- a/app/controllers/authenticate_controller.rb +++ b/app/controllers/authenticate_controller.rb @@ -10,32 +10,47 @@ def authenticate_via_get ) # Allow an authenticator to define the params it's expecting - auth_token = handler.call( + response = handler.call( parameters: params.permit(handler.params_allowed).to_h.symbolize_keys, request_ip: request.ip - ) + ).bind do |auth_token| + return render_authn_token(auth_token) + end - render_authn_token(auth_token) + error_response(response) rescue => e log_backtrace(e) raise e end def authenticate_via_post - auth_token = Authentication::Handler::AuthenticationHandler.new( + response = Authentication::Handler::AuthenticationHandler.new( authenticator_type: params[:authenticator] ).call( parameters: params, request_body: request.body.read, request_ip: request.ip - ) + ).bind do |auth_token| + return render_authn_token(auth_token) + end - render_authn_token(auth_token) + error_response(response) rescue => e log_backtrace(e) raise e end + def error_response(response) + logger.info(LogMessages::Authentication::AuthenticationError.new(response.exception)) + logger.info("Exception: #{response.exception.class.name}: #{response.exception.message}") + [*response.exception.backtrace].each { |line| logger.info(line) } + + render( + json: { error: response.exception.message }, + status: response.status + ) + end + def oidc_authenticate_code_redirect # TODO: need a mechanism for an authenticator strategy to define the required # params. This will likely need to be done via the Handler. diff --git a/app/controllers/providers_controller.rb b/app/controllers/providers_controller.rb index 7e0be9597d..2090c6766c 100644 --- a/app/controllers/providers_controller.rb +++ b/app/controllers/providers_controller.rb @@ -5,14 +5,28 @@ def index namespace = Authentication::Util::NamespaceSelector.select( authenticator_type: params[:authenticator] ) + authenticator_klass = "#{namespace}::DataObjects::Authenticator".constantize + validations = "#{namespace}::Validations::AuthenticatorConfiguration".constantize.new( + utils: ::Util::ContractUtils + ) + validator = DB::Validation.new(validations: validations) + + authenticators = DB::Repository::AuthenticatorRepository.new.find_all( + account: params[:account], + type: params[:authenticator] + ).bind do |authenticators_data| + authenticators_data.map do |authenticator_data| + # perform validation on each record + verified_data = validator.validate(data: authenticator_data) + if verified_data.success? + authenticator_klass.new(**verified_data.result) + end + end.compact + end + render( json: "#{namespace}::Views::ProviderContext".constantize.new.call( - authenticators: DB::Repository::AuthenticatorRepository.new( - data_object: "#{namespace}::DataObjects::Authenticator".constantize - ).find_all( - account: params[:account], - type: params[:authenticator] - ) + authenticators: authenticators ) ) end diff --git a/app/db/repository/authenticator_repository.rb b/app/db/repository/authenticator_repository.rb index a43e9a25bc..ff111309b0 100644 --- a/app/db/repository/authenticator_repository.rb +++ b/app/db/repository/authenticator_repository.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module DB module Repository # This class is responsible for loading the variables associated with a @@ -13,42 +15,62 @@ module Repository # class AuthenticatorRepository def initialize( - data_object:, - validations: nil, resource_repository: ::Resource, + available_authenticators: Authentication::InstalledAuthenticators, logger: Rails.logger ) @resource_repository = resource_repository - @data_object = data_object - @validations = validations + @available_authenticators = available_authenticators @logger = logger + + @success = ::SuccessResponse + @failure = ::FailureResponse end def find_all(type:, account:) - authenticator_webservices(type: type, account: account).map do |webservice| + authenticators = authenticator_webservices(type: type, account: account).map do |webservice| service_id = service_id_from_resource_id(webservice.id) begin - load_authenticator(account: account, service_id: service_id, type: type) + load_authenticator_variables(account: account, service_id: service_id, type: type) rescue => e @logger.info("failed to load #{type} authenticator '#{service_id}' do to validation failure: #{e.message}") nil end end.compact + @success.new(authenticators) end - def find(type:, account:, service_id:) - webservice_identifier = [type, service_id].compact.join('/') + def find(type:, account:, service_id: nil, &block) + identifier = [type, service_id].compact.join('/') + webservice = @resource_repository.where( Sequel.like( :resource_id, - "#{account}:webservice:conjur/#{webservice_identifier}" + "#{account}:webservice:conjur/#{identifier}" ) ).first unless webservice - raise Errors::Authentication::Security::WebserviceNotFound.new(webservice_identifier, account) + return @failure.new( + "Failed to find a webservice: '#{account}:webservice:conjur/#{identifier}'", + exception: Errors::Authentication::Security::WebserviceNotFound.new(identifier, account) + ) end - load_authenticator(account: account, service_id: service_id, type: type) + begin + @success.new( + load_authenticator_variables( + account: account, + service_id: service_id, + type: type + ) + ) + rescue => e + @failure.new( + e.message, + exception: e, + level: :debug + ) + end end private @@ -72,7 +94,7 @@ def service_id_from_resource_id(id) full_id.split('/')[2] end - def load_authenticator(type:, account:, service_id:) + def load_authenticator_variables(type:, account:, service_id:) identifier = [type, service_id].compact.join('/') variables = @resource_repository.where( Sequel.like( @@ -80,7 +102,7 @@ def load_authenticator(type:, account:, service_id:) "#{account}:variable:conjur/#{identifier}/%" ) ).eager(:secrets).all - args_list = {}.tap do |args| + {}.tap do |args| args[:account] = account args[:service_id] = service_id variables.each do |variable| @@ -91,23 +113,6 @@ def load_authenticator(type:, account:, service_id:) args[variable.resource_id.split('/')[-1].underscore.to_sym] = value end end - - # Validate the variables against the authenticator contract - result = @validations.call(args_list) - if result.success? - @data_object.new(**result.to_h) - else - errors = result.errors - @logger.info(errors.to_h.inspect) - - # If contract fails, raise the first defined exception... - error = errors.first - raise(error.meta[:exception]) if error.meta[:exception].present? - - # Otherwise, it's a validation error so raise the appropriate exception - raise(Errors::Conjur::RequiredSecretMissing, - "#{account}:variable:conjur/#{type}/#{service_id}/#{error.path.first.to_s.dasherize}") - end end end end diff --git a/app/db/repository/authenticator_role_repository.rb b/app/db/repository/authenticator_role_repository.rb index da0b4f66c9..06414bc2c4 100644 --- a/app/db/repository/authenticator_role_repository.rb +++ b/app/db/repository/authenticator_role_repository.rb @@ -17,30 +17,37 @@ def initialize(authenticator:, role_contract: nil, role: Role, logger: Rails.log @role = role @logger = logger @role_contract = role_contract + + @success = ::SuccessResponse + @failure = ::FailureResponse end def find(role_identifier:) role = @role[role_identifier.identifier] unless role.present? - raise(Errors::Authentication::Security::RoleNotFound, role_identifier.role_for_error) + return @failure.new( + "Failed to find role for: '#{role_identifier.identifier}'", + exception: Errors::Authentication::Security::RoleNotFound.new(role_identifier.role_for_error), + status: :bad_request + ) end - return role unless role.resource? + return @success.new(role) unless role.resource? - relevant_annotations = relevant_annotations( + relevant_annotations( annotations: {}.tap { |h| role.resource.annotations.each {|a| h[a.name] = a.value }} - ) - - validate_role_annotations_against_contract( - annotations: relevant_annotations - ) - - annotations_match?( - role_annotations: relevant_annotations, - target_annotations: role_identifier.annotations - ) - - role + ).bind do |relevant_annotations| + validate_role_annotations_against_contract( + annotations: relevant_annotations + ).bind do + annotations_match?( + role_annotations: relevant_annotations, + target_annotations: role_identifier.annotations + ).bind do + @success.new(role) + end + end + end end private @@ -48,11 +55,16 @@ def find(role_identifier:) def validate_role_annotations_against_contract(annotations:) # If authenticator requires annotations, verify some are present if @authenticator.annotations_required && annotations.empty? - raise(Errors::Authentication::Constraints::RoleMissingAnyRestrictions) + return @failure.new( + 'This authenticator requires a role to include authenticator annotations', + exception: Errors::Authentication::Constraints::RoleMissingAnyRestrictions, + status: :unauthorized + ) end # Only run contract validations if they are present - return if @role_contract.nil? + # This isn't functional, but how best to return something of use...? + return @success.new(annotations) if @role_contract.nil? annotations.each do |annotation, value| annotation_valid = @role_contract.new(authenticator: @authenticator, utils: ::Util::ContractUtils).call( @@ -62,10 +74,16 @@ def validate_role_annotations_against_contract(annotations:) ) next if annotation_valid.success? - raise(annotation_valid.errors.first.meta[:exception]) + return @failure.new( + annotation_valid.errors.first, + exception: annotation_valid.errors.first.meta[:exception], + status: :unauthorized + ) end + @success.new(annotations) end + # Need to account for the following two options: # Annotations relevant to specific authenticator # - !host @@ -82,9 +100,13 @@ def validate_role_annotations_against_contract(annotations:) def relevant_annotations(annotations:) # Verify that at least one service specific auth token is present - if annotations.keys.any?{|k,_|k.include?(@authenticator.type.to_s) } && - !annotations.keys.any?{|k,_|k.include?("#{@authenticator.type}/#{@authenticator.service_id}") } - raise(Errors::Authentication::Constraints::RoleMissingAnyRestrictions) + if annotations.keys.any? { |k, _| k.include?(@authenticator.type.to_s) } && + !annotations.keys.any? { |k, _| k.include?("#{@authenticator.type}/#{@authenticator.service_id}") } + return @failure.new( + 'Role mush include some restrications', + exception: Errors::Authentication::Constraints::RoleMissingAnyRestrictions, + status: :unauthorized + ) end generic = annotations @@ -98,12 +120,12 @@ def relevant_annotations(annotations:) .select{|k, _| k.match?(%r{^authn-jwt/#{@authenticator.service_id}/})} .transform_keys{|k| k.split('/').last} - generic.merge(specific) + @success.new(generic.merge(specific)) end def annotations_match?(role_annotations:, target_annotations:) # If there are no annotations to match, just return - return if target_annotations.empty? + return @success.new(role_annotations) if target_annotations.empty? role_annotations.each do |annotation, value| next unless annotation.present? @@ -115,14 +137,24 @@ def annotations_match?(role_annotations:, target_annotations:) end unless target_annotations.key?(annotation) - raise(Errors::Authentication::AuthnJwt::JwtTokenClaimIsMissing, annotation) + return @failure.new( + "Role annotation: '#{annotation}' is not present in the authorization payload", + exception: Errors::Authentication::AuthnJwt::JwtTokenClaimIsMissing.new(annotation), + status: :unauthorized + ) end - raise(Errors::Authentication::ResourceRestrictions::InvalidResourceRestrictions, annotation) + return @failure.new( + "Role annotation: '#{annotation}' is invalid", + Errors::Authentication::ResourceRestrictions::InvalidResourceRestrictions.new(annotation), + status: :unauthorized + ) end @logger.debug(LogMessages::Authentication::ResourceRestrictions::ValidatedResourceRestrictions.new) @logger.debug(LogMessages::Authentication::AuthnJwt::ValidateRestrictionsPassed.new) + + @success.new(role_annotations) end end end diff --git a/app/db/validation.rb b/app/db/validation.rb new file mode 100644 index 0000000000..13794ac83e --- /dev/null +++ b/app/db/validation.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module DB + # This class provides a generic mechanism for running Dry-RB contracts + # against provided data. + class Validation + def initialize(validations:, logger: Rails.logger) + @validations = validations + @logger = logger + + @success = ::SuccessResponse + @failure = ::FailureResponse + end + + def validate(data:) + return @success.new(data) if @validations.blank? + + result = @validations.call(**data) + if result.success? + @success.new(result.to_h) + else + errors = result.errors + # Print all errors + @logger.info(errors.to_h.inspect) + + # If contract fails, return the first defined exception... + error = errors.first + @failure.new(error, exception: error.meta[:exception]) + end + end + end +end diff --git a/app/domain/authentication/authn_api_key/v2/strategy.rb b/app/domain/authentication/authn_api_key/v2/strategy.rb index 18b37d81b7..b1cf7a75ce 100644 --- a/app/domain/authentication/authn_api_key/v2/strategy.rb +++ b/app/domain/authentication/authn_api_key/v2/strategy.rb @@ -15,17 +15,18 @@ def initialize(authenticator:, logger: Rails.logger, credentials: ::Credentials, @logger = logger @credentials = credentials @role = role + @success = ::SuccessResponse @failure = ::FailureResponse end + # Parameter `id` is guaranteed to be present based on the + # upstream routes file. def callback(request_body:, parameters:) - # TODO: Check that `id` is present in the parameters list - role_id = parameters['id'] api_key = request_body - role_id = if role_id.match?(/^host\/./) + full_role_id = if role_id.match?(/^host\/./) id = role_id.split('/')[1..-1].join "#{@authenticator.account}:host:#{id}" else @@ -33,17 +34,17 @@ def callback(request_body:, parameters:) end role_identifier = Authentication::Base::RoleIdentifier.new( - identifier: role_id + identifier: full_role_id ) - if @role[role_id].nil? + if @role[full_role_id].nil? return @failure.new( role_identifier, exception: Errors::Authentication::Security::RoleNotFound.new(role_id) ) end - role_credentials = @credentials[role_id] + role_credentials = @credentials[full_role_id] if role_credentials.nil? return @failure.new( role_identifier, diff --git a/app/domain/authentication/authn_oidc/v2/client.rb b/app/domain/authentication/authn_oidc/v2/client.rb index e15ab6d655..fbf37e28e2 100644 --- a/app/domain/authentication/authn_oidc/v2/client.rb +++ b/app/domain/authentication/authn_oidc/v2/client.rb @@ -16,6 +16,9 @@ def initialize( @discovery_configuration = discovery_configuration @cache = cache @logger = logger + + @success = ::SuccessResponse + @failure = ::FailureResponse end # Writing certificates to the default system cert store requires @@ -56,21 +59,11 @@ def callback(code:, nonce:, code_verifier: nil) begin bearer_token = oidc_client.access_token!(**access_token_args) rescue Rack::OAuth2::Client::Error => e - # Only handle the expected errors related to access token retrieval. - case e.message - when /PKCE verification failed/, # Okta's PKCE failure msg - /challenge mismatch/ # Identity's PKCE failure msg - raise Errors::Authentication::AuthnOidc::TokenRetrievalFailed, - 'PKCE verification failed' - when /The authorization code is invalid or has expired/, # Okta's reused code msg - /supplied code does not match known request/ # Identity's reused code msg - 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 + return @failure.new( + e.message, + exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(e.message), + status: :bad_request + ) end id_token = bearer_token.id_token || bearer_token.access_token @@ -80,10 +73,16 @@ def callback(code:, nonce:, code_verifier: nil) id_token, discovery_information.jwks ) - rescue StandardError => e - attempts += 1 - raise e if attempts > 1 + rescue => e + attempts += 1 + if attempts > 1 + return @failure.new( + 'JWKS signing check failed', + exception: e, + status: :unauthorized + ) + end # 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. @@ -97,17 +96,14 @@ def callback(code:, nonce:, code_verifier: nil) 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 + @success.new(decoded_id_token) + rescue => e + @failure.new( + e.message, + exception: e, + status: :bad_request + ) end - decoded_id_token end # callback_with_temporary_cert wraps the callback method with commands diff --git a/app/domain/authentication/authn_oidc/v2/data_objects/authenticator.rb b/app/domain/authentication/authn_oidc/v2/data_objects/authenticator.rb index b542b1fb7d..c29cf6bd2d 100644 --- a/app/domain/authentication/authn_oidc/v2/data_objects/authenticator.rb +++ b/app/domain/authentication/authn_oidc/v2/data_objects/authenticator.rb @@ -2,10 +2,9 @@ module Authentication module AuthnOidc module V2 module DataObjects - class Authenticator + class Authenticator < Authentication::Base::DataObject - REQUIRED_VARIABLES = %i[provider_uri client_id client_secret claim_mapping].freeze - OPTIONAL_VARIABLES = %i[redirect_uri response_type provider_scope name token_ttl ca_cert].freeze + REQUIRES_ROLE_ANNOTIONS = false attr_reader( :provider_uri, @@ -19,6 +18,8 @@ class Authenticator :ca_cert ) + # rubocop:disable Metrics/ParameterLists + # rubocop:disable Lint/MissingSuper def initialize( provider_uri:, client_id:, @@ -30,7 +31,7 @@ def initialize( name: nil, response_type: 'code', provider_scope: nil, - token_ttl: 'PT60M', + token_ttl: '', ca_cert: nil ) @account = account @@ -43,9 +44,13 @@ def initialize( @name = name @provider_scope = provider_scope @redirect_uri = redirect_uri - @token_ttl = token_ttl @ca_cert = ca_cert + + # Set TTL to 60 minutes by default + @token_ttl = token_ttl.present? ? token_ttl : 'PT60M' end + # rubocop:enable Metrics/ParameterLists + # rubocop:enable Lint/MissingSuper def scope (%w[openid email profile] + [*@provider_scope.to_s.split(' ')]).uniq.join(' ') @@ -54,17 +59,6 @@ def scope def name @name || @service_id.titleize end - - def resource_id - "#{account}:webservice:conjur/authn-oidc/#{service_id}" - end - - # Returns the validity duration, in seconds, of an instance's access tokens. - def token_ttl - ActiveSupport::Duration.parse(@token_ttl) - rescue ActiveSupport::Duration::ISO8601Parser::ParsingError - raise Errors::Authentication::DataObjects::InvalidTokenTTL.new(resource_id, @token_ttl) - end end end end diff --git a/app/domain/authentication/authn_oidc/v2/strategy.rb b/app/domain/authentication/authn_oidc/v2/strategy.rb index 0b70d72af6..f4abc0bcd1 100644 --- a/app/domain/authentication/authn_oidc/v2/strategy.rb +++ b/app/domain/authentication/authn_oidc/v2/strategy.rb @@ -1,7 +1,12 @@ +# frozen_string_literal: true + module Authentication module AuthnOidc module V2 class Strategy + REQUIRED_PARAMS = %i[code nonce].freeze + ALLOWED_PARAMS = (REQUIRED_PARAMS + %i[code_verifier]).freeze + def initialize( authenticator:, client: Authentication::AuthnOidc::V2::Client, @@ -10,35 +15,72 @@ def initialize( @authenticator = authenticator @client = client.new(authenticator: authenticator) @logger = logger + + @success = ::SuccessResponse + @failure = ::FailureResponse end - def callback(args) - # NOTE: `code_verifier` param is optional - %i[code nonce].each do |param| - unless args[param].present? - raise Errors::Authentication::RequestBody::MissingRequestParam, param.to_s + # rubocop:disable Lint/UnusedMethodArgument + def callback(parameters:, request_body: nil) + validate_parameters(parameters).bind do |params| + validate_jwt_token(params).bind do |jwt_token| + resolve_identity(jwt: jwt_token).bind do |identity| + @success.new( + Authentication::Base::RoleIdentifier.new( + identifier: "#{@authenticator.account}:user:#{identity}" + ) + ) + end end end - identity = resolve_identity( - jwt: @client.callback_with_temporary_cert( - code: args[:code], - nonce: args[:nonce], - code_verifier: args[:code_verifier], - cert_string: @authenticator.ca_cert - ) - ) - unless identity.present? - raise Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty.new( - @authenticator.claim_mapping, - "claim-mapping" - ) + end + # rubocop:enable Lint/UnusedMethodArgument + + # Called by status handler. This handles checking as much of the strategy + # integrity as possible without performing an actual authentication. + def verify_status + @client.discover + end + + private + + def validate_parameters(parameters) + REQUIRED_PARAMS.each do |param| + unless parameters[param].present? + return @failure.new( + "Missing parameter: '#{param}'", + exception: Errors::Authentication::RequestBody::MissingRequestParam.new( + param.to_s + ), + status: :bad_request + ) + end end - identity + @success.new(parameters.select {|item| ALLOWED_PARAMS.include?(item) } ) + end + + def validate_jwt_token(parameters) + @client.callback_with_temporary_cert( + code: parameters[:code], + nonce: parameters[:nonce], + code_verifier: parameters[:code_verifier], + cert_string: @authenticator.ca_cert + ) end def resolve_identity(jwt:) - jwt.raw_attributes.with_indifferent_access[@authenticator.claim_mapping] + identity = jwt.raw_attributes.with_indifferent_access[@authenticator.claim_mapping] + return @success.new(identity) if identity.present? + + @failure.new( + 'Claim was not found', + exception: Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty.new( + @authenticator.claim_mapping, + 'claim-mapping' + ), + status: :unauthorized + ) end end end diff --git a/app/domain/authentication/authn_oidc/v2/validations/authenticator_configuration.rb b/app/domain/authentication/authn_oidc/v2/validations/authenticator_configuration.rb new file mode 100644 index 0000000000..e68b73157d --- /dev/null +++ b/app/domain/authentication/authn_oidc/v2/validations/authenticator_configuration.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Authentication + module AuthnOidc + module V2 + module Validations + + # This class validates the configuration of the OIDC Authenticator as defined + # by the authenticator's variables. + # + # All required and optional variables should be defined here, as well as any + # the validation of those input's values. + # + # This validations are executed against the data loaded from Conjur variables when + # the authenicator is loaded via the AuthenticatorRepository. + class AuthenticatorConfiguration < Dry::Validation::Contract + option :utils + + schema do + required(:account).value(:string) + required(:service_id).value(:string) + required(:provider_uri).value(:string) + required(:client_id).value(:string) + required(:client_secret).value(:string) + required(:claim_mapping).value(:string) + + optional(:redirect_uri).value(:string) + optional(:response_type).value(:string) + optional(:provider_scope).value(:string) + optional(:name).value(:string) + optional(:token_ttl).value(:string) + optional(:provider_scope).value(:string) + optional(:ca_cert).value(:string) + end + + # Verify that `provider_uri` has a secret value set if variable is present + rule(:provider_uri, :service_id, :account) do + if values[:provider_uri].empty? + utils.failed_response( + key: key, + error: Errors::Conjur::RequiredSecretMissing.new( + "#{values[:account]}:variable:conjur/authn-oidc/#{values[:service_id]}/provider-uri" + ) + ) + end + end + end + end + end + end +end diff --git a/app/domain/authentication/base/data_object.rb b/app/domain/authentication/base/data_object.rb index 3982c2a61a..a241393b8d 100644 --- a/app/domain/authentication/base/data_object.rb +++ b/app/domain/authentication/base/data_object.rb @@ -3,13 +3,14 @@ module Authentication module Base class DataObject - def type - # TODO: dynamically find this based on class name. - # 'authn-jwt' @type ||= self.class.to_s.split('::')[1].underscore.dasherize end + def identifier + [type, @service_id].compact.join('/') + end + def resource_id [ @account, @@ -22,7 +23,6 @@ def resource_id ].join(':') end - def token_ttl ttl = @token_ttl.present? ? @token_ttl : 'PT8M' ActiveSupport::Duration.parse(ttl.to_s) @@ -30,7 +30,6 @@ def token_ttl raise Errors::Authentication::DataObjects::InvalidTokenTTL.new(resource_id, @token_ttl) end - def annotations_required # binding.pry requires_role_annotations = self.class::REQUIRES_ROLE_ANNOTIONS diff --git a/app/domain/authentication/handler/authentication_handler.rb b/app/domain/authentication/handler/authentication_handler.rb index d29b085b45..1a58049381 100644 --- a/app/domain/authentication/handler/authentication_handler.rb +++ b/app/domain/authentication/handler/authentication_handler.rb @@ -5,7 +5,7 @@ module Handler class AuthenticationHandler def initialize( authenticator_type:, - authn_repo: DB::Repository::AuthenticatorRepository, + authenticator_repository: DB::Repository::AuthenticatorRepository.new, namespace_selector: Authentication::Util::NamespaceSelector, logger: Rails.logger, audit_logger: ::Audit.logger, @@ -13,7 +13,8 @@ def initialize( available_authenticators: Authentication::InstalledAuthenticators, role_repository: DB::Repository::AuthenticatorRoleRepository, authorization: RBAC::Permission.new, - token_factory: TokenFactory.new + token_factory: TokenFactory.new, + validator: DB::Validation ) @authenticator_type = authenticator_type @logger = logger @@ -23,153 +24,185 @@ def initialize( @role_repository = role_repository @authorization = authorization @token_factory = token_factory + @authenticator_repository = authenticator_repository + @validator = validator # Dynamically load authenticator specific classes - @namespace = namespace_selector.select( + namespace = namespace_selector.select( authenticator_type: authenticator_type ) - @strategy = "#{@namespace}::Strategy".constantize - @authn_repo = authn_repo.new( - data_object: "#{@namespace}::DataObjects::Authenticator".constantize, - validations: set_if_present { "#{@namespace}::Validations::AuthenticatorConfiguration".constantize.new(utils: ::Util::ContractUtils) } - ) + @strategy = "#{namespace}::Strategy".constantize + @authenticator_klass = "#{namespace}::DataObjects::Authenticator".constantize + @authenticator_validation = Util::KlassLoader.set_if_present do + "#{namespace}::Validations::AuthenticatorConfiguration".constantize.new( + utils: ::Util::ContractUtils + ) + end + # @role_validation = Util::KlassLoader.set_if_present { "#{@namespace}::Validations::RoleMappingValidations".constantize.new(utils: ::Util::ContractUtils) } + + @success = ::SuccessResponse + @failure = ::FailureResponse end def call(request_ip:, parameters:, request_body: nil, action: nil) - authenticator_identifier = [parameters[:authenticator], parameters[:service_id]].compact.join('/') - authenticator = retrieve_authenticator( - identifier: authenticator_identifier, - account: parameters[:account], - service_id: parameters[:service_id] - ) - - role_identifier_response = @strategy.new( - authenticator: authenticator - ).callback(parameters: parameters, request_body: request_body) - - if role_identifier_response.success? - role_identifier = role_identifier_response.result - else - if role_identifier_response.message.is_a?(Authentication::Base::RoleIdentifier) - role = role_identifier_response.message.role_identifier + service_id = parameters[:service_id] + account = parameters[:account] + role_for_audit = nil + identified_authenticator = nil + + response = retrieve_authenticator(service_id: service_id, account: account).bind do |authenticator| + identified_authenticator = authenticator + identify_role(authenticator: authenticator, parameters: parameters, request_body: request_body).bind do |role_identifier| + retrieve_role(authenticator: authenticator, role_identifier: role_identifier).bind do |role| + role_for_audit = role + check_usage_permitted(role: role, authenticator: authenticator).bind do |check_permitted_role| + check_origin_permitted(role: check_permitted_role, request_ip: request_ip).bind do |check_allowed_role| + log_audit_success(authenticator, check_allowed_role.role_id, request_ip, authenticator.type) + issue_authentication_token(account: account, login: check_allowed_role.login, ttl: authenticator.token_ttl).bind do |token| + log_audit_success(authenticator, role.role_id, request_ip, authenticator.type) + return @success.new(token) + end + end + end + end end - raise role_identifier_response.exception end - role = @role_repository.new( - authenticator: authenticator, - role_contract: @role_contract - ).find( - role_identifier: role_identifier - ) - - # Add an error message (this may actually never be hit as we raise - # upstream if there is a problem with authentication & lookup) - raise Errors::Authorization::AuthenticationFailed unless role - - permitted?( - role: role, - authenticator_identifier: authenticator_identifier, - account: parameters[:account] - ) - - unless role.valid_origin?(request_ip) - raise Errors::Authentication::InvalidOrigin + if role_for_audit.present? + role_identifier = role_for_audit.is_a?(String) ? role_for_audit : role_for_audit&.role_id + log_audit_failure(identified_authenticator, role_identifier, request_ip, authenticator&.type, e) end - log_audit_success(authenticator, role.role_id, request_ip, authenticator.type) - - @token_factory.signed_token( - account: parameters[:account], - username: role.login, - user_ttl: authenticator.token_ttl - ) + response rescue => e - role_identifier = role.is_a?(String) ? role : role&.role_id - log_audit_failure(authenticator, role_identifier, request_ip, authenticator&.type, e) - handle_error(e) + @failure.new(e.message, exception: e) end - private - def params_allowed allowed = %i[authenticator service_id account] allowed += @strategy::ALLOWED_PARAMS if @strategy.const_defined?('ALLOWED_PARAMS') allowed end - def set_if_present(&block) - block.call - rescue NameError - nil + private + + def issue_authentication_token(account:, login:, ttl:) + @success.new( + @token_factory.signed_token( + account: account, + username: login, + user_ttl: ttl + ) + ) + end + + def check_origin_permitted(role:, request_ip:) + if role.valid_origin?(request_ip) + @success.new(role) + else + @failure.new( + role, + status: :forbidden, + exception: Errors::Authentication::InvalidOrigin + ) + end end - def permitted?(role:, authenticator_identifier:, account:) - return true if @available_authenticators.native_authenticators.include?(authenticator_identifier) + def check_usage_permitted(role:, authenticator:) + return @success.new(role) if @available_authenticators.native_authenticators.include?(authenticator.identifier) # Verify that the identified role is permitted to use this authenticator @authorization.permitted?( - role_id: role.id, - resource_id: "#{account}:webservice:conjur/#{authenticator_identifier}", + role: role, + resource_id: authenticator.resource_id, privilege: :authenticate ) end + def retrieve_role(authenticator:, role_identifier:) + @role_repository.new( + authenticator: authenticator + ).find( + role_identifier: role_identifier + ) + end + + def identify_role(authenticator:, parameters:, request_body:) + @strategy.new( + authenticator: authenticator + ).callback(parameters: parameters, request_body: request_body) + end + + def retrieve_authenticator(service_id:, account:) + identifier = [@authenticator_type, service_id].compact.join('/') - def retrieve_authenticator(identifier:, service_id:, account:) # verify authenticator is whitelisted.... unless @available_authenticators.enabled_authenticators.include?(identifier) - raise Errors::Authentication::Security::AuthenticatorNotWhitelisted, identifier + return @failure.new( + "Authenticator: '#{identifier}' is no enabled.", + status: :bad_request, + exception: Errors::Authentication::Security::AuthenticatorNotWhitelisted.new(identifier) + ) end + # If this is a native authenticator (like API Key), it won't be stored + # as webservice, so just load the authenticator. if @available_authenticators.native_authenticators.include?(identifier) - set_if_present do - "#{@namespace}::DataObjects::Authenticator".constantize.new( - account: account - ) - end + @success.new(@authenticator_klass.new(account: account)) else - # Load Authenticator policy and values (validates data stored as variables) - @authn_repo.find( + # Load Authenticator policy and variables + @authenticator_repository.find( type: @authenticator_type, account: account, service_id: service_id - ) + ).bind do |authenticator_data| + + # validate data against authenticator specific validations + @validator.new( + validations: @authenticator_validation + ).validate(data: authenticator_data).bind do |validated_authenticator_data| + + # Instantiate and return authenticator data object for future use + @success.new(@authenticator_klass.new(**validated_authenticator_data)) + rescue => e + @failure.new(e.message, exception: e) + end + end end end - def handle_error(err) - # Log authentication errors (but don't raise...) - authentication_error = LogMessages::Authentication::AuthenticationError.new(err.inspect) - @logger.info(authentication_error) - - @logger.info("#{err.class.name}: #{err.message}") - err.backtrace.each {|l| @logger.info(l) } - - case err - when Errors::Authentication::Security::RoleNotAuthorizedOnResource - raise ApplicationController::Forbidden - - when Errors::Authentication::RequestBody::MissingRequestParam, - Errors::Authentication::Security::RoleNotFound, - Errors::Authentication::Security::AuthenticatorNotWhitelisted, - Errors::Authentication::AuthnOidc::TokenVerificationFailed, - Errors::Authentication::AuthnOidc::TokenRetrievalFailed, - Rack::OAuth2::Client::Error # Code value mismatch - raise ApplicationController::BadRequest - - when Errors::Conjur::RequestedResourceNotFound, - Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty - raise ApplicationController::Unauthorized - - when Errors::Authentication::Jwt::TokenExpired - raise ApplicationController::Unauthorized.new(err.message, true) - - else - raise ApplicationController::Unauthorized - end - end + # def handle_error(err) + # # Log authentication errors (but don't raise...) + # authentication_error = LogMessages::Authentication::AuthenticationError.new(err.inspect) + # @logger.info(authentication_error) + + # @logger.info("#{err.class.name}: #{err.message}") + # err.backtrace.each {|l| @logger.info(l) } + + # case err + # when Errors::Authentication::Security::RoleNotAuthorizedOnResource + # raise ApplicationController::Forbidden + + # when Errors::Authentication::RequestBody::MissingRequestParam, + # Errors::Authentication::Security::RoleNotFound, + # Errors::Authentication::Security::AuthenticatorNotWhitelisted, + # Errors::Authentication::AuthnOidc::TokenVerificationFailed, + # Errors::Authentication::AuthnOidc::TokenRetrievalFailed, + # Rack::OAuth2::Client::Error # Code value mismatch + # raise ApplicationController::BadRequest + + # when Errors::Conjur::RequestedResourceNotFound, + # Errors::Authentication::AuthnOidc::IdTokenClaimNotFoundOrEmpty + # raise ApplicationController::Unauthorized + + # when Errors::Authentication::Jwt::TokenExpired + # raise ApplicationController::Unauthorized.new(err.message, true) + + # else + # raise ApplicationController::Unauthorized + # end + # end def log_audit_success(service, role_id, client_ip, type) @audit_logger.log( diff --git a/app/domain/authentication/util/klass_loader.rb b/app/domain/authentication/util/klass_loader.rb new file mode 100644 index 0000000000..506bfbff78 --- /dev/null +++ b/app/domain/authentication/util/klass_loader.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Authentication + module Util + + # This is a utility to handle detection of Authenticator and Role Annotation + # validations. This enables us to optionally add validations for a particular + # authenticator. + # + # It returns either the provided block or nil. + class KlassLoader + class << self + def set_if_present(&block) + block.call + rescue NameError + nil + end + end + end + end +end diff --git a/app/domain/rbac/permission.rb b/app/domain/rbac/permission.rb index f5eccbe255..8382d702a1 100644 --- a/app/domain/rbac/permission.rb +++ b/app/domain/rbac/permission.rb @@ -2,24 +2,38 @@ module RBAC class Permission - def initialize(role_library: Role, resource_library: Resource) - @role_library = role_library + def initialize(resource_library: ::Resource) @resource_library = resource_library + + @success = ::SuccessResponse + @failure = ::FailureResponse end - def permitted?(role_id:, resource_id:, privilege:) - role = @role_library[role_id] + def permitted?(role:, resource_id:, privilege:) resource = @resource_library[resource_id] - return true if role.present? && role.allowed_to?(privilege, resource) + unless resource.present? + return @failure.new( + "Resource '#{resource_id}' was not found", + status: :unauthorized, + exception: Errors::Conjur::RequiredResourceMissing.new(resource_id) + ) + end - raise( - Errors::Authentication::Security::RoleNotAuthorizedOnResource.new( - [role.kind, role.identifier].join('/'), - privilege, - resource.id + # binding.pry + if role.present? && role.allowed_to?(privilege, resource) + @success.new(role) + else + @failure.new( + role, + status: :forbidden, + exception: Errors::Authentication::Security::RoleNotAuthorizedOnResource.new( + [role.try(:kind), role.try(:identifier)].join('/'), + privilege, + resource_id + ) ) - ) + end end end end diff --git a/app/domain/responses.rb b/app/domain/responses.rb index 014312d926..4a3a9cad94 100644 --- a/app/domain/responses.rb +++ b/app/domain/responses.rb @@ -26,13 +26,14 @@ def bind(&_block) # Responsible for handling "failed" requests. # Log level and Response code are both option. class FailureResponse - attr_reader :message, :status, :exception + attr_reader :message, :status, :exception, :backtrace def initialize(message, level: :warn, status: :unauthorized, exception: nil) @message = message @level = level @status = status @exception = exception + @backtrace = caller # Add stack trace end def success? diff --git a/ci/oauth/keycloak/fetch_certificate b/ci/oauth/keycloak/fetch_certificate index 5b103f3cfc..3ea58cce23 100755 --- a/ci/oauth/keycloak/fetch_certificate +++ b/ci/oauth/keycloak/fetch_certificate @@ -15,4 +15,6 @@ openssl s_client \ hash=$(openssl x509 -hash -in /etc/ssl/certs/keycloak.pem --noout) +rm -f "/etc/ssl/certs/${hash}.0" + ln -s /etc/ssl/certs/keycloak.pem "/etc/ssl/certs/${hash}.0" || true diff --git a/config/routes.rb b/config/routes.rb index b994c458f3..88b0912bef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -38,7 +38,7 @@ def matches?(request) end # New OIDC endpoint - get '/:authenticator(/:service_id)/:account/authenticate' => 'authenticate#oidc_authenticate_code_redirect' + get '/:authenticator(/:service_id)/:account/authenticate' => 'authenticate#authenticate_via_get' post '/authn-gcp/:account/authenticate' => 'authenticate#authenticate_gcp' post '/authn-oidc(/:service_id)/:account/authenticate' => 'authenticate#authenticate_oidc' diff --git a/cucumber/authenticators_oidc/features/authn_oidc_v2.feature b/cucumber/authenticators_oidc/features/authn_oidc_v2.feature index 4b6daff86e..adfbc46b09 100644 --- a/cucumber/authenticators_oidc/features/authn_oidc_v2.feature +++ b/cucumber/authenticators_oidc/features/authn_oidc_v2.feature @@ -264,10 +264,10 @@ Feature: OIDC Authenticator V2 - Users can authenticate with OIDC authenticator Given I save my place in the log file And I fetch a code for username "alice" and password "alice" from "keycloak2" When I authenticate via OIDC V2 with code and service-id "non-exist" - Then it is not found + Then it is a bad request And The following appears in the log after my savepoint: """ - Errors::Conjur::RequestedResourceNotFound: CONJ00123E Resource + Errors::Authentication::Security::AuthenticatorNotWhitelisted: CONJ00004E 'authn-oidc/non-exist' is not enabled """ @smoke diff --git a/dev/start b/dev/start index 54c33a8954..e2b1882be9 100755 --- a/dev/start +++ b/dev/start @@ -334,7 +334,7 @@ enable_oidc_authenticators() { echo "Configuring Keycloak as OpenID provider for manual testing" # We enable an OIDC authenticator without a service-id to test that it's # invalid. - enabled_authenticators="$enabled_authenticators,authn-oidc/keycloak,authn-oidc,authn-oidc/keycloak2" + enabled_authenticators="$enabled_authenticators,authn-oidc/keycloak,authn-oidc,authn-oidc/keycloak2,authn-oidc/keycloak2-long-lived" fi if [[ $ENABLE_OIDC_OKTA = true ]]; then From aaae5b09cac2c09d8115d4c8c3cf3f27c27c81bc Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Fri, 27 Oct 2023 23:33:44 -0600 Subject: [PATCH 5/8] Refactor to remove OIDC gem --- .../authentication/authn_oidc/v2/client.rb | 610 +++++++++++------- .../authn_oidc/v2/network_transport.rb | 67 ++ .../authentication/authn_oidc/v2/strategy.rb | 23 +- 3 files changed, 448 insertions(+), 252 deletions(-) create mode 100644 app/domain/authentication/authn_oidc/v2/network_transport.rb diff --git a/app/domain/authentication/authn_oidc/v2/client.rb b/app/domain/authentication/authn_oidc/v2/client.rb index fbf37e28e2..93fda1d594 100644 --- a/app/domain/authentication/authn_oidc/v2/client.rb +++ b/app/domain/authentication/authn_oidc/v2/client.rb @@ -1,287 +1,407 @@ +# frozen_string_literal: true + module Authentication module AuthnOidc module V2 - class Client + class OidcClient def initialize( authenticator:, - client: ::OpenIDConnect::Client, - oidc_id_token: ::OpenIDConnect::ResponseObject::IdToken, - discovery_configuration: ::OpenIDConnect::Discovery::Provider::Config, + client: NetworkTransporter, cache: Rails.cache, logger: Rails.logger ) @authenticator = authenticator - @client = client - @oidc_id_token = oidc_id_token - @discovery_configuration = discovery_configuration @cache = cache @logger = logger + @client = client.new( + authenticator: authenticator + ) + @success = ::SuccessResponse @failure = ::FailureResponse end - # Writing certificates to the default system cert store requires - # superuser privilege. Instead, Conjur will use ${CONJUR_ROOT}/tmp/certs. - def self.default_cert_dir(dir: Dir, fileutils: FileUtils) - if @default_cert_dir.blank? - conjur_root = __dir__.slice(0..(__dir__.index('/app'))) - @default_cert_dir = File.join(conjur_root, "tmp/certs") + def exchange(code:, nonce:, code_verifier: nil) + exchange_token(code: code, nonce: nonce, code_verifier: code_verifier).bind do |bearer_token| + decode_token(bearer_token).bind do |decoded_token| + verify_token(token: decoded_token, nonce: nonce).bind do |verified_token| + @success.new(verified_token) + end + end end - - fileutils.mkdir_p(@default_cert_dir) unless dir.exist?(@default_cert_dir.to_s) - - @default_cert_dir 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 + # 'jwks_uri' - GET + # 'token_endpoint' - POST + # Public method so strategy can call it to verify configuration + def configuration + @configuration ||= @client.get('.well-known/openid-configuration') end - def callback(code:, nonce:, code_verifier: nil) - oidc_client.authorization_code = code - access_token_args = { client_auth_method: :basic } - access_token_args[:code_verifier] = code_verifier if code_verifier.present? - begin - bearer_token = oidc_client.access_token!(**access_token_args) - rescue Rack::OAuth2::Client::Error => e - return @failure.new( - e.message, - exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(e.message), - status: :bad_request - ) - 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 - ) + private - rescue => e - attempts += 1 - if attempts > 1 - return @failure.new( - 'JWKS signing check failed', - exception: e, - status: :unauthorized - ) - end - # 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 + def exchange_token(code:, nonce:, code_verifier:) + args = { + grant_type: 'authorization_code', + scope: true, + code: code, + nonce: nonce + } + args[:code_verifier] = code_verifier if code_verifier.present? + args[:redirect_uri] = ERB::Util.url_encode(@authenticator.redirect_uri) if @authenticator.redirect_uri.present? begin - decoded_id_token.verify!( - issuer: @authenticator.provider_uri, - client_id: @authenticator.client_id, - nonce: nonce + response = @client.post( + url: configuration['token_endpoint'].gsub("#{@authenticator.provider_uri}/", ''), + body: args.map { |k, v| "#{k}=#{v}" }.join('&'), + headers: { 'Authorization' => "Basic #{Base64.strict_encode64([@authenticator.client_id, @authenticator.client_secret].join(':'))}" } ) - @success.new(decoded_id_token) rescue => e - @failure.new( + return @failure.new( e.message, - exception: e, + exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(e.message), status: :bad_request ) end - end - - # callback_with_temporary_cert wraps the callback method with commands - # to write & clean up a given certificate or cert chain in a given - # directory. By default, ${CONJUR_ROOT}/tmp/certs is used. - # - # The temporary certificate file name is "x.n", where x is the hash of - # the certificate subject name, and n is incrememnted from 0 in case of - # collision. - # - # Unlike self.discover, which wraps a single ::OpenIDConnect method, - # callback_with_temporary_cert wraps the entire callback method, which - # includes multiple calls to the OIDC provider, including at least one - # discover! call. The temporary certs will apply to all required - # operations. - def callback_with_temporary_cert( - code:, - nonce:, - code_verifier: nil, - cert_dir: Authentication::AuthnOidc::V2::Client.default_cert_dir, - cert_string: nil - ) - c = -> { callback(code: code, nonce: nonce, code_verifier: code_verifier) } - - return c.call if cert_string.blank? - - begin - certs_a = ::Conjur::CertUtils.parse_certs(cert_string) - rescue OpenSSL::X509::CertificateError => e - raise Errors::Authentication::AuthnOidc::InvalidCertificate, e.message - end - raise Errors::Authentication::AuthnOidc::InvalidCertificate, "provided string does not contain a certificate" if certs_a.empty? - - symlink_a = [] - - Dir.mktmpdir do |tmp_dir| - certs_a.each_with_index do |cert, idx| - tmp_file = File.join(tmp_dir, "conjur-oidc-client.#{idx}.pem") - File.write(tmp_file, cert.to_s) - n = 0 - hash = cert.subject.hash.to_s(16) - while true - symlink = File.join(cert_dir, "#{hash}.#{n}") - break unless File.exist?(symlink) - - n += 1 - end - - File.symlink(tmp_file, symlink) - symlink_a << symlink - end - - if OpenIDConnect.http_config.nil? || OpenIDConnect.http_client.ssl.ca_path != cert_dir - config_proc = proc do |config| - config.ssl.ca_path = cert_dir - config.ssl.verify_mode = OpenSSL::SSL::VERIFY_PEER - end - - # OpenIDConnect gem only accepts a single Faraday configuration - # through calls to its .http_config method, and future calls to - # the #http_config method return the first config instance. - # - # On the first call to OpenIDConnect.http_config, it will pass the - # new Faraday configuration to its dependency gems that also have - # nil configs. We can't be certain that each gem is configured - # with the same Faraday config and need them synchronized, so we - # inject them manually. - OpenIDConnect.class_variable_set(:@@http_config, config_proc) - WebFinger.instance_variable_set(:@http_config, config_proc) - SWD.class_variable_set(:@@http_config, config_proc) - Rack::OAuth2.class_variable_set(:@@http_config, config_proc) - end - - c.call - ensure - symlink_a.each{ |s| File.unlink(s) if s.present? && File.symlink?(s) } - end + @success.new(response['id_token'] || response['access_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 - self.class.discover( - provider_uri: @authenticator.provider_uri, - discovery_configuration: @discovery_configuration, - cert_string: @authenticator.ca_cert - ) - rescue 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 + def decode_token(encoded_token) + @success.new( + JWT.decode( + encoded_token, + nil, + true, # Verify the signature of this token + algorithms: %w[RS256 RS384 RS512], + iss: @authenticator.provider_uri, + verify_iss: true, + aud: @authenticator.client_id, + verify_aud: true, + jwks: fetch_jwks + ).first + ) + rescue => e + @failure.new(e.message, exception: e, status: :bad_request) end - # discover wraps ::OpenIDConnect::Discovery::Provider::Config.discover! - # with commands to write & clean up a given certificate or cert chain in - # a given directory. By default, ${CONJUR_ROOT}/tmp/certs is used. - # - # The temporary certificate file name is "x.n", where x is the hash of - # the certificate subject name, and n is incremented from 0 in case of - # collision. - # - # discover is a class method, because there are a few contexts outside - # this class where the underlying discover! method is used. Call it by - # running Authentication::AuthnOIDC::V2::Client.discover(...). - def self.discover( - provider_uri:, - discovery_configuration: ::OpenIDConnect::Discovery::Provider::Config, - cert_dir: default_cert_dir, - cert_string: nil, - jwks: false - ) - case jwks - when false - d = -> { discovery_configuration.discover!(provider_uri) } - when true - d = -> { discovery_configuration.discover!(provider_uri).jwks } + def verify_token(token:, nonce:) + unless token['nonce'] == nonce + return @failure.new('nonce does not match') end - return d.call if cert_string.blank? - - begin - certs_a = ::Conjur::CertUtils.parse_certs(cert_string) - rescue OpenSSL::X509::CertificateError => e - raise Errors::Authentication::AuthnOidc::InvalidCertificate, e.message - end - raise Errors::Authentication::AuthnOidc::InvalidCertificate, "provided string does not contain a certificate" if certs_a.empty? - - symlink_a = [] - - Dir.mktmpdir do |tmp_dir| - certs_a.each_with_index do |cert, idx| - tmp_file = File.join(tmp_dir, "conjur-oidc-client.#{idx}.pem") - File.write(tmp_file, cert.to_s) - - n = 0 - hash = cert.subject.hash.to_s(16) - while true - symlink = File.join(cert_dir, "#{hash}.#{n}") - break unless File.exist?(symlink) - - n += 1 - end - - File.symlink(tmp_file, symlink) - symlink_a << symlink - end - - if OpenIDConnect.http_config.nil? || OpenIDConnect.http_client.ssl.ca_path != cert_dir - config_proc = proc do |config| - config.ssl.ca_path = cert_dir - config.ssl.verify_mode = OpenSSL::SSL::VERIFY_PEER - end - - # OpenIDConnect gem only accepts a single Faraday configuration - # through calls to its .http_config method, and future calls to - # the #http_config method return the first config instance. - # - # On the first call to OpenIDConnect.http_config, it will pass the - # new Faraday configuration to its dependency gems that also have - # nil configs. We can't be certain that each gem is configured - # with the same Faraday config and need them synchronized, so we - # inject them manually. - OpenIDConnect.class_variable_set(:@@http_config, config_proc) - WebFinger.instance_variable_set(:@http_config, config_proc) - SWD.class_variable_set(:@@http_config, config_proc) - Rack::OAuth2.class_variable_set(:@@http_config, config_proc) - end + @success.new(token) + end - d.call - ensure - symlink_a.each{ |s| File.unlink(s) if s.present? && File.symlink?(s) } - end + def fetch_jwks + @client.get( + configuration['jwks_uri'].gsub("#{@authenticator.provider_uri}/", '') + ) end + + # def exchange_code_for_token(code:, nonce:, code_verifier: nil) + # args = { + # grant_type: 'authorization_code', + # scope: true, + # code: code, + # nonce: nonce + # } + # args[:code_verifier] = code_verifier if code_verifier.present? + # args[:redirect_uri] = ERB::Util.url_encode(@authenticator.redirect_uri) if @authenticator.redirect_uri.present? + + # response = @client.post( + # url: configuration['token_endpoint'].gsub("#{@authenticator.provider_uri}/", ''), + # body: args.map { |k, v| "#{k}=#{v}" }.join('&'), + # headers: { 'Authorization' => "Basic #{Base64.strict_encode64([@authenticator.client_id, @authenticator.client_secret].join(':'))}" } + # ) + + # bearer_token = response['id_token'] || response['access_token'] + + # decoded_jwt = JWT.decode( + # bearer_token, + # nil, + # true, # Verify the signature of this token + # algorithms: %w[RS256 RS384 RS512], + # iss: @authenticator.provider_uri, + # verify_iss: true, + # aud: @authenticator.client_id, + # verify_aud: true, + # jwks: fetch_jwks + # ).first + + # unless decoded_jwt['nonce'] == nonce + # return @failure.new('nonce does not match') + # end + + # @success.new(decoded_jwt) + # end + + + # # Writing certificates to the default system cert store requires + # # superuser privilege. Instead, Conjur will use ${CONJUR_ROOT}/tmp/certs. + # def self.default_cert_dir(dir: Dir, fileutils: FileUtils) + # if @default_cert_dir.blank? + # conjur_root = __dir__.slice(0..(__dir__.index('/app'))) + # @default_cert_dir = File.join(conjur_root, "tmp/certs") + # end + + # fileutils.mkdir_p(@default_cert_dir) unless dir.exist?(@default_cert_dir.to_s) + + # @default_cert_dir + # 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 + # end + + # def callback(code:, nonce:, code_verifier: nil) + # response = exchange_code_for_token(code: code, nonce: nonce, code_verifier: code_verifier) + # # binding.pry + + # # oidc_client.authorization_code = code + # # access_token_args = { client_auth_method: :basic } + # # access_token_args[:code_verifier] = code_verifier if code_verifier.present? + # # begin + # # bearer_token = oidc_client.access_token!(**access_token_args) + # # rescue Rack::OAuth2::Client::Error => e + # # return @failure.new( + # # e.message, + # # exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(e.message), + # # status: :bad_request + # # ) + # # 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 => e + # # attempts += 1 + # # if attempts > 1 + # # return @failure.new( + # # 'JWKS signing check failed', + # # exception: e, + # # status: :unauthorized + # # ) + # # end + # # # 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 + # # ) + # # @success.new(decoded_id_token) + # # rescue => e + # # @failure.new( + # # e.message, + # # exception: e, + # # status: :bad_request + # # ) + # # end + # end + + # # callback_with_temporary_cert wraps the callback method with commands + # # to write & clean up a given certificate or cert chain in a given + # # directory. By default, ${CONJUR_ROOT}/tmp/certs is used. + # # + # # The temporary certificate file name is "x.n", where x is the hash of + # # the certificate subject name, and n is incrememnted from 0 in case of + # # collision. + # # + # # Unlike self.discover, which wraps a single ::OpenIDConnect method, + # # callback_with_temporary_cert wraps the entire callback method, which + # # includes multiple calls to the OIDC provider, including at least one + # # discover! call. The temporary certs will apply to all required + # # operations. + # def callback_with_temporary_cert( + # code:, + # nonce:, + # code_verifier: nil, + # cert_dir: Authentication::AuthnOidc::V2::Client.default_cert_dir, + # cert_string: nil + # ) + # c = -> { callback(code: code, nonce: nonce, code_verifier: code_verifier) } + + # return c.call if cert_string.blank? + + # begin + # certs_a = ::Conjur::CertUtils.parse_certs(cert_string) + # rescue OpenSSL::X509::CertificateError => e + # raise Errors::Authentication::AuthnOidc::InvalidCertificate, e.message + # end + # raise Errors::Authentication::AuthnOidc::InvalidCertificate, "provided string does not contain a certificate" if certs_a.empty? + + # symlink_a = [] + + # Dir.mktmpdir do |tmp_dir| + # certs_a.each_with_index do |cert, idx| + # tmp_file = File.join(tmp_dir, "conjur-oidc-client.#{idx}.pem") + # File.write(tmp_file, cert.to_s) + + # n = 0 + # hash = cert.subject.hash.to_s(16) + # while true + # symlink = File.join(cert_dir, "#{hash}.#{n}") + # break unless File.exist?(symlink) + + # n += 1 + # end + + # File.symlink(tmp_file, symlink) + # symlink_a << symlink + # end + + # if OpenIDConnect.http_config.nil? || OpenIDConnect.http_client.ssl.ca_path != cert_dir + # config_proc = proc do |config| + # config.ssl.ca_path = cert_dir + # config.ssl.verify_mode = OpenSSL::SSL::VERIFY_PEER + # end + + # # OpenIDConnect gem only accepts a single Faraday configuration + # # through calls to its .http_config method, and future calls to + # # the #http_config method return the first config instance. + # # + # # On the first call to OpenIDConnect.http_config, it will pass the + # # new Faraday configuration to its dependency gems that also have + # # nil configs. We can't be certain that each gem is configured + # # with the same Faraday config and need them synchronized, so we + # # inject them manually. + # OpenIDConnect.class_variable_set(:@@http_config, config_proc) + # WebFinger.instance_variable_set(:@http_config, config_proc) + # SWD.class_variable_set(:@@http_config, config_proc) + # Rack::OAuth2.class_variable_set(:@@http_config, config_proc) + # end + + # c.call + # ensure + # symlink_a.each{ |s| File.unlink(s) if s.present? && File.symlink?(s) } + # end + # 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 + # self.class.discover( + # provider_uri: @authenticator.provider_uri, + # discovery_configuration: @discovery_configuration, + # cert_string: @authenticator.ca_cert + # ) + # rescue 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 + + # # discover wraps ::OpenIDConnect::Discovery::Provider::Config.discover! + # # with commands to write & clean up a given certificate or cert chain in + # # a given directory. By default, ${CONJUR_ROOT}/tmp/certs is used. + # # + # # The temporary certificate file name is "x.n", where x is the hash of + # # the certificate subject name, and n is incremented from 0 in case of + # # collision. + # # + # # discover is a class method, because there are a few contexts outside + # # this class where the underlying discover! method is used. Call it by + # # running Authentication::AuthnOIDC::V2::Client.discover(...). + # def self.discover( + # provider_uri:, + # discovery_configuration: ::OpenIDConnect::Discovery::Provider::Config, + # cert_dir: default_cert_dir, + # cert_string: nil, + # jwks: false + # ) + # case jwks + # when false + # d = -> { discovery_configuration.discover!(provider_uri) } + # when true + # d = -> { discovery_configuration.discover!(provider_uri).jwks } + # end + + # return d.call if cert_string.blank? + + # begin + # certs_a = ::Conjur::CertUtils.parse_certs(cert_string) + # rescue OpenSSL::X509::CertificateError => e + # raise Errors::Authentication::AuthnOidc::InvalidCertificate, e.message + # end + # raise Errors::Authentication::AuthnOidc::InvalidCertificate, "provided string does not contain a certificate" if certs_a.empty? + + # symlink_a = [] + + # Dir.mktmpdir do |tmp_dir| + # certs_a.each_with_index do |cert, idx| + # tmp_file = File.join(tmp_dir, "conjur-oidc-client.#{idx}.pem") + # File.write(tmp_file, cert.to_s) + + # n = 0 + # hash = cert.subject.hash.to_s(16) + # while true + # symlink = File.join(cert_dir, "#{hash}.#{n}") + # break unless File.exist?(symlink) + + # n += 1 + # end + + # File.symlink(tmp_file, symlink) + # symlink_a << symlink + # end + + # if OpenIDConnect.http_config.nil? || OpenIDConnect.http_client.ssl.ca_path != cert_dir + # config_proc = proc do |config| + # config.ssl.ca_path = cert_dir + # config.ssl.verify_mode = OpenSSL::SSL::VERIFY_PEER + # end + + # # OpenIDConnect gem only accepts a single Faraday configuration + # # through calls to its .http_config method, and future calls to + # # the #http_config method return the first config instance. + # # + # # On the first call to OpenIDConnect.http_config, it will pass the + # # new Faraday configuration to its dependency gems that also have + # # nil configs. We can't be certain that each gem is configured + # # with the same Faraday config and need them synchronized, so we + # # inject them manually. + # OpenIDConnect.class_variable_set(:@@http_config, config_proc) + # WebFinger.instance_variable_set(:@http_config, config_proc) + # SWD.class_variable_set(:@@http_config, config_proc) + # Rack::OAuth2.class_variable_set(:@@http_config, config_proc) + # end + + # d.call + # ensure + # symlink_a.each{ |s| File.unlink(s) if s.present? && File.symlink?(s) } + # end + # end end end end diff --git a/app/domain/authentication/authn_oidc/v2/network_transport.rb b/app/domain/authentication/authn_oidc/v2/network_transport.rb new file mode 100644 index 0000000000..2222b62d6f --- /dev/null +++ b/app/domain/authentication/authn_oidc/v2/network_transport.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Authentication + module AuthnOidc + module V2 + class NetworkTransporter + def initialize(authenticator:) + @authenticator = authenticator + end + + def get(url) + with_ssl do |ssl_options| + network_call(ssl_options) do |client| + client.get(url).body + end + end + end + + def post(url:, body:, headers: {}) + with_ssl do |ssl_options| + network_call(ssl_options) do |client| + client.post(url, body, headers).body + end + end + end + + private + + def with_ca(ssl_options, &block) + Dir.mkdir('./tmp/certificates') unless Dir.exist?('./tmp/certificates') + Tempfile.create('ca', './tmp/certificates/') do |ca_certificate| + ca_certificate.write(@authenticator.ca_cert) + ca_certificate.flush + ssl_options[:ca_file] = ca_certificate.path + + block.call(ssl_options) + + ensure + File.delete(ca_certificate.path) if File.exist?(ca_certificate.path) + end + end + + def with_ssl(&block) + ssl_options = {} + if @authenticator.ca_cert.present? + with_ca(ssl_options) do |updated_ssl_options| + block.call(updated_ssl_options) + end + else + block.call(ssl_options) + end + end + + def network_call(ssl_options, &block) + client = Faraday.new(@authenticator.provider_uri, ssl: ssl_options) do |conn| + conn.response(:json, content_type: /\bjson$/) + conn.response(:raise_error) + + conn.adapter(Faraday.default_adapter) + end + + block.call(client) + end + end + end + end +end diff --git a/app/domain/authentication/authn_oidc/v2/strategy.rb b/app/domain/authentication/authn_oidc/v2/strategy.rb index f4abc0bcd1..c6cba84cb8 100644 --- a/app/domain/authentication/authn_oidc/v2/strategy.rb +++ b/app/domain/authentication/authn_oidc/v2/strategy.rb @@ -9,11 +9,12 @@ class Strategy def initialize( authenticator:, - client: Authentication::AuthnOidc::V2::Client, + oidc_client: OidcClient, + # client: Authentication::AuthnOidc::V2::Client, logger: Rails.logger ) @authenticator = authenticator - @client = client.new(authenticator: authenticator) + @oidc_client = oidc_client.new(authenticator: authenticator) @logger = logger @success = ::SuccessResponse @@ -40,7 +41,8 @@ def callback(parameters:, request_body: nil) # Called by status handler. This handles checking as much of the strategy # integrity as possible without performing an actual authentication. def verify_status - @client.discover + # @client.discover + @oidc_client.configuration end private @@ -61,16 +63,23 @@ def validate_parameters(parameters) end def validate_jwt_token(parameters) - @client.callback_with_temporary_cert( + @oidc_client.exchange( code: parameters[:code], nonce: parameters[:nonce], - code_verifier: parameters[:code_verifier], - cert_string: @authenticator.ca_cert + code_verifier: parameters[:code_verifier] ) + # @client.callback_with_temporary_cert( + # code: parameters[:code], + # nonce: parameters[:nonce], + # code_verifier: parameters[:code_verifier], + # cert_string: @authenticator.ca_cert + # ) end def resolve_identity(jwt:) - identity = jwt.raw_attributes.with_indifferent_access[@authenticator.claim_mapping] + # binding.pry + # identity = jwt.raw_attributes.with_indifferent_access[@authenticator.claim_mapping] + identity = jwt[@authenticator.claim_mapping] return @success.new(identity) if identity.present? @failure.new( From 9241fef6f946e282fd2ec754143ae094aa0f901d Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Mon, 30 Oct 2023 09:43:20 -0600 Subject: [PATCH 6/8] Extract data transport and create an internal OIDC client --- ...rk_transport.rb => network_transporter.rb} | 19 ++++++++++++++++- .../v2/{client.rb => oidc_client.rb} | 21 +++++++++++-------- 2 files changed, 30 insertions(+), 10 deletions(-) rename app/domain/authentication/authn_oidc/v2/{network_transport.rb => network_transporter.rb} (75%) rename app/domain/authentication/authn_oidc/v2/{client.rb => oidc_client.rb} (97%) diff --git a/app/domain/authentication/authn_oidc/v2/network_transport.rb b/app/domain/authentication/authn_oidc/v2/network_transporter.rb similarity index 75% rename from app/domain/authentication/authn_oidc/v2/network_transport.rb rename to app/domain/authentication/authn_oidc/v2/network_transporter.rb index 2222b62d6f..eb28ea7bdc 100644 --- a/app/domain/authentication/authn_oidc/v2/network_transport.rb +++ b/app/domain/authentication/authn_oidc/v2/network_transporter.rb @@ -6,9 +6,15 @@ module V2 class NetworkTransporter def initialize(authenticator:) @authenticator = authenticator + + @success = ::SuccessResponse + @failure = ::FailureResponse end def get(url) + # make_call do |client| + # client.post(url, body, headers).body + # end with_ssl do |ssl_options| network_call(ssl_options) do |client| client.get(url).body @@ -19,13 +25,24 @@ def get(url) def post(url:, body:, headers: {}) with_ssl do |ssl_options| network_call(ssl_options) do |client| - client.post(url, body, headers).body + @success.new(client.post(url, body, headers).body) end end + rescue => e + @failure.new(e.message, exception: e, status: :bad_request) end private + def make_call(&block) + with_ssl do |ssl_options| + network_call(ssl_options) do |client| + block.call(client) + # client.post(url, body, headers).body + end + end + end + def with_ca(ssl_options, &block) Dir.mkdir('./tmp/certificates') unless Dir.exist?('./tmp/certificates') Tempfile.create('ca', './tmp/certificates/') do |ca_certificate| diff --git a/app/domain/authentication/authn_oidc/v2/client.rb b/app/domain/authentication/authn_oidc/v2/oidc_client.rb similarity index 97% rename from app/domain/authentication/authn_oidc/v2/client.rb rename to app/domain/authentication/authn_oidc/v2/oidc_client.rb index 93fda1d594..19bea1a317 100644 --- a/app/domain/authentication/authn_oidc/v2/client.rb +++ b/app/domain/authentication/authn_oidc/v2/oidc_client.rb @@ -51,21 +51,24 @@ def exchange_token(code:, nonce:, code_verifier:) args[:code_verifier] = code_verifier if code_verifier.present? args[:redirect_uri] = ERB::Util.url_encode(@authenticator.redirect_uri) if @authenticator.redirect_uri.present? - begin + # begin + # binding.pry response = @client.post( url: configuration['token_endpoint'].gsub("#{@authenticator.provider_uri}/", ''), body: args.map { |k, v| "#{k}=#{v}" }.join('&'), headers: { 'Authorization' => "Basic #{Base64.strict_encode64([@authenticator.client_id, @authenticator.client_secret].join(':'))}" } - ) - rescue => e - return @failure.new( - e.message, + ).bind do |response| + return @success.new(response['id_token'] || response['access_token']) + end + # rescue => e + # binding.pry + @failure.new( + response.message, exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(e.message), - status: :bad_request + status: :unauthorized ) - end - - @success.new(response['id_token'] || response['access_token']) + # end + # binding.pry end def decode_token(encoded_token) From 0518eecda5318d25991b3c3e6348d85693437303 Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Mon, 30 Oct 2023 15:54:14 -0600 Subject: [PATCH 7/8] Rework Network Transport to use default Ruby HTTP library --- .../authn_oidc/v2/network_transporter.rb | 116 +++--- .../authn_oidc/v2/oidc_client.rb | 389 +++--------------- 2 files changed, 122 insertions(+), 383 deletions(-) diff --git a/app/domain/authentication/authn_oidc/v2/network_transporter.rb b/app/domain/authentication/authn_oidc/v2/network_transporter.rb index eb28ea7bdc..2568624846 100644 --- a/app/domain/authentication/authn_oidc/v2/network_transporter.rb +++ b/app/domain/authentication/authn_oidc/v2/network_transporter.rb @@ -4,79 +4,97 @@ module Authentication module AuthnOidc module V2 class NetworkTransporter - def initialize(authenticator:) - @authenticator = authenticator + def initialize(hostname:, ca_certificate: nil, temp_directory: './tmp/certificates', http: Net::HTTP) + # Set as empty so we can check it later when making network calls + @ca_certificate = nil + if ca_certificate.present? + @temp_directory = temp_directory + create_directory_if_missing(temp_directory) + @ca_certificate = ca_certificate + end + + # Set the default hostname + @uri = URI(hostname) + # Stripping path if present + @uri.path = '' + + @http = http @success = ::SuccessResponse @failure = ::FailureResponse end - def get(url) - # make_call do |client| - # client.post(url, body, headers).body - # end - with_ssl do |ssl_options| - network_call(ssl_options) do |client| - client.get(url).body - end - end - end - - def post(url:, body:, headers: {}) - with_ssl do |ssl_options| - network_call(ssl_options) do |client| - @success.new(client.post(url, body, headers).body) + def get(path) + http_client.start do |http| + request = Net::HTTP::Get.new(@uri.to_s + URI(path).path) + response = http.request(request) + if response.code == '200' + @success.new(JSON.parse(response.body)) + else + @failure.new(response) end end rescue => e @failure.new(e.message, exception: e, status: :bad_request) end - private - - def make_call(&block) - with_ssl do |ssl_options| - network_call(ssl_options) do |client| - block.call(client) - # client.post(url, body, headers).body + def post(path:, body: '', basic_auth: []) + http_client.start do |http| + request = Net::HTTP::Post.new(@uri + URI(path).path) + request.body = body + request.basic_auth(*basic_auth) unless basic_auth.empty? + response = http.request(request) + begin + if response.code == '200' + @success.new(JSON.parse(response.body)) + else + @failure.new(response) + end + rescue => e + @failure.new(e.message, exception: e, status: :bad_request) end end end - def with_ca(ssl_options, &block) - Dir.mkdir('./tmp/certificates') unless Dir.exist?('./tmp/certificates') - Tempfile.create('ca', './tmp/certificates/') do |ca_certificate| - ca_certificate.write(@authenticator.ca_cert) - ca_certificate.flush - ssl_options[:ca_file] = ca_certificate.path + private - block.call(ssl_options) + def http_client + @http_client ||= begin + http = @http.new(@uri.host, @uri.port) + return http unless @uri.instance_of?(URI::HTTPS) - ensure - File.delete(ca_certificate.path) if File.exist?(ca_certificate.path) - end - end + # Enable SSL support + http.use_ssl = true + http.verify_mode = OpenSSL::SSL::VERIFY_PEER - def with_ssl(&block) - ssl_options = {} - if @authenticator.ca_cert.present? - with_ca(ssl_options) do |updated_ssl_options| - block.call(updated_ssl_options) + store = OpenSSL::X509::Store.new + # If CA Certificate is available, we write it to a tempfile for + # import. This allows us to handle certificate chains. + if @ca_certificate.present? + with_ca_certificate(@ca_certificate) do |file| + store.add_file(file.path) + end + else + # Auto-include system CAs unless a CA has been defined + store.set_default_paths end - else - block.call(ssl_options) + http.cert_store = store + + # return the http object + http end end - def network_call(ssl_options, &block) - client = Faraday.new(@authenticator.provider_uri, ssl: ssl_options) do |conn| - conn.response(:json, content_type: /\bjson$/) - conn.response(:raise_error) - - conn.adapter(Faraday.default_adapter) + def with_ca_certificate(certificate_content, &block) + Tempfile.create('ca', @temp_directory) do |ca_certificate| + ca_certificate.write(certificate_content) + ca_certificate.close + block.call(ca_certificate) end + end - block.call(client) + def create_directory_if_missing(path) + Dir.mkdir(path) unless Dir.exist?(path) end end end diff --git a/app/domain/authentication/authn_oidc/v2/oidc_client.rb b/app/domain/authentication/authn_oidc/v2/oidc_client.rb index 19bea1a317..5f644fd614 100644 --- a/app/domain/authentication/authn_oidc/v2/oidc_client.rb +++ b/app/domain/authentication/authn_oidc/v2/oidc_client.rb @@ -15,7 +15,8 @@ def initialize( @logger = logger @client = client.new( - authenticator: authenticator + hostname: @authenticator.provider_uri, + ca_certificate: @authenticator.ca_cert ) @success = ::SuccessResponse @@ -35,8 +36,20 @@ def exchange(code:, nonce:, code_verifier: nil) # 'jwks_uri' - GET # 'token_endpoint' - POST # Public method so strategy can call it to verify configuration - def configuration - @configuration ||= @client.get('.well-known/openid-configuration') + def oidc_configuration + @oidc_configuration ||= begin + response = @client.get("#{@authenticator.provider_uri}/.well-known/openid-configuration").bind do |response| + return @success.new(response) + end + @failure.new( + "Authn-OIDC '#{@authenticator.service_id}' provider-uri: '#{@authenticator.provider_uri}' is unreachable", + exception: Errors::Authentication::OAuth::ProviderDiscoveryFailed.new( + @authenticator.provider_uri, + response.message + ), + status: :unauthorized + ) + end end private @@ -51,40 +64,45 @@ def exchange_token(code:, nonce:, code_verifier:) args[:code_verifier] = code_verifier if code_verifier.present? args[:redirect_uri] = ERB::Util.url_encode(@authenticator.redirect_uri) if @authenticator.redirect_uri.present? - # begin - # binding.pry + oidc_configuration.bind do |config| response = @client.post( - url: configuration['token_endpoint'].gsub("#{@authenticator.provider_uri}/", ''), + path: config['token_endpoint'], body: args.map { |k, v| "#{k}=#{v}" }.join('&'), - headers: { 'Authorization' => "Basic #{Base64.strict_encode64([@authenticator.client_id, @authenticator.client_secret].join(':'))}" } - ).bind do |response| - return @success.new(response['id_token'] || response['access_token']) + basic_auth: [@authenticator.client_id, @authenticator.client_secret] + ).bind do |token| + bearer_token = token['id_token'] || token['access_token'] + return @success.new(bearer_token) if bearer_token.present? + + @failure.new( + 'Bearer Token is empty', + exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(response.message), + status: :bad_request + ) end - # rescue => e - # binding.pry @failure.new( response.message, - exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(e.message), - status: :unauthorized + exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(response.message), + status: :bad_request ) - # end - # binding.pry + end end def decode_token(encoded_token) - @success.new( - JWT.decode( - encoded_token, - nil, - true, # Verify the signature of this token - algorithms: %w[RS256 RS384 RS512], - iss: @authenticator.provider_uri, - verify_iss: true, - aud: @authenticator.client_id, - verify_aud: true, - jwks: fetch_jwks - ).first - ) + fetch_jwks.bind do |jwks| + return @success.new( + JWT.decode( + encoded_token, + nil, + true, # Verify the signature of this token + algorithms: %w[RS256 RS384 RS512], + iss: @authenticator.provider_uri, + verify_iss: true, + aud: @authenticator.client_id, + verify_aud: true, + jwks: jwks + ).first + ) + end rescue => e @failure.new(e.message, exception: e, status: :bad_request) end @@ -98,313 +116,16 @@ def verify_token(token:, nonce:) end def fetch_jwks - @client.get( - configuration['jwks_uri'].gsub("#{@authenticator.provider_uri}/", '') - ) + oidc_configuration.bind do |configuration| + @client.get( + configuration['jwks_uri'] + ).bind do |response| + @success.new(response) + end + rescue => e + @failure.new(e.message, exception: e, status: :bad_request) + end end - - # def exchange_code_for_token(code:, nonce:, code_verifier: nil) - # args = { - # grant_type: 'authorization_code', - # scope: true, - # code: code, - # nonce: nonce - # } - # args[:code_verifier] = code_verifier if code_verifier.present? - # args[:redirect_uri] = ERB::Util.url_encode(@authenticator.redirect_uri) if @authenticator.redirect_uri.present? - - # response = @client.post( - # url: configuration['token_endpoint'].gsub("#{@authenticator.provider_uri}/", ''), - # body: args.map { |k, v| "#{k}=#{v}" }.join('&'), - # headers: { 'Authorization' => "Basic #{Base64.strict_encode64([@authenticator.client_id, @authenticator.client_secret].join(':'))}" } - # ) - - # bearer_token = response['id_token'] || response['access_token'] - - # decoded_jwt = JWT.decode( - # bearer_token, - # nil, - # true, # Verify the signature of this token - # algorithms: %w[RS256 RS384 RS512], - # iss: @authenticator.provider_uri, - # verify_iss: true, - # aud: @authenticator.client_id, - # verify_aud: true, - # jwks: fetch_jwks - # ).first - - # unless decoded_jwt['nonce'] == nonce - # return @failure.new('nonce does not match') - # end - - # @success.new(decoded_jwt) - # end - - - # # Writing certificates to the default system cert store requires - # # superuser privilege. Instead, Conjur will use ${CONJUR_ROOT}/tmp/certs. - # def self.default_cert_dir(dir: Dir, fileutils: FileUtils) - # if @default_cert_dir.blank? - # conjur_root = __dir__.slice(0..(__dir__.index('/app'))) - # @default_cert_dir = File.join(conjur_root, "tmp/certs") - # end - - # fileutils.mkdir_p(@default_cert_dir) unless dir.exist?(@default_cert_dir.to_s) - - # @default_cert_dir - # 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 - # end - - # def callback(code:, nonce:, code_verifier: nil) - # response = exchange_code_for_token(code: code, nonce: nonce, code_verifier: code_verifier) - # # binding.pry - - # # oidc_client.authorization_code = code - # # access_token_args = { client_auth_method: :basic } - # # access_token_args[:code_verifier] = code_verifier if code_verifier.present? - # # begin - # # bearer_token = oidc_client.access_token!(**access_token_args) - # # rescue Rack::OAuth2::Client::Error => e - # # return @failure.new( - # # e.message, - # # exception: Errors::Authentication::AuthnOidc::TokenRetrievalFailed.new(e.message), - # # status: :bad_request - # # ) - # # 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 => e - # # attempts += 1 - # # if attempts > 1 - # # return @failure.new( - # # 'JWKS signing check failed', - # # exception: e, - # # status: :unauthorized - # # ) - # # end - # # # 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 - # # ) - # # @success.new(decoded_id_token) - # # rescue => e - # # @failure.new( - # # e.message, - # # exception: e, - # # status: :bad_request - # # ) - # # end - # end - - # # callback_with_temporary_cert wraps the callback method with commands - # # to write & clean up a given certificate or cert chain in a given - # # directory. By default, ${CONJUR_ROOT}/tmp/certs is used. - # # - # # The temporary certificate file name is "x.n", where x is the hash of - # # the certificate subject name, and n is incrememnted from 0 in case of - # # collision. - # # - # # Unlike self.discover, which wraps a single ::OpenIDConnect method, - # # callback_with_temporary_cert wraps the entire callback method, which - # # includes multiple calls to the OIDC provider, including at least one - # # discover! call. The temporary certs will apply to all required - # # operations. - # def callback_with_temporary_cert( - # code:, - # nonce:, - # code_verifier: nil, - # cert_dir: Authentication::AuthnOidc::V2::Client.default_cert_dir, - # cert_string: nil - # ) - # c = -> { callback(code: code, nonce: nonce, code_verifier: code_verifier) } - - # return c.call if cert_string.blank? - - # begin - # certs_a = ::Conjur::CertUtils.parse_certs(cert_string) - # rescue OpenSSL::X509::CertificateError => e - # raise Errors::Authentication::AuthnOidc::InvalidCertificate, e.message - # end - # raise Errors::Authentication::AuthnOidc::InvalidCertificate, "provided string does not contain a certificate" if certs_a.empty? - - # symlink_a = [] - - # Dir.mktmpdir do |tmp_dir| - # certs_a.each_with_index do |cert, idx| - # tmp_file = File.join(tmp_dir, "conjur-oidc-client.#{idx}.pem") - # File.write(tmp_file, cert.to_s) - - # n = 0 - # hash = cert.subject.hash.to_s(16) - # while true - # symlink = File.join(cert_dir, "#{hash}.#{n}") - # break unless File.exist?(symlink) - - # n += 1 - # end - - # File.symlink(tmp_file, symlink) - # symlink_a << symlink - # end - - # if OpenIDConnect.http_config.nil? || OpenIDConnect.http_client.ssl.ca_path != cert_dir - # config_proc = proc do |config| - # config.ssl.ca_path = cert_dir - # config.ssl.verify_mode = OpenSSL::SSL::VERIFY_PEER - # end - - # # OpenIDConnect gem only accepts a single Faraday configuration - # # through calls to its .http_config method, and future calls to - # # the #http_config method return the first config instance. - # # - # # On the first call to OpenIDConnect.http_config, it will pass the - # # new Faraday configuration to its dependency gems that also have - # # nil configs. We can't be certain that each gem is configured - # # with the same Faraday config and need them synchronized, so we - # # inject them manually. - # OpenIDConnect.class_variable_set(:@@http_config, config_proc) - # WebFinger.instance_variable_set(:@http_config, config_proc) - # SWD.class_variable_set(:@@http_config, config_proc) - # Rack::OAuth2.class_variable_set(:@@http_config, config_proc) - # end - - # c.call - # ensure - # symlink_a.each{ |s| File.unlink(s) if s.present? && File.symlink?(s) } - # end - # 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 - # self.class.discover( - # provider_uri: @authenticator.provider_uri, - # discovery_configuration: @discovery_configuration, - # cert_string: @authenticator.ca_cert - # ) - # rescue 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 - - # # discover wraps ::OpenIDConnect::Discovery::Provider::Config.discover! - # # with commands to write & clean up a given certificate or cert chain in - # # a given directory. By default, ${CONJUR_ROOT}/tmp/certs is used. - # # - # # The temporary certificate file name is "x.n", where x is the hash of - # # the certificate subject name, and n is incremented from 0 in case of - # # collision. - # # - # # discover is a class method, because there are a few contexts outside - # # this class where the underlying discover! method is used. Call it by - # # running Authentication::AuthnOIDC::V2::Client.discover(...). - # def self.discover( - # provider_uri:, - # discovery_configuration: ::OpenIDConnect::Discovery::Provider::Config, - # cert_dir: default_cert_dir, - # cert_string: nil, - # jwks: false - # ) - # case jwks - # when false - # d = -> { discovery_configuration.discover!(provider_uri) } - # when true - # d = -> { discovery_configuration.discover!(provider_uri).jwks } - # end - - # return d.call if cert_string.blank? - - # begin - # certs_a = ::Conjur::CertUtils.parse_certs(cert_string) - # rescue OpenSSL::X509::CertificateError => e - # raise Errors::Authentication::AuthnOidc::InvalidCertificate, e.message - # end - # raise Errors::Authentication::AuthnOidc::InvalidCertificate, "provided string does not contain a certificate" if certs_a.empty? - - # symlink_a = [] - - # Dir.mktmpdir do |tmp_dir| - # certs_a.each_with_index do |cert, idx| - # tmp_file = File.join(tmp_dir, "conjur-oidc-client.#{idx}.pem") - # File.write(tmp_file, cert.to_s) - - # n = 0 - # hash = cert.subject.hash.to_s(16) - # while true - # symlink = File.join(cert_dir, "#{hash}.#{n}") - # break unless File.exist?(symlink) - - # n += 1 - # end - - # File.symlink(tmp_file, symlink) - # symlink_a << symlink - # end - - # if OpenIDConnect.http_config.nil? || OpenIDConnect.http_client.ssl.ca_path != cert_dir - # config_proc = proc do |config| - # config.ssl.ca_path = cert_dir - # config.ssl.verify_mode = OpenSSL::SSL::VERIFY_PEER - # end - - # # OpenIDConnect gem only accepts a single Faraday configuration - # # through calls to its .http_config method, and future calls to - # # the #http_config method return the first config instance. - # # - # # On the first call to OpenIDConnect.http_config, it will pass the - # # new Faraday configuration to its dependency gems that also have - # # nil configs. We can't be certain that each gem is configured - # # with the same Faraday config and need them synchronized, so we - # # inject them manually. - # OpenIDConnect.class_variable_set(:@@http_config, config_proc) - # WebFinger.instance_variable_set(:@http_config, config_proc) - # SWD.class_variable_set(:@@http_config, config_proc) - # Rack::OAuth2.class_variable_set(:@@http_config, config_proc) - # end - - # d.call - # ensure - # symlink_a.each{ |s| File.unlink(s) if s.present? && File.symlink?(s) } - # end - # end end end end From 7b1399a770912fa2b07588d2990c35d75e8cb0de Mon Sep 17 00:00:00 2001 From: Jason Vanderhoof Date: Mon, 30 Oct 2023 15:55:13 -0600 Subject: [PATCH 8/8] Update status endpoint to check variables --- .../authn_oidc/authenticator.rb | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/app/domain/authentication/authn_oidc/authenticator.rb b/app/domain/authentication/authn_oidc/authenticator.rb index 17cfcccc06..45201b2d45 100644 --- a/app/domain/authentication/authn_oidc/authenticator.rb +++ b/app/domain/authentication/authn_oidc/authenticator.rb @@ -28,29 +28,28 @@ def status(authenticator_status_input:) # term, we need to port the V1 functionality to V2. Once that # is done, the following check can be removed. - # Attempt to load the V2 version of the OIDC Authenticator - authenticator = DB::Repository::AuthenticatorRepository.new( - data_object: Authentication::AuthnOidc::V2::DataObjects::Authenticator - ).find( + DB::Repository::AuthenticatorRepository.new.find( type: authenticator_status_input.authenticator_name, account: authenticator_status_input.account, service_id: authenticator_status_input.service_id - ) - # If successful, validate the new set of required variables - if authenticator.present? - Authentication::AuthnOidc::ValidateStatus.new( - required_variable_names: %w[provider-uri client-id client-secret claim-mapping], - optional_variable_names: %w[ca-cert] - ).( - account: authenticator_status_input.account, - service_id: authenticator_status_input.service_id - ) - else - # Otherwise, perform the default check - Authentication::AuthnOidc::ValidateStatus.new.( - account: authenticator_status_input.account, - service_id: authenticator_status_input.service_id - ) + ).bind do |authenticator_data| + # check if this authenticator appears to be a Code Redirect authenticator + if authenticator_data.key?(:client_id) + Authentication::AuthnOidc::ValidateStatus.new( + required_variable_names: %w[provider-uri client-id client-secret claim-mapping], + optional_variable_names: %w[ca-cert] + ).( + account: authenticator_status_input.account, + service_id: authenticator_status_input.service_id + ) + + # Otherwise, use the old style check + else + Authentication::AuthnOidc::ValidateStatus.new.( + account: authenticator_status_input.account, + service_id: authenticator_status_input.service_id + ) + end end end end