Skip to content

Commit 88e759f

Browse files
committed
Huge improvements
* Unblock hashing (tested in Postman) * Reconsider DI scopes * Remove redundant code
1 parent 71c1e23 commit 88e759f

27 files changed

Lines changed: 373 additions & 366 deletions

File tree

config/local/.secrets.toml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,22 @@ PASSWORD = "changethis"
1010
# Recommended: Use a cryptographically secure random generator to create a
1111
# string of at least 32 characters including numbers, letters, and symbols
1212
PEPPER = "REPLACE_THIS_WITH_YOUR_OWN_SECRET_PEPPER_VALUE"
13+
# https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#introduction
14+
HASHER_WORK_FACTOR = 11
15+
# CPU-bound & GIL released: per-worker ≈ max(1, floor(effective vCPUs / workers))
16+
HASHER_MAX_THREADS = 8
1317

1418
[security.auth]
1519
# Recommended: Use a cryptographically secure random generator to create a
1620
# string of at least 32 characters including numbers, letters, and symbols
1721
JWT_SECRET = "REPLACE_THIS_WITH_YOUR_OWN_SECRET_VALUE"
18-
# JWT_ALGORITHM can be set to "HS256", "HS384", "HS512", "RS256", "RS384", "RS512"
22+
# Can be set to "HS256", "HS384", "HS512", "RS256", "RS384", "RS512"
1923
JWT_ALGORITHM = "HS256"
20-
# SESSION_TTL_MIN must be at least 1 (number of minutes)
24+
# Must be at least 1 (number of minutes)
2125
SESSION_TTL_MIN = 5
22-
# SESSION_REFRESH_THRESHOLD must be a number (fraction, 0 < fraction < 1)
26+
# Must be a number (fraction, 0 < fraction < 1)
2327
SESSION_REFRESH_THRESHOLD = 0.2
2428

2529
[security.cookies]
26-
# Secure can be set to 0 or 1
27-
# Choose 1 for production (secure=True, samesite="Strict")
30+
# Can be set to 0 or 1, choose 1 for production (secure=True, samesite="Strict")
2831
SECURE = 0

config/local/config.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@ MAX_OVERFLOW = 10
1919

2020
# Logs
2121
[logs]
22-
# Level can be set to "DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"
22+
# Can be set to "DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"
2323
LEVEL = "DEBUG"

src/app/application/commands/create_user.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ async def execute(self, request_data: CreateUserRequest) -> CreateUserResponse:
8080

8181
username = Username(request_data.username)
8282
password = RawPassword(request_data.password)
83-
user = self._user_service.create_user(username, password, request_data.role)
83+
user = await self._user_service.create_user(
84+
username, password, request_data.role
85+
)
8486

8587
self._user_command_gateway.add(user)
8688

src/app/application/commands/set_user_password.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ async def execute(self, request_data: SetUserPasswordRequest) -> None:
9292
),
9393
)
9494

95-
self._user_service.change_password(user, password)
95+
await self._user_service.change_password(user, password)
9696
await self._transaction_manager.commit()
9797

9898
log.info("Set user password: done. Target user ID: '%s'.", user.id_.value)

src/app/domain/ports/password_hasher.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66

77
class PasswordHasher(Protocol):
88
@abstractmethod
9-
def hash(self, raw_password: RawPassword) -> bytes: ...
9+
async def hash(self, raw_password: RawPassword) -> bytes: ...
1010

1111
@abstractmethod
12-
def verify(self, *, raw_password: RawPassword, hashed_password: bytes) -> bool: ...
12+
async def verify(
13+
self,
14+
raw_password: RawPassword,
15+
hashed_password: bytes,
16+
) -> bool: ...

