Skip to content
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

Handle unconfirmed email #378

Merged
merged 1 commit into from
Sep 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config/test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,6 @@ config :coherence,
registration_permitted_attributes: ["email","name","password", "password_confirmation","current_password"],
invitation_permitted_attributes: ["name","email"],
password_reset_permitted_attributes: ["reset_password_token","password","password_confirmation"],
session_permitted_attributes: ["remember","email", "password"]
session_permitted_attributes: ["remember","email", "password"],
confirm_email_updates: true

2 changes: 2 additions & 0 deletions lib/coherence/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ defmodule Coherence.Config do
* :invitation_permitted_attributes - List of allowed invitation parameter attribues as strings
* :password_reset_permitted_attributes - List of allowed password reset atributes as stings,
* :session_permitted_attributes - List of allowed session attributes as strings
* :confirm_email_updates - All email updates should be confirmed by email (using the unconfirmed_email field)

## Examples

Expand Down Expand Up @@ -125,6 +126,7 @@ defmodule Coherence.Config do
:invitation_permitted_attributes,
:password_reset_permitted_attributes,
:session_permitted_attributes,
{:confirm_email_updates, false}
]
|> Enum.each(fn
{key, default} ->
Expand Down
13 changes: 11 additions & 2 deletions lib/coherence/controllers/confirmation_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,19 @@ defmodule Coherence.ConfirmationController do
}
)
else
changeset = Controller.changeset(:confirmation, user_schema, user, %{
attrs = case Config.get(:confirm_email_updates) do
true ->
%{
email: user.unconfirmed_email,
unconfirmed_email: nil
}
_ ->
%{}
end
changeset = Ecto.Changeset.change(user, Map.merge(attrs, %{
confirmation_token: nil,
confirmed_at: NaiveDateTime.utc_now(),
})
}))
case Config.repo.update(changeset) do
{:ok, _user} ->
conn
Expand Down
9 changes: 8 additions & 1 deletion lib/coherence/controllers/controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ defmodule Coherence.Controller do
end
end


