Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion migrations_lockfile.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ releases: 0004_cleanup_failed_safe_deletes

replays: 0007_organizationmember_replay_access

sentry: 1018_encrypt_integration_metadata
sentry: 1019_add_issued_grant_type_to_apitoken

social_auth: 0003_social_auth_json_field

Expand Down
33 changes: 33 additions & 0 deletions src/sentry/migrations/1019_add_issued_grant_type_to_apitoken.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Generated by Django 5.2.8 on 2025-01-15

from django.db import migrations, models

from sentry.new_migrations.migrations import CheckedMigration


class Migration(CheckedMigration):
# This flag is used to mark that a migration shouldn't be automatically run in production.
# This should only be used for operations where it's safe to run the migration after your
# code has deployed. So this should not be used for most operations that alter the schema
# of a table.
# Here are some things that make sense to mark as post deployment:
# - Large data migrations. Typically we want these to be run manually so that they can be
# monitored and not block the deploy for a long period of time while they run.
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
# run this outside deployments so that we don't block them. Note that while adding an index
# is a schema change, it's completely safe to run the operation after the code has deployed.
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment

is_post_deployment = False

dependencies = [
("sentry", "1018_encrypt_integration_metadata"),
]

operations = [
migrations.AddField(
model_name="apitoken",
name="issued_grant_type",
field=models.CharField(max_length=50, null=True, blank=True),
),
]
11 changes: 10 additions & 1 deletion src/sentry/models/apitoken.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,12 @@ class ApiToken(ReplicatedControlModel, HasApiScopes):
hashed_refresh_token = models.CharField(max_length=128, unique=True, null=True)
expires_at = models.DateTimeField(null=True, default=default_expiration)
date_added = models.DateTimeField(default=timezone.now)
# Track how the token was originally issued (e.g., "device_code", "authorization_code").
# Used to determine refresh behavior per RFC 6749 §6:
# - Tokens issued via device_code (public client per RFC 8628 §5.6) can refresh without client_secret
# - Tokens issued via authorization_code (confidential client) MUST provide client_secret for refresh
# Null for tokens created before this field was added (treated as confidential for backward compatibility).
issued_grant_type = models.CharField(max_length=50, null=True, blank=True)

objects: ClassVar[ApiTokenManager] = ApiTokenManager(cache_fields=("token",))

Expand Down Expand Up @@ -439,6 +445,7 @@ def from_grant(
user=grant.user,
scope_list=grant.get_scopes(),
scoping_organization_id=grant.organization_id,
issued_grant_type="authorization_code",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use/expand the existing GrantType enum instead of adding magic strings?

)

# Remove the ApiGrant from the database to prevent reuse of the same
Expand Down Expand Up @@ -580,7 +587,9 @@ def default_flush(self, value: bool) -> None:
self._default_flush = value


def is_api_token_auth(auth: object) -> TypeGuard[AuthenticatedToken | ApiToken | ApiTokenReplica]:
def is_api_token_auth(
auth: object,
) -> TypeGuard[AuthenticatedToken | ApiToken | ApiTokenReplica]:
""":returns True when an API token is hitting the API."""
from sentry.auth.services.auth import AuthenticatedToken
from sentry.hybridcloud.models.apitokenreplica import ApiTokenReplica
Expand Down
129 changes: 92 additions & 37 deletions src/sentry/web/frontend/oauth_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,44 +147,19 @@
},
)

# Device flow supports public clients per RFC 8628 §5.6.
# Device flow and refresh token support public clients per RFC 8628 §5.6.
# Public clients only provide client_id to identify themselves.
# If client_secret is provided, we still validate it for confidential clients.
if grant_type == GrantTypes.DEVICE_CODE:
if not client_id:
return self.error(
request=request,
name="invalid_client",
reason="missing client_id",
status=401,
)

# Build query - validate secret only if provided (confidential client)
query = {"client_id": client_id}
if client_secret:
query["client_secret"] = client_secret

