Skip to content

Commit 836a149

Browse files
committed
fix(oauth): Address code review feedback
- Extract magic numbers into named constants (DEVICE_CODE_BYTES, USER_CODE_GROUP_LENGTH) - Add user_code to ApiDeviceCode.__str__ for better debugging - Add "You can now close this tab" UX message to completion page - Use constants in _normalize_user_code instead of hardcoded values
1 parent 2585a55 commit 836a149

File tree

3 files changed

+17
-6
lines changed

3 files changed

+17
-6
lines changed

src/sentry/models/apidevicecode.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
# Reference: RFC 8628 §5.1
2626
USER_CODE_ALPHABET = "BCDFGHJKLMNPQRSTVWXZ"
2727
USER_CODE_LENGTH = 8
28+
USER_CODE_GROUP_LENGTH = USER_CODE_LENGTH // 2 # Characters per group in "XXXX-XXXX" format
29+
30+
# Device code entropy: 32 bytes = 256 bits
31+
DEVICE_CODE_BYTES = 32
2832

2933

3034
def default_expiration():
@@ -33,7 +37,7 @@ def default_expiration():
3337

3438
def generate_device_code():
3539
"""Generate a cryptographically secure device code (256-bit entropy)."""
36-
return secrets.token_hex(nbytes=32)
40+
return secrets.token_hex(nbytes=DEVICE_CODE_BYTES)
3741

3842

3943
def generate_user_code():
@@ -45,7 +49,7 @@ def generate_user_code():
4549
Reference: RFC 8628 §5.1
4650
"""
4751
chars = [secrets.choice(USER_CODE_ALPHABET) for _ in range(USER_CODE_LENGTH)]
48-
return f"{''.join(chars[:4])}-{''.join(chars[4:])}"
52+
return f"{''.join(chars[:USER_CODE_GROUP_LENGTH])}-{''.join(chars[USER_CODE_GROUP_LENGTH:])}"
4953

5054

5155
# Maximum retries for generating unique codes
@@ -127,7 +131,7 @@ class Meta:
127131
db_table = "sentry_apidevicecode"
128132

129133
def __str__(self) -> str:
130-
return f"device_code={self.id}, application={self.application.id}, status={self.status}"
134+
return f"id={self.id}, user_code={self.user_code}, application={self.application_id}, status={self.status}"
131135

132136
def get_scopes(self) -> list[str]:
133137
"""Return the list of requested scopes."""

src/sentry/templates/sentry/oauth-device-complete.html

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
<div class="alert alert-success">
99
<h4>Success!</h4>
1010
<p>{{ message }}</p>
11+
<p style="margin-top: 1em;">You can now close this tab and return to your device.</p>
1112
</div>
1213
{% else %}
1314
<div class="alert alert-info">
1415
<p>{{ message }}</p>
16+
<p style="margin-top: 1em;">You can now close this tab.</p>
1517
</div>
1618
{% endif %}
1719

src/sentry/web/frontend/oauth_device.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@
1010

1111
from sentry.models.apiapplication import ApiApplicationStatus
1212
from sentry.models.apiauthorization import ApiAuthorization
13-
from sentry.models.apidevicecode import ApiDeviceCode, DeviceCodeStatus
13+
from sentry.models.apidevicecode import (
14+
USER_CODE_GROUP_LENGTH,
15+
USER_CODE_LENGTH,
16+
ApiDeviceCode,
17+
DeviceCodeStatus,
18+
)
1419
from sentry.ratelimits import backend as ratelimiter
1520
from sentry.users.services.user.service import user_service
1621
from sentry.utils import metrics
@@ -46,8 +51,8 @@ def _normalize_user_code(user_code: str) -> str:
4651
Handles case variations, missing dashes, and extra whitespace.
4752
"""
4853
normalized = user_code.replace("-", "").upper().strip()
49-
if len(normalized) == 8:
50-
return f"{normalized[:4]}-{normalized[4:]}"
54+
if len(normalized) == USER_CODE_LENGTH:
55+
return f"{normalized[:USER_CODE_GROUP_LENGTH]}-{normalized[USER_CODE_GROUP_LENGTH:]}"
5156
return user_code.upper().strip()
5257

5358

0 commit comments

Comments
 (0)