Skip to content

Commit 22a57ad

Browse files
committed
use option to control rollout rate
1 parent 45aecd6 commit 22a57ad

File tree

3 files changed

+62
-10
lines changed

3 files changed

+62
-10
lines changed

src/sentry/api/authentication.py

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

33
import hashlib
4+
import random
45
from collections.abc import Callable, Iterable
56
from typing import Any, ClassVar
67

@@ -307,6 +308,17 @@ def authenticate(self, request: Request):
307308
return self.transform_auth(user_id, None)
308309

309310

311+
class TokenStrLookupRequired(Exception):
312+
"""
313+
Used in combination with `apitoken.use-and-update-hash-rate` option.
314+
315+
If raised, calling code should peform API token lookups based on its
316+
plaintext value and not its hashed value.
317+
"""
318+
319+
pass
320+
321+
310322
@AuthenticationSiloLimit(SiloMode.REGION, SiloMode.CONTROL)
311323
class UserAuthTokenAuthentication(StandardAuthentication):
312324
token_name = b"bearer"
@@ -328,11 +340,17 @@ def _find_or_update_token_by_hash(self, token_str: str) -> ApiToken | ApiTokenRe
328340

329341
hashed_token = hashlib.sha256(token_str.encode()).hexdigest()
330342

343+
rate = options.get("apitoken.use-and-update-hash-rate")
344+
random_rate = random.random()
345+
331346
if SiloMode.get_current_mode() == SiloMode.REGION:
332347
try:
333-
# Try to find the token by its hashed value first
334-
return ApiTokenReplica.objects.get(hashed_token=hashed_token)
335-
except ApiTokenReplica.DoesNotExist:
348+
if rate > random_rate:
349+
# Try to find the token by its hashed value first
350+
return ApiTokenReplica.objects.get(hashed_token=hashed_token)
351+
else:
352+
raise TokenStrLookupRequired
353+
except (ApiTokenReplica.DoesNotExist, TokenStrLookupRequired):
336354
try:
337355
# If we can't find it by hash, use the plaintext string
338356
return ApiTokenReplica.objects.get(token=token_str)
@@ -342,10 +360,13 @@ def _find_or_update_token_by_hash(self, token_str: str) -> ApiToken | ApiTokenRe
342360
else:
343361
try:
344362
# Try to find the token by its hashed value first
345-
return ApiToken.objects.select_related("user", "application").get(
346-
hashed_token=hashed_token
347-
)
348-
except ApiToken.DoesNotExist:
363+
if rate > random_rate:
364+
return ApiToken.objects.select_related("user", "application").get(
365+
hashed_token=hashed_token
366+
)
367+
else:
368+
raise TokenStrLookupRequired
369+
except (ApiToken.DoesNotExist, TokenStrLookupRequired):
349370
try:
350371
# If we can't find it by hash, use the plaintext string
351372
api_token = ApiToken.objects.select_related("user", "application").get(
@@ -355,9 +376,11 @@ def _find_or_update_token_by_hash(self, token_str: str) -> ApiToken | ApiTokenRe
355376
# If the token does not exist by plaintext either, it is not a valid token
356377
raise AuthenticationFailed("Invalid token")
357378
else:
358-
# Update it with the hashed value if found by plaintext
359-
api_token.hashed_token = hashed_token
360-
api_token.save(update_fields=["hashed_token"])
379+
if rate > random_rate:
380+
# Update it with the hashed value if found by plaintext
381+
api_token.hashed_token = hashed_token
382+
api_token.save(update_fields=["hashed_token"])
383+
361384
return api_token
362385

363386
def accepts_auth(self, auth: list[bytes]) -> bool:

src/sentry/options/defaults.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,14 @@
301301
flags=FLAG_ALLOW_EMPTY | FLAG_PRIORITIZE_DISK | FLAG_AUTOMATOR_MODIFIABLE,
302302
)
303303

304+
# Controls the rate of using the hashed value of User API tokens for lookups when logging in
305+
# and also updates tokens which are not hashed
306+
register(
307+
"apitoken.use-and-update-hash-rate",
308+
default=0.0,
309+
flags=FLAG_AUTOMATOR_MODIFIABLE,
310+
)
311+
304312
register(
305313
"api.rate-limit.org-create",
306314
default=5,

tests/sentry/api/test_authentication.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ def test_no_match(self):
207207
self.auth.authenticate(request)
208208

209209
@override_options({"apitoken.save-hash-on-create": False})
210+
@override_options({"apitoken.use-and-update-hash-rate", 1.0})
210211
def test_token_hashed_with_option_off(self):
211212
# see https://github.com/getsentry/sentry/pull/65941
212213
# the UserAuthTokenAuthentication middleware was updated to hash tokens as
@@ -228,6 +229,25 @@ def test_token_hashed_with_option_off(self):
228229
api_token.refresh_from_db()
229230
assert api_token.hashed_token == expected_hash
230231

232+
@override_options({"apitoken.save-hash-on-create": False})
233+
@override_options({"apitoken.use-and-update-hash-rate", 0.0})
234+
def test_token_not_hashed_with_0_rate(self):
235+
api_token = ApiToken.objects.create(user=self.user, token_type=AuthTokenType.USER)
236+
237+
# we haven't authenticated to the API endpoint yet, so this value should be empty
238+
assert api_token.hashed_token is None
239+
240+
request = HttpRequest()
241+
request.META["HTTP_AUTHORIZATION"] = f"Bearer {api_token.token}"
242+
243+
# trigger the authentication middleware
244+
result = self.auth.authenticate(request)
245+
assert result is not None
246+
247+
# check for the expected hash value
248+
api_token.refresh_from_db()
249+
assert api_token.hashed_token is None
250+
231251

232252
@no_silo_test
233253
class TestTokenAuthenticationReplication(TestCase):
@@ -237,6 +257,7 @@ def setUp(self):
237257
self.auth = UserAuthTokenAuthentication()
238258

239259
@override_options({"apitoken.save-hash-on-create": False})
260+
@override_options({"apitoken.use-and-update-hash-rate", 1.0})
240261
def test_hash_is_replicated(self):
241262
api_token = ApiToken.objects.create(user=self.user, token_type=AuthTokenType.USER)
242263
expected_hash = hashlib.sha256(api_token.token.encode()).hexdigest()

0 commit comments

Comments
 (0)