Skip to content

Commit

Permalink
Make emails case-insensitive
Browse files Browse the repository at this point in the history
  • Loading branch information
ku1ik committed May 2, 2024
1 parent 8cb17bf commit c1e9634
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 14 deletions.
9 changes: 6 additions & 3 deletions lib/asciinema/accounts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ defmodule Asciinema.Accounts do
result =
%User{}
|> cast(attrs, [:email])
|> validate_format(:email, @valid_email_re)
|> validate_required([:email])
|> update_change(:email, &String.downcase/1)
|> validate_format(:email, @valid_email_re)
|> add_contraints()
|> Repo.insert()

Expand Down Expand Up @@ -79,6 +80,8 @@ defmodule Asciinema.Accounts do
:terminal_font_family,
:asciicasts_private_by_default
])
|> validate_required([:email])
|> update_change(:email, &String.downcase/1)
|> validate_format(:email, @valid_email_re)
|> validate_format(:username, @valid_username_re)
|> validate_length(:username, min: 2, max: 16)
Expand All @@ -100,7 +103,7 @@ defmodule Asciinema.Accounts do

user
|> change_user(params)
|> validate_required([:username, :email])
|> validate_required([:username])
|> Repo.update()
end

Expand Down Expand Up @@ -139,7 +142,7 @@ defmodule Asciinema.Accounts do

def lookup_user(identifier) when is_binary(identifier) do
if String.contains?(identifier, "@") do
{:email, Repo.get_by(User, email: identifier)}
{:email, Repo.get_by(User, email: String.downcase(identifier))}
else
{:username, find_user_by_username(identifier)}
end
Expand Down
21 changes: 21 additions & 0 deletions priv/repo/migrations/20240502083959_lowercase_emails.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
defmodule Asciinema.Repo.Migrations.LowercaseEmails do
use Ecto.Migration

@get_dup_emails "SELECT LOWER(email) AS e FROM users WHERE email IS NOT NULL GROUP BY e HAVING COUNT(*) > 1"
@get_users_for_email "SELECT u.id, COUNT(a.id) AS asciicast_count FROM users u LEFT OUTER JOIN asciicasts a ON (a.user_id = u.id) WHERE LOWER(u.email) = $1 GROUP BY u.id ORDER BY asciicast_count DESC, name, username"
@update_email "UPDATE users SET email = CONCAT('_', $2::int, '_', email) WHERE id = $1"

def change do
execute(fn ->
for [email] <- repo().query!(@get_dup_emails).rows do
for {[id | _], i} <- Enum.with_index(repo().query!(@get_users_for_email, [email]).rows) do
if i > 0 do
repo().query!(@update_email, [id, i])
end
end
end
end, fn -> :ok end)

execute "UPDATE users SET email=LOWER(email) WHERE email IS NOT NULL", ""
end
end
11 changes: 7 additions & 4 deletions test/asciinema/accounts_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,16 @@ defmodule Asciinema.AccountsTest do
%{user: fixture(:user)}
end

def assert_success(user, attrs) do
assert {:ok, _} = Accounts.update_user(user, attrs)
def success(user, attrs) do
assert {:ok, user} = Accounts.update_user(user, attrs)

user
end

test "success", %{user: user} do
assert_success(user, %{email: "[email protected]"})
assert_success(user, %{username: "newone"})
assert success(user, %{email: "[email protected]"})
assert success(user, %{email: "[email protected]"}).email == "[email protected]"
assert success(user, %{username: "newone"})
end

def assert_validation_error(user, attrs) do
Expand Down
15 changes: 8 additions & 7 deletions test/asciinema_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ defmodule AsciinemaTest do
test "succeeds when email not taken" do
assert {:ok, _} = Asciinema.create_user(%{email: "[email protected]"})
assert {:error, :email_taken} = Asciinema.create_user(%{email: "[email protected]"})
assert {:error, :email_taken} = Asciinema.create_user(%{email: "[email protected]"})
end
end

Expand All @@ -26,29 +27,29 @@ defmodule AsciinemaTest do
end

test "existing user, by email" do
user = insert(:user)
insert(:user, email: "[email protected]")

assert Asciinema.send_login_email(user.email, true, Routes) == :ok
assert Asciinema.send_login_email("[email protected]", true, Routes) == :ok

assert_enqueued(
worker: Asciinema.Emails.Job,
args: %{"type" => "login", "to" => user.email, "url" => "http://login"}
args: %{"type" => "login", "to" => "[email protected]", "url" => "http://login"}
)
end

test "existing user, by username" do
user = insert(:user)
insert(:user, username: "foobar", email: "[email protected]")

assert Asciinema.send_login_email(user.username, true, Routes) == :ok
assert Asciinema.send_login_email("foobar", true, Routes) == :ok

assert_enqueued(
worker: Asciinema.Emails.Job,
args: %{"type" => "login", "to" => user.email, "url" => "http://login"}
args: %{"type" => "login", "to" => "[email protected]", "url" => "http://login"}
)
end

test "non-existing user, by email" do
assert Asciinema.send_login_email("[email protected]", true, Routes) ==
assert Asciinema.send_login_email("[email protected]", true, Routes) ==
:ok

assert_enqueued(
Expand Down
12 changes: 12 additions & 0 deletions test/controllers/login_controller_test.exs
Original file line number Diff line number Diff line change
@@ -1,29 +1,41 @@
defmodule Asciinema.LoginControllerTest do
use AsciinemaWeb.ConnCase
use Oban.Testing, repo: Asciinema.Repo

@honeypot_detection_header "x-melliculum"

test "with valid email", %{conn: conn} do
conn = post conn, "/login", %{login: %{email: "[email protected]", username: ""}}
assert redirected_to(conn, 302) == "/login/sent"
assert get_resp_header(conn, @honeypot_detection_header) == []
assert_enqueued(worker: Asciinema.Emails.Job, args: %{"to" => "[email protected]"})
end

test "with valid email (uppercase)", %{conn: conn} do
conn = post conn, "/login", %{login: %{email: "[email protected]", username: ""}}
assert redirected_to(conn, 302) == "/login/sent"
assert get_resp_header(conn, @honeypot_detection_header) == []
assert_enqueued(worker: Asciinema.Emails.Job, args: %{"to" => "[email protected]"})
end

test "with invalid email", %{conn: conn} do
conn = post conn, "/login", %{login: %{email: "new@", username: ""}}
assert html_response(conn, 200) =~ "correct email"
assert get_resp_header(conn, @honeypot_detection_header) == []
refute_enqueued(worker: Asciinema.Emails.Job)
end

test "as bot with username", %{conn: conn} do
conn = post conn, "/login", %{login: %{email: "[email protected]", username: "bot"}}
assert redirected_to(conn, 302) == "/login/sent"
assert List.first(get_resp_header(conn, @honeypot_detection_header)) == "machina"
refute_enqueued(worker: Asciinema.Emails.Job)
end

test "as bot with terms", %{conn: conn} do
conn = post conn, "/login", %{login: %{email: "[email protected]", username: "", terms: "1"}}
assert redirected_to(conn, 302) == "/login/sent"
assert List.first(get_resp_header(conn, @honeypot_detection_header)) == "machina"
refute_enqueued(worker: Asciinema.Emails.Job)
end
end

0 comments on commit c1e9634

Please sign in to comment.