Skip to content

Commit 2bbba99

Browse files
committed
replace people.email (plaintext) with Fields.EmailEncrypted for privacy/security #285 also comment out all insecure/unused code - to be removed shortly
1 parent da0af7e commit 2bbba99

14 files changed

+324
-285
lines changed

lib/auth/accounts.ex

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ defmodule Auth.Accounts do
2323
2424
"""
2525
def get_person_by_email(email) when is_binary(email) do
26-
Repo.get_by(Person, email: email)
26+
Repo.get_by(Person, email_hash: email)
2727
end
2828

2929
@doc """
@@ -40,7 +40,7 @@ defmodule Auth.Accounts do
4040
"""
4141
def get_person_by_email_and_password(email, password)
4242
when is_binary(email) and is_binary(password) do
43-
person = Repo.get_by(Person, email: email)
43+
person = Repo.get_by(Person, email_hash: email)
4444
if Person.valid_password?(person, password), do: person
4545
end
4646

@@ -267,27 +267,27 @@ defmodule Auth.Accounts do
267267
end
268268
end
269269

270-
@doc """
271-
Confirms a person by the given token.
272-
273-
If the token matches, the person account is marked as confirmed
274-
and the token is deleted.
275-
"""
276-
def confirm_person(token) do
277-
with {:ok, query} <- PersonToken.verify_email_token_query(token, "confirm"),
278-
%Person{} = person <- Repo.one(query),
279-
{:ok, %{person: person}} <- Repo.transaction(confirm_person_multi(person)) do
280-
{:ok, person}
281-
else
282-
_ -> :error
283-
end
284-
end
285-
286-
defp confirm_person_multi(person) do
287-
Ecto.Multi.new()
288-
|> Ecto.Multi.update(:person, Person.confirm_changeset(person))
289-
|> Ecto.Multi.delete_all(:tokens, PersonToken.person_and_contexts_query(person, ["confirm"]))
290-
end
270+
# @doc """
271+
# Confirms a person by the given token.
272+
273+
# If the token matches, the person account is marked as confirmed
274+
# and the token is deleted.
275+
# """
276+
# def confirm_person(token) do
277+
# with {:ok, query} <- PersonToken.verify_email_token_query(token, "confirm"),
278+
# %Person{} = person <- Repo.one(query),
279+
# {:ok, %{person: person}} <- Repo.transaction(confirm_person_multi(person)) do
280+
# {:ok, person}
281+
# else
282+
# _ -> :error
283+
# end
284+
# end
285+
286+
# defp confirm_person_multi(person) do
287+
# Ecto.Multi.new()
288+
# |> Ecto.Multi.update(:person, Person.confirm_changeset(person))
289+
# |> Ecto.Multi.delete_all(:tokens, PersonToken.person_and_contexts_query(person, ["confirm"]))
290+
# end
291291

292292
## Reset password
293293

@@ -307,26 +307,26 @@ defmodule Auth.Accounts do
307307
PersonNotifier.deliver_reset_password_instructions(person, reset_password_url_fun.(encoded_token))
308308
end
309309

310-
@doc """
311-
Gets the person by reset password token.
310+
# @doc """
311+
# Gets the person by reset password token.
312312

313-
## Examples
313+
# ## Examples
314314

315-
iex> get_person_by_reset_password_token("validtoken")
316-
%Person{}
315+
# iex> get_person_by_reset_password_token("validtoken")
316+
# %Person{}
317317

318-
iex> get_person_by_reset_password_token("invalidtoken")
319-
nil
318+
# iex> get_person_by_reset_password_token("invalidtoken")
319+
# nil
320320

321-
"""
322-
def get_person_by_reset_password_token(token) do
323-
with {:ok, query} <- PersonToken.verify_email_token_query(token, "reset_password"),
324-
%Person{} = person <- Repo.one(query) do
325-
person
326-
else
327-
_ -> nil
328-
end
329-
end
321+
# """
322+
# def get_person_by_reset_password_token(token) do
323+
# with {:ok, query} <- PersonToken.verify_email_token_query(token, "reset_password"),
324+
# %Person{} = person <- Repo.one(query) do
325+
# person
326+
# else
327+
# _ -> nil
328+
# end
329+
# end
330330

