Feature Proposal: Audit Logging

[tompave]

Feature Proposal: Audit Logging

[tompave]

Hello!

Thank you for using the library and suggesting this feature. Especially, thank you for taking the time to write such a detailed proposal.

I appreciate the value of an audit log for when feature flags are toggled, but I wouldn't feel comfortable adding so many new responsibilities to the library. Audit logging is also quite difficult to do right and it's not something where one size fits all, so I am a bit resistant to the idea of shipping an opinionated implementation with FWF.

That doesn't mean that it can't be done with a third party library or with a bit of user code in the host application, though!

You seem to have spent some time thinking about the suggested design, that is, your FunWithFlags.add_audit_log/4 function as an opinionated interface to persist some data. I can see why it would work for your use case, but what if other people would prefer to send the audit log to a log drain, a time-series DB, or to a queue of some kind? (e.g. Kafka, Rabbit, SQS, etc) The same thing applies to the UI. Even though the idea is good, I'm not convinced that this kind of opinionated functionality belongs into FWF, sorry.

So my suggestion is that this sort of thing could be isolated in a third party library or implemented in the host application.

Hopefully, the design of FWF would make it easy, and I can give you some pointers.

The first thing I'd like to point out is that you need to consider your trust model. Your proposal focuses on auditing interactions through FWF.UI, and it doesn't seem concerned with flags being toggled with code. The reason is that since add_audit_log/4 is a separate function, there is no guarantee that will be invoked when toggling flags. Sure, if it were integrated in FWF.UI it could be assumed that the two calls always happen together. (Although you correctly point our the need for transactions.) That alone wouldn't be enough however. FWF.UI is a convenient web interface for human operators, but flags can also be toggled in application code. There is no guarantee that a developer won't insert a rogue line somewhere to toggle a flag, or more simply do it within a iex -S mix session, without also calling FunWithFlags.add_audit_log/4. So the first question would be: are you concerned about that?

If you're not, then good news. Adding audit logging to FWF.UI is quite simple. Just add a plug.

For example, extending the FWF.UI Readme:

defmodule MyApp.AuditLog.Plug do
  def init(opts), do: opts

  def call(conn, _opts) do
    # 1. Skip if it's an asset request.
    # 2. Get from the `conn` the flag and gate names and the action.
    # 3. Get the user, based on the logic of your app.
    # 3. Log what you need, e.g. write to the DB or to a queue.
    # 4. Maybe use `Plug.Conn.register_before_send/2` for some more logging?
    # 5. Done. Return the conn.
    conn
  end
end

# ...

pipeline :flag_auditing do
  plug MyApp.AuditLog.Plug
end

scope path: "/feature-flags" do
  pipe_through :mounted_and_protected_apps
  pipe_through :flag_auditing
  forward "/", FunWithFlags.UI.Router, namespace: "feature-flags"
end

You might point out that this doesn't give you the atomicity guarantees of a transaction, but I'd argue that it's better. This approach would allow you to have audit logs that follow the pattern: "user Foo is about to perform action Bar". Then with Plug.Conn.register_before_send/2 you can look into the conn struct and add logs like "the action was/was-not successful". You can even store unique IDs in the conn to ensure that the before and after logs are matched or end up together.

On top of that, you have full control over how the user is retrieved.

If, on the other hand, you also want to audit-log when flags are toggled outside of FWF.UI, then you must put your audit-logging at a deeper level. That means it has to go directly into FWF.

A simple way to do it would be to add to FWF a way to register a callback that happens before/after/around the action of updating any persisted flag data. That would work, but I'd say it's not necessary. The same thing can be achieved with a custom persistence adapter. Just implement your own adapter that wraps one of the builtin adapters, and add your audit-logging code there:

defmodule MyApp.FunWithFlags.PersistentStoreWithAuditLog do
  @behaviour FunWithFlags.Store.Persistent
  @base FunWithFlags.Store.Persistent.Ecto

  @impl true
  def put(flag_name, gate) do
    user = maybe_read_it_from_the_process_dictionary()
    MyApp.Auditing.add_audit_log(:toggle, flag_name, gate.type, user)
    @base.put(flag_name, gate)
  end

  # No idea if this would work or if you need to expand them all!
  defdelegate everything_else, to: @base
end

Of course, a determined bad actor can still find a way to bypass that and write directly to the datastore, but at least that should cover the common cases!

The one problem with this kind of "deep" audit logging is that it's not easy to inject data to identify the user who is toggling a flag. The code could just be invoked in too many different contexts: a Phoenix or Plug endpoint, generic Elixir code, iex -S mix. Even ignoring that, without adding some sort of metadata/context parameter to the FWF functions (which would require a breaking change, therefore better not), it's not easy. In the snippet above I suggest to store it into the process dictionary, which could be an option.

[mbramson]

Thanks for the extremely detailed response! Like you pointed out, while a callback would be great... getting the user data there without using something gnarly like the process dictionary is a challenge. Wrapping it in a plug is probably the way to go.