Skip to content

Commit 3ae4a10

Browse files
authored
feat: hash support for user auth token middleware (#65941)
- Update the `UserAuthTokenAuthentication` middleware to support hashed token values. As tokens are used, it will store the appropriate hash value. - Introduce a temporary option, `apitoken.save-hash-on-create`. This will be used in the model logic and in pre and post backfill migration tests in the future.
1 parent bd52d45 commit 3ae4a10

File tree

6 files changed

+138
-20
lines changed

6 files changed

+138
-20
lines changed

src/sentry/api/authentication.py

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import hashlib
34
from collections.abc import Callable, Iterable
45
from typing import Any, ClassVar
56

@@ -298,6 +299,55 @@ def authenticate(self, request: Request):
298299
class UserAuthTokenAuthentication(StandardAuthentication):
299300
token_name = b"bearer"
300301

302+
def _find_or_update_token_by_hash(self, token_str: str) -> ApiToken | ApiTokenReplica:
303+
"""
304+
Find token by hash or update token's hash value if only found via plaintext.
305+
306+
1. Hash provided plaintext token.
307+
2. Perform lookup based on hashed value.
308+
3. If found, return the token.
309+
4. If not found, search for the token based on its plaintext value.
310+
5. If found, update the token's hashed value and return the token.
311+
6. If not found via hash or plaintext value, raise AuthenticationFailed
312+
313+
Returns `ApiTokenReplica` if running in REGION silo or
314+
`ApiToken` if running in CONTROL silo.
315+
"""
316+
317+
hashed_token = hashlib.sha256(token_str.encode()).hexdigest()
318+
319+
if SiloMode.get_current_mode() == SiloMode.REGION:
320+
try:
321+
# Try to find the token by its hashed value first
322+
return ApiTokenReplica.objects.get(hashed_token=hashed_token)
323+
except ApiTokenReplica.DoesNotExist:
324+
try:
325+
# If we can't find it by hash, use the plaintext string
326+
return ApiTokenReplica.objects.get(token=token_str)
327+
except ApiTokenReplica.DoesNotExist:
328+
# If the token does not exist by plaintext either, it is not a valid token
329+
raise AuthenticationFailed("Invalid token")
330+
else:
331+
try:
332+
# Try to find the token by its hashed value first
333+
return ApiToken.objects.select_related("user", "application").get(
334+
hashed_token=hashed_token
335+
)
336+
except ApiToken.DoesNotExist:
337+
try:
338+
# If we can't find it by hash, use the plaintext string
339+
api_token = ApiToken.objects.select_related("user", "application").get(
340+
token=token_str
341+
)
342+
except ApiToken.DoesNotExist:
343+
# If the token does not exist by plaintext either, it is not a valid token
344+
raise AuthenticationFailed("Invalid token")
345+
else:
346+
# Update it with the hashed value if found by plaintext
347+
api_token.hashed_token = hashed_token
348+
api_token.save(update_fields=["hashed_token"])
349+
return api_token
350+
301351
def accepts_auth(self, auth: list[bytes]) -> bool:
302352
if not super().accepts_auth(auth):
303353
return False
@@ -320,26 +370,16 @@ def authenticate_token(self, request: Request, token_str: str) -> tuple[Any, Any
320370
application_is_inactive = False
321371

322372
if not token:
323-
if SiloMode.get_current_mode() == SiloMode.REGION:
324-
try:
325-
atr = token = ApiTokenReplica.objects.get(token=token_str)
326-
except ApiTokenReplica.DoesNotExist:
327-
raise AuthenticationFailed("Invalid token")
328-
user = user_service.get_user(user_id=atr.user_id)
329-
application_is_inactive = not atr.application_is_active
330-
else:
331-
try:
332-
at = token = (
333-
ApiToken.objects.filter(token=token_str)
334-
.select_related("user", "application")
335-
.get()
336-
)
337-
except ApiToken.DoesNotExist:
338-
raise AuthenticationFailed("Invalid token")
339-
user = at.user
373+
token = self._find_or_update_token_by_hash(token_str)
374+
if isinstance(token, ApiTokenReplica): # we're running as a REGION silo
375+
user = user_service.get_user(user_id=token.user_id)
376+
application_is_inactive = not token.application_is_active
377+
else: # the token returned is an ApiToken from the CONTROL silo
378+
user = token.user
340379
application_is_inactive = (
341-
at.application is not None and not at.application.is_active
380+
token.application is not None and not token.application.is_active
342381
)
382+
343383
elif isinstance(token, SystemToken):
344384
user = token.user
345385

@@ -389,9 +429,9 @@ def authenticate_token(self, request: Request, token_str: str) -> tuple[Any, Any
389429
raise AuthenticationFailed("Invalid org token")
390430
else:
391431
try:
392-
token = OrgAuthToken.objects.filter(
432+
token = OrgAuthToken.objects.get(
393433
token_hashed=token_hashed, date_deactivated__isnull=True
394-
).get()
434+
)
395435
except OrgAuthToken.DoesNotExist:
396436
raise AuthenticationFailed("Invalid org token")
397437

src/sentry/options/defaults.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,12 @@
271271
type=Bool,
272272
flags=FLAG_ALLOW_EMPTY | FLAG_PRIORITIZE_DISK | FLAG_AUTOMATOR_MODIFIABLE,
273273
)
274+
register(
275+
"apitoken.save-hash-on-create",
276+
default=True,
277+
type=Bool,
278+
flags=FLAG_ALLOW_EMPTY | FLAG_PRIORITIZE_DISK | FLAG_AUTOMATOR_MODIFIABLE,
279+
)
274280

275281
register(
276282
"api.rate-limit.org-create",

src/sentry/services/hybrid_cloud/auth/model.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class RpcApiToken(RpcModel):
3434
application_id: int | None = None
3535
application_is_active: bool = False
3636
token: str = ""
37+
hashed_token: str | None = None
3738
expires_at: datetime.datetime | None = None
3839
allowed_origins: list[str] = Field(default_factory=list)
3940
scope_list: list[str] = Field(default_factory=list)

src/sentry/services/hybrid_cloud/auth/serial.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ def serialize_api_token(at: ApiToken) -> RpcApiToken:
8787
organization_id=at.organization_id,
8888
application_is_active=at.application_id is None or at.application.is_active,
8989
token=at.token,
90+
hashed_token=at.hashed_token,
9091
expires_at=at.expires_at,
9192
allowed_origins=list(at.get_allowed_origins()),
9293
scope_list=at.get_scopes(),

src/sentry/services/hybrid_cloud/replica/impl.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ def upsert_replicated_api_token(self, *, api_token: RpcApiToken, region_name: st
160160
organization=organization,
161161
application_is_active=api_token.application_is_active,
162162
token=api_token.token,
163+
hashed_token=api_token.hashed_token,
163164
expires_at=api_token.expires_at,
164165
apitoken_id=api_token.id,
165166
scope_list=api_token.scope_list,

tests/sentry/api/test_authentication.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import hashlib
12
import uuid
23
from datetime import UTC, datetime
34

@@ -30,8 +31,11 @@
3031
)
3132
from sentry.silo import SiloMode
3233
from sentry.testutils.cases import TestCase
34+
from sentry.testutils.helpers import override_options
35+
from sentry.testutils.outbox import outbox_runner
3336
from sentry.testutils.pytest.fixtures import django_db_all
3437
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test, no_silo_test
38+
from sentry.types.token import AuthTokenType
3539
from sentry.utils.security.orgauthtoken_token import hash_token
3640

3741

@@ -202,6 +206,71 @@ def test_no_match(self):
202206
with pytest.raises(AuthenticationFailed):
203207
self.auth.authenticate(request)
204208

209+
@override_options({"apitoken.save-hash-on-create": False})
210+
def test_token_hashed_with_option_off(self):
211+
# see https://github.com/getsentry/sentry/pull/65941
212+
# the UserAuthTokenAuthentication middleware was updated to hash tokens as
213+
# they were used, this test verifies the hash
214+
api_token = ApiToken.objects.create(user=self.user, token_type=AuthTokenType.USER)
215+
expected_hash = hashlib.sha256(api_token.token.encode()).hexdigest()
216+
217+
# we haven't authenticated to the API endpoint yet, so this value should be empty
218+
assert api_token.hashed_token is None
219+
220+
request = HttpRequest()
221+
request.META["HTTP_AUTHORIZATION"] = f"Bearer {api_token.token}"
222+
223+
# trigger the authentication middleware, and thus the hashing
224+
result = self.auth.authenticate(request)
225+
assert result is not None
226+
227+
# check for the expected hash value
228+
api_token.refresh_from_db()
229+
assert api_token.hashed_token == expected_hash
230+
231+
232+
@no_silo_test
233+
class TestTokenAuthenticationReplication(TestCase):
234+
def setUp(self):
235+
super().setUp()
236+
237+
self.auth = UserAuthTokenAuthentication()
238+
239+
@override_options({"apitoken.save-hash-on-create": False})
240+
def test_hash_is_replicated(self):
241+
api_token = ApiToken.objects.create(user=self.user, token_type=AuthTokenType.USER)
242+
expected_hash = hashlib.sha256(api_token.token.encode()).hexdigest()
243+
244+
# we haven't authenticated to the API endpoint yet, so this value should be empty
245+
assert api_token.hashed_token is None
246+
247+
request = HttpRequest()
248+
request.META["HTTP_AUTHORIZATION"] = f"Bearer {api_token.token}"
249+
250+
with assume_test_silo_mode(SiloMode.REGION):
251+
with outbox_runner():
252+
# make sure the token was replicated
253+
api_token_replica = ApiTokenReplica.objects.get(apitoken_id=api_token.id)
254+
assert api_token.token == api_token_replica.token
255+
assert (
256+
api_token_replica.hashed_token is None
257+
) # we don't expect to have a hashed value yet
258+
259+
# trigger the authentication middleware, and thus the hashing backfill
260+
result = self.auth.authenticate(request)
261+
assert result is not None
262+
263+
# check for the expected hash value
264+
api_token.refresh_from_db()
265+
assert api_token.hashed_token == expected_hash
266+
267+
# ApiTokenReplica should also be updated
268+
api_token_replica.refresh_from_db()
269+
assert api_token_replica.hashed_token == expected_hash
270+
271+
# just for good measure
272+
assert api_token.hashed_token == api_token_replica.hashed_token
273+
205274

206275
@django_db_all
207276
@pytest.mark.parametrize("internal", [True, False])

0 commit comments

Comments
 (0)