331331
@doc """
332332
Resets the person password.

lib/auth/accounts/person.ex

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ defmodule Auth.Accounts.Person do
33
import Ecto.Changeset
44

55
schema "people" do
6-
field :email, :string
6+
field :confirmed_at, :naive_datetime
7+
field :email, Fields.EmailEncrypted
8+
field :email_hash, Fields.EmailHash
79
field :password, :string, virtual: true, redact: true
810
field :hashed_password, :string, redact: true
9-
field :confirmed_at, :naive_datetime
1011

1112
timestamps()
1213
end
@@ -41,11 +42,23 @@ defmodule Auth.Accounts.Person do
4142
|> validate_password(opts)
4243
end
4344

45+
defp put_email_hash(changeset) when not is_nil(changeset.changes.email) do
46+
# dbg(changeset)
47+
put_change(changeset, :email_hash, String.downcase(changeset.changes.email))
48+
end
49+
50+
defp put_email_hash(changeset) do
51+
# dbg(changeset)
52+
changeset
53+
end
54+
55+
4456
defp validate_email(changeset, opts) do
4557
changeset
4658
|> validate_required([:email])
4759
|> validate_format(:email, ~r/^[^\s]+@[^\s]+$/, message: "must have the @ sign and no spaces")
4860
|> validate_length(:email, max: 160)
61+
|> put_email_hash()
4962
|> maybe_validate_unique_email(opts)
5063
end
5164

@@ -80,8 +93,8 @@ defmodule Auth.Accounts.Person do
8093
defp maybe_validate_unique_email(changeset, opts) do
8194
if Keyword.get(opts, :validate_email, true) do
8295
changeset
83-
|> unsafe_validate_unique(:email, Auth.Repo)
84-
|> unique_constraint(:email)
96+
|> unsafe_validate_unique(:email_hash, Auth.Repo)
97+
|> unique_constraint(:email_hash)
8598
else
8699
changeset
87100
end

lib/auth/accounts/person_token.ex

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ defmodule Auth.Accounts.PersonToken do
1717
field :token, :binary
1818
field :context, :string
1919
field :sent_to, :string
20+
# field :sent_to, Fields.EmailEncrypted
21+
# field :email_hash, Fields.EmailHash
2022
belongs_to :person, Auth.Accounts.Person
2123

2224
timestamps(updated_at: false)
@@ -107,29 +109,29 @@ defmodule Auth.Accounts.PersonToken do
107109
for resetting the password. For verifying requests to change the email,
108110
see `verify_change_email_token_query/2`.
109111
"""
110-
def verify_email_token_query(token, context) do
111-
case Base.url_decode64(token, padding: false) do
112-
{:ok, decoded_token} ->
113-
hashed_token = :crypto.hash(@hash_algorithm, decoded_token)
114-
days = days_for_context(context)
115-
116-
query =
117-
from token in token_and_context_query(hashed_token, context),
118-
join: person in assoc(token, :person),
119-
where: token.inserted_at > ago(^days, "day") and token.sent_to == person.email,
120-
select: person
121-
122-
{:ok, query}
123-
# phx.gen.auth boilerplate code not yet covered by tests ...
124-
# coveralls-ignore-start
125-
:error ->
126-
:error
127-
# coveralls-ignore-stop
128-
end
129-
end
130-
131-
defp days_for_context("confirm"), do: @confirm_validity_in_days
132-
defp days_for_context("reset_password"), do: @reset_password_validity_in_days
112+
# def verify_email_token_query(token, context) do
113+
# case Base.url_decode64(token, padding: false) do
114+
# {:ok, decoded_token} ->
115+
# hashed_token = :crypto.hash(@hash_algorithm, decoded_token)
116+
# days = days_for_context(context)
117+
118+
# query =
119+
# from token in token_and_context_query(hashed_token, context),
120+
# join: person in assoc(token, :person),
121+
# where: token.inserted_at > ago(^days, "day") and token.sent_to == person.email,
122+
# select: person
123+
124+
# {:ok, query}
125+
# # phx.gen.auth boilerplate code not yet covered by tests ...
126+
# # coveralls-ignore-start
127+
# :error ->
128+
# :error
129+
# # coveralls-ignore-stop
130+
# end
131+
# end
132+
133+
# defp days_for_context("confirm"), do: @confirm_validity_in_days
134+
# defp days_for_context("reset_password"), do: @reset_password_validity_in_days
133135

134136
@doc """
135137
Checks if the token is valid and returns its underlying lookup query.

lib/auth_web/controllers/person_confirmation_controller.ex

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,27 @@ defmodule AuthWeb.PersonConfirmationController do
3030

3131
# Do not log in the person after confirmation to avoid a
3232
# leaked token giving the person access to the account.
33-
def update(conn, %{"token" => token}) do
34-
case Accounts.confirm_person(token) do
35-
{:ok, _} ->
36-
conn
37-
|> put_flash(:info, "Person confirmed successfully.")
38-
|> redirect(to: ~p"/")
39-
40-
:error ->
41-
# If there is a current person and the account was already confirmed,
42-
# then odds are that the confirmation link was already visited, either
43-
# by some automation or by the person themselves, so we redirect without
44-
# a warning message.
45-
case conn.assigns do
46-
%{current_person: %{confirmed_at: confirmed_at}} when not is_nil(confirmed_at) ->
47-
redirect(conn, to: ~p"/")
48-
49-
%{} ->
50-
conn
51-
|> put_flash(:error, "Person confirmation link is invalid or it has expired.")
52-
|> redirect(to: ~p"/")
53-
end
54-
end
55-
end
33+
# def update(conn, %{"token" => token}) do
34+
# case Accounts.confirm_person(token) do
35+
# {:ok, _} ->
36+
# conn
37+
# |> put_flash(:info, "Person confirmed successfully.")
38+
# |> redirect(to: ~p"/")
39+
40+
# :error ->
41+
# # If there is a current person and the account was already confirmed,
42+
# # then odds are that the confirmation link was already visited, either
43+
# # by some automation or by the person themselves, so we redirect without
44+
# # a warning message.
45+
# case conn.assigns do
46+
# %{current_person: %{confirmed_at: confirmed_at}} when not is_nil(confirmed_at) ->
47+
# redirect(conn, to: ~p"/")
48+
49+
# %{} ->
50+
# conn
51+
# |> put_flash(:error, "Person confirmation link is invalid or it has expired.")
52+
# |> redirect(to: ~p"/")
53+
# end
54+
# end
55+
# end
5656
end

lib/auth_web/controllers/person_reset_password_controller.ex

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ defmodule AuthWeb.PersonResetPasswordController do
33

44
alias Auth.Accounts
55

6-
plug :get_person_by_reset_password_token when action in [:edit, :update]
6+
# plug :get_person_by_reset_password_token when action in [:edit, :update]
77

88
def new(conn, _params) do
99
render(conn, :new)
@@ -13,6 +13,7 @@ defmodule AuthWeb.PersonResetPasswordController do
1313
if person = Accounts.get_person_by_email(email) do
1414
Accounts.deliver_person_reset_password_instructions(
1515
person,
16+
# ~p"/people/reset_password/"
1617
&url(~p"/people/reset_password/#{&1}")
1718
)
1819
end
@@ -25,34 +26,34 @@ defmodule AuthWeb.PersonResetPasswordController do
2526
|> redirect(to: ~p"/")
2627
end
2728

28-
def edit(conn, _params) do
29-
render(conn, :edit, changeset: Accounts.change_person_password(conn.assigns.person))
30-
end
31-
32-
# Do not log in the person after reset password to avoid a
33-
# leaked token giving the person access to the account.
34-
def update(conn, %{"person" => person_params}) do
35-
case Accounts.reset_person_password(conn.assigns.person, person_params) do
36-
{:ok, _} ->
37-
conn
38-
|> put_flash(:info, "Password reset successfully.")
39-
|> redirect(to: ~p"/people/log_in")
40-
41-
{:error, changeset} ->
42-
render(conn, :edit, changeset: changeset)
43-
end
44-
end
45-
46-
defp get_person_by_reset_password_token(conn, _opts) do
47-
%{"token" => token} = conn.params
48-
49-
if person = Accounts.get_person_by_reset_password_token(token) do
50-
conn |> assign(:person, person) |> assign(:token, token)
51-
else
52-
conn
53-
|> put_flash(:error, "Reset password link is invalid or it has expired.")
54-
|> redirect(to: ~p"/")
55-
|> halt()
56-
end
57-
end
29+
# def edit(conn, _params) do
30+
# render(conn, :edit, changeset: Accounts.change_person_password(conn.assigns.person))
31+
# end
32+
33+
# # Do not log in the person after reset password to avoid a
34+
# # leaked token giving the person access to the account.
35+
# def update(conn, %{"person" => person_params}) do
36+
# case Accounts.reset_person_password(conn.assigns.person, person_params) do
37+
# {:ok, _} ->
38+
# conn
39+
# |> put_flash(:info, "Password reset successfully.")
40+
# |> redirect(to: ~p"/people/log_in")
41+
42+
# {:error, changeset} ->
43+
# render(conn, :edit, changeset: changeset)
44+
# end
45+
# end
46+
47+
# defp get_person_by_reset_password_token(conn, _opts) do
48+
# %{"token" => token} = conn.params
49+
50+
# if person = Accounts.get_person_by_reset_password_token(token) do
51+
# conn |> assign(:person, person) |> assign(:token, token)
52+
# else
53+
# conn
54+
# |> put_flash(:error, "Reset password link is invalid or it has expired.")
55+
# |> redirect(to: ~p"/")
56+
# |> halt()
57+
# end
58+
# end
5859
end

lib/auth_web/router.ex

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ defmodule AuthWeb.Router do
5757
post "/people/log_in", PersonSessionController, :create
5858
get "/people/reset_password", PersonResetPasswordController, :new
5959
post "/people/reset_password", PersonResetPasswordController, :create
60-
get "/people/reset_password/:token", PersonResetPasswordController, :edit
61-
put "/people/reset_password/:token", PersonResetPasswordController, :update
60+
# get "/people/reset_password/:token", PersonResetPasswordController, :edit
61+
# put "/people/reset_password/:token", PersonResetPasswordController, :update
6262
end
6363

6464
scope "/", AuthWeb do
@@ -76,6 +76,6 @@ defmodule AuthWeb.Router do
7676
get "/people/confirm", PersonConfirmationController, :new
7777
post "/people/confirm", PersonConfirmationController, :create
7878
get "/people/confirm/:token", PersonConfirmationController, :edit
79-
post "/people/confirm/:token", PersonConfirmationController, :update
79+
# post "/people/confirm/:token", PersonConfirmationController, :update
8080
end
8181
end

mix.exs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ defmodule Auth.MixProject do
44
def project do
55
[
66
app: :auth,
7-
version: "0.1.0",
7+
version: "2.0.0",
88
elixir: "~> 1.14",
99
elixirc_paths: elixirc_paths(Mix.env()),
1010
start_permanent: Mix.env() == :prod,
@@ -67,6 +67,10 @@ defmodule Auth.MixProject do
6767
{:jason, "~> 1.2"},
6868
{:plug_cowboy, "~> 2.5"},
6969

70+
# Field Validation and Encryption: github.com/dwyl/fields
71+
{:fields, "~> 2.10.3"},
72+
73+
7074
# Check test coverage: github.com/parroty/excoveralls
7175
{:excoveralls, "~> 0.14.3", only: :test},
7276
]

0 commit comments

Comments
 (0)