Merge pull request #412 from Conjur-Enterprise/CNJR-5833

CNJR-5833 Slice 1A

.gitignore

@@ -74,3 +74,4 @@ conjur_git_commit
 dev/policies/authenticators/authn-oidc/identity-users.yml
 
 VERSION
+key.txt

app/controllers/application_controller.rb

@@ -62,7 +62,9 @@ class UnprocessableEntity < RuntimeError
   rescue_from Sequel::ForeignKeyConstraintViolation, with: :foreign_key_constraint_violation
   rescue_from Exceptions::EnhancedPolicyError, with: :enhanced_policy_error
   rescue_from Exceptions::InvalidPolicyObject, with: :policy_invalid
-  rescue_from NoMethodError, with: :policy_invalid
+  rescue_from Conjur::PolicyParser::Invalid, with: :policy_invalid
+  rescue_from Conjur::PolicyParser::ResolverError, with: :policy_invalid
+  rescue_from NoMethodError, with: :validation_failed
   rescue_from ArgumentError, with: :argument_error
   rescue_from ActionController::ParameterMissing, with: :argument_error
   rescue_from UnprocessableEntity, with: :unprocessable_entity
@@ -171,7 +173,8 @@ def validation_failed e
   def policy_invalid e
     logger.debug("#{e}\n#{e.backtrace.join("\n")}")
 
