Skip to content

Commit 617ac41

Browse files
committed
fix(oauth): Support public clients for device flow per RFC 8628 §5.6
The OAuth 2.0 Device Authorization Grant (RFC 8628) is designed for headless clients like CLIs, native apps, and IoT devices that cannot securely store credentials. Per RFC 8628 §5.6 (Non-Confidential Clients): > Device clients are generally incapable of maintaining the > confidentiality of their credentials, as users in possession of > the device can reverse-engineer it and extract the credentials. > Therefore, unless additional measures are taken, they should be > treated as public clients. And per RFC 8628 §3.4 (Device Access Token Request): > client_id: REQUIRED if the client is not authenticating with the > authorization server as described in Section 3.2.1. of [RFC6749]. The previous implementation required client_secret for ALL grant types, including device_code. This broke the fundamental design of the device flow, which is meant for public clients that authenticate with only client_id. Changes: - Move grant_type validation before credential check - For device_code grant: only require client_id (public client mode) - If client_secret is provided: still validate it (confidential client) - Other grant types: unchanged (still require client_id + client_secret) This allows CLIs to authenticate without embedding secrets while still supporting confidential clients that choose to provide credentials.
1 parent f6f233f commit 617ac41

File tree

2 files changed

+251
-50
lines changed

2 files changed

+251
-50
lines changed

src/sentry/web/frontend/oauth_token.py

Lines changed: 118 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717
from sentry import options
1818
from sentry.locks import locks
1919
from sentry.models.apiapplication import ApiApplication, ApiApplicationStatus
20-
from sentry.models.apidevicecode import DEFAULT_INTERVAL, ApiDeviceCode, DeviceCodeStatus
20+
from sentry.models.apidevicecode import (
21+
DEFAULT_INTERVAL,
22+
ApiDeviceCode,
23+
DeviceCodeStatus,
24+
)
2125
from sentry.models.apigrant import ApiGrant, ExpiredGrantError, InvalidGrantError
2226
from sentry.models.apitoken import ApiToken
2327
from sentry.ratelimits import backend as ratelimiter
@@ -104,6 +108,8 @@ def post(self, request: Request) -> HttpResponse:
104108
Client authentication
105109
- Either Authorization header (Basic) or form fields `client_id`/`client_secret`
106110
(RFC 6749 §2.3.1). Only one method may be used per request.
111+
- For device_code grant: supports public clients per RFC 8628 §5.6, which only
112+
require `client_id`. If `client_secret` is provided, it will be validated.
107113
108114
Request format
109115
- Requests are `application/x-www-form-urlencoded` as defined in RFC 6749 §3.2.
@@ -120,8 +126,22 @@ def post(self, request: Request) -> HttpResponse:
120126
"""
121127
grant_type = request.POST.get("grant_type")
122128

129+
# Validate grant_type first (needed to determine auth requirements)
130+
if not grant_type:
131+
return self.error(
132+
request=request, name="invalid_request", reason="missing grant_type"
133+
)
134+
if grant_type not in [
135+
GrantTypes.AUTHORIZATION,
136+
GrantTypes.REFRESH,
137+
GrantTypes.DEVICE_CODE,
138+
]:
139+
return self.error(request=request, name="unsupported_grant_type")
140+
123141
# Determine client credentials from header or body (mutually exclusive).
124-
(client_id, client_secret), cred_error = self._extract_basic_auth_credentials(request)
142+
(client_id, client_secret), cred_error = self._extract_basic_auth_credentials(
143+
request
144+
)
125145
if cred_error is not None:
126146
return cred_error
127147

@@ -131,42 +151,78 @@ def post(self, request: Request) -> HttpResponse:
131151
tags={
132152
"client_id_exists": bool(client_id),
133153
"client_secret_exists": bool(client_secret),
154+
"grant_type": grant_type,
134155
},
135156
)
136157

137-
if not client_id or not client_secret:
138-
return self.error(
139-
request=request,
140-
name="invalid_client",
141-
reason="missing client credentials",
142-
status=401,
143-
)
158+
# Device flow supports public clients per RFC 8628 §5.6.
159+
# Public clients only provide client_id to identify themselves.
160+
# If client_secret is provided, we still validate it for confidential clients.
161+
if grant_type == GrantTypes.DEVICE_CODE:
162+
if not client_id:
163+
return self.error(
164+
request=request,
165+
name="invalid_client",
166+
reason="missing client_id",
167+
status=401,
168+
)
144169

145-
if not grant_type:
146-
return self.error(request=request, name="invalid_request", reason="missing grant_type")
147-
if grant_type not in [GrantTypes.AUTHORIZATION, GrantTypes.REFRESH, GrantTypes.DEVICE_CODE]:
148-
return self.error(request=request, name="unsupported_grant_type")
170+
# Build query - validate secret only if provided (confidential client)
171+
query = {"client_id": client_id}
172+
if client_secret:
173+
query["client_secret"] = client_secret
149174

150-
try:
151-
# Note: We don't filter by status here to distinguish between invalid
152-
# credentials (unknown client) and inactive applications. This allows
153-
# proper grant cleanup per RFC 6749 §10.5 and clearer metrics.
154-
application = ApiApplication.objects.get(
155-
client_id=client_id,
156-
client_secret=client_secret,
157-
)
158-
except ApiApplication.DoesNotExist:
159-
metrics.incr(
160-
"oauth_token.post.invalid",
161-
sample_rate=1.0,
162-
)
163-
logger.warning("Invalid client_id / secret pair", extra={"client_id": client_id})
164-
return self.error(
165-
request=request,
166-
name="invalid_client",
167-
reason="invalid client_id or client_secret",
168-
status=401,
169-
)
175+
try:
176+
application = ApiApplication.objects.get(**query)
177+
except ApiApplication.DoesNotExist:
178+
metrics.incr("oauth_token.post.invalid", sample_rate=1.0)
179+
if client_secret:
180+
logger.warning(
181+
"Invalid client_id / secret pair",
182+
extra={"client_id": client_id},
183+
)
184+
reason = "invalid client_id or client_secret"
185+
else:
186+
logger.warning("Invalid client_id", extra={"client_id": client_id})
187+
reason = "invalid client_id"
188+
return self.error(
189+
request=request,
190+
name="invalid_client",
191+
reason=reason,
192+
status=401,
193+
)
194+
else:
195+
# Other grant types require confidential client authentication
196+
if not client_id or not client_secret:
197+
return self.error(
198+
request=request,
199+
name="invalid_client",
200+
reason="missing client credentials",
201+
status=401,
202+
)
203+
204+
try:
205+
# Note: We don't filter by status here to distinguish between invalid
206+
# credentials (unknown client) and inactive applications. This allows
207+
# proper grant cleanup per RFC 6749 §10.5 and clearer metrics.
208+
application = ApiApplication.objects.get(
209+
client_id=client_id,
210+
client_secret=client_secret,
211+
)
212+
except ApiApplication.DoesNotExist:
213+
metrics.incr(
214+
"oauth_token.post.invalid",
215+
sample_rate=1.0,
216+
)
217+
logger.warning(
218+
"Invalid client_id / secret pair", extra={"client_id": client_id}
219+
)
220+
return self.error(
221+
request=request,
222+
name="invalid_client",
223+
reason="invalid client_id or client_secret",
224+
status=401,
225+
)
170226

171227
# Check application status separately from credential validation.
172228
# This preserves metric clarity and provides consistent error handling.
@@ -186,7 +242,9 @@ def post(self, request: Request) -> HttpResponse:
186242
# Use unguarded_write because deleting the grant triggers SET_NULL on
187243
# SentryAppInstallation.api_grant, which is a cross-model write
188244
with unguarded_write(using=router.db_for_write(ApiGrant)):
189-
ApiGrant.objects.filter(application=application, code=code).delete()
245+
ApiGrant.objects.filter(
246+
application=application, code=code
247+
).delete()
190248
# For device_code, invalidate the device code
191249
elif grant_type == GrantTypes.DEVICE_CODE:
192250
device_code_value = request.POST.get("device_code")
@@ -223,11 +281,17 @@ def post(self, request: Request) -> HttpResponse:
223281
)
224282

225283
if grant_type == GrantTypes.AUTHORIZATION:
226-
token_data = self.get_access_tokens(request=request, application=application)
284+
token_data = self.get_access_tokens(
285+
request=request, application=application
286+
)
227287
elif grant_type == GrantTypes.DEVICE_CODE:
228-
return self.handle_device_code_grant(request=request, application=application)
288+
return self.handle_device_code_grant(
289+
request=request, application=application
290+
)
229291
else:
230-
token_data = self.get_refresh_token(request=request, application=application)
292+
token_data = self.get_refresh_token(
293+
request=request, application=application
294+
)
231295
if "error" in token_data:
232296
return self.error(
233297
request=request,
@@ -292,22 +356,28 @@ def _extract_basic_auth_credentials(
292356
# avoid excessive memory use on decode.
293357
b64 = param.strip()
294358
if len(b64) > MAX_BASIC_AUTH_B64_LEN:
295-
logger.warning("Invalid Basic auth header: too long", extra={"client_id": None})
359+
logger.warning(
360+
"Invalid Basic auth header: too long", extra={"client_id": None}
361+
)
296362
return (None, None), self.error(
297363
request=request,
298364
name="invalid_client",
299365
reason="invalid basic auth (too long)",
300366
status=401,
301367
)
302368
try:
303-
decoded = base64.b64decode(b64.encode("ascii"), validate=True).decode("utf-8")
369+
decoded = base64.b64decode(
370+
b64.encode("ascii"), validate=True
371+
).decode("utf-8")
304372
# format: client_id:client_secret (client_secret may be empty)
305373
if ":" not in decoded:
306374
raise ValueError("missing colon in basic credentials")
307375
client_id, client_secret = decoded.split(":", 1)
308376
return (client_id, client_secret), None
309377
except Exception:
310-
logger.warning("Invalid Basic auth header", extra={"client_id": None})
378+
logger.warning(
379+
"Invalid Basic auth header", extra={"client_id": None}
380+
)
311381
return (None, None), self.error(
312382
request=request,
313383
name="invalid_client",
@@ -340,9 +410,15 @@ def get_access_tokens(self, request: Request, application: ApiApplication) -> di
340410
code_verifier=request.POST.get("code_verifier"),
341411
)
342412
except InvalidGrantError as e:
343-
return {"error": "invalid_grant", "reason": str(e) if str(e) else "invalid grant"}
413+
return {
414+
"error": "invalid_grant",
415+
"reason": str(e) if str(e) else "invalid grant",
416+
}
344417
except ExpiredGrantError as e:
345-
return {"error": "invalid_grant", "reason": str(e) if str(e) else "grant expired"}
418+
return {
419+
"error": "invalid_grant",
420+
"reason": str(e) if str(e) else "grant expired",
421+
}
346422

347423
token_data = {"token": api_token}
348424

0 commit comments

Comments
 (0)