diff --git a/src/sentry/api/bases/user.py b/src/sentry/api/bases/user.py index a1aa7127663958..3946b449404754 100644 --- a/src/sentry/api/bases/user.py +++ b/src/sentry/api/bases/user.py @@ -2,12 +2,12 @@ from sentry.api.base import Endpoint from sentry.api.exceptions import ResourceDoesNotExist -from sentry.api.permissions import ScopedPermission -from sentry.models import User +from sentry.api.permissions import SentryPermission +from sentry.models import Organization, OrganizationStatus, User from sentry.auth.superuser import is_active_superuser -class UserPermission(ScopedPermission): +class UserPermission(SentryPermission): def has_object_permission(self, request, view, user=None): if user is None: user = request.user @@ -20,6 +20,36 @@ def has_object_permission(self, request, view, user=None): return False +class OrganizationUserPermission(UserPermission): + scope_map = { + 'DELETE': ['member:admin'], + } + + def has_org_permission(self, request, user): + """ + Org can act on a user account, + if the user is a member of only one org + e.g. reset org member's 2FA + """ + + try: + organization = Organization.objects.get( + status=OrganizationStatus.VISIBLE, + member_set__user=user + ) + + self.determine_access(request, organization) + allowed_scopes = set(self.scope_map.get(request.method, [])) + return any(request.access.has_scope(s) for s in allowed_scopes) + except (Organization.DoesNotExist, Organization.MultipleObjectsReturned): + return False + + def has_object_permission(self, request, view, user=None): + if super(OrganizationUserPermission, self).has_object_permission(request, view, user): + return True + return self.has_org_permission(request, user) + + class UserEndpoint(Endpoint): permission_classes = (UserPermission, ) diff --git a/src/sentry/api/endpoints/organization_member_details.py b/src/sentry/api/endpoints/organization_member_details.py index ba0fc930fae44b..4a7c75cbdb813b 100644 --- a/src/sentry/api/endpoints/organization_member_details.py +++ b/src/sentry/api/endpoints/organization_member_details.py @@ -9,7 +9,8 @@ from sentry.api.bases.organization import ( OrganizationEndpoint, OrganizationPermission) from sentry.api.exceptions import ResourceDoesNotExist -from sentry.api.serializers import serialize, RoleSerializer, OrganizationMemberWithTeamsSerializer +from sentry.api.serializers import ( + DetailedUserSerializer, serialize, RoleSerializer, OrganizationMemberWithTeamsSerializer) from sentry.api.serializers.rest_framework import ListField from sentry.auth.superuser import is_active_superuser from sentry.models import ( @@ -105,6 +106,7 @@ def _serialize_member(self, member, request, allowed_roles=None): if request.access.has_scope('member:admin'): context['invite_link'] = member.get_invite_link() + context['user'] = serialize(member.user, request.user, DetailedUserSerializer()) context['isOnlyOwner'] = self._is_only_owner(member) context['roles'] = serialize( diff --git a/src/sentry/api/endpoints/user_authenticator_details.py b/src/sentry/api/endpoints/user_authenticator_details.py index b4629d701ca8a5..ba8de0c574de14 100644 --- a/src/sentry/api/endpoints/user_authenticator_details.py +++ b/src/sentry/api/endpoints/user_authenticator_details.py @@ -4,7 +4,7 @@ from rest_framework import status from rest_framework.response import Response -from sentry.api.bases.user import UserEndpoint +from sentry.api.bases.user import UserEndpoint, OrganizationUserPermission from sentry.api.decorators import sudo_required from sentry.api.serializers import serialize from sentry.models import Authenticator @@ -12,6 +12,8 @@ class UserAuthenticatorDetailsEndpoint(UserEndpoint): + permission_classes = (OrganizationUserPermission, ) + @sudo_required def get(self, request, user, auth_id): """ diff --git a/src/sentry/api/serializers/models/user.py b/src/sentry/api/serializers/models/user.py index 1b270bddb9f6ce..5d14a40e2e6395 100644 --- a/src/sentry/api/serializers/models/user.py +++ b/src/sentry/api/serializers/models/user.py @@ -10,6 +10,8 @@ from sentry.models import ( AuthIdentity, Authenticator, + OrganizationMember, + OrganizationStatus, User, UserAvatar, UserOption, @@ -150,10 +152,18 @@ def get_attrs(self, item_list, user): user__in=item_list, ), 'user_id') + memberships = manytoone_to_dict(OrganizationMember.objects.filter( + user__in=item_list, + organization__status=OrganizationStatus.VISIBLE, + ), 'user_id') + for item in item_list: attrs[item]['authenticators'] = authenticators[item.id] attrs[item]['permissions'] = permissions[item.id] + # org can reset 2FA if the user is only in one org + attrs[item]['canReset2fa'] = len(memberships[item.id]) == 1 + return attrs def serialize(self, obj, attrs, user): @@ -174,4 +184,5 @@ def serialize(self, obj, attrs, user): 'dateUsed': a.last_used_at, } for a in attrs['authenticators'] ] + d['canReset2fa'] = attrs['canReset2fa'] return d diff --git a/src/sentry/static/sentry/app/actionCreators/account.jsx b/src/sentry/static/sentry/app/actionCreators/account.jsx index 85eac708f1830a..0ede89661dcb0f 100644 --- a/src/sentry/static/sentry/app/actionCreators/account.jsx +++ b/src/sentry/static/sentry/app/actionCreators/account.jsx @@ -29,3 +29,9 @@ export function logout(api) { method: 'DELETE', }); } + +export function removeAuthenticator(api, userId, authId) { + return api.requestPromise(`/users/${userId}/authenticators/${authId}/`, { + method: 'DELETE', + }); +} diff --git a/src/sentry/static/sentry/app/views/settings/organizationMembers/organizationMemberDetail.jsx b/src/sentry/static/sentry/app/views/settings/organizationMembers/organizationMemberDetail.jsx index 5d8ca6d4d14786..3ea51251b39cbd 100644 --- a/src/sentry/static/sentry/app/views/settings/organizationMembers/organizationMemberDetail.jsx +++ b/src/sentry/static/sentry/app/views/settings/organizationMembers/organizationMemberDetail.jsx @@ -1,21 +1,35 @@ +import * as Sentry from '@sentry/browser'; + import {browserHistory} from 'react-router'; import React from 'react'; import {resendMemberInvite, updateMember} from 'app/actionCreators/members'; -import {t} from 'app/locale'; +import {addErrorMessage, addSuccessMessage} from 'app/actionCreators/indicator'; +import {t, tct} from 'app/locale'; import AsyncView from 'app/views/asyncView'; import Button from 'app/components/button'; +import Confirm from 'app/components/confirm'; import DateTime from 'app/components/dateTime'; +import Field from 'app/views/settings/components/forms/field'; import IndicatorStore from 'app/stores/indicatorStore'; import NotFound from 'app/components/errors/notFound'; import {Panel, PanelBody, PanelHeader, PanelItem} from 'app/components/panels'; +import {removeAuthenticator} from 'app/actionCreators/account'; import SentryTypes from 'app/sentryTypes'; import SettingsPageHeader from 'app/views/settings/components/settingsPageHeader'; +import Tooltip from 'app/components/tooltip'; import recreateRoute from 'app/utils/recreateRoute'; import RoleSelect from './inviteMember/roleSelect'; import TeamSelect from './inviteMember/teamSelect'; +const NOT_ENROLLED = t('Not enrolled in two-factor authentication'); +const NO_PERMISSION = t('You do not have permission to perform this action'); +const TWO_FACTOR_REQUIRED = t( + 'Cannot be reset since two-factor is required for this organization' +); +const MULTIPLE_ORGS = t('Cannot be reset since user is in more than one organization'); + class OrganizationMemberDetail extends AsyncView { static contextTypes = { organization: SentryTypes.Organization, @@ -142,16 +156,62 @@ class OrganizationMemberDetail extends AsyncView { }); }; + handle2faReset = () => { + let {user} = this.state.member; + let {slug} = this.getOrganization(); + + let requests = user.authenticators.map(auth => + removeAuthenticator(this.api, user.id, auth.id) + ); + + Promise.all(requests) + .then(() => { + this.props.router.push(`/settings/${slug}/members/`); + addSuccessMessage(t('All authenticators have been removed')); + }) + .catch(err => { + addErrorMessage(t('Error removing authenticators')); + Sentry.captureException(err); + }); + }; + + showResetButton = () => { + let {member} = this.state; + let {require2FA} = this.getOrganization(); + let {user} = member; + + if (!user || !user.authenticators || require2FA) return false; + let hasAuth = user.authenticators.length >= 1; + return hasAuth && user.canReset2fa; + }; + + getTooltip = () => { + let {member} = this.state; + let {require2FA} = this.getOrganization(); + let {user} = member; + + if (!user) return ''; + + if (!user.authenticators) return NO_PERMISSION; + if (!user.authenticators.length) return NOT_ENROLLED; + if (!user.canReset2fa) return MULTIPLE_ORGS; + if (require2FA) return TWO_FACTOR_REQUIRED; + + return ''; + }; + renderBody() { let {error, member} = this.state; let {teams, access} = this.getOrganization(); if (!member) return ; - let email = member.email; let inviteLink = member.invite_link; let canEdit = access.includes('org:write'); - let canResend = !member.expired; + + let {email, expired, pending} = member; + let canResend = !expired; + let showAuth = !pending; return (
@@ -246,6 +306,44 @@ class OrganizationMemberDetail extends AsyncView { + {showAuth && ( + + {t('Authentication')} + + + + + + + + + + + + + )} + { + expect(wrapper.find(button).text()).toEqual('Reset two-factor authentication'); + expect(wrapper.find(button).prop('disabled')).toBe(false); + + expect(wrapper.find(tooltip).prop('title')).toEqual(''); + expect(wrapper.find(tooltip).prop('disabled')).toBe(true); + }; + + const expectButtonDisabled = title => { + expect(wrapper.find(button).text()).toEqual('Reset two-factor authentication'); + expect(wrapper.find(button).prop('disabled')).toBe(true); + + expect(wrapper.find(tooltip).prop('title')).toEqual(title); + expect(wrapper.find(tooltip).prop('disabled')).toBe(false); + }; + + it('does not show for pending member', function() { + wrapper = mount( + , + routerContext + ); + expect(wrapper.find(button)).toHaveLength(0); + }); + + it('shows tooltip for joined member without permission to edit', function() { + wrapper = mount( + , + routerContext + ); + expectButtonDisabled('You do not have permission to perform this action'); + }); + + it('shows tooltip for member without 2fa', function() { + wrapper = mount( + , + routerContext + ); + expectButtonDisabled('Not enrolled in two-factor authentication'); + }); + + it('can reset member 2FA', function() { + let deleteMocks = has2fa.user.authenticators.map(auth => { + return MockApiClient.addMockResponse({ + url: `/users/${has2fa.user.id}/authenticators/${auth.id}/`, + method: 'DELETE', + }); + }); + + wrapper = mount( + , + routerContext + ); + + expectButtonEnabled(); + wrapper.find(button).simulate('click'); + wrapper.find('Button[data-test-id="confirm-modal"]').simulate('click'); + deleteMocks.map(deleteMock => { + expect(deleteMock).toHaveBeenCalled(); + }); + }); + + it('shows tooltip for member in multiple orgs', function() { + wrapper = mount( + , + routerContext + ); + expectButtonDisabled('Cannot be reset since user is in more than one organization'); + }); + + it('shows tooltip for member in 2FA required org', function() { + organization.require2FA = true; + MockApiClient.addMockResponse({ + url: `/organizations/${organization.slug}/members/${has2fa.id}/`, + body: has2fa, + }); + + wrapper = mount( + , + routerContext + ); + expectButtonDisabled( + 'Cannot be reset since two-factor is required for this organization' + ); + }); + }); }); diff --git a/tests/sentry/api/endpoints/test_organization_member_details.py b/tests/sentry/api/endpoints/test_organization_member_details.py index 0f94d68b4e5fa6..71f3d5f1fcef0e 100644 --- a/tests/sentry/api/endpoints/test_organization_member_details.py +++ b/tests/sentry/api/endpoints/test_organization_member_details.py @@ -2,10 +2,13 @@ from django.core import mail from django.core.urlresolvers import reverse +from django.db.models import F from mock import patch from sentry.models import ( - AuthProvider, OrganizationMember, OrganizationMemberTeam) + Authenticator, AuthProvider, Organization, OrganizationMember, OrganizationMemberTeam, + RecoveryCodeInterface, TotpInterface +) from sentry.testutils import APITestCase @@ -466,6 +469,166 @@ def test_cannot_lower_superior_role(self): assert owner_om.role == 'owner' +class ResetOrganizationMember2faTest(APITestCase): + def setUp(self): + self.owner = self.create_user() + self.org = self.create_organization(owner=self.owner) + + self.member = self.create_user() + self.member_om = self.create_member( + organization=self.org, + user=self.member, + role='member', + teams=[], + ) + self.login_as(self.member) + totp = TotpInterface() + totp.enroll(self.member) + self.interface_id = totp.authenticator.id + assert Authenticator.objects.filter(user=self.member).exists() + + def assert_can_get_authenticators(self): + path = reverse( + 'sentry-api-0-organization-member-details', args=[self.org.slug, self.member_om.id] + ) + resp = self.client.get(path) + assert resp.status_code == 200 + data = resp.data + + assert len(data['user']['authenticators']) == 1 + assert data['user']['has2fa'] is True + assert data['user']['canReset2fa'] is True + + def assert_cannot_get_authenticators(self): + path = reverse( + 'sentry-api-0-organization-member-details', args=[self.org.slug, self.member_om.id] + ) + resp = self.client.get(path) + assert resp.status_code == 200 + data = resp.data + + assert 'authenticators' not in data['user'] + assert 'canReset2fa' not in data['user'] + + def assert_can_remove_authenticators(self): + path = reverse( + 'sentry-api-0-user-authenticator-details', args=[self.member.id, self.interface_id] + ) + resp = self.client.delete(path) + assert resp.status_code == 204 + assert not Authenticator.objects.filter(user=self.member).exists() + + def assert_cannot_remove_authenticators(self): + path = reverse( + 'sentry-api-0-user-authenticator-details', args=[self.member.id, self.interface_id] + ) + resp = self.client.delete(path) + assert resp.status_code == 403 + assert Authenticator.objects.filter(user=self.member).exists() + + def test_org_owner_can_reset_member_2fa(self): + self.login_as(self.owner) + + self.assert_can_get_authenticators() + self.assert_can_remove_authenticators() + + def test_owner_must_have_org_membership(self): + owner = self.create_user() + self.create_organization(owner=owner) + self.login_as(owner) + + path = reverse( + 'sentry-api-0-organization-member-details', args=[self.org.slug, self.member_om.id] + ) + resp = self.client.get(path) + assert resp.status_code == 403 + + self.assert_cannot_remove_authenticators() + + def test_org_manager_can_reset_member_2fa(self): + manager = self.create_user() + self.create_member( + organization=self.org, + user=manager, + role='manager', + teams=[], + ) + self.login_as(manager) + + self.assert_can_get_authenticators() + self.assert_can_remove_authenticators() + + def test_org_admin_cannot_reset_member_2fa(self): + admin = self.create_user() + self.create_member( + organization=self.org, + user=admin, + role='admin', + teams=[], + ) + self.login_as(admin) + + self.assert_cannot_get_authenticators() + self.assert_cannot_remove_authenticators() + + def test_org_member_cannot_reset_member_2fa(self): + member = self.create_user() + self.create_member( + organization=self.org, + user=member, + role='member', + teams=[], + ) + self.login_as(member) + + self.assert_cannot_get_authenticators() + self.assert_cannot_remove_authenticators() + + def test_cannot_reset_member_2fa__has_multiple_org_membership(self): + self.create_organization(owner=self.member) + self.login_as(self.owner) + + path = reverse( + 'sentry-api-0-organization-member-details', args=[self.org.slug, self.member_om.id] + ) + resp = self.client.get(path) + assert resp.status_code == 200 + data = resp.data + + assert len(data['user']['authenticators']) == 1 + assert data['user']['has2fa'] is True + assert data['user']['canReset2fa'] is False + + self.assert_cannot_remove_authenticators() + + def test_cannot_reset_member_2fa__org_requires_2fa(self): + self.login_as(self.owner) + TotpInterface().enroll(self.owner) + + self.org.update(flags=F('flags').bitor(Organization.flags.require_2fa)) + assert self.org.flags.require_2fa.is_set is True + + self.assert_cannot_remove_authenticators() + + def test_owner_can_only_reset_member_2fa(self): + self.login_as(self.owner) + + path = reverse( + 'sentry-api-0-user-authenticator-details', args=[self.member.id, self.interface_id] + ) + resp = self.client.get(path) + assert resp.status_code == 403 + + # cannot regenerate recovery codes + recovery = RecoveryCodeInterface() + recovery.enroll(self.user) + path = reverse( + 'sentry-api-0-user-authenticator-details', args=[self.member.id, recovery.authenticator.id] + ) + resp = self.client.put(path) + assert resp.status_code == 403 + + class DeleteOrganizationMemberTest(APITestCase): def test_simple(self): self.login_as(user=self.user) diff --git a/tests/sentry/api/serializers/test_user.py b/tests/sentry/api/serializers/test_user.py index 564b541563ebdd..3db705608288cf 100644 --- a/tests/sentry/api/serializers/test_user.py +++ b/tests/sentry/api/serializers/test_user.py @@ -84,3 +84,8 @@ def test_simple(self): assert len(result['authenticators']) == 1 assert result['authenticators'][0]['id'] == six.text_type(auth.id) assert result['permissions'] == ['foo'] + assert result['canReset2fa'] is True + + self.create_organization(owner=user) + result = serialize(user, user, DetailedUserSerializer()) + assert result['canReset2fa'] is False