-    error = { code: "policy_invalid", message: e.message }
+    msg = e.message == nil ? e.to_s : e.message
+    error = { code: "policy_invalid", message: msg }
 
     if e.instance_of?(Conjur::PolicyParser::Invalid)
       error[:innererror] = {

app/controllers/policies_controller.rb

@@ -54,15 +54,13 @@ def get
     raise e
   end
 
-  # A Conjur policy can be interpreted in various ways.
-  # A production strategy guides the interpretation.
-  # Loader::Orchestrate is a production that loads Conjur policy to database.
-  # Loader::Validate is a production that parses Conjur policy, resulting in error reports.
-  # The dryRun strategy parses Conjur policy, reports any errors, 
-  #   and if valid, reports the effective policy changes.
+  # A Conjur policy can be interpreted under various strategies.
+  # Loader::Validate is a strategy that parses policy to look for lexical errors.
+  # Loader::DryRun is a strategy that rehearses policy application and reports what would change.
+  # Loader::Orchestrate is a strategy that actually does apply policy and store it to database.
 
-  def production_type
-    return :validation if params["dryRun"]&.strip&.downcase == "true"
+  def strategy_type
+    return :rehearsal if params["dryRun"]&.strip&.downcase == "true"
 
     :orchestration
   end
@@ -103,89 +101,134 @@ def find_or_create_root_policy
 
   private
 
+  def enhance_error(error)
+    enhanced = error
+    unless error.instance_of?(Exceptions::EnhancedPolicyError)
+      enhanced = Exceptions::EnhancedPolicyError.new(
+        original_error: error
+      )
+    end
+    enhanced
+  end
+
   # Policy processing is a function of the policy mode request (load/update/replace) and
-  # is interpreted using the Orchestrate strategy.
+  # is interpreted using strategies to match the user command.
 
   def load_policy(mode, mode_class, delete_permitted:)
-    # Authorization to load
+    # A single instance of PolicyResult tracks policy progress and
+    # is passed to successive operations.
+    policy_result = PolicyResult.new
+    policy_mode = nil
+
+    # Declare above any vars that the lambdas need to recognize as local.
+
+    # Has a policy error been encountered yet?
+    policy_erred = lambda {
+      !policy_result.nil? &&
+      !policy_result.policy_parse.nil? &&
+      !policy_result.policy_parse.error.nil?
+    }
+
+    # policy_mode is used to call loader methods
+    get_policy_mode = lambda { |strategy_class|
+      # mode = f(strategy, parse, version)
+      policy_mode = mode_class.from_policy(
+        policy_result.policy_parse,
+        policy_result.policy_version,
+        strategy_class
+      )
+    }
+
+    # We wrap policy operations (parsing, loading, applying business rules)
+    # in rescue blocks and capture the exceptions as the error results.
+    # This prevents exceptions from rising uncontrollably (which would be a
+    # problem in situations such as the dry-run rollback).
+    evaluate_policy = lambda { |strategy_class|
+      # load, raise, enhance
+      begin
+        get_policy_mode.call(strategy_class)
+        policy_mode.call_pr(policy_result) unless policy_erred.call
+      rescue => e
+        policy_result.error=(enhance_error(e))
+      end
+    }
+
+    # Evaluate policy until success or an error is encountered.
+    # If errored, skip remaining evaluation and raise the error
+    # to redirect to strategy-based report generation.
+
     authorize(mode)
     mode_class.authorize(current_user, resource)
 
-    # Parse the policy
-    policy_parse = parse_submitted_policy
+    parse_submitted_policy(policy_result)
 
-    # If this is not a dry run, then save the policy. This has to occur here,
-    # as creating the policy_mode requires the policy_version
-    policy_version = unless production_type == :validation
-      save_submitted_policy(policy_parse, delete_permitted)
-    end
+    if strategy_type == :rehearsal
+      evaluate_policy.call(Loader::Validate)
+      raise policy_result.error if policy_erred.call
 
-    # Reporting on all policy errors requires an instance of the policy_mode
-    # to call the #report method.
-    policy_mode = mode_class.from_policy(
-      policy_parse,
-      policy_version,
-      production_class
-    )
+      Sequel::Model.db.transaction(savepoint: true) do
+        save_submitted_policy(policy_result, delete_permitted) unless policy_erred.call
+        evaluate_policy.call(Loader::DryRun)
+
+        raise Sequel::Rollback
+      end
 
-    # Process the rest of the policy, either to load it or further validate it
-    policy_result = policy_mode.call
+    else # :orchestration
+      save_submitted_policy(policy_result, delete_permitted) unless policy_erred.call
+      evaluate_policy.call(Loader::Orchestrate)
+
+    end
 
-    raise policy_result.error if policy_result.error
+    raise policy_result.error if policy_erred.call
+
+    # Success
+    audit_success(policy_result.policy_version)
 
-    audit_success(policy_version)
     render(
       json: policy_mode.report(policy_result),
-      status: success_status
+      status: strategy_type == :orchestration ? :created : :ok
     )
+
+  # Error triage:
+  # - Audit the original
+  # - Report the enhanced
+  # - Raise the original
+
   # Sequel::ForeignKeyConstraintViolation and Exceptions::RecordNotFound are not
   # currently handled by the enhanced error framework, so we pass it directly up
   # to the application controller.
   rescue Sequel::ForeignKeyConstraintViolation, Exceptions::RecordNotFound => e
     audit_failure(e, mode)
-    raise
+    raise e
+
   rescue => e
-    # Processing errors can be explained the same as parsing errors,
-    # but check whether the original is safe.
-    load_err = e
+    original_error = e
     if e.instance_of?(Exceptions::EnhancedPolicyError)
       if e.original_error
-        load_err = e.original_error
+        original_error = e.original_error
       end
     end
 
-    # Errors caught here include those due to mode processing.
-    audit_failure(e, mode)
+    audit_failure(original_error, mode)
 
-    # If an error occurred before the policy_mode was instantiated, raise it
-    # for the application controller to handle
-    unless production_type == :validation
-      raise load_err
-    end
+    # Render Orchestration errors through ApplicationController
+    raise original_error if strategy_type == :orchestration
 
-    # Render the errors according the mode (load or validate)
     render(
       json: policy_mode.report(policy_result),
       status: :unprocessable_entity
     )
-  end
 
-  def success_status
-    production_type == :validation ? :ok : :created
-  end
-
-  def production_class
-    production_type == :validation ? Loader::Validate : Loader::Orchestrate
   end
 
   # Auditing
 
   def audit_success(policy_version)
-    case production_type
-    when :validation
-      audit_validation_success("validate")
-    else
+    case strategy_type
+    when :orchestration
       audit_load_success(policy_version)
+    else
+      audit_validation_success("validate")
     end
   end
 
@@ -245,40 +288,45 @@ def retry_delay
     rand(1..8)
   end
 
-  # Generate a version and parse the policy
-  def save_submitted_policy(policy_parse, delete_permitted)
-    # (SYSK: as with parse_submitted_policy, PolicyVersion
-    # calls Commands::Policy::Parse, but it also generates
-    # a policy version number and binds them in a db record.)
-
-    policy_version = PolicyVersion.new(
-      role: current_user,
-      policy: resource,
-      policy_text: request.raw_post,
-      client_ip: request.ip,
-      policy_parse: policy_parse
-    )
-    policy_version.delete_permitted = delete_permitted
-    policy_version.save
-
-    policy_version
+  # Generate a version and parse the policy.
+  # Returns a PolicyResult; errors related to version creation are packaged inside.
+  # Raises no exceptions.
+  def save_submitted_policy(policy_result, delete_permitted)
+    version = nil
+    begin
+      version = PolicyVersion.new(
+        role: current_user,
+        policy: resource,
+        policy_text: request.raw_post,
+        client_ip: request.ip,
+        policy_parse: policy_result.policy_parse
+      )
+      version.delete_permitted = delete_permitted
+      version.save
+    rescue => e
+      policy_result.policy_parse = (PolicyParse.new([], enhance_error(e)))
+    end
+    policy_result.policy_version = (version)
   end
 
   # Parse the policy; no version is generated.
-  def parse_submitted_policy
+  # Returns a PolicyParse; errors, if any, are packaged inside.
+  # Raises no exceptions.
+  def parse_submitted_policy(policy_result)
     # Commands::Policy::Parse catches errors related to policy validation
 
     policy = resource
     is_root = policy.kind == "policy" && policy.identifier == "root"
 
-    Commands::Policy::Parse.new.call(
+    parse = Commands::Policy::Parse.new.call(
       account: policy.account,
       policy_id: policy.identifier,
       owner_id: policy.owner.id,
       policy_text: request.raw_post,
       policy_filename: nil,  # filename is historical and no longer informative
       root_policy: is_root
     )
+    policy_result.policy_parse = (parse)
   end
 
   def publish_event

app/domain/commands/policy/explain_error.rb

@@ -168,7 +168,7 @@ def catch_variable_error_messages(msg)
           # From conjur/gems/policy-parser/lib/conjur/policy/resolver.rb
         elsif msg.end_with?('has a blank id')
           # #{record.class.simple_name.underscore} has a blank id
-          advice = ''
+          advice = "Each resource must be identified using the 'id' field"
 
         elsif msg.start_with?('Invalid relative reference')
           # Invalid relative reference: #{id}

app/models/loader/create_policy.rb

@@ -27,19 +27,38 @@ def self.from_policy(
       )
     end
 
+    def call_pr(policy_result)
+      result = call
+      policy_result.created_roles = (result.created_roles)
+      policy_result.diff = (result.diff)
+    end
+
     def call
+      @loader.snapshot_public_schema_before
       @loader.setup_db_for_new_policy
       @loader.delete_shadowed_and_duplicate_rows
       @loader.store_policy_in_db
+
+      diff
+
+      # Destroy the temp schema used for diffing
+      @loader.drop_snapshot_public_schema_before
       @loader.release_db_connection
 
       PolicyResult.new(
         policy_parse: @loader.policy_parse,
         policy_version: @loader.policy_version,
-        created_roles: credential_roles
+        created_roles: credential_roles,
+        diff: diff
       )
     end
 
+    # This cache needs to be hydrated before the transaction is rolled back
+    # and/or before the temp schema is dropped.
+    def diff
+      @cached_diff ||= @loader.get_diff
+    end
+
     def new_roles
       @loader.new_roles
     end