fix: Initial credential creation show after as rather a value (#1476)

* Fixes when a credential is created, an audit_event is generated. It should display a hash of the credential body and include the initial label as you can see in the updated event below and after the created event in this image. --------- Co-authored-by: Taylor Downs <[email protected]> Co-authored-by: Stuart Corbishley <[email protected]>
OpenFn · Dec 11, 2023 · ec394c1 · ec394c1
1 parent 0140930
commit ec394c1
Show file tree

Hide file tree

Showing 20 changed files with 517 additions and 160 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,8 @@ and this project adheres to
   conflict [#1329](https://github.com/OpenFn/Lightning/issues/1329)
 - Demo script sets up example Runs with their log lines in a consistant order.
   [#1487](https://github.com/OpenFn/Lightning/issues/1487)
+- Initial credential creation `changes` show `after` as `null` rather a value
+  [#1118](https://github.com/OpenFn/Lightning/issues/1118)
 
 ## [v0.11.0] - 2023-12-06
 
@@ -98,6 +100,7 @@ and this project adheres to
 - Fix for missing data in 'created' audit trail events for webhook auth methods
   [#1500](https://github.com/OpenFn/Lightning/issues/1500)
 
+
 ## [v0.10.4] - 2023-11-30
 
 ### Added

diff --git a/lib/lightning/auditing.ex b/lib/lightning/auditing.ex
@@ -5,7 +5,7 @@ defmodule Lightning.Auditing do
 
   import Ecto.Query
   alias Lightning.Repo
-  alias Lightning.Auditing.Model, as: Audit
+  alias Lightning.Auditing.Audit
   alias Lightning.Accounts.User
 
   def list_all(params \\ %{}) do

diff --git a/lib/lightning/auditing/model.ex → lib/lightning/auditing/audit.ex b/lib/lightning/auditing/model.ex → lib/lightning/auditing/audit.ex
@@ -1,10 +1,12 @@
-defmodule Lightning.Auditing.Model do
+defmodule Lightning.Auditing.Audit do
   @moduledoc """
   Macro module to add common model behaviour to a given Ecto model
   """
   require Ecto.Query
   require Logger
 
+  @callback update_changes(changes :: map()) :: map()
+
   # coveralls-ignore-start
   defmacro __using__(opts) do
     repo = Keyword.fetch!(opts, :repo)
@@ -14,6 +16,17 @@ defmodule Lightning.Auditing.Model do
     if Enum.empty?(events),
       do: raise(ArgumentError, message: "No events provided to Audit")
 
+    update_changes_func =
+      quote do
+        @behaviour Lightning.Auditing.Audit
+
+        def update_changes(changes) do
+          changes
+        end
+
+        defoverridable update_changes: 1
+      end
+
     save_function =
       quote do
         # Output:
@@ -45,7 +58,8 @@ defmodule Lightning.Auditing.Model do
               unquote(event_name),
               item_id,
               actor_id,
-              changes
+              changes,
+              &update_changes/1
             )
           end
         end
@@ -56,13 +70,17 @@ defmodule Lightning.Auditing.Model do
         import Ecto.Query
 
         def base_query do
-          from(unquote(__MODULE__),
-            where: [item_type: unquote(item)]
-          )
+          from(unquote(__MODULE__), where: [item_type: unquote(item)])
         end
       end
 
-    [base_query, save_function, event_signature, event_log_functions]
+    [
+      base_query,
+      save_function,
+      event_signature,
+      event_log_functions,
+      update_changes_func
+    ]
   end
 
   # coveralls-ignore-stop
@@ -80,23 +98,11 @@ defmodule Lightning.Auditing.Model do
     end
 
     @doc false
-    def changeset(changes, attrs \\ %{}) do
+    def changeset(changes, attrs, update_changes_fun) do
       changes
       |> cast(attrs, [:before, :after])
-      |> update_change(:before, &encrypt_body/1)
-      |> update_change(:after, &encrypt_body/1)
-    end
-
-    defp encrypt_body(changes) when is_map(changes) do
-      if Map.has_key?(changes, :body) do
-        changes
-        |> Map.update(:body, nil, fn val ->
-          {:ok, val} = Lightning.Encrypted.Map.dump(val)
-          val |> Base.encode64()
-        end)
-      else
-        changes
-      end
+      |> update_change(:before, update_changes_fun)
+      |> update_change(:after, update_changes_fun)
     end
   end
 
@@ -117,10 +123,18 @@ defmodule Lightning.Auditing.Model do
   end
 
   @doc false
-  def changeset(%__MODULE__{} = audit, attrs) do
+  def changeset(
+        %__MODULE__{} = audit,
+        attrs,
+        update_changes_fun \\ fn x -> x end
+      ) do
     audit
     |> cast(attrs, [:event, :item_id, :actor_id, :item_type])
-    |> cast_embed(:changes)
+    |> cast_embed(:changes,
+      with: fn schema, changes ->
+        Changes.changeset(schema, changes, update_changes_fun)
+      end
+    )
     |> validate_required([:event, :actor_id])
   end
 
@@ -136,7 +150,6 @@ defmodule Lightning.Auditing.Model do
           {:ok, :no_changes}
           | {:ok, Ecto.Schema.t()}
           | {:error, Ecto.Changeset.t()}
-
   def save(:no_changes, _repo) do
     {:ok, :no_changes}
   end
@@ -160,13 +173,21 @@ defmodule Lightning.Auditing.Model do
           String.t(),
           Ecto.UUID.t(),
           Ecto.UUID.t(),
-          Ecto.Changeset.t() | map() | nil
+          Ecto.Changeset.t() | map() | nil,
+          update_changes_fun :: (map() -> map())
         ) ::
           :no_changes | Ecto.Changeset.t()
 
-  def event(item_type, event, item_id, actor_id, changes \\ %{})
+  def event(
+        item_type,
+        event,
+        item_id,
+        actor_id,
+        changes \\ %{},
+        update_fun \\ fn x -> x end
+      )
 
-  def event(_, _, _, _, %Ecto.Changeset{changes: changes} = _changeset)
+  def event(_, _, _, _, %Ecto.Changeset{changes: changes}, _update_fun)
       when map_size(changes) == 0 do
     :no_changes
   end
@@ -176,7 +197,8 @@ defmodule Lightning.Auditing.Model do
         event,
         item_id,
         actor_id,
-        %Ecto.Changeset{data: %subject_schema{} = data, changes: changes}
+        %Ecto.Changeset{data: %subject_schema{} = data, changes: changes},
+        update_fun
       ) do
     change_keys = changes |> Map.keys() |> MapSet.new()
 
@@ -186,26 +208,35 @@ defmodule Lightning.Auditing.Model do
       |> MapSet.intersection(change_keys)
       |> MapSet.to_list()
 
-    changes = %{
-      before: Map.take(data, field_keys),
-      after: Map.take(changes, field_keys)
-    }
+    before_change = Map.take(data, field_keys)
+
+    after_change = Map.take(changes, field_keys)
+
+    changes =
+      %{
+        before: if(event == "created", do: nil, else: before_change),
+        after: if(after_change === %{}, do: nil, else: after_change)
+      }
 
-    audit_changeset(item_type, event, item_id, actor_id, changes)
+    audit_changeset(item_type, event, item_id, actor_id, changes, update_fun)
   end
 
-  def event(item_type, event, item_id, actor_id, changes)
+  def event(item_type, event, item_id, actor_id, changes, update_fun)
       when is_map(changes) do
-    audit_changeset(item_type, event, item_id, actor_id, changes)
+    audit_changeset(item_type, event, item_id, actor_id, changes, update_fun)
   end
 
-  defp audit_changeset(item_type, event, item_id, actor_id, changes) do
-    changeset(%__MODULE__{}, %{
-      item_type: item_type,
-      event: event,
-      item_id: item_id,
-      actor_id: actor_id,
-      changes: changes
-    })
+  defp audit_changeset(item_type, event, item_id, actor_id, changes, update_fun) do
+    changeset(
+      %__MODULE__{},
+      %{
+        item_type: item_type,
+        event: event,
+        item_id: item_id,
+        actor_id: actor_id,
+        changes: changes
+      },
+      update_fun
+    )
   end
 end
diff --git a/lib/lightning/credentials.ex b/lib/lightning/credentials.ex
@@ -124,10 +124,11 @@ defmodule Lightning.Credentials do
     changeset = %Credential{} |> change_credential(attrs) |> cast_body_change()
 
     Multi.new()
-    |> Multi.insert(:credential, changeset)
-    |> Multi.insert(:audit, fn %{credential: credential} ->
-      Audit.event("created", credential.id, credential.user_id)
-    end)
+    |> Multi.insert(
+      :credential,
+      changeset
+    )
+    |> derive_events(changeset)
     |> Repo.transaction()
     |> case do
       {:error, _op, changeset, _changes} ->
@@ -239,7 +240,8 @@ defmodule Lightning.Credentials do
 
   defp derive_events(
          multi,
-         %Ecto.Changeset{data: %Credential{}} = changeset
+         %Ecto.Changeset{data: %Credential{__meta__: %{state: state}}} =
+           changeset
        ) do
     case changeset.changes do
       map when map_size(map) == 0 ->
@@ -257,7 +259,7 @@ defmodule Lightning.Credentials do
           :audit,
           fn %{credential: credential} ->
             Audit.event(
-              "updated",
+              if(state == :built, do: "created", else: "updated"),
               credential.id,
               credential.user_id,
               changeset

diff --git a/lib/lightning/credentials/audit.ex b/lib/lightning/credentials/audit.ex
@@ -2,7 +2,7 @@ defmodule Lightning.Credentials.Audit do
   @moduledoc """
   Model for storing changes to Credentials
   """
-  use Lightning.Auditing.Model,
+  use Lightning.Auditing.Audit,
     repo: Lightning.Repo,
     item: "credential",
     events: [
@@ -12,4 +12,16 @@ defmodule Lightning.Credentials.Audit do
       "removed_from_project",
       "deleted"
     ]
+
+  def update_changes(changes) when is_map(changes) do
+    if Map.has_key?(changes, :body) do
+      changes
+      |> Map.update(:body, nil, fn val ->
+        {:ok, val} = Lightning.Encrypted.Map.dump(val)
+        Base.encode64(val)
+      end)
+    else
+      changes
+    end
+  end
 end
diff --git a/lib/lightning/projects/project_credential.ex b/lib/lightning/projects/project_credential.ex
@@ -19,7 +19,7 @@ defmodule Lightning.Projects.ProjectCredential do
   schema "project_credentials" do
     belongs_to :credential, Credential
     belongs_to :project, Project
-    field :delete, :boolean, virtual: true
+    field :delete, :boolean, virtual: true, default: false
 
     timestamps()
   end

diff --git a/lib/lightning/setup_utils.ex b/lib/lightning/setup_utils.ex
@@ -854,7 +854,7 @@ defmodule Lightning.SetupUtils do
       Lightning.Attempt,
       Lightning.AttemptRun,
       Lightning.AuthProviders.AuthConfig,
-      Lightning.Auditing.Model,
+      Lightning.Auditing.Audit,
       Lightning.Projects.ProjectCredential,
       Lightning.WorkOrder,
       Lightning.Invocation.Run,

diff --git a/lib/lightning/webhook_auth_methods.ex b/lib/lightning/webhook_auth_methods.ex
@@ -162,12 +162,7 @@ defmodule Lightning.WebhookAuthMethods do
     Multi.new()
     |> Multi.insert(:auth_method, changeset)
     |> Multi.insert(:audit, fn %{auth_method: auth_method} ->
-      WebhookAuthMethodAudit.event("created", auth_method.id, user.id, %{
-        before: %{name: nil},
-        after: %{
-          name: auth_method.name
-        }
-      })
+      WebhookAuthMethodAudit.event("created", auth_method.id, user.id, changeset)
     end)
     |> Repo.transaction()
     |> case do
@@ -182,14 +177,15 @@ defmodule Lightning.WebhookAuthMethods do
   @spec create_auth_method(Trigger.t(), map(), actor: User.t()) ::
           {:ok, WebhookAuthMethod.t()} | {:error, Ecto.Changeset.t()}
   def create_auth_method(%Trigger{} = trigger, params, actor: %User{} = user) do
-    Multi.new()
-    |> Multi.insert(:auth_method, fn _changes ->
+    changeset =
       %WebhookAuthMethod{}
       |> WebhookAuthMethod.changeset(params)
       |> Ecto.Changeset.put_assoc(:triggers, [trigger])
-    end)
+
+    Multi.new()
+    |> Multi.insert(:auth_method, changeset)
     |> Multi.insert(:created_audit, fn %{auth_method: auth_method} ->
-      WebhookAuthMethodAudit.event("created", auth_method.id, user.id)
+      WebhookAuthMethodAudit.event("created", auth_method.id, user.id, changeset)
     end)
     |> Multi.insert(:add_to_trigger_audit, fn %{auth_method: auth_method} ->
       WebhookAuthMethodAudit.event(

diff --git a/lib/lightning/workflows/webhook_auth_method_audit.ex b/lib/lightning/workflows/webhook_auth_method_audit.ex
@@ -0,0 +1,27 @@
+defmodule Lightning.Workflows.WebhookAuthMethodAudit do
+  @moduledoc """
+  Model for storing changes to WebhookAuthMethod
+  """
+  use Lightning.Auditing.Audit,
+    repo: Lightning.Repo,
+    item: "webhook_auth_method",
+    events: [
+      "created",
+      "updated",
+      "added_to_trigger",
+      "removed_from_trigger",
+      "deleted"
+    ]
+
+  @impl true
+  def update_changes(changes) when is_map(changes) do
+    Enum.into(changes, %{}, fn
+      {key, val} when key in [:username, :password, :api_key] ->
+        {:ok, encrypted_val} = Lightning.Encrypted.Binary.dump(val)
+        {key, Base.encode64(encrypted_val)}
+
+      other ->
+        other
+    end)
+  end
+end
diff --git a/lib/lightning/workflows/webhook_auth_methods.ex b/lib/lightning/workflows/webhook_auth_methods.ex