Skip to content

feat(sentry-apps): hide clientSecret in 1 day after creation #69289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 5 additions & 1 deletion src/sentry/api/serializers/models/sentry_app.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
from collections.abc import Mapping
from datetime import timedelta
from typing import Any

from django.utils import timezone

from sentry.api.serializers import Serializer, register, serialize
from sentry.app import env
from sentry.auth.staff import is_active_staff
Expand Down Expand Up @@ -91,13 +94,14 @@ def serialize(self, obj, attrs, user, access):
is_active_superuser(env.request) or is_active_staff(env.request)
)
if elevated_user or owner.id in user_org_ids:
has_secret = obj.date_added > timezone.now() - timedelta(days=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think show_secret might be a better name for this, because that's what this is helping determine and holding the value of right?

client_secret = (
obj.application.client_secret if obj.show_auth_info(access) else MASKED_VALUE
)
data.update(
{
"clientId": obj.application.client_id,
"clientSecret": client_secret,
"clientSecret": client_secret if has_secret else None,
"owner": {"id": owner.id, "slug": owner.slug},
}
)
Expand Down
23 changes: 22 additions & 1 deletion tests/sentry/api/serializers/test_sentry_app.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
from datetime import datetime, timedelta

from sentry.api.serializers import serialize
from sentry.api.serializers.models.sentry_app import SentryAppSerializer
from sentry.auth import access
from sentry.models.avatars.sentry_app_avatar import SentryAppAvatar
from sentry.testutils.cases import TestCase
from sentry.testutils.silo import control_silo_test
from sentry.testutils.helpers.datetime import freeze_time
from sentry.testutils.silo import control_silo_test, no_silo_test


@control_silo_test
Expand Down Expand Up @@ -59,3 +63,20 @@ def test_with_avatar(self):
assert result["avatars"][0]["avatarUuid"] == "abc123"
assert result["avatars"][0]["avatarType"] == "upload"
assert result["avatars"][0]["avatarUrl"] == "http://testserver/sentry-app-avatar/abc123/"


@no_silo_test
class SentryAppHiddenClientSecretSerializerTest(TestCase):
def test_hidden_client_secret(self):
sentry_app = self.create_sentry_app(
name="Tesla App", organization=self.organization, published=True, scopes=("org:write",)
)

acc = access.from_user(self.user, self.organization)
result = serialize(sentry_app, self.user, SentryAppSerializer(), access=acc)
assert result["clientSecret"] is not None

now = datetime.now()
with freeze_time(now + timedelta(hours=25)):
result = serialize(sentry_app, self.user, SentryAppSerializer(), access=acc)
assert result["clientSecret"] is None
Loading