src/app/domain/services/user.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def __init__(
2222
self._user_id_generator = user_id_generator
2323
self._password_hasher = password_hasher
2424

25-
def create_user(
25+
async def create_user(
2626
self,
2727
username: Username,
2828
raw_password: RawPassword,
@@ -37,7 +37,8 @@ def create_user(
3737
raise RoleAssignmentNotPermittedError(role)
3838

3939
user_id = UserId(self._user_id_generator.generate())
40-
password_hash = UserPasswordHash(self._password_hasher.hash(raw_password))
40+
password_hash_value = await self._password_hasher.hash(raw_password)
41+
password_hash = UserPasswordHash(password_hash_value)
4142
return User(
4243
id_=user_id,
4344
username=username,
@@ -46,14 +47,15 @@ def create_user(
4647
is_active=is_active,
4748
)
4849

49-
def is_password_valid(self, user: User, raw_password: RawPassword) -> bool:
50-
return self._password_hasher.verify(
50+
async def is_password_valid(self, user: User, raw_password: RawPassword) -> bool:
51+
return await self._password_hasher.verify(
5152
raw_password=raw_password,
5253
hashed_password=user.password_hash.value,
5354
)
5455

55-
def change_password(self, user: User, raw_password: RawPassword) -> None:
56-
hashed_password = UserPasswordHash(self._password_hasher.hash(raw_password))
56+
async def change_password(self, user: User, raw_password: RawPassword) -> None:
57+
password_hash_value = await self._password_hasher.hash(raw_password)
58+
hashed_password = UserPasswordHash(password_hash_value)
5759
user.password_hash = hashed_password
5860

5961
def toggle_user_activation(self, user: User, *, is_active: bool) -> bool:

src/app/infrastructure/adapters/password_hasher_bcrypt.py

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,64 @@
1+
import asyncio
12
import base64
23
import hashlib
34
import hmac
4-
from typing import NewType
5+
import logging
56

67
import bcrypt
78

89
from app.domain.ports.password_hasher import PasswordHasher
910
from app.domain.value_objects.raw_password import RawPassword
11+
from app.infrastructure.adapters.types import HasherThreadPoolExecutor
1012

11-
PasswordPepper = NewType("PasswordPepper", str)
13+
log = logging.getLogger(__name__)
1214

1315

1416
class BcryptPasswordHasher(PasswordHasher):
15-
def __init__(self, pepper: PasswordPepper):
17+
def __init__(
18+
self,
19+
pepper: str,
20+
work_factor: int,
21+
executor: HasherThreadPoolExecutor,
22+
):
1623
self._pepper = pepper
24+
self._work_factor = work_factor
25+
self._executor = executor
1726

18-
def hash(self, raw_password: RawPassword) -> bytes:
27+
async def hash(self, raw_password: RawPassword) -> bytes:
28+
loop = asyncio.get_running_loop()
29+
return await loop.run_in_executor(self._executor, self.hash_sync, raw_password)
30+
31+
async def verify(self, raw_password: RawPassword, hashed_password: bytes) -> bool:
32+
loop = asyncio.get_running_loop()
33+
return await loop.run_in_executor(
34+
self._executor,
35+
self.verify_sync,
36+
raw_password,
37+
hashed_password,
38+
)
39+
40+
def hash_sync(self, raw_password: RawPassword) -> bytes:
1941
"""
20-
Bcrypt is limited to 72-character passwords. Adding a pepper may surpass this character count.
21-
To keep the input within the 72-character limit, pre-hashing can be employed.
22-
One option is using HMAC-SHA256, which produces a fixed-length digest of the peppered password.
23-
However, pre-hashing may introduce null bytes, which `bcrypt` cannot process correctly.
24-
This issue can be resolved by applying `base64` encoding to the digest.
25-
The resulting `base64(hmac-sha256(password, pepper))` string is then ready for bcrypt hashing.
26-
Salt is added to this string before passing it to `bcrypt` for the final hashing step.
27-
Inspired by: https://blog.ircmaxell.com/2015/03/security-issue-combining-bcrypt-with.html
42+
Pre-hashing:
43+
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pre-hashing-passwords-with-bcrypt
44+
Work factor:
45+
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#introduction
2846
"""
29-
base64_hmac_password: bytes = self._add_pepper(raw_password, self._pepper)
30-
salt: bytes = bcrypt.gensalt()
31-
return bcrypt.hashpw(base64_hmac_password, salt)
47+
log.debug("hash")
48+
base64_hmac_peppered: bytes = self._add_pepper(raw_password, self._pepper)
49+
salt: bytes = bcrypt.gensalt(rounds=self._work_factor)
50+
return bcrypt.hashpw(base64_hmac_peppered, salt)
51+
52+
def verify_sync(self, raw_password: RawPassword, hashed_password: bytes) -> bool:
53+
log.debug("verify")
54+
base64_hmac_peppered: bytes = self._add_pepper(raw_password, self._pepper)
55+
return bcrypt.checkpw(base64_hmac_peppered, hashed_password)
3256

3357
@staticmethod
34-
def _add_pepper(raw_password: RawPassword, pepper: PasswordPepper) -> bytes:
58+
def _add_pepper(raw_password: RawPassword, pepper: str) -> bytes:
3559
hmac_password: bytes = hmac.new(
3660
key=pepper.encode(),
3761
msg=raw_password.value.encode(),
38-
digestmod=hashlib.sha256,
62+
digestmod=hashlib.sha384,
3963
).digest()
4064
return base64.b64encode(hmac_password)
41-
42-
def verify(self, *, raw_password: RawPassword, hashed_password: bytes) -> bool:
43-
base64_hmac_password: bytes = self._add_pepper(raw_password, self._pepper)
44-
return bcrypt.checkpw(base64_hmac_password, hashed_password)
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
from concurrent.futures import ThreadPoolExecutor
12
from typing import NewType
23

34
from sqlalchemy.ext.asyncio import AsyncSession
45

56
MainAsyncSession = NewType("MainAsyncSession", AsyncSession)
7+
HasherThreadPoolExecutor = NewType("HasherThreadPoolExecutor", ThreadPoolExecutor)

src/app/infrastructure/auth/handlers/change_password.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,13 @@ async def execute(self, request_data: ChangePasswordRequest) -> None:
6262
if current_password == new_password:
6363
raise AuthenticationChangeError(AUTH_PASSWORD_NEW_SAME_AS_CURRENT)
6464

65-
if not self._user_service.is_password_valid(current_user, current_password):
65+
if not await self._user_service.is_password_valid(
66+
current_user,
67+
current_password,
68+
):
6669
raise ReAuthenticationError(AUTH_PASSWORD_INVALID)
6770

68-
self._user_service.change_password(current_user, new_password)
71+
await self._user_service.change_password(current_user, new_password)
6972
await self._transaction_manager.commit()
7073

7174
log.info("Change password: done. User ID: '%s'.", current_user.id_.value)

src/app/infrastructure/auth/handlers/sign_up.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ async def execute(self, request_data: SignUpRequest) -> SignUpResponse:
7474
username = Username(request_data.username)
7575
password = RawPassword(request_data.password)
7676

77-
user = self._user_service.create_user(username, password)
77+
user = await self._user_service.create_user(username, password)
7878

7979
self._user_command_gateway.add(user)
8080

0 commit comments

Comments
 (0)