Skip to content

Commit 0761667

Browse files
getsentry-botdashed
andcommitted
Revert "fix(oauth): Add state validation to prevent promo code conflicts (#95742)"
This reverts commit ea60b81. Co-authored-by: dashed <[email protected]>
1 parent 46aea93 commit 0761667

File tree

5 files changed

+11
-1615
lines changed

5 files changed

+11
-1615
lines changed

src/sentry/identity/oauth2.py

Lines changed: 11 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -242,89 +242,25 @@ def get_authorize_params(self, state, redirect_uri):
242242
"redirect_uri": redirect_uri,
243243
}
244244

245-
def _start_new_oauth_flow(
246-
self, request: HttpRequest, pipeline: IdentityPipeline
247-
) -> HttpResponseRedirect:
248-
"""Start a new OAuth flow by generating state, building redirect URL, and binding state."""
249-
state = secrets.token_hex()
250-
params = self.get_authorize_params(
251-
state=state, redirect_uri=absolute_uri(_redirect_url(pipeline))
252-
)
253-
redirect_uri = f"{self.get_authorize_url()}?{urlencode(params)}"
254-
255-
pipeline.bind_state("state", state)
256-
if request.subdomain:
257-
pipeline.bind_state("subdomain", request.subdomain)
258-
259-
return HttpResponseRedirect(redirect_uri)
260-
261245
@method_decorator(csrf_exempt)
262246
def dispatch(self, request: HttpRequest, pipeline: IdentityPipeline) -> HttpResponseBase:
263247
with record_event(IntegrationPipelineViewType.OAUTH_LOGIN, pipeline.provider.key).capture():
264-
# Detect parameter pollution attacks for critical OAuth parameters
265-
code_values = request.GET.getlist("code")
266-
error_values = request.GET.getlist("error")
267-
state_values = request.GET.getlist("state")
268-
269-
# Check for parameter pollution on critical OAuth parameters
270-
pollution_detected = (
271-
len(code_values) > 1 or len(error_values) > 1 or len(state_values) > 1
272-
)
248+
for param in ("code", "error", "state"):
249+
if param in request.GET:
250+
return pipeline.next_step()
273251

274-
if pollution_detected:
275-
logger.warning(
276-
"OAuth parameter pollution attack detected",
277-
extra={
278-
"provider": pipeline.provider.key,
279-
"code_count": len(code_values),
280-
"error_count": len(error_values),
281-
"state_count": len(state_values),
282-
"remote_addr": request.META.get("REMOTE_ADDR"),
283-
"user_agent": request.META.get("HTTP_USER_AGENT"),
284-
"referer": request.META.get("HTTP_REFERER"),
285-
},
286-
)
287-
# Start new OAuth flow to prevent potential parameter pollution attack
288-
return self._start_new_oauth_flow(request, pipeline)
289-
290-
# Safe to get single values now that we've validated no pollution
291-
oauth_code = code_values[0] if code_values else None
292-
oauth_error = error_values[0] if error_values else None
293-
oauth_state = state_values[0] if state_values else None
294-
stored_state = pipeline.fetch_state("state")
295-
296-
# Only treat as OAuth callback if:
297-
# 1. We have a code OR error parameter (but not both) AND
298-
# 2. We have exactly ONE state parameter AND
299-
# 3. The state matches what we stored
300-
is_oauth_callback = (
301-
(oauth_code or oauth_error)
302-
and not (oauth_code and oauth_error) # RFC 6749: MUST NOT have both code and error
303-
and oauth_state
304-
and oauth_state == stored_state
305-
)
252+
state = secrets.token_hex()
306253

307-
# Log OAuth callback detection for monitoring
308-
logger.debug(
309-
"OAuth callback detection",
310-
extra={
311-
"provider": pipeline.provider.key,
312-
"has_code": bool(oauth_code),
313-
"has_error": bool(oauth_error),
314-
"has_state": bool(oauth_state),
315-
"code_count": len(code_values),
316-
"error_count": len(error_values),
317-
"state_count": len(state_values),
318-
"state_matches": oauth_state == stored_state if oauth_state else False,
319-
"is_callback": is_oauth_callback,
320-
},
254+
params = self.get_authorize_params(
255+
state=state, redirect_uri=absolute_uri(_redirect_url(pipeline))
321256
)
257+
redirect_uri = f"{self.get_authorize_url()}?{urlencode(params)}"
322258

323-
if is_oauth_callback:
324-
return pipeline.next_step()
259+
pipeline.bind_state("state", state)
260+
if request.subdomain:
261+
pipeline.bind_state("subdomain", request.subdomain)
325262

326-
# Otherwise, start new OAuth flow
327-
return self._start_new_oauth_flow(request, pipeline)
263+
return HttpResponseRedirect(redirect_uri)
328264

329265

330266
class OAuth2CallbackView:

0 commit comments

Comments
 (0)