Skip to content
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

ref(feedback): Add enhanced privacy to feedback alerts #12418

Merged
merged 1 commit into from
Mar 18, 2019
Merged
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
4 changes: 4 additions & 0 deletions src/sentry/plugins/sentry_mail/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,9 @@ def handle_user_report(self, payload, project, **kwargs):
if not participants:
return

org = group.organization
enhanced_privacy = org.flags.enhanced_privacy

context = {
'project': project,
'project_link': absolute_uri(u'/{}/{}/'.format(
Expand All @@ -387,6 +390,7 @@ def handle_user_report(self, payload, project, **kwargs):
)),
'group': group,
'report': payload['report'],
'enhanced_privacy': enhanced_privacy,
}

subject_prefix = self.get_option('subject_prefix', project) or self._subject_prefix()
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/templates/sentry/emails/activity/generic.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ <h2>{{ activity_name }}</h2>

{% endblock %}

{% if group %}
{% if group and not enhanced_privacy %}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we hide the group information here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The enhanced privacy setting says it hides PII and source code. Does the stacktrace in the group information count as source code?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, definitely would. Sounds good to hide it here then!

{% include "sentry/emails/group_header.html" %}
{% endif %}

Expand Down
34 changes: 21 additions & 13 deletions src/sentry/templates/sentry/emails/activity/new-user-feedback.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,25 @@
{% block activity %}
<h3>New Feedback from {{ report.name }}</h3>

<table class="note">
<tr>
<td class="avatar-column">
{% email_avatar report.name report.email size 48 %}
</td>
<td class="notch-column">
<img width="7" height="48" src="{% absolute_asset_url 'sentry' 'images/email/avatar-notch.png' %}">
</td>
<td>
<div class="note-body">{{ report.comments|urlize|linebreaks }}</div>
</td>
</tr>
</table>
{% if enhanced_privacy %}
<div class="notice">
Details about this feedback are not shown in this notification since enhanced privacy
controls are enabled. For more details about this feedback, <a href="{{ link }}">view on Sentry</a>.
</div>

{% else %}
<table class="note">
<tr>
<td class="avatar-column">
{% email_avatar report.name report.email size 48 %}
</td>
<td class="notch-column">
<img width="7" height="48" src="{% absolute_asset_url 'sentry' 'images/email/avatar-notch.png' %}">
</td>
<td>
<div class="note-body">{{ report.comments|urlize|linebreaks }}</div>
</td>
</tr>
</table>
{% endif %}
{% endblock %}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@

{{ report.name }} left a new comment:
Copy link
Member

Choose a reason for hiding this comment

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

Is it ok to display the reporter's name if enhanced privacy is enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably our call. The company requesting this said they're fine with the username -- only concerned about the content of the feedback being hidden.

Copy link
Member

Choose a reason for hiding this comment

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

It's probably fine for now then, maybe a product decision.


{% if enhanced_privacy %}
Details about this feedback are not shown in this notification since enhanced privacy
controls are enabled. For more details about this feedback, view on Sentry.

{% else %}
{{ report.comments }}

{% endif %}

## Details

Expand Down
22 changes: 19 additions & 3 deletions src/sentry/web/frontend/debug/debug_new_user_feedback_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@

from django.views.generic import View

from sentry.models import Group, Organization, Project
from sentry.models import Organization, Project

from .mail import MailPreview

from sentry.utils.http import absolute_uri
from sentry.utils.samples import create_sample_event


class DebugNewUserFeedbackEmailView(View):
def get(self, request):
Expand All @@ -20,11 +23,21 @@ def get(self, request):
slug='project',
name='My Project',
)
group = Group(
id=1,

event = create_sample_event(
project=project,
platform='python',
event_id='595',
timestamp=1452683305,
)

group = event.group
link = absolute_uri(u'/{}/{}/issues/{}/feedback/'.format(
project.organization.slug,
project.slug,
group.id,
))

return MailPreview(
html_template='sentry/emails/activity/new-user-feedback.html',
text_template='sentry/emails/activity/new-user-feedback.txt',
Expand All @@ -35,5 +48,8 @@ def get(self, request):
'email': 'homer.simpson@example.com',
'comments': 'I hit a bug.\n\nI went to https://example.com, hit the any key, and then it stopped working. DOH!',
},
'link': link,
'reason': 'are subscribed to this issue',
'enhanced_privacy': False,
},
).render(request)
43 changes: 38 additions & 5 deletions tests/sentry/plugins/mail/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import six
from django.contrib.auth.models import AnonymousUser
from django.core import mail
from django.db.models import F
from django.utils import timezone
from exam import fixture
from mock import Mock
Expand All @@ -20,8 +21,8 @@
from sentry.digests.notifications import build_digest, event_to_record
from sentry.interfaces.stacktrace import Stacktrace
from sentry.models import (
Activity, Event, Group, GroupSubscription, OrganizationMember, OrganizationMemberTeam,
ProjectOwnership, Rule, UserOption, UserReport
Activity, Event, Group, GroupSubscription, Organization, OrganizationMember,
OrganizationMemberTeam, ProjectOwnership, Rule, UserOption, UserReport
)
from sentry.ownership.grammar import Owner, Matcher, dump_schema
from sentry.plugins import Notification
Expand Down Expand Up @@ -424,17 +425,19 @@ class MailPluginSignalsTest(TestCase):
def plugin(self):
return MailPlugin()

def test_user_feedback(self):
def create_report(self):
user_foo = self.create_user('foo@example.com')
self.project.teams.first().organization.member_set.create(user=user_foo)

report = UserReport.objects.create(
return UserReport.objects.create(
project=self.project,
group=self.group,
name='Homer Simpson',
email='homer.simpson@example.com'
)

self.project.teams.first().organization.member_set.create(user=user_foo)
def test_user_feedback(self):
report = self.create_report()

with self.tasks():
self.plugin.handle_signal(
Expand All @@ -446,9 +449,39 @@ def test_user_feedback(self):
)

assert len(mail.outbox) == 1
msg = mail.outbox[0]

# email includes issue metadata
assert 'group-header' in msg.alternatives[0][0]
assert 'enhanced privacy' not in msg.body

assert msg.subject == u'[Sentry] {} - New Feedback from Homer Simpson'.format(
self.group.qualified_short_id,
)
assert msg.to == [self.user.email]

def test_user_feedback__enhanced_privacy(self):
self.organization.update(flags=F('flags').bitor(Organization.flags.enhanced_privacy))
assert self.organization.flags.enhanced_privacy.is_set is True

report = self.create_report()

with self.tasks():
self.plugin.handle_signal(
name='user-reports.created',
project=self.project,
payload={
'report': serialize(report, AnonymousUser(), UserReportWithGroupSerializer()),
},
)

assert len(mail.outbox) == 1
msg = mail.outbox[0]

# email does not include issue metadata
assert 'group-header' not in msg.alternatives[0][0]
assert 'enhanced privacy' in msg.body

assert msg.subject == u'[Sentry] {} - New Feedback from Homer Simpson'.format(
self.group.qualified_short_id,
)
Expand Down