-
Notifications
You must be signed in to change notification settings - Fork 2
validate_secure_password #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
validate_secure_password #10
Conversation
danielberkompas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me so far! I'm reading the NIST guidelines this afternoon; I'll comment I find anything more we should add.
lib/authority_ecto/changeset.ex
Outdated
| @password_blacklist File.cwd!() | ||
| |> Path.join("password_blacklist.txt") | ||
| |> File.read!() | ||
| |> String.split("\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should probably go in priv, and I think you can read it this way:
@password_blacklist "priv/password_blacklist.txt"
|> File.stream!
|> Stream.map(&String.trim/1)
|> Enum.into([])It's slightly more efficient.
|
This regular expression can be used to exclude repeated characters like "aaaaa": To detect sequences, it looks like we'll need to write code. Elixir binary pattern matching, here we come! |
|
Binary matching could definitely work here, but I think this might be simpler: @consecutive_alphanum "abcdefghijklmnopqrstuvwxyz01234567890"
defp consecutive?(value, size) do
blacklist = chunk_letters(@consecutive_alphanum, size)
value |> chunk_letters(size) |> Enum.any?(&(&1 in blacklist))
end
defp chunk_letters(value, size) do
value |> String.to_charlist |> Enum.chunk_every(size, 1, :discard)
end |
|
Yeah, I wrote a function using pattern matching but yours is better. We can use your function to exclude passwords with >= 3 consecutive characters. def validate_consecutive(changeset, field, opts \\ []) do
value = get_change(changeset, field)
max = opts[:max] || 3
msg = opts[:message] || "contains more than %{max} consecutive characters"
if value && consecutive?(value, max) do
add_error(changeset, field, msg, [validation: :consecutive, max: max])
else
changeset
end
end |
| @@ -0,0 +1,34 @@ | |||
| defmodule Authority.Ecto.Password do | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps this should be marked @moduledoc false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree with that.
0d1eedb to
a7c51b1
Compare
|
We add the |
Resolves #3.