Skip to content

Commit 7df14e5

Browse files
Fix password reset email flow (#142)
## Summary - Recreate the password reset fixes on top of develop so the diff is based on the correct target branch - Return UserNotFoundException for unknown password-reset email requests - Improve password reset email payload and sender validation - Reduce reset code TTL from 10 minutes to 5 minutes - Address CodeRabbit feedback for commit timing, row locking, timezone-aware datetimes, settings init, and migration defaults ## Tests - .venv\Scripts\python.exe -m pytest wacruit\src\tests\auth\test_auth_password_reset_service.py wacruit\src\tests\mail\test_mail_service.py - 21 passed, 1 warning ## Notes - Previous PR #141 was closed because its branch history was not based on develop. - Local .env.test OCI mail settings were not included.
1 parent 6edc143 commit 7df14e5

11 files changed

Lines changed: 128 additions & 32 deletions

File tree

wacruit/src/apps/auth/exceptions.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def __init__(self):
88

99
class InvalidTokenException(WacruitException):
1010
def __init__(self):
11-
super().__init__(status_code=401, detail="잘못된 토큰입니다.")
11+
super().__init__(status_code=401, detail="유효하지 않은 토큰입니다.")
1212

1313

1414
class EmailConflictException(WacruitException):
@@ -18,14 +18,14 @@ def __init__(self):
1818

1919
class InvalidPasswordResetCodeException(WacruitException):
2020
def __init__(self):
21-
super().__init__(status_code=400, detail="인증 번호가 올바르지 않습니다.")
21+
super().__init__(status_code=400, detail="인증번호가 올바르지 않습니다.")
2222

2323

2424
class ExpiredPasswordResetCodeException(WacruitException):
2525
def __init__(self):
26-
super().__init__(status_code=400, detail="만료된 인증 번호입니다.")
26+
super().__init__(status_code=400, detail="만료된 인증번호입니다.")
2727

2828

2929
class PasswordResetCodeNotVerifiedException(WacruitException):
3030
def __init__(self):
31-
super().__init__(status_code=400, detail="인증 번호 확인이 필요합니다.")
31+
super().__init__(status_code=400, detail="인증번호 확인이 필요합니다.")

wacruit/src/apps/auth/models.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class PasswordResetVerification(DeclarativeBase):
2626
expires_at: Mapped[datetime] = mapped_column(DateTime(timezone=True))
2727
verified_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True))
2828
used_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True))
29-
attempt_count: Mapped[int] = mapped_column(default=0)
29+
attempt_count: Mapped[int] = mapped_column(default=0, server_default="0")
3030
created_at: Mapped[datetime] = mapped_column(
3131
DateTime(timezone=True),
3232
server_default=CURRENT_TIMESTAMP,

wacruit/src/apps/auth/repositories.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from pydantic import EmailStr
66
from sqlalchemy.orm import Session
77

8+
from wacruit.src.apps.auth.exceptions import UserNotFoundException
89
from wacruit.src.apps.auth.models import BlockedToken
910
from wacruit.src.apps.auth.models import PasswordResetVerification
1011
from wacruit.src.apps.user.models import User
@@ -46,6 +47,15 @@ def replace_active_password_reset_for_email(
4647
expires_at: datetime,
4748
) -> PasswordResetVerification:
4849
with self.transaction:
50+
# Lock the user row to serialize password reset requests per email.
51+
locked_user = (
52+
self.session.query(User)
53+
.where(User.email == email)
54+
.with_for_update()
55+
.one_or_none()
56+
)
57+
if locked_user is None:
58+
raise UserNotFoundException()
4959
(
5060
self.session.query(PasswordResetVerification)
5161
.where(
@@ -60,6 +70,9 @@ def replace_active_password_reset_for_email(
6070
self.session.add(verification)
6171
return verification
6272

73+
def commit(self) -> None:
74+
self.session.commit()
75+
6376
def get_latest_password_reset_verification(
6477
self, email: EmailStr
6578
) -> PasswordResetVerification | None:

wacruit/src/apps/auth/services.py

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from datetime import datetime
22
from datetime import timedelta
3+
from datetime import timezone
34
import secrets
45
from typing import Annotated
56

@@ -22,10 +23,20 @@
2223
from wacruit.src.apps.user.models import User
2324

2425
PASSWORD_RESET_CODE_LENGTH = 6
25-
PASSWORD_RESET_CODE_TTL_MINUTES = 10
26+
PASSWORD_RESET_CODE_TTL_MINUTES = 5
2627
PASSWORD_RESET_MAX_ATTEMPTS = 5
2728

2829

30+
def _utc_now() -> datetime:
31+
return datetime.now(timezone.utc)
32+
33+
34+
def _as_utc(value: datetime) -> datetime:
35+
if value.tzinfo is None:
36+
return value.replace(tzinfo=timezone.utc)
37+
return value.astimezone(timezone.utc)
38+
39+
2940
class AuthService:
3041
def __init__(
3142
self,
@@ -90,7 +101,7 @@ def issue_token(self, user_id: int, expiration_hour: int, token_type: str) -> st
90101
header = {"alg": "HS256"}
91102
payload = {
92103
"sub": user_id,
93-
"exp": int((datetime.now() + timedelta(hours=expiration_hour)).timestamp()),
104+
"exp": int((_utc_now() + timedelta(hours=expiration_hour)).timestamp()),
94105
"token_type": token_type,
95106
}
96107

@@ -105,10 +116,10 @@ def check_available_email(self, email: EmailStr) -> bool:
105116
def send_password_reset_email(self, email: EmailStr) -> None:
106117
user = self.auth_repository.get_user_by_email(email)
107118
if user is None:
108-
return
119+
raise UserNotFoundException()
109120

110121
code = self._generate_password_reset_code()
111-
now = datetime.now()
122+
now = _utc_now()
112123
expires_at = now + timedelta(minutes=PASSWORD_RESET_CODE_TTL_MINUTES)
113124

114125
self.auth_repository.replace_active_password_reset_for_email(
@@ -120,11 +131,12 @@ def send_password_reset_email(self, email: EmailStr) -> None:
120131
),
121132
now,
122133
)
134+
self.auth_repository.commit()
123135
self.email_service.send_password_reset_code(str(email), code)
124136

125137
def verify_password_reset_code(self, email: EmailStr, code: str) -> None:
126138
verification = self._get_valid_password_reset_verification(email, code)
127-
verification.verified_at = datetime.now()
139+
verification.verified_at = _utc_now()
128140
self.auth_repository.update_password_reset_verification(verification)
129141

130142
def reset_password(self, email: EmailStr, code: str, new_password: str) -> None:
@@ -140,7 +152,7 @@ def reset_password(self, email: EmailStr, code: str, new_password: str) -> None:
140152
verification.id,
141153
email,
142154
PasswordService.hash_password(new_password),
143-
datetime.now(),
155+
_utc_now(),
144156
)
145157
if not consumed:
146158
raise InvalidPasswordResetCodeException()
@@ -161,8 +173,8 @@ def _get_valid_password_reset_verification(
161173
if verification is None or verification.used_at is not None:
162174
raise InvalidPasswordResetCodeException()
163175

164-
now = datetime.now()
165-
if verification.expires_at < now:
176+
now = _utc_now()
177+
if _as_utc(verification.expires_at) < now:
166178
raise ExpiredPasswordResetCodeException()
167179

168180
if verification.attempt_count >= PASSWORD_RESET_MAX_ATTEMPTS:

wacruit/src/apps/auth/views.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@ def check_available_email(
6060
@v3_router.post(
6161
"/password-reset/email",
6262
status_code=200,
63-
responses=responses_from(MailConfigException, MailSendFailedException),
63+
responses=responses_from(
64+
MailConfigException,
65+
MailSendFailedException,
66+
UserNotFoundException,
67+
),
6468
)
6569
def send_password_reset_email(
6670
req: PasswordResetEmailRequest,

wacruit/src/apps/mail/config.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ class Config(BaseSettings.Config):
2525
env_prefix = "EMAIL_"
2626
env_file = settings.env_files
2727

28-
def __init__(self):
29-
super().__init__()
28+
def __init__(self, **values):
29+
super().__init__(**values)
3030
secret_manager = OCISecretManager()
3131
if secret_manager.is_available():
3232
self._load_from_vault(secret_manager)

wacruit/src/apps/mail/services.py

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from html import escape
12
from typing import Optional
23
import uuid
34

@@ -22,15 +23,31 @@ def __init__(self) -> None:
2223
self._client: Optional[EmailDPClient] = None
2324

2425
def send_password_reset_code(self, to_email: str, code: str) -> None:
25-
subject = "[Waffle Studio] 비밀번호 재설정 인증 번호"
26+
subject = "[Waffle Studio] 비밀번호 재설정 인증번호"
2627
content = (
27-
"비밀번호 재설정을 위한 인증 번호입니다.\n\n"
28-
f"인증 번호: {code}\n\n"
29-
"인증 번호는 10분 동안 유효합니다."
28+
"비밀번호 재설정을 위한 인증번호입니다.\n\n"
29+
f"인증번호: {code}\n\n"
30+
"인증번호는 5분 동안 유효합니다."
31+
)
32+
html_content = (
33+
"<p>비밀번호 재설정을 위한 인증번호입니다.</p>"
34+
f"<p><strong>인증번호: {escape(code)}</strong></p>"
35+
"<p>인증번호는 5분 동안 유효합니다.</p>"
36+
)
37+
self.send_email(
38+
to_email=to_email,
39+
subject=subject,
40+
content=content,
41+
html_content=html_content,
3042
)
31-
self.send_email(to_email=to_email, subject=subject, content=content)
3243

33-
def send_email(self, to_email: str, subject: str, content: str) -> None:
44+
def send_email(
45+
self,
46+
to_email: str,
47+
subject: str,
48+
content: str,
49+
html_content: str | None = None,
50+
) -> None:
3451
if not mail_config.compartment_id or not mail_config.from_email:
3552
raise MailConfigException()
3653
if not self._is_valid_from_email(mail_config.from_email):
@@ -51,6 +68,7 @@ def send_email(self, to_email: str, subject: str, content: str) -> None:
5168
recipients=Recipients(to=[EmailAddress(email=to_email)]),
5269
subject=subject,
5370
body_text=content,
71+
body_html=html_content,
5472
reply_to=(
5573
[EmailAddress(email=mail_config.reply_to)]
5674
if mail_config.reply_to
@@ -78,6 +96,8 @@ def _get_client(self) -> EmailDPClient:
7896
try:
7997
signer = oci.auth.signers.InstancePrincipalsSecurityTokenSigner()
8098
except Exception: # noqa: PLW0718
99+
if not (settings.is_local or settings.is_test):
100+
raise
81101
config = oci.config.from_file()
82102
self._client = EmailDPClient(config, **client_kwargs)
83103
else:
@@ -91,8 +111,12 @@ def _get_client(self) -> EmailDPClient:
91111

92112
def _generate_message_id(self) -> str:
93113
domain = mail_config.from_email.rsplit("@", 1)[-1]
94-
return f"<{uuid.uuid4()}@{domain}>"
114+
return f"{uuid.uuid4()}@{domain}"
95115

96116
def _is_valid_from_email(self, email: str) -> bool:
117+
if email != email.strip() or any(char.isspace() for char in email):
118+
return False
119+
if email.count("@") != 1:
120+
return False
97121
local_part, separator, domain = email.partition("@")
98122
return bool(local_part and separator and domain and "@" not in domain)

wacruit/src/database/migrations/versions/2026_05_17_0000-2f9c8e0a1b34_add_password_reset_verification.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ def upgrade() -> None:
2424
sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False),
2525
sa.Column("verified_at", sa.DateTime(timezone=True), nullable=True),
2626
sa.Column("used_at", sa.DateTime(timezone=True), nullable=True),
27-
sa.Column("attempt_count", sa.Integer(), nullable=False),
27+
sa.Column(
28+
"attempt_count",
29+
sa.Integer(),
30+
server_default=sa.text("0"),
31+
nullable=False,
32+
),
2833
sa.Column(
2934
"created_at",
3035
sa.DateTime(timezone=True),

wacruit/src/tests/auth/conftest.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from datetime import datetime
2+
from datetime import timezone
23

34
from pydantic import EmailStr
45
import pytest
@@ -13,6 +14,7 @@ class FakeAuthRepository:
1314
def __init__(self) -> None:
1415
self.users: dict[str, User] = {}
1516
self.verifications: list[PasswordResetVerification] = []
17+
self.commit_count = 0
1618

1719
def get_user_by_email(self, email: EmailStr) -> User | None:
1820
return self.users.get(str(email))
@@ -28,11 +30,14 @@ def create_password_reset_verification(
2830
self, verification: PasswordResetVerification
2931
) -> PasswordResetVerification:
3032
verification.id = len(self.verifications) + 1
31-
verification.created_at = datetime.now()
33+
verification.created_at = datetime.now(timezone.utc)
3234
verification.attempt_count = verification.attempt_count or 0
3335
self.verifications.append(verification)
3436
return verification
3537

38+
def commit(self) -> None:
39+
self.commit_count += 1
40+
3641
def replace_active_password_reset_for_email(
3742
self,
3843
email: EmailStr,

wacruit/src/tests/auth/test_auth_password_reset_service.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
from datetime import datetime
22
from datetime import timedelta
3+
from datetime import timezone
34

45
from pydantic import EmailStr
56
import pytest
67

78
from wacruit.src.apps.auth.exceptions import ExpiredPasswordResetCodeException
89
from wacruit.src.apps.auth.exceptions import InvalidPasswordResetCodeException
910
from wacruit.src.apps.auth.exceptions import PasswordResetCodeNotVerifiedException
11+
from wacruit.src.apps.auth.exceptions import UserNotFoundException
1012
from wacruit.src.apps.auth.models import PasswordResetVerification
1113
from wacruit.src.apps.auth.services import AuthService
1214
from wacruit.src.apps.common.security import PasswordService
@@ -30,6 +32,10 @@ def test_send_password_reset_email_creates_verification(
3032
)
3133
assert verification is not None
3234
assert PasswordService.verify_password("123456", verification.code_hash)
35+
now = datetime.now(timezone.utc)
36+
assert now + timedelta(minutes=4, seconds=50) < verification.expires_at
37+
assert verification.expires_at < now + timedelta(minutes=5, seconds=10)
38+
assert auth_repository.commit_count == 1
3339
assert fake_email_service.sent_password_reset_codes == [(user.email, "123456")]
3440

3541

@@ -53,7 +59,7 @@ def test_send_password_reset_email_replaces_active_verification(
5359
EmailStr(user.email)
5460
)
5561
assert first_verification is not None
56-
assert first_verification.expires_at <= datetime.now()
62+
assert first_verification.expires_at <= datetime.now(timezone.utc)
5763
assert latest_verification is not None
5864
assert latest_verification != first_verification
5965
assert PasswordService.verify_password("654321", latest_verification.code_hash)
@@ -63,14 +69,15 @@ def test_send_password_reset_email_replaces_active_verification(
6369
]
6470

6571

66-
def test_send_password_reset_email_ignores_unknown_email(
72+
def test_send_password_reset_email_rejects_unknown_email(
6773
auth_service: AuthService,
6874
auth_repository: FakeAuthRepository,
6975
fake_email_service: FakeEmailService,
7076
):
7177
email = EmailStr("unknown@email.com")
7278

73-
auth_service.send_password_reset_email(email)
79+
with pytest.raises(UserNotFoundException):
80+
auth_service.send_password_reset_email(email)
7481

7582
assert auth_repository.get_latest_password_reset_verification(email) is None
7683
assert fake_email_service.sent_password_reset_codes == []
@@ -119,7 +126,7 @@ def test_verify_password_reset_code_rejects_expired_code(
119126
verification = PasswordResetVerification(
120127
email=user.email,
121128
code_hash=PasswordService.hash_password("123456"),
122-
expires_at=datetime.now() - timedelta(minutes=1),
129+
expires_at=datetime.now(timezone.utc) - timedelta(minutes=1),
123130
)
124131
auth_repository.create_password_reset_verification(verification)
125132

@@ -180,7 +187,7 @@ def use_verification_before_consume(
180187
) -> bool:
181188
verification = auth_repository.get_latest_password_reset_verification(email)
182189
assert verification is not None
183-
verification.used_at = datetime.now()
190+
verification.used_at = datetime.now(timezone.utc)
184191
return consume_password_reset_verification(
185192
verification_id,
186193
email,

0 commit comments

Comments
 (0)