[security] improve logging/notifications around account security actions (#4771)

- add general logging behind 'sentry.security' logger
- add mfa-removed event
- add mfa-added event
- add password-changed event
- refactor avatar helpers
- add WIP tag for changesets (Danger)
- add system.security-email option
- add security/support email options to UI

CHANGES

@@ -6,6 +6,7 @@ Version 8.13 (Unreleased)
 - start using ReleaseProject and Release.organization instead of Release.project
 - Project quotas are no longer available, and must now be configured via the organizational rate limits.
 - Quotas implementation now requires a tuple of maximum rate and interval window.
+- Added security emails for adding and removing MFA and password changes.
 - Added the ability to download an apple compatible crash report for cocoa events.
 - Add memory and storage information for apple devices
 - The legacy API keys feature is now disabled by default.

Dangerfile

@@ -91,7 +91,7 @@ if securityMatches.any?
 end
 
 # Make it more obvious that a PR is a work in progress and shouldn"t be merged yet
-warn("PR is classed as Work in Progress") if github.pr_title.include? "[WIP]"
+warn("PR is classed as Work in Progress") if github.pr_title.include? "[WIP]" || github.pr_body.include?("#wip")
 
 # Warn when there is a big PR
 warn("Big PR -- consider splitting it up into multiple changesets") if git.lines_of_code > @S_BIG_PR_LINES

src/sentry/api/endpoints/user_authenticator_details.py

@@ -1,10 +1,12 @@
 from __future__ import absolute_import
 
+from django.db import transaction
 from rest_framework.response import Response
 
 from sentry.api.bases.user import UserEndpoint
 from sentry.api.permissions import SuperuserPermission
 from sentry.models import Authenticator
+from sentry.security import capture_security_activity
 
 
 class UserAuthenticatorDetailsEndpoint(UserEndpoint):
@@ -14,8 +16,53 @@ class UserAuthenticatorDetailsEndpoint(UserEndpoint):
     permission_classes = (SuperuserPermission,)
 
     def delete(self, request, user, auth_id):
-        Authenticator.objects.filter(
-            user=user,
-            id=auth_id,
-        ).delete()
+        try:
+            authenticator = Authenticator.objects.get(
+                user=user,
+                id=auth_id,
+            )
+        except Authenticator.DoesNotExist:
+            return Response(status=404)
+
+        with transaction.atomic():
+            authenticator.delete()
+
+            # if we delete an actual authenticator and all that
+            # remainds are backup interfaces, then we kill them in the
+            # process.
+            if not authenticator.interface.is_backup_interface:
+                interfaces = Authenticator.objects.all_interfaces_for_user(user)
+                backup_interfaces = [
+                    x for x in interfaces
+                    if x.is_backup_interface
+                ]
+                if len(backup_interfaces) == len(interfaces):
+                    for iface in backup_interfaces:
+                        iface.authenticator.delete()
+
+                    # wait to generate entries until all pending writes
+                    # have been sent to db
+                    for iface in backup_interfaces:
+                        capture_security_activity(
+                            account=request.user,
+                            type='mfa-removed',
+                            actor=request.user,
+                            ip_address=request.META['REMOTE_ADDR'],
+                            context={
+                                'authenticator': iface.authenticator,
+                            },
+                            send_email=False,
+                        )
+
+            capture_security_activity(
+                account=user,
+                type='mfa-removed',
+                actor=request.user,
+                ip_address=request.META['REMOTE_ADDR'],
+                context={
+                    'authenticator': authenticator,
+                },
+                send_email=not authenticator.interface.is_backup_interface,
+            )
+
         return Response(status=204)

src/sentry/models/useravatar.py

@@ -6,7 +6,7 @@ from django.db import models
 from PIL import Image
 from six import BytesIO
 
-from sentry.db.models import FlexibleForeignKey, Model
+from sentry.db.models import BaseManager, FlexibleForeignKey, Model
 from sentry.utils.cache import cache
 
 
@@ -30,6 +30,8 @@ class UserAvatar(Model):
     ident = models.CharField(max_length=32, unique=True, db_index=True)
     avatar_type = models.PositiveSmallIntegerField(default=0, choices=AVATAR_TYPES)
 
+    objects = BaseManager(cache_fields=['user'])
+
     class Meta:
         app_label = 'sentry'
         db_table = 'sentry_useravatar'

src/sentry/options/defaults.py

@@ -21,6 +21,7 @@ from sentry.utils.types import Dict, String, Sequence
 # System
 register('system.admin-email', flags=FLAG_REQUIRED)
 register('system.support-email', flags=FLAG_ALLOW_EMPTY | FLAG_PRIORITIZE_DISK)
+register('system.security-email', flags=FLAG_ALLOW_EMPTY | FLAG_PRIORITIZE_DISK)
 register('system.databases', type=Dict, flags=FLAG_NOSTORE)
 # register('system.debug', default=False, flags=FLAG_NOSTORE)
 register('system.rate-limit', default=0, flags=FLAG_ALLOW_EMPTY | FLAG_PRIORITIZE_DISK)

src/sentry/security/__init__.py

@@ -0,0 +1,37 @@
+from __future__ import absolute_import, print_function
+
+import logging
+
+from django.utils import timezone
+
+from .emails import generate_security_email
+
+logger = logging.getLogger('sentry.security')
+
+
+def capture_security_activity(account, type, actor, ip_address, context=None,
+                              send_email=True, current_datetime=None):
+    if current_datetime is None:
+        current_datetime = timezone.now()
+
+    logger_context = {
+        'ip_address': ip_address,
+        'user_id': account.id,
+        'actor_id': actor.id,
+    }
+
+    if type == 'mfa-removed' or type == 'mfa-added':
+        logger_context['authenticator_id'] = context['authenticator'].id
+
+    logger.info('user.{}'.format(type), extra=logger_context)
+
+    if send_email:
+        msg = generate_security_email(
+            account=account,
+            type=type,
+            actor=actor,
+            ip_address=ip_address,
+            context=context,
+            current_datetime=current_datetime,
+        )
+        msg.send_async([account.email])

src/sentry/security/emails.py

@@ -0,0 +1,42 @@
+from __future__ import absolute_import, print_function
+
+from django.utils import timezone
+
+from sentry.utils.email import MessageBuilder
+
+
+def generate_security_email(account, type, actor, ip_address, context=None,
+                            current_datetime=None):
+    if current_datetime is None:
+        current_datetime = timezone.now()
+
+    subject = 'Security settings changed'
+    if type == 'mfa-removed':
+        assert 'authenticator' in context
+        template = 'sentry/emails/mfa-removed.txt'
+        html_template = 'sentry/emails/mfa-removed.html'
+    elif type == 'mfa-added':
+        assert 'authenticator' in context
+        template = 'sentry/emails/mfa-added.txt'
+        html_template = 'sentry/emails/mfa-added.html'
+    elif type == 'password-changed':
+        template = 'sentry/emails/password-changed.txt'
+        html_template = 'sentry/emails/password-changed.html'
+    else:
+        raise ValueError('unknown type: {}'.format(type))
+
+    new_context = {
+        'account': account,
+        'actor': actor,
+        'ip_address': ip_address,
+        'datetime': current_datetime,
+    }
+    if context:
+        new_context.update(context)
+
+    return MessageBuilder(
+        subject=subject,
+        context=new_context,
+        template=template,
+        html_template=html_template,
+    )

src/sentry/static/sentry/app/options.jsx

@@ -33,6 +33,24 @@ const definitions = [
     component: EmailField,
     defaultValue: () => ConfigStore.get('user').email,
   },
+  {
+    key: 'system.support-email',
+    label: t('Support Email'),
+    placeholder: 'support@example.com',
+    help: t('The support contact for this Sentry installation.'),
+    // TODO(dcramer): this should not be hardcoded to a component
+    component: EmailField,
+    defaultValue: () => ConfigStore.get('user').email,
+  },
+  {
+    key: 'system.security-email',
+    label: t('Security Email'),
+    placeholder: 'security@example.com',
+    help: t('The security contact for this Sentry installation.'),
+    // TODO(dcramer): this should not be hardcoded to a component
+    component: EmailField,
+    defaultValue: () => ConfigStore.get('user').email,
+  },
   {
     key: 'system.rate-limit',
     label: t('Rate Limit'),

src/sentry/static/sentry/app/views/adminSettings.jsx

@@ -12,6 +12,8 @@ import {Form} from '../components/forms';
 const optionsAvailable = [
   'system.url-prefix',
   'system.admin-email',
+  'system.support-email',
+  'system.security-email',
   'system.rate-limit',
   'auth.ip-rate-limit',
   'auth.user-rate-limit',
@@ -74,6 +76,8 @@ const SettingsList = React.createClass({
         <h4>General</h4>
         {fields['system.url-prefix']}
         {fields['system.admin-email']}
+        {fields['system.support-email']}
+        {fields['system.security-email']}
         {fields['system.rate-limit']}
 
         <h4>Security &amp; Abuse</h4>

src/sentry/templates/sentry/account/settings.html

@@ -2,8 +2,10 @@
 
 {% load crispy_forms_tags %}
 {% load i18n %}
+{% load sentry_avatars %}
 {% load sentry_helpers %}
 {% load sentry_features %}
+
 {% block wrapperclass %}narrow account-settings{% endblock %}
 
 {% block title %}{% trans "Account Settings" %} | {{ block.super }}{% endblock %}
@@ -32,12 +34,7 @@
             <div>
               <a class="avatar-edit-link" href="{% url 'sentry-account-settings-avatar' %}">
                 <span class="avatar">
-                {% letter_avatar_svg request.user.get_display_name request.user.get_label size 118 %}
-                {% if request.user.get_avatar_type == 'upload' %}
-                    <img src="{% profile_photo_url request.user.id size 118 %}">
-                {% elif request.user.get_avatar_type == 'gravatar' %}
-                    <img src="{% gravatar_url user.email size 118 default 'blank' %}">
-                {% endif %}
+                {% avatar request.user 118 %}
                 </span>
                 <span class="icon-settings"></span>
               </a>