diff --git a/src/sentry/api/serializers/models/team.py b/src/sentry/api/serializers/models/team.py index 72037b6338a22d..a7e8b24dea429b 100644 --- a/src/sentry/api/serializers/models/team.py +++ b/src/sentry/api/serializers/models/team.py @@ -3,7 +3,6 @@ import itertools from collections import defaultdict -from django.conf import settings from sentry.app import env from sentry.api.serializers import Serializer, register, serialize @@ -17,19 +16,7 @@ class TeamSerializer(Serializer): def get_attrs(self, item_list, user): request = env.request - if (request.is_superuser() and request.user == user) or settings.SENTRY_PUBLIC: - inactive_memberships = frozenset( - OrganizationMemberTeam.objects.filter( - team__in=item_list, - organizationmember__user=user, - is_active=False, - ).values_list('team', flat=True) - ) - memberships = frozenset([ - t.id for t in item_list - if t.id not in inactive_memberships - ]) - elif user.is_authenticated(): + if user.is_authenticated(): memberships = frozenset( OrganizationMemberTeam.objects.filter( organizationmember__user=user, @@ -50,11 +37,22 @@ def get_attrs(self, item_list, user): else: access_requests = frozenset() + is_superuser = request.is_superuser() and request.user == user result = {} for team in item_list: + is_member = team.id in memberships + if is_member: + has_access = True + elif is_superuser: + has_access = True + elif team.organization.flags.allow_joinleave: + has_access = True + else: + has_access = False result[team] = { 'pending_request': team.id in access_requests, - 'is_member': team.id in memberships, + 'is_member': is_member, + 'has_access': has_access, } return result @@ -65,6 +63,7 @@ def serialize(self, obj, attrs, user): 'name': obj.name, 'dateCreated': obj.date_added, 'isMember': attrs['is_member'], + 'hasAccess': attrs['has_access'], 'isPending': attrs['pending_request'], } diff --git a/src/sentry/auth/access.py b/src/sentry/auth/access.py index b584cbf4643126..c96eebc0dabe4d 100644 --- a/src/sentry/auth/access.py +++ b/src/sentry/auth/access.py @@ -2,6 +2,8 @@ __all__ = ['from_user', 'from_member', 'DEFAULT'] +import warnings + from django.conf import settings from sentry.models import AuthIdentity, AuthProvider, OrganizationMember @@ -10,7 +12,10 @@ class BaseAccess(object): is_active = False sso_is_valid = False + # teams with valid access teams = () + # teams with valid membership + memberships = () scopes = frozenset() def has_scope(self, scope): @@ -19,12 +24,22 @@ def has_scope(self, scope): return scope in self.scopes def has_team(self, team): + warnings.warn('has_team() is deprecated in favor of has_team_access', + DeprecationWarning) + return self.has_team_access(team) + + def has_team_access(self, team): if not self.is_active: return False return team in self.teams + def has_team_membership(self, team): + if not self.is_active: + return False + return team in self.memberships + def has_team_scope(self, team, scope): - return self.has_team(team) and self.has_scope(scope) + return self.has_team_access(team) and self.has_scope(scope) def to_django_context(self): return { @@ -37,8 +52,9 @@ class Access(BaseAccess): # TODO(dcramer): this is still a little gross, and ideally backend access # would be based on the same scopes as API access so theres clarity in # what things mean - def __init__(self, scopes, is_active, teams, sso_is_valid): + def __init__(self, scopes, is_active, teams, memberships, sso_is_valid): self.teams = teams + self.memberships = memberships self.scopes = scopes self.is_active = is_active @@ -50,10 +66,12 @@ def from_request(request, organization): return DEFAULT if request.is_superuser(): + team_list = list(organization.team_set.all()) return Access( scopes=settings.SENTRY_SCOPES, is_active=True, - teams=organization.team_set.all(), + teams=team_list, + memberships=team_list, sso_is_valid=True, ) return from_user(request.user, organization) @@ -74,6 +92,9 @@ def from_user(user, organization): except OrganizationMember.DoesNotExist: return DEFAULT + # ensure cached relation + om.organization = organization + return from_member(om) @@ -100,11 +121,18 @@ def from_member(member): else: sso_is_valid = auth_identity.is_valid(member) + team_memberships = member.get_teams() + if member.organization.flags.allow_joinleave: + team_access = list(member.organization.team_set.all()) + else: + team_access = team_memberships + return Access( is_active=True, sso_is_valid=sso_is_valid, scopes=member.get_scopes(), - teams=member.get_teams(), + memberships=team_memberships, + teams=team_access, ) @@ -121,6 +149,10 @@ def is_active(self): def teams(self): return () + @property + def memberships(self): + return () + @property def scopes(self): return frozenset() diff --git a/src/sentry/static/sentry/app/views/projectDetails.jsx b/src/sentry/static/sentry/app/views/projectDetails.jsx index 8b6e3f1ffaf7e7..b8468c8b533c29 100644 --- a/src/sentry/static/sentry/app/views/projectDetails.jsx +++ b/src/sentry/static/sentry/app/views/projectDetails.jsx @@ -91,9 +91,9 @@ const ProjectDetails = React.createClass({ return; } let [activeTeam, activeProject] = this.identifyProject(); - let isMember = activeTeam && activeTeam.isMember; + let hasAccess = activeTeam && activeTeam.hasAccess; - if (activeProject && isMember) { + if (activeProject && hasAccess) { // TODO(dcramer): move member list to organization level this.api.request(this.getMemberListEndpoint(), { success: (data) => { @@ -108,7 +108,7 @@ const ProjectDetails = React.createClass({ error: false, errorType: null }); - } else if (isMember === false) { + } else if (activeTeam && activeTeam.isMember) { this.setState({ project: activeProject, team: activeTeam, diff --git a/src/sentry/testutils/cases.py b/src/sentry/testutils/cases.py index 8a5047a4aba2f2..e445ac21a0bf23 100644 --- a/src/sentry/testutils/cases.py +++ b/src/sentry/testutils/cases.py @@ -276,7 +276,10 @@ class PermissionTestCase(TestCase): def setUp(self): super(PermissionTestCase, self).setUp() self.owner = self.create_user(is_superuser=False) - self.organization = self.create_organization(owner=self.owner) + self.organization = self.create_organization( + owner=self.owner, + flags=0, # disable default allow_joinleave access + ) self.team = self.create_team(organization=self.organization) def assert_can_access(self, user, path, method='GET'): diff --git a/tests/sentry/api/endpoints/test_organization_access_request_details.py b/tests/sentry/api/endpoints/test_organization_access_request_details.py index 223df4313fac39..b08fde4e14a4db 100644 --- a/tests/sentry/api/endpoints/test_organization_access_request_details.py +++ b/tests/sentry/api/endpoints/test_organization_access_request_details.py @@ -112,10 +112,14 @@ def test_team_admin_can_approve(self): assert resp.status_code == 204 - def test_teamless_admin_cannot_approve(self): + def test_teamless_admin_cannot_approve_with_closed_membership(self): self.login_as(user=self.user) - organization = self.create_organization(name='foo', owner=self.user) + organization = self.create_organization( + name='foo', + owner=self.user, + flags=0, # kill allow_joinleave + ) user = self.create_user('bar@example.com') member = self.create_member( organization=organization, diff --git a/tests/sentry/auth/test_access.py b/tests/sentry/auth/test_access.py index ef380b550bb2d5..e507bb3f8fbc60 100644 --- a/tests/sentry/auth/test_access.py +++ b/tests/sentry/auth/test_access.py @@ -4,7 +4,7 @@ from mock import Mock from sentry.auth import access -from sentry.models import AuthProvider +from sentry.models import AuthProvider, Organization from sentry.testutils import TestCase @@ -18,7 +18,8 @@ def test_no_access(self): assert not result.is_active assert result.sso_is_valid assert not result.scopes - assert not result.has_team(team) + assert not result.has_team_access(team) + assert not result.has_team_membership(team) def test_owner_all_teams(self): user = self.create_user() @@ -33,11 +34,15 @@ def test_owner_all_teams(self): assert result.is_active assert result.sso_is_valid assert result.scopes == member.get_scopes() - assert result.has_team(team) + assert result.has_team_access(team) + assert result.has_team_membership(team) - def test_member_no_teams(self): + def test_member_no_teams_closed_membership(self): user = self.create_user() - organization = self.create_organization(owner=self.user) + organization = self.create_organization( + owner=self.user, + flags=0, # disable default allow_joinleave + ) member = self.create_member( organization=organization, user=user, role='member', @@ -48,7 +53,27 @@ def test_member_no_teams(self): assert result.is_active assert result.sso_is_valid assert result.scopes == member.get_scopes() - assert not result.has_team(team) + assert not result.has_team_access(team) + assert not result.has_team_membership(team) + + def test_member_no_teams_open_membership(self): + user = self.create_user() + organization = self.create_organization( + owner=self.user, + flags=Organization.flags.allow_joinleave, + ) + member = self.create_member( + organization=organization, user=user, + role='member', teams=(), + ) + team = self.create_team(organization=organization) + + result = access.from_user(user, organization) + assert result.is_active + assert result.sso_is_valid + assert result.scopes == member.get_scopes() + assert result.has_team_access(team) + assert not result.has_team_membership(team) def test_team_restricted_org_member_access(self): user = self.create_user() @@ -64,7 +89,8 @@ def test_team_restricted_org_member_access(self): assert result.is_active assert result.sso_is_valid assert result.scopes == member.get_scopes() - assert result.has_team(team) + assert result.has_team_access(team) + assert result.has_team_membership(team) def test_unlinked_sso(self): user = self.create_user() @@ -106,4 +132,5 @@ def test_no_access(self): assert not result.is_active assert result.sso_is_valid assert not result.scopes - assert not result.has_team(Mock()) + assert not result.has_team_access(Mock()) + assert not result.has_team_membership(Mock()) diff --git a/tests/sentry/web/frontend/test_organization_members.py b/tests/sentry/web/frontend/test_organization_members.py index 947d84442e4d52..7fe2e36595028c 100644 --- a/tests/sentry/web/frontend/test_organization_members.py +++ b/tests/sentry/web/frontend/test_organization_members.py @@ -58,7 +58,11 @@ def test_renders_with_context(self): ] def test_shows_access_requests_for_team_admin(self): - organization = self.create_organization(name='foo', owner=self.user) + organization = self.create_organization( + name='foo', + owner=self.user, + flags=0, # kill default allow_joinleave + ) team_1 = self.create_team(name='foo', organization=organization) team_2 = self.create_team(name='bar', organization=organization)