add 100% test coverage for Towerops.Accounts
- add 23 new ExUnit tests covering all previously uncovered code paths - fix touch_changeset bug: DateTime.utc_now() -> DateTime.utc_now(:second) to avoid microsecond errors with :utc_datetime fields - add defensive coverage tests using PostgreSQL triggers to exercise Multi transaction catch-all clauses and partial insert_all paths - coverage: 88.67% -> 100.00% (169 -> 192 tests)
This commit is contained in:
parent
41708651a0
commit
3b0487fd75
3 changed files with 514 additions and 1 deletions
|
|
@ -122,7 +122,7 @@ defmodule Towerops.Accounts.BrowserSession do
|
|||
to keep track of when the session was last used.
|
||||
"""
|
||||
def touch_changeset(browser_session) do
|
||||
change(browser_session, %{last_activity_at: DateTime.utc_now()})
|
||||
change(browser_session, %{last_activity_at: DateTime.utc_now(:second)})
|
||||
end
|
||||
|
||||
@doc """
|
||||
|
|
|
|||
116
test/towerops/accounts_defensive_coverage_test.exs
Normal file
116
test/towerops/accounts_defensive_coverage_test.exs
Normal file
|
|
@ -0,0 +1,116 @@
|
|||
defmodule Towerops.AccountsDefensiveCoverageTest do
|
||||
@moduledoc """
|
||||
Tests for defensive code paths in Towerops.Accounts that require
|
||||
database-level manipulation (triggers/constraints) to exercise.
|
||||
|
||||
These tests use raw SQL to create temporary triggers that are
|
||||
rolled back by the Ecto sandbox after each test.
|
||||
"""
|
||||
use Towerops.DataCase
|
||||
|
||||
import Towerops.AccountsFixtures
|
||||
|
||||
alias Towerops.Accounts
|
||||
|
||||
describe "register_user/1 Multi catch-all (L136)" do
|
||||
test "returns error when consent grant fails due to FK violation" do
|
||||
# Create a BEFORE INSERT trigger on user_consents that deletes the user,
|
||||
# causing the FK constraint to fail. The FK is declared in the changeset
|
||||
# so Ecto catches it and returns {:error, changeset}.
|
||||
Repo.query!("""
|
||||
CREATE OR REPLACE FUNCTION test_delete_user_on_consent_insert()
|
||||
RETURNS TRIGGER AS $$
|
||||
BEGIN
|
||||
DELETE FROM users WHERE id = NEW.user_id;
|
||||
RETURN NEW;
|
||||
END;
|
||||
$$ LANGUAGE plpgsql;
|
||||
""")
|
||||
|
||||
Repo.query!("""
|
||||
CREATE TRIGGER test_consent_fk_trigger
|
||||
BEFORE INSERT ON user_consents
|
||||
FOR EACH ROW
|
||||
EXECUTE FUNCTION test_delete_user_on_consent_insert();
|
||||
""")
|
||||
|
||||
attrs = %{
|
||||
"email" => unique_user_email(),
|
||||
"password" => valid_user_password(),
|
||||
"privacy_policy_consent" => "true",
|
||||
"terms_of_service_consent" => "true"
|
||||
}
|
||||
|
||||
assert {:error, changeset} = Accounts.register_user(attrs)
|
||||
assert %Ecto.Changeset{} = changeset
|
||||
end
|
||||
end
|
||||
|
||||
describe "register_user_with_organization/1 Multi catch-all (L177)" do
|
||||
test "returns error when consent grant fails due to FK violation" do
|
||||
Repo.query!("""
|
||||
CREATE OR REPLACE FUNCTION test_delete_user_on_consent_insert()
|
||||
RETURNS TRIGGER AS $$
|
||||
BEGIN
|
||||
DELETE FROM users WHERE id = NEW.user_id;
|
||||
RETURN NEW;
|
||||
END;
|
||||
$$ LANGUAGE plpgsql;
|
||||
""")
|
||||
|
||||
Repo.query!("""
|
||||
CREATE TRIGGER test_consent_fk_trigger
|
||||
BEFORE INSERT ON user_consents
|
||||
FOR EACH ROW
|
||||
EXECUTE FUNCTION test_delete_user_on_consent_insert();
|
||||
""")
|
||||
|
||||
attrs = %{
|
||||
"email" => unique_user_email(),
|
||||
"password" => valid_user_password(),
|
||||
"privacy_policy_consent" => "true",
|
||||
"terms_of_service_consent" => "true",
|
||||
"organization_name" => "Test Org"
|
||||
}
|
||||
|
||||
assert {:error, changeset} = Accounts.register_user_with_organization(attrs)
|
||||
assert %Ecto.Changeset{} = changeset
|
||||
end
|
||||
end
|
||||
|
||||
describe "generate_recovery_codes/1 partial insert (L514)" do
|
||||
test "returns error when insert_all inserts fewer than 12 codes" do
|
||||
user = user_fixture()
|
||||
|
||||
# Create a BEFORE INSERT trigger that skips half the recovery code rows
|
||||
# by returning NULL for even-numbered rows
|
||||
Repo.query!("""
|
||||
CREATE SEQUENCE IF NOT EXISTS test_recovery_counter START 1;
|
||||
""")
|
||||
|
||||
Repo.query!("""
|
||||
CREATE OR REPLACE FUNCTION test_skip_recovery_codes()
|
||||
RETURNS TRIGGER AS $$
|
||||
DECLARE
|
||||
val int;
|
||||
BEGIN
|
||||
val := nextval('test_recovery_counter');
|
||||
IF val % 2 = 0 THEN
|
||||
RETURN NULL;
|
||||
END IF;
|
||||
RETURN NEW;
|
||||
END;
|
||||
$$ LANGUAGE plpgsql;
|
||||
""")
|
||||
|
||||
Repo.query!("""
|
||||
CREATE TRIGGER test_skip_recovery_trigger
|
||||
BEFORE INSERT ON user_recovery_codes
|
||||
FOR EACH ROW
|
||||
EXECUTE FUNCTION test_skip_recovery_codes();
|
||||
""")
|
||||
|
||||
assert {:error, :generation_failed} = Accounts.generate_recovery_codes(user.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -5,7 +5,10 @@ defmodule Towerops.AccountsTest do
|
|||
|
||||
alias Towerops.Accounts
|
||||
alias Towerops.Accounts.User
|
||||
alias Towerops.Accounts.UserRecoveryCode
|
||||
alias Towerops.Accounts.UserToken
|
||||
alias Towerops.GeoIP.Block
|
||||
alias Towerops.GeoIP.Location
|
||||
|
||||
describe "get_user_by_email/1" do
|
||||
test "does not return the user if the email does not exist" do
|
||||
|
|
@ -1889,6 +1892,83 @@ defmodule Towerops.AccountsTest do
|
|||
end
|
||||
end
|
||||
|
||||
describe "deliver_user_confirmation_instructions/2" do
|
||||
test "sends token through notification" do
|
||||
user = unconfirmed_user_fixture()
|
||||
|
||||
token =
|
||||
extract_user_token(fn url ->
|
||||
Accounts.deliver_user_confirmation_instructions(user, url)
|
||||
end)
|
||||
|
||||
{:ok, token} = Base.url_decode64(token, padding: false)
|
||||
assert user_token = Repo.get_by(UserToken, token: :crypto.hash(:sha256, token))
|
||||
assert user_token.user_id == user.id
|
||||
assert user_token.sent_to == user.email
|
||||
assert user_token.context == "confirm"
|
||||
end
|
||||
end
|
||||
|
||||
describe "confirm_user/1" do
|
||||
test "confirms user with valid token" do
|
||||
user = unconfirmed_user_fixture()
|
||||
refute user.confirmed_at
|
||||
|
||||
token =
|
||||
extract_user_token(fn url ->
|
||||
Accounts.deliver_user_confirmation_instructions(user, url)
|
||||
end)
|
||||
|
||||
assert {:ok, confirmed_user} = Accounts.confirm_user(token)
|
||||
assert confirmed_user.confirmed_at
|
||||
assert confirmed_user.id == user.id
|
||||
end
|
||||
|
||||
test "deletes confirmation tokens after confirming" do
|
||||
user = unconfirmed_user_fixture()
|
||||
|
||||
token =
|
||||
extract_user_token(fn url ->
|
||||
Accounts.deliver_user_confirmation_instructions(user, url)
|
||||
end)
|
||||
|
||||
assert {:ok, _} = Accounts.confirm_user(token)
|
||||
# Token should be consumed
|
||||
refute Repo.get_by(UserToken, user_id: user.id, context: "confirm")
|
||||
end
|
||||
|
||||
test "returns error for invalid token" do
|
||||
assert :error = Accounts.confirm_user("invalid-token")
|
||||
end
|
||||
|
||||
test "returns error for expired token" do
|
||||
user = unconfirmed_user_fixture()
|
||||
|
||||
token =
|
||||
extract_user_token(fn url ->
|
||||
Accounts.deliver_user_confirmation_instructions(user, url)
|
||||
end)
|
||||
|
||||
{1, nil} = Repo.update_all(UserToken, set: [inserted_at: ~N[2020-01-01 00:00:00]])
|
||||
assert :error = Accounts.confirm_user(token)
|
||||
end
|
||||
end
|
||||
|
||||
describe "register_user_with_organization/1 error paths" do
|
||||
test "returns error when organization creation fails" do
|
||||
attrs = %{
|
||||
"email" => unique_user_email(),
|
||||
"password" => valid_user_password(),
|
||||
"privacy_policy_consent" => "true",
|
||||
"terms_of_service_consent" => "true",
|
||||
"organization_name" => "A"
|
||||
}
|
||||
|
||||
assert {:error, changeset} = Accounts.register_user_with_organization(attrs)
|
||||
assert "should be at least 2 character(s)" in errors_on(changeset).name
|
||||
end
|
||||
end
|
||||
|
||||
describe "register_user/1 with consent params" do
|
||||
test "grants privacy_policy consent when checkbox is true" do
|
||||
email = unique_user_email()
|
||||
|
|
@ -1941,5 +2021,322 @@ defmodule Towerops.AccountsTest do
|
|||
assert Accounts.has_consent?(user.id, "privacy_policy")
|
||||
assert Accounts.has_consent?(user.id, "terms_of_service")
|
||||
end
|
||||
|
||||
test "grants consent with atom key params" do
|
||||
email = unique_user_email()
|
||||
|
||||
{:ok, user} =
|
||||
Accounts.register_user(%{
|
||||
email: email,
|
||||
password: valid_user_password(),
|
||||
privacy_policy_consent: true,
|
||||
terms_of_service_consent: true
|
||||
})
|
||||
|
||||
consents = Accounts.list_user_consents(user.id)
|
||||
assert length(consents) == 2
|
||||
end
|
||||
end
|
||||
|
||||
describe "login_user_by_magic_link/1 additional cases" do
|
||||
test "confirms unconfirmed user without password and logs them in" do
|
||||
# Create user, set confirmed_at and hashed_password to nil
|
||||
user = unconfirmed_user_fixture()
|
||||
|
||||
user =
|
||||
user
|
||||
|> Ecto.Changeset.change(hashed_password: nil)
|
||||
|> Repo.update!()
|
||||
|
||||
refute user.confirmed_at
|
||||
refute user.hashed_password
|
||||
|
||||
{encoded_token, _hashed_token} = generate_user_magic_link_token(user)
|
||||
|
||||
assert {:ok, {logged_in_user, expired_tokens}} =
|
||||
Accounts.login_user_by_magic_link(encoded_token)
|
||||
|
||||
assert logged_in_user.id == user.id
|
||||
assert logged_in_user.confirmed_at
|
||||
assert is_list(expired_tokens)
|
||||
end
|
||||
|
||||
test "returns error for malformed token that fails verification" do
|
||||
assert {:error, :not_found} = Accounts.login_user_by_magic_link("not-valid-base64!!!")
|
||||
end
|
||||
end
|
||||
|
||||
describe "verify_user_mfa/2 without TOTP" do
|
||||
test "returns totp_not_enabled when user has no TOTP" do
|
||||
user = user_fixture(enable_totp: false)
|
||||
refute Accounts.totp_enabled?(user)
|
||||
|
||||
assert {:error, :totp_not_enabled} = Accounts.verify_user_mfa(user, "123456")
|
||||
end
|
||||
end
|
||||
|
||||
describe "verify_totp/2 failure logging" do
|
||||
test "logs debug info when TOTP verification fails" do
|
||||
import ExUnit.CaptureLog
|
||||
|
||||
secret = Accounts.generate_totp_secret()
|
||||
|
||||
# Temporarily enable info logging to exercise the failure log path
|
||||
original_level = Logger.level()
|
||||
Logger.configure(level: :info)
|
||||
|
||||
log =
|
||||
capture_log(fn ->
|
||||
refute Accounts.verify_totp(secret, "000000")
|
||||
end)
|
||||
|
||||
Logger.configure(level: original_level)
|
||||
|
||||
assert log =~ "TOTP verification failed"
|
||||
assert log =~ "provided=000000"
|
||||
end
|
||||
end
|
||||
|
||||
describe "verify_totp/2 with base32 secret" do
|
||||
test "works with base32-encoded secret" do
|
||||
raw_secret = Accounts.generate_totp_secret()
|
||||
base32_secret = Base.encode32(raw_secret, case: :upper, padding: false)
|
||||
code = NimbleTOTP.verification_code(raw_secret)
|
||||
|
||||
assert Accounts.verify_totp(base32_secret, code)
|
||||
end
|
||||
end
|
||||
|
||||
describe "get_user_token_id_by_value/1 with valid token" do
|
||||
test "returns token id for valid session token" do
|
||||
user = user_fixture()
|
||||
# get_user_token_id_by_value hashes the input with SHA256 before lookup
|
||||
# So we insert a token whose stored value IS the SHA256 hash of a known value
|
||||
known_value = "test_session_token_value"
|
||||
hashed_value = :crypto.hash(:sha256, known_value)
|
||||
|
||||
user_token =
|
||||
Repo.insert!(%UserToken{
|
||||
token: hashed_value,
|
||||
context: "session",
|
||||
user_id: user.id,
|
||||
authenticated_at: DateTime.utc_now(:second)
|
||||
})
|
||||
|
||||
assert Accounts.get_user_token_id_by_value(known_value) == user_token.id
|
||||
end
|
||||
end
|
||||
|
||||
describe "get_browser_session_by_token_value/1 with valid token" do
|
||||
test "returns session for valid token value" do
|
||||
user = user_fixture()
|
||||
# get_browser_session_by_token_value hashes the input with SHA256 before lookup
|
||||
known_value = "test_browser_session_token"
|
||||
hashed_value = :crypto.hash(:sha256, known_value)
|
||||
|
||||
user_token =
|
||||
Repo.insert!(%UserToken{
|
||||
token: hashed_value,
|
||||
context: "session",
|
||||
user_id: user.id,
|
||||
authenticated_at: DateTime.utc_now(:second)
|
||||
})
|
||||
|
||||
{:ok, session} =
|
||||
Accounts.create_browser_session(%{
|
||||
user_id: user.id,
|
||||
user_token_id: user_token.id,
|
||||
ip_address: "10.0.0.1",
|
||||
user_agent: "TestBrowser/1.0",
|
||||
last_activity_at: DateTime.utc_now(:second),
|
||||
expires_at: DateTime.add(DateTime.utc_now(:second), 14, :day)
|
||||
})
|
||||
|
||||
found = Accounts.get_browser_session_by_token_value(known_value)
|
||||
assert found.id == session.id
|
||||
end
|
||||
end
|
||||
|
||||
describe "touch_browser_session/1 via function" do
|
||||
test "updates last_activity_at timestamp" do
|
||||
user = user_fixture()
|
||||
{_token, user_token} = Accounts.generate_user_session_token_with_record(user)
|
||||
past = DateTime.add(DateTime.utc_now(:second), -3600, :second)
|
||||
|
||||
{:ok, session} =
|
||||
Accounts.create_browser_session(%{
|
||||
user_id: user.id,
|
||||
user_token_id: user_token.id,
|
||||
ip_address: "10.0.0.1",
|
||||
user_agent: "TestBrowser/1.0",
|
||||
last_activity_at: past,
|
||||
expires_at: DateTime.add(DateTime.utc_now(:second), 14, :day)
|
||||
})
|
||||
|
||||
assert {:ok, updated} = Accounts.touch_browser_session(session)
|
||||
assert DateTime.after?(updated.last_activity_at, past)
|
||||
end
|
||||
end
|
||||
|
||||
describe "record_login_attempt/1 edge cases" do
|
||||
test "skips GeoIP enrichment when ip_address is nil" do
|
||||
attrs = %{
|
||||
email: "test@example.com",
|
||||
success: true,
|
||||
method: "password",
|
||||
ip_address: nil
|
||||
}
|
||||
|
||||
# ip_address is required, so this returns a changeset error
|
||||
# but the nil ip_address path in record_login_attempt is exercised
|
||||
assert {:error, changeset} = Accounts.record_login_attempt(attrs)
|
||||
assert "can't be blank" in errors_on(changeset).ip_address
|
||||
end
|
||||
|
||||
test "enriches with GeoIP data when available" do
|
||||
user = user_fixture()
|
||||
# Insert test GeoIP data
|
||||
# 1.2.3.4 = 1*16777216 + 2*65536 + 3*256 + 4 = 16909060
|
||||
now = DateTime.utc_now(:second)
|
||||
|
||||
Repo.insert!(%Location{
|
||||
geoname_id: 99_999,
|
||||
country_code: "XX",
|
||||
country_name: "Test Country",
|
||||
city_name: "Test City",
|
||||
subdivision_1_name: "Test State",
|
||||
inserted_at: now,
|
||||
updated_at: now
|
||||
})
|
||||
|
||||
Repo.insert!(%Block{
|
||||
network: "1.2.3.0/24",
|
||||
start_ip_int: 16_909_056,
|
||||
end_ip_int: 16_909_311,
|
||||
geoname_id: 99_999,
|
||||
inserted_at: now,
|
||||
updated_at: now
|
||||
})
|
||||
|
||||
attrs = %{
|
||||
user_id: user.id,
|
||||
email: user.email,
|
||||
success: true,
|
||||
method: "password",
|
||||
ip_address: "1.2.3.4"
|
||||
}
|
||||
|
||||
assert {:ok, attempt} = Accounts.record_login_attempt(attrs)
|
||||
assert attempt.country_code == "XX"
|
||||
assert attempt.country_name == "Test Country"
|
||||
assert attempt.city_name == "Test City"
|
||||
assert attempt.subdivision_1_name == "Test State"
|
||||
end
|
||||
end
|
||||
|
||||
describe "create_browser_session/1 with GeoIP data" do
|
||||
test "enriches session with GeoIP location data" do
|
||||
user = user_fixture()
|
||||
{_token, user_token} = Accounts.generate_user_session_token_with_record(user)
|
||||
now = DateTime.utc_now(:second)
|
||||
|
||||
# Insert test GeoIP data for 2.3.4.5
|
||||
# 2.3.4.0 = 2*16777216 + 3*65536 + 4*256 = 33752064
|
||||
# 2.3.4.255 = 33752064 + 255 = 33752319
|
||||
Repo.insert!(%Location{
|
||||
geoname_id: 88_888,
|
||||
country_code: "YY",
|
||||
country_name: "Another Country",
|
||||
city_name: "Another City",
|
||||
subdivision_1_name: "Another State",
|
||||
inserted_at: now,
|
||||
updated_at: now
|
||||
})
|
||||
|
||||
Repo.insert!(%Block{
|
||||
network: "2.3.4.0/24",
|
||||
start_ip_int: 33_752_064,
|
||||
end_ip_int: 33_752_319,
|
||||
geoname_id: 88_888,
|
||||
inserted_at: now,
|
||||
updated_at: now
|
||||
})
|
||||
|
||||
attrs = %{
|
||||
user_id: user.id,
|
||||
user_token_id: user_token.id,
|
||||
ip_address: "2.3.4.5",
|
||||
user_agent: "TestBrowser/1.0",
|
||||
last_activity_at: now,
|
||||
expires_at: DateTime.add(now, 14, :day)
|
||||
}
|
||||
|
||||
assert {:ok, session} = Accounts.create_browser_session(attrs)
|
||||
assert session.country_code == "YY"
|
||||
assert session.country_name == "Another Country"
|
||||
assert session.city_name == "Another City"
|
||||
assert session.subdivision_1_name == "Another State"
|
||||
end
|
||||
end
|
||||
|
||||
describe "verify_totp_only/2 rejects numeric recovery code match" do
|
||||
test "returns recovery_code_not_allowed when numeric code matches a recovery code" do
|
||||
# Create user with TOTP enabled
|
||||
user = user_fixture(enable_totp: true)
|
||||
assert Accounts.totp_enabled?(user)
|
||||
|
||||
# Manually insert a recovery code whose hash matches the numeric string "123456"
|
||||
# This simulates an unlikely scenario where a recovery code hash collides with a numeric input
|
||||
numeric_code = "123456"
|
||||
code_hash = UserRecoveryCode.hash_code(numeric_code)
|
||||
now = DateTime.utc_now(:second)
|
||||
|
||||
Repo.insert!(%UserRecoveryCode{
|
||||
user_id: user.id,
|
||||
code_hash: code_hash,
|
||||
created_at: now
|
||||
})
|
||||
|
||||
# verify_totp_only sees "123456" as all-numeric, ≤ 6 digits, so passes it to verify_user_mfa
|
||||
# TOTP check fails (wrong code), recovery code check succeeds (hash matches)
|
||||
# verify_user_mfa returns {:ok, user, :recovery_code}
|
||||
# verify_totp_only maps that to {:error, :recovery_code_not_allowed}
|
||||
assert {:error, :recovery_code_not_allowed} = Accounts.verify_totp_only(user, numeric_code)
|
||||
end
|
||||
end
|
||||
|
||||
describe "revoke_browser_session/3 edge cases" do
|
||||
test "returns error when user_token has already been deleted" do
|
||||
user = user_fixture()
|
||||
{_token1, user_token1} = Accounts.generate_user_session_token_with_record(user)
|
||||
{_token2, user_token2} = Accounts.generate_user_session_token_with_record(user)
|
||||
|
||||
{:ok, _session1} =
|
||||
Accounts.create_browser_session(%{
|
||||
user_id: user.id,
|
||||
user_token_id: user_token1.id,
|
||||
ip_address: "10.0.0.1",
|
||||
user_agent: "TestBrowser/1.0",
|
||||
last_activity_at: DateTime.utc_now(:second),
|
||||
expires_at: DateTime.add(DateTime.utc_now(:second), 14, :day)
|
||||
})
|
||||
|
||||
{:ok, session2} =
|
||||
Accounts.create_browser_session(%{
|
||||
user_id: user.id,
|
||||
user_token_id: user_token2.id,
|
||||
ip_address: "10.0.0.2",
|
||||
user_agent: "TestBrowser/2.0",
|
||||
last_activity_at: DateTime.utc_now(:second),
|
||||
expires_at: DateTime.add(DateTime.utc_now(:second), 14, :day)
|
||||
})
|
||||
|
||||
# Delete the user_token directly
|
||||
Repo.delete!(user_token2)
|
||||
|
||||
# Now trying to revoke should return not_found (token gone)
|
||||
assert {:error, :not_found} =
|
||||
Accounts.revoke_browser_session(session2.id, user.id, user_token1.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue