Commit f69cbf47 authored by Ivan Tashkinov's avatar Ivan Tashkinov

[#114] Added :user_id component to email confirmation path to improve the security.

Added tests for `confirm_email` action.
parent 8adcd1e8
......@@ -70,6 +70,7 @@ def account_confirmation_email(user) do
Router.Helpers.confirm_email_url(
Endpoint,
:confirm_email,
user.id,
to_string(user.info.confirmation_token)
)
......
......@@ -396,10 +396,6 @@ def get_or_fetch_by_nickname(nickname) do
end
end
def get_by_confirmation_token(token) do
Repo.one(from(u in User, where: fragment("? ->> 'confirmation_token' = ?", u.info, ^token)))
end
def get_followers_query(%User{id: id, follower_address: follower_address}) do
from(
u in User,
......
......@@ -282,7 +282,12 @@ defmodule Pleroma.Web.Router do
post("/account/register", TwitterAPI.Controller, :register)
post("/account/password_reset", TwitterAPI.Controller, :password_reset)
get("/account/confirm_email/:token", TwitterAPI.Controller, :confirm_email, as: :confirm_email)
get(
"/account/confirm_email/:user_id/:token",
TwitterAPI.Controller,
:confirm_email,
as: :confirm_email
)
post("/account/resend_confirmation_email", TwitterAPI.Controller, :resend_confirmation_email)
......
......@@ -382,9 +382,11 @@ def password_reset(conn, params) do
end
end
def confirm_email(conn, %{"token" => token}) do
with %User{} = user <- User.get_by_confirmation_token(token),
def confirm_email(conn, %{"user_id" => uid, "token" => token}) do
with %User{} = user <- Repo.get(User, uid),
true <- user.local,
true <- user.info.confirmation_pending,
true <- user.info.confirmation_token == token,
info_change <- User.Info.confirmation_changeset(user.info, :confirmed),
changeset <- Changeset.change(user) |> Changeset.put_embed(:info, info_change),
{:ok, _} <- User.update_and_set_cache(changeset) do
......
......@@ -873,7 +873,7 @@ test "it returns 500 when user is not local", %{conn: conn, user: user} do
end
end
describe "GET /api/account/confirm_email/:token" do
describe "GET /api/account/confirm_email/:id/:token" do
setup do
user = insert(:user)
info_change = User.Info.confirmation_changeset(user.info, :unconfirmed)
......@@ -890,19 +890,31 @@ test "it returns 500 when user is not local", %{conn: conn, user: user} do
end
test "it redirects to root url", %{conn: conn, user: user} do
conn = get(conn, "/api/account/confirm_email/#{user.info.confirmation_token}")
conn = get(conn, "/api/account/confirm_email/#{user.id}/#{user.info.confirmation_token}")
assert 302 == conn.status
end
test "it confirms the user account", %{conn: conn, user: user} do
get(conn, "/api/account/confirm_email/#{user.info.confirmation_token}")
get(conn, "/api/account/confirm_email/#{user.id}/#{user.info.confirmation_token}")
user = Repo.get(User, user.id)
refute user.info.confirmation_pending
refute user.info.confirmation_token
end
test "it returns 500 if user cannot be found by id", %{conn: conn, user: user} do
conn = get(conn, "/api/account/confirm_email/0/#{user.info.confirmation_token}")
assert 500 == conn.status
end
test "it returns 500 if token is invalid", %{conn: conn, user: user} do
conn = get(conn, "/api/account/confirm_email/#{user.id}/wrong_token")
assert 500 == conn.status
end
end
describe "POST /api/account/resend_confirmation_email" do
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment