cloak icon indicating copy to clipboard operation
cloak copied to clipboard

Throw on missing cipher configuration?

Open cybrox opened this issue 5 years ago • 2 comments
trafficstars

First of all, thank your for maintaining cloak_ecto!

I was wondering, in terms of usability, wouldn't it make sense for the library to throw an error when no cipher is configured?
As far as I can see, the library is unusable without a suitable configuration.

My reasoning for this is, I just spent an hour attempting to figure out the following error:

** (Ecto.ChangeError) value `"test"` for `MyModel.encrypted_api_token` in `update` does not match type MyApp.Encrypted.Binary
    (ecto 3.5.3) lib/ecto/repo/schema.ex:889: Ecto.Repo.Schema.dump_field!/6
    (ecto 3.5.3) lib/ecto/repo/schema.ex:898: anonymous fn/6 in Ecto.Repo.Schema.dump_fields!/5
    (stdlib 3.12) maps.erl:232: :maps.fold_1/3
    (ecto 3.5.3) lib/ecto/repo/schema.ex:896: Ecto.Repo.Schema.dump_fields!/5
    (ecto 3.5.3) lib/ecto/repo/schema.ex:829: Ecto.Repo.Schema.dump_changes!/6
    (ecto 3.5.3) lib/ecto/repo/schema.ex:334: anonymous fn/15 in Ecto.Repo.Schema.do_update/4
    (ecto 3.5.3) lib/ecto/repo/schema.ex:177: Ecto.Repo.Schema.update!/4
    (elixir 1.11.1) lib/enum.ex:1399: Enum."-map/2-lists^map/1-0-"/2

However, I searched in all the wrong places. I double checked all my database types, checked if my ecto adapter (myxql) supported the correct data type, updated ecto, etc. etc... After a lot of digging, I ended up adding an inspect to lib/cloak_ecto/type.ex in dump/1's error clause and got this helpful message %Cloak.InvalidConfig{message: "could not encrypt due to missing configuration"}}

As it turns out, I had put the following in my vault module:

  @impl GenServer
  def init(config) do
    config =
      Keyword.put(config, :cyphers,
        default: {Cloak.Ciphers.AES.GCM, tag: "AES.GCM.V1", key: decode_env!("CLOAK_KEY")}
      )

    {:ok, config}
  end

After changing :cyphers to :ciphers, as it is spelled in the docs, everything worked fine :facepalm:

So I was wondering, wouldn't it make sense to throw an error when no cipher configuration is provided at all?

cybrox avatar Oct 27 '20 16:10 cybrox

Also a thanks from me for cloak_ecto! And ➕ 1 on this. I ran into a similar error and was thrown off until I eventually found this GitHub issue. Thanks for considering!

paulstatezny avatar Apr 12 '21 21:04 paulstatezny

@cybrox @paulstatezny This is actually an issue for the cloak package, because that's where this error would need to be implemented. It does already return an error when you call encrypt with invalid configuration:

https://github.com/danielberkompas/cloak/blob/ead16b16e34ad8393b1f64b54cbd796da4af6975/lib/cloak/vault.ex#L295-L302

But Cloak.Ecto swallows it in the dump function:

https://github.com/danielberkompas/cloak_ecto/blob/2d7ceb1ea9cbe94bdb72a13a6a01a57d753c7377/lib/cloak_ecto/type.ex#L34-L43

The only way I can think of to fix this is to make the vault raise an error or log a warning when it boots up, if no valid configuration is given.

danielberkompas avatar Jun 17 '22 16:06 danielberkompas