try:
application = ApiApplication.objects.get(**query)
except ApiApplication.DoesNotExist:
metrics.incr("oauth_token.post.invalid", sample_rate=1.0)
if client_secret:
logger.warning(
"Invalid client_id / secret pair",
extra={"client_id": client_id},
)
reason = "invalid client_id or client_secret"
else:
logger.warning("Invalid client_id", extra={"client_id": client_id})
reason = "invalid client_id"
return self.error(
request=request,
name="invalid_client",
reason=reason,
status=401,
)
if grant_type in (GrantTypes.DEVICE_CODE, GrantTypes.REFRESH):
application, error = self._authenticate_public_client(request, client_id, client_secret)
if error:
return error
assert application is not None # Type narrowing: error check above guarantees this
# For refresh_token, we need to verify the client authentication matches
# how the token was issued. This check happens in get_refresh_token().
# Pass whether client_secret was provided so we can enforce RFC 6749 §6.
else:
# Other grant types require confidential client authentication
# authorization_code grant requires confidential client authentication
if not client_id or not client_secret:
return self.error(
request=request,
Expand Down Expand Up @@ -273,7 +248,11 @@
elif grant_type == GrantTypes.DEVICE_CODE:
return self.handle_device_code_grant(request=request, application=application)
else:
token_data = self.get_refresh_token(request=request, application=application)
token_data = self.get_refresh_token(
request=request,
application=application,
client_secret_provided=bool(client_secret),
)
if "error" in token_data:
return self.error(
request=request,
Expand Down Expand Up @@ -368,6 +347,61 @@
# Neither header nor body provided credentials.
return (None, None), None

def _authenticate_public_client(
self,
request: Request,
client_id: str | None,
client_secret: str | None,
) -> tuple[ApiApplication | None, HttpResponse | None]:
"""Authenticate a public client for device flow or refresh token grants.
Public clients (per RFC 8628 §5.6) only require client_id to identify
themselves. If client_secret is provided, it will be validated to support
confidential clients using these grant types.
Args:
request: The HTTP request (for error responses)
client_id: The client identifier (required)
client_secret: Optional client secret (validated if provided)
Returns:
(application, None) on success
(None, error_response) on failure
"""
if not client_id:
return None, self.error(
request=request,
name="invalid_client",
reason="missing client_id",
status=401,
)

# Build query - validate secret only if provided (confidential client)
query: dict[str, str] = {"client_id": client_id}
if client_secret:
query["client_secret"] = client_secret

try:
application = ApiApplication.objects.get(**query)
return application, None
except ApiApplication.DoesNotExist:
metrics.incr("oauth_token.post.invalid", sample_rate=1.0)
if client_secret:
logger.warning(
"Invalid client_id / secret pair",
extra={"client_id": client_id},
)
reason = "invalid client_id or client_secret"
else:
logger.warning("Invalid client_id", extra={"client_id": client_id})
reason = "invalid client_id"
return None, self.error(
request=request,
name="invalid_client",
reason=reason,
status=401,
)

def get_access_tokens(self, request: Request, application: ApiApplication) -> dict:
code = request.POST.get("code")
try:
Expand Down Expand Up @@ -418,7 +452,12 @@

return token_data

def get_refresh_token(self, request: Request, application: ApiApplication) -> dict:
def get_refresh_token(
self,
request: Request,
application: ApiApplication,
client_secret_provided: bool = True,
) -> dict:
refresh_token_code = request.POST.get("refresh_token")
scope = request.POST.get("scope")

Expand All @@ -435,6 +474,21 @@
)
except ApiToken.DoesNotExist:
return {"error": "invalid_grant", "reason": "invalid request"}

# RFC 6749 §6: "If the client type is confidential or the client was issued
# client credentials, the client MUST authenticate."
#
# Per RFC 8628 §5.6, device flow clients are public clients.
# Tokens issued via device_code can be refreshed without client_secret.
# All other tokens (authorization_code, or legacy null) require client_secret.
if not client_secret_provided:
if refresh_token.issued_grant_type != "device_code":
# Confidential client token - client_secret is required
return {
"error": "invalid_client",
"reason": "client_secret required for this token",
}

refresh_token.refresh()

return {"token": refresh_token}
Expand Down Expand Up @@ -587,6 +641,7 @@
user_id=device_code.user.id,
scope_list=device_code.scope_list,
scoping_organization_id=device_code.organization_id,
issued_grant_type="device_code",
)

# Delete the device code (one-time use)
Expand Down
Loading
Loading