From 2fe4a36271106f22a19ef996f5432e58d249b2b9 Mon Sep 17 00:00:00 2001
From: Michael Kohl <citizen428@dev.to>
Date: Wed, 22 Jul 2020 08:23:42 +0700
Subject: [PATCH] [deploy] Add audit logs for credit card actions (#9416)

* Add AuditLog entries for credit card actions

* Update docs

* Update category and slug

* Update audit log example in docs
---
 .../stripe_active_cards_controller.rb         | 20 +++++++++++++++
 docs/backend/audit-log.md                     | 20 +++++++++++++++
 spec/requests/stripe_active_cards_spec.rb     | 25 +++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/app/controllers/stripe_active_cards_controller.rb b/app/controllers/stripe_active_cards_controller.rb
index d24f0b2b0..f62776520 100644
--- a/app/controllers/stripe_active_cards_controller.rb
+++ b/app/controllers/stripe_active_cards_controller.rb
@@ -1,6 +1,9 @@
 class StripeActiveCardsController < ApplicationController
   before_action :authenticate_user!
 
+  AUDIT_LOG_CATEGORY = "user.credit_card.edit".freeze
+  private_constant :AUDIT_LOG_CATEGORY
+
   def create
     authorize :stripe_active_card
 
@@ -8,6 +11,7 @@ class StripeActiveCardsController < ApplicationController
 
     if Payments::Customer.create_source(customer.id, stripe_params[:stripe_token])
       flash[:settings_notice] = "Your billing information has been updated"
+      audit_log("add")
     else
       DatadogStatsClient.increment("stripe.errors", tags: ["action:create_card", "user_id:#{current_user.id}"])
 
@@ -29,6 +33,7 @@ class StripeActiveCardsController < ApplicationController
 
     if Payments::Customer.save(customer)
       flash[:settings_notice] = "Your billing information has been updated"
+      audit_log("update")
     else
       DatadogStatsClient.increment("stripe.errors", tags: ["action:update_card", "user_id:#{current_user.id}"])
       flash[:error] = "There was a problem updating your billing info."
@@ -54,6 +59,7 @@ class StripeActiveCardsController < ApplicationController
       Payments::Customer.save(customer)
 
       flash[:settings_notice] = "Your card has been successfully removed."
+      audit_log("remove")
     end
 
     redirect_to user_settings_path(:billing)
@@ -82,4 +88,18 @@ class StripeActiveCardsController < ApplicationController
   def stripe_params
     params.permit(%i[stripe_token])
   end
+
+  def audit_log(user_action)
+    AuditLog.create(
+      category: AUDIT_LOG_CATEGORY,
+      user: current_user,
+      roles: current_user.roles_name,
+      slug: "credit_card_#{user_action}",
+      data: {
+        action: action_name,
+        controller: controller_name,
+        user_action: user_action
+      },
+    )
+  end
 end
diff --git a/docs/backend/audit-log.md b/docs/backend/audit-log.md
index 1dc5ce9e2..502ca8bb9 100644
--- a/docs/backend/audit-log.md
+++ b/docs/backend/audit-log.md
@@ -43,3 +43,23 @@ the internal controller.
 
 It's a good idea to add a similar `after_action` to any controller action that
 might benefit from increased transparency.
+
+Additionally, the `AuditLog` is used to track important actions performed on a
+user's account, e.g. adding or removing a credit card:
+
+```ruby
+#<AuditLog:0x00000001193ce348> {
+    category: "user.credit_card.edit",
+  created_at: Tue, 21 Jul 2020 06:35:13 +03 +03:00,
+        data: {
+         "action" => "create",
+     "controller" => "stripe_active_cards",
+    "user_action" => "add"
+  },
+          id: 4,
+       roles: [],
+        slug: "credit_card_add",
+  updated_at: Tue, 21 Jul 2020 06:35:13 +03 +03:00,
+     user_id: 53
+}
+```
diff --git a/spec/requests/stripe_active_cards_spec.rb b/spec/requests/stripe_active_cards_spec.rb
index 24cb73ec3..2693d86e2 100644
--- a/spec/requests/stripe_active_cards_spec.rb
+++ b/spec/requests/stripe_active_cards_spec.rb
@@ -33,6 +33,12 @@ RSpec.describe "StripeActiveCards", type: :request do
       expect(card.is_a?(Stripe::Card)).to eq(true)
     end
 
+    it "creates an AuditLog entry for successful creates" do
+      expect do
+        post stripe_active_cards_path(stripe_token: stripe_helper.generate_card_token)
+      end.to change(AuditLog, :count).by(1)
+    end
+
     it "does not add a card if there is a card error" do
       StripeMock.prepare_card_error(:incorrect_number, :create_source)
 
@@ -93,6 +99,17 @@ RSpec.describe "StripeActiveCards", type: :request do
       expect(Payments::Customer.get(customer.id).default_source).to eq(new_card.id)
     end
 
+    it "creates an AuditLog entry for successful updates" do
+      customer, = create_user_with_card(user, card_token)
+      # add a second source
+      new_card_token = stripe_helper.generate_card_token
+      new_card = Payments::Customer.create_source(customer.id, new_card_token)
+
+      expect do
+        put stripe_active_card_path(id: new_card.id)
+      end.to change(AuditLog, :count).by(1)
+    end
+
     it "does not update the customer default souce if the source ID is unknown" do
       customer, source = create_user_with_card(user, card_token)
 
@@ -166,6 +183,14 @@ RSpec.describe "StripeActiveCards", type: :request do
         customer = Payments::Customer.get(user.stripe_id_code)
         expect(Payments::Customer.get_sources(customer).count).to eq(0)
       end
+
+      it "creates an AuditLog entry for successful deletes" do
+        _, source = create_user_with_card(user, card_token)
+
+        expect do
+          delete stripe_active_card_path(id: source.id)
+        end.to change(AuditLog, :count).by(1)
+      end
     end
 
     context "when an invalid request is made" do