Cloak.Ecto password validate_length not working, when hash

Hi,

I am using Cloak.Ecto to encrypt username and email, and hash the user password(user_password_hash).

In the changeset, I pipe the cs to validate_length(:user_password, min: 8)
and then to validate_password (a private function).

Curiously, the validate_password triggers an invalid credentials message, but, the validate_length (which is before in the pipeline), does not trigger an error when supplied with less than 8 characters.

I don’t know if this is because Cloak.Ecto has anything to do with it.

Part of my schema:

Encrypted fields

field :user_email, Circles.EncryptedBinary
field :user_email_hash, Cloak.Ecto.SHA256
field :user_name, Circles.EncryptedBinary
field :user_password_hash, Cloak.Ecto.SHA256

@required_fields ~w(user_name user_email user_password)a
@mail_regex ~r/^[A-Za-z0-9._%±]+@[A-Za-z0-9.-]+.[A-Za-z]+$/
def registration_changeset(%Aiuser{} = aiuser, attrs \ %{}) do
aiuser
|> Map.merge(attrs)
|> cast(attrs, @required_fields)
|> validate_required(@required_fields)
|> validate_email()
|> validate_length(:user_password, min: 8)
|> put_hashed_fields()
|> new_user_id()
|> unique_constraint(:user_email_hash, name: :aiuser_user_email_hash_index)
|> unique_constraint(:user_id, name: :aiuser_pkey)
end

@required_for_login ~w[user_email user_password]a
def login_changeset(%Aiuser{} = aiuser, attrs \ %{}) do
aiuser
|> Map.merge(attrs)
|> cast(attrs, @required_for_login)
|> validate_required(@required_for_login)
|> validate_email()
|> validate_length(:user_password, min: 8)
|> put_hashed_fields()
|> validate_password()
end


Erlang/OTP 23 [erts-11.0.4] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [hipe]

Elixir 1.11.3 (compiled with Erlang/OTP 23)

phoenix 1.5.7
cloak 1.0.3
cloak_ecto 1.1.1

Thanks for any ideas.
Gusti.

1 Like

Sorry, there is also the following in the schema:

field(:user_password, :string, virtual: true)

It’s hard to guess what’s going on without the rest of the private functions here - validate_password, put_hashed_fields, new_user_id

But those functions are all after the “validate_length”. Do they matter anyway?

Shouldn’t validate_length throw an error before, and the rest of the pipe not called?

If the input is too short, validate_length adds an error to the changeset but the flow of execution continues.

If there’s code that shouldn’t run on an invalid changeset, it should be guarded with a pattern-match:

def do_thing_only_for_valid(%{valid?: true} = changeset) do
   # do thing with changeset
end

# no-op if valid? isn't set
def do_thing_only_for_valid(changeset), do: changeset
1 Like

When I run:

iex(17)> log = Aiuser.login_changeset(%Aiuser{}, %{user_email: “anysample.com”, user_password: “”})

ecto.Changeset<
action: nil,
changes: %{user_email_hash: “anysample.com”, user_password_hash: “”},
errors: [
user_password: {“invalid credentials”, },
user_email: {“invalid email”, },
user_password: {“can’t be blank”, [validation: :required]}
],
data: #Circles.Aiuser<>,
valid?: false

When I run:

iex(18)> log = Aiuser.login_changeset(%Aiuser{}, %{user_email: “any@sample.com”, user_password: “”})

ecto.Changeset<
action: nil,
changes: %{user_email_hash: “any@sample.com”, user_password_hash: “”},
errors: [
user_password: {“invalid credentials”, },
user_password: {“can’t be blank”, [validation: :required]}
],
data: #Circles.Aiuser<>,
valid?: false

But when I run:
iex(19)> log = Aiuser.login_changeset(%Aiuser{}, %{user_email: “any@sample.com”, user_password: “a”})

ecto.Changeset<
action: nil,
changes: %{user_email_hash: “any@sample.com”, user_password_hash: “a”},
errors: [user_password: {“invalid credentials”, }],
data: #Circles.Aiuser<>,
valid?: false

… it does not add the error that the password is too short. The “invalid credentials” is added by the “validate_password” pipe.

Does “validate_length” work on virtual fields?

I have other schemas where the “validate_length” works OK.
The only difference in this schema is that I have some fields from Cloak.Ecto.

I wonder if Cloak.Ecto is interfering with the validation.

I found that if I remove the

|> Map.merge(attrs)

line from my changeset, the validations work OK.

I copied the changeset example from somewhere, so I don’t know if the Map.merge is needed at all, or under what circumstances might it make sense.