@doc """
Plug to redirect already logged in users.
"""
Expand Down Expand Up @@ -293,6 +292,14 @@ defmodule Coherence.Controller do
end
apply(module, fun, [model, params, :password])
end
def changeset(:registration, module, model, params) do
fun =
case Application.get_env(:coherence, :changeset, {nil, :changeset}) do
{_mod, fun} -> fun
_ -> :changeset
end
apply(module, fun, [model, params, :registration])
end
def changeset(which, module, model, params) do
{mod, fun, args} =
case Application.get_env :coherence, :changeset do
Expand Down
10 changes: 7 additions & 3 deletions lib/coherence/controllers/registration_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,18 @@ defmodule Coherence.RegistrationController do
@spec update(conn, params) :: conn
def update(conn, %{"registration" => user_params} = params) do
user_schema = Config.user_schema
user = Coherence.current_user(conn)
initial_user = Coherence.current_user(conn)
:registration
|> Controller.changeset(user_schema, user, Controller.permit(user_params,
|> Controller.changeset(user_schema, initial_user, Controller.permit(user_params,
Config.registration_permitted_attributes() ||
Schema.permitted_attributes_default(:registration)))
|> Schemas.update
|> case do
{:ok, user} ->
if Config.get(:confirm_email_updates) &&
user.unconfirmed_email != initial_user.unconfirmed_email do
send_confirmation(conn, user, user_schema)
end
Config.auth_module
|> apply(Config.update_login, [conn, user, [id_key: Config.schema_key]])
|> respond_with(
Expand All @@ -126,7 +130,7 @@ defmodule Coherence.RegistrationController do
}
)
{:error, changeset} ->
respond_with(conn, :registration_update_error, %{user: user, changeset: changeset})
respond_with(conn, :registration_update_error, %{user: initial_user, changeset: changeset})
end
end

Expand Down
4 changes: 3 additions & 1 deletion lib/coherence/schema.ex
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,9 @@ defmodule Coherence.Schema do
field :confirmation_token, :string
field :confirmed_at, :naive_datetime
field :confirmation_sent_at, :naive_datetime
# field :unconfirmed_email, :string
if Coherence.Config.get(:confirm_email_updates) do
field :unconfirmed_email, :string
end
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/mix/mix_utils.ex
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ defmodule Coherence.Mix.Utils do
"# confirmable",
"add :confirmation_token, :string",
"add :confirmed_at, :utc_datetime",
"add :confirmation_sent_at, :utc_datetime"
"add :confirmation_sent_at, :utc_datetime",
"add :unconfirmed_email, :string"
]
]
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,19 @@ defmodule <%= web_base %>.Coherence.ConfirmationController do
}
)
else
changeset = Controller.changeset(:confirmation, user_schema, user, %{
attrs = case Config.get(:confirm_email_updates) do
true ->
%{
email: user.unconfirmed_email,
unconfirmed_email: nil
}
_ ->
%{}
end
changeset = Ecto.Changeset.change(user, Map.merge(attrs, %{
confirmation_token: nil,
confirmed_at: NaiveDateTime.utc_now(),
})
}))
case Config.repo.update(changeset) do
{:ok, _user} ->
conn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,18 @@ defmodule <%= web_base %>.Coherence.RegistrationController do
@spec update(conn, params) :: conn
def update(conn, %{"registration" => user_params} = params) do
user_schema = Config.user_schema
user = Coherence.current_user(conn)
initial_user = Coherence.current_user(conn)
:registration
|> Controller.changeset(user_schema, user, Controller.permit(user_params,
|> Controller.changeset(user_schema, initial_user, Controller.permit(user_params,
Config.registration_permitted_attributes() ||
Schema.permitted_attributes_default(:registration)))
|> Schemas.update
|> case do
{:ok, user} ->
if Config.get(:confirm_email_updates) &&
user.unconfirmed_email != initial_user.unconfirmed_email do
send_confirmation(conn, user, user_schema)
end
Config.auth_module
|> apply(Config.update_login, [conn, user, [id_key: Config.schema_key]])
|> respond_with(
Expand All @@ -126,7 +130,7 @@ defmodule <%= web_base %>.Coherence.RegistrationController do
}
)
{:error, changeset} ->
respond_with(conn, :registration_update_error, %{user: user, changeset: changeset})
respond_with(conn, :registration_update_error, %{user: initial_user, changeset: changeset})
end
end

Expand Down
23 changes: 20 additions & 3 deletions priv/templates/coh.install/emails/coherence/user_email.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,25 @@ defmodule <%= web_base %>.Coherence.UserEmail do
end

def confirmation(user, url) do
{template, subject, email} = if Config.get(:confirm_email_updates) && user.unconfirmed_email do
{
"reconfirmation.html",
dgettext("coherence", "%{site_name} - Confirm your new email", site_name: site_name()),
unconfirmed_email(user)
}
else
{
"confirmation.html",
dgettext("coherence", "%{site_name} - Confirm your new account", site_name: site_name()),
user_email(user)
}
end
%Email{}
|> from(from_email())
|> to(user_email(user))
|> to(email)
|> add_reply_to()
|> subject(dgettext("coherence", "%{site_name} - Confirm your new account", site_name: site_name()))
|> render_body("confirmation.html", %{url: url, name: first_name(user.name)})
|> subject(subject)
|> render_body(template, %{url: url, name: first_name(user.name)})
end

def invitation(invitation, url) do
Expand Down Expand Up @@ -65,6 +78,10 @@ defmodule <%= web_base %>.Coherence.UserEmail do
{user.name, user.email}
end

def unconfirmed_email(user) do
{user.name, user.unconfirmed_email}
end

defp from_email do
case Coherence.Config.email_from do
nil ->
Expand Down
13 changes: 13 additions & 0 deletions priv/templates/coh.install/models/coherence/user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,17 @@ defmodule <%= user_schema %> do
|> cast(params, ~w(password password_confirmation reset_password_token reset_password_sent_at))
|> validate_coherence_password_reset(params)
end

def changeset(model, params, :registration) do
changeset =
model
|> changeset(params)
if Config.get(:confirm_email_updates) && Map.get(params, "email", false) && model.id do
changeset
|> put_change(:unconfirmed_email, get_change(changeset, :email))
|> delete_change(:email)
else
changeset
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<div>
<p><%%= dgettext "coherence", "Hello %{name}!", name: @name %><p>
<p>
<%%= dgettext "coherence", "Your new email has been correctly saved. Click the link below to confirm you new email." %>
</p>
<p>
<a href="<%%= @url %>"><%%= dgettext "coherence", "Confirm my Email" %></a>
</p>
<p><%%= dgettext "coherence", "Thank you!" %></p>
</div>

47 changes: 47 additions & 0 deletions test/controllers/confirmation_controller.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
defmodule CoherenceTest.ConfirmationController do
use TestCoherence.ConnCase
import TestCoherenceWeb.Router.Helpers

setup %{conn: conn} do
Application.put_env :coherence, :opts, [:confirmable, :registerable]
user = %TestCoherence.User{
name: "John Doe",
email: "[email protected]",
password_hash: "superhash",
unconfirmed_email: "[email protected]",
confirmation_token: "foobar",
confirmation_sent_at: Timex.now
} |> TestCoherence.Repo.insert!
{:ok, conn: conn, user: user}
end

describe "edit" do
test "should confirm valid confirmation token", %{conn: conn} do
conn = get conn, confirmation_path(conn, :edit, "foobar")
assert html_response(conn, 302)
user = get_user_by_email("[email protected]")
assert user.confirmation_token == nil
refute user.confirmed_at == nil
end

test "should set email from unconfirmed_email if confirm_email_updates is true", %{conn: conn} do
Application.put_env :coherence, :confirm_email_updates, true
conn = get conn, confirmation_path(conn, :edit, "foobar")
assert html_response(conn, 302)
user = get_user_by_email("[email protected]")
assert user.unconfirmed_email == nil
assert user.confirmation_token == nil
refute user.confirmed_at == nil
end

test "should not set email from unconfirmed_email if confirm_email_updates is false", %{conn: conn} do
Application.put_env :coherence, :confirm_email_updates, false
conn = get conn, confirmation_path(conn, :edit, "foobar")
assert html_response(conn, 302)
user = get_user_by_email("[email protected]")
refute user.unconfirmed_email == nil
assert user.confirmation_token == nil
refute user.confirmed_at == nil
end
end
end
47 changes: 39 additions & 8 deletions test/controllers/registration_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ defmodule CoherenceTest.RegistrationController do
use TestCoherence.ConnCase
import TestCoherenceWeb.Router.Helpers

setup %{conn: conn} do
setup %{conn: conn} = context do
Application.put_env :coherence, :opts, [:confirmable, :registerable]
user = insert_user()
Application.put_env :coherence, :confirm_email_updates, Map.get(context, :confirm_email_updates, false)
user = insert_user(%{name: "John Doe"})
conn = assign conn, :current_user, user
{:ok, conn: conn, user: user}
end
Expand Down Expand Up @@ -63,6 +64,7 @@ defmodule CoherenceTest.RegistrationController do
end
end

@moduletag report: [:confirm_email_updates]
describe "update" do
test "can update registration with valid current password", %{conn: conn, user: user} do
params = %{"registration" => %{"current_password" => user.password}}
Expand All @@ -84,13 +86,42 @@ defmodule CoherenceTest.RegistrationController do
errors = conn.assigns.changeset.errors
assert errors[:current_password] == {"invalid current password", []}
end

test "mass assignment not allowed", %{conn: conn, user: user} do
params = %{"registration" => %{"current_password" => user.password, "current_sign_in_ip" => "mass_asignment"}}
conn = put conn, registration_path(conn, :update), params
assert conn.private[:phoenix_flash] == %{"info" => "Account updated successfully."}
assert html_response(conn, 302)
%{:current_sign_in_ip => current_sign_in_ip} = get_user_by_email(user.email)
refute current_sign_in_ip == params["registration"]["current_sign_in_ip"]
end
conn = put conn, registration_path(conn, :update), params
assert conn.private[:phoenix_flash] == %{"info" => "Account updated successfully."}
assert html_response(conn, 302)
%{:current_sign_in_ip => current_sign_in_ip} = get_user_by_email(user.email)
refute current_sign_in_ip == params["registration"]["current_sign_in_ip"]
end

test "can update email directly", %{conn: conn} do
params = %{"registration" => %{"email" => "[email protected]"}}
conn = put conn, registration_path(conn, :update), params
assert html_response(conn, 302)
%{:email => email} = get_user_by_name("John Doe")
assert email == params["registration"]["email"]
end

@tag confirm_email_updates: true
test "cannot update email directly but unconfirmed_email", %{conn: conn} do
params = %{"registration" => %{"email" => "[email protected]"}}
conn = put conn, registration_path(conn, :update), params
assert html_response(conn, 302)
user = get_user_by_name("John Doe")
assert user.unconfirmed_email == params["registration"]["email"]
refute user.email == params["registration"]["email"]
end

@tag confirm_email_updates: true
test "should not set unconfirmed email if email is the same", %{conn: conn, user: user} do
params = %{"registration" => %{"email" => user.email}}
conn = put conn, registration_path(conn, :update), params
assert html_response(conn, 302)
user = get_user_by_name("John Doe")
refute user.unconfirmed_email
assert user.email == user.email
end
end
end
11 changes: 10 additions & 1 deletion test/support/email.exs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@ defmodule TestCoherenceWeb.Coherence.UserEmail do


def confirmation(user, url) do
email = if Config.get(:confirm_email_updates) && user.unconfirmed_email do
unconfirmed_email(user)
else
user_email(user)
end
%Email{}
|> from(from_email())
|> to(user_email(user))
|> to(email)
|> add_reply_to
|> subject("#{site_name()} - Confirm your new account")
|> render_body("confirmation.html", %{url: url, name: first_name(user.name)})
Expand Down Expand Up @@ -73,6 +78,10 @@ defmodule TestCoherenceWeb.Coherence.UserEmail do
{user.name, user.email}
end

def unconfirmed_email(user) do
{user.name, user.unconfirmed_email}
end

defp from_email do
case Coherence.Config.email_from do
nil ->
Expand Down
1 change: 1 addition & 0 deletions test/support/migrations.exs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ defmodule TestCoherence.Migrations do
add :confirmation_token, :string
add :confirmed_at, :utc_datetime
add :confirmation_sent_at, :utc_datetime
add :unconfirmed_email, :string
# rememberable
add :remember_created_at, :utc_datetime
timestamps()
Expand Down
Loading