From d718829f5de996f6b20260102a6c373fa3e08b8a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 16 Feb 2017 15:16:47 -0500 Subject: [PATCH 01/30] Initial LDAP group member iteration support Add interface for group member iteration on internal auth providers and implement support in the LDAP interface. --- data/users/__init__.py | 10 ++++++ data/users/externalldap.py | 73 +++++++++++++++++++++++++++++++++++--- data/users/federated.py | 60 +++++++++++++++++-------------- test/test_ldap.py | 32 +++++++++++++++-- 4 files changed, 141 insertions(+), 34 deletions(-) diff --git a/data/users/__init__.py b/data/users/__init__.py index 1f083e8ff..669ba7918 100644 --- a/data/users/__init__.py +++ b/data/users/__init__.py @@ -186,6 +186,16 @@ class UserAuthentication(object): """ Verifies that the given username and password credentials are valid. """ return self.state.verify_credentials(username_or_email, password) + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): + """ Returns a tuple of an iterator over all the members of the group matching the given lookup + args dictionary, or the error that occurred if the initial call failed or is unsupported. + The format of the lookup args dictionary is specific to the implementation. + Each result in the iterator is a tuple of (UserInformation, error_message), and only + one will be not-None. + """ + return self.state.iterate_group_members(group_lookup_args, page_size=page_size, + disable_pagination=disable_pagination) + def verify_and_link_user(self, username_or_email, password, basic_auth=False): """ Verifies that the given username and password credentials are valid and, if so, creates or links the database user to the federated identity. """ diff --git a/data/users/externalldap.py b/data/users/externalldap.py index 8cd82ac28..6642e7279 100644 --- a/data/users/externalldap.py +++ b/data/users/externalldap.py @@ -2,6 +2,8 @@ import ldap import logging import os +from ldap.controls import SimplePagedResultsControl + from collections import namedtuple from data.users.federated import FederatedUsers, UserInformation from util.itertoolrecipes import take @@ -10,6 +12,7 @@ logger = logging.getLogger(__name__) _DEFAULT_NETWORK_TIMEOUT = 10.0 # seconds _DEFAULT_TIMEOUT = 10.0 # seconds +_DEFAULT_PAGE_SIZE = 500 class LDAPConnectionBuilder(object): @@ -84,6 +87,7 @@ class LDAPUsers(FederatedUsers): # Create the set of full DN paths. self._user_dns = [get_full_rdn(relative_dn) for relative_dn in relative_user_dns] + self._base_dn = ','.join(base_dn) def _get_ldap_referral_dn(self, referral_exception): logger.debug('Got referral: %s', referral_exception.args[0]) @@ -174,7 +178,7 @@ class LDAPUsers(FederatedUsers): with_mail = [result for result in with_dns if result.attrs.get(self._email_attr)] return (with_mail[0] if with_mail else with_dns[0], None) - def _credential_for_user(self, response): + def _build_user_information(self, response): if not response.get(self._uid_attr): return (None, 'Missing uid field "%s" in user record' % self._uid_attr) @@ -194,7 +198,7 @@ class LDAPUsers(FederatedUsers): logger.debug('Found user for LDAP username or email %s', username_or_email) _, found_response = found_user - return self._credential_for_user(found_response) + return self._build_user_information(found_response) def query_users(self, query, limit=20): """ Queries LDAP for matching users. """ @@ -208,7 +212,7 @@ class LDAPUsers(FederatedUsers): final_results = [] for result in results[0:limit]: - credentials, err_msg = self._credential_for_user(result.attrs) + credentials, err_msg = self._build_user_information(result.attrs) if err_msg is not None: continue @@ -253,4 +257,65 @@ class LDAPUsers(FederatedUsers): logger.debug('Invalid LDAP credentials') return (None, 'Invalid password') - return self._credential_for_user(found_response) + return self._build_user_information(found_response) + + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): + try: + with self._ldap.get_connection(): + pass + except ldap.INVALID_CREDENTIALS: + return (None, 'LDAP Admin dn or password is invalid') + + group_dn = group_lookup_args['group_dn'] + page_size = page_size or _DEFAULT_PAGE_SIZE + return (self._iterate_members(group_dn, page_size, disable_pagination), None) + + def _iterate_members(self, group_dn, page_size, disable_pagination): + with self._ldap.get_connection() as conn: + lc = ldap.controls.libldap.SimplePagedResultsControl(criticality=True, size=page_size, + cookie='') + + search_flt = '(memberOf=%s,%s)' % (group_dn, self._base_dn) + + for user_search_dn in self._user_dns: + # Conduct the initial search for users that are a member of the group. + if disable_pagination: + msgid = conn.search(user_search_dn, ldap.SCOPE_SUBTREE, search_flt) + else: + msgid = conn.search_ext(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, serverctrls=[lc]) + + while True: + if disable_pagination: + _, rdata = conn.result(msgid) + else: + _, rdata, _, serverctrls = conn.result3(msgid) + + # Yield any users found. + for userdata in rdata: + yield self._build_user_information(userdata[1]) + + # If pagination is disabled, nothing more to do. + if disable_pagination: + break + + # Filter down the controls with which the server responded, looking for the paging + # control type. If not found, then the server does not support pagination and we already + # got all of the results. + pctrls = [control for control in serverctrls + if control.controlType == ldap.controls.SimplePagedResultsControl.controlType] + + if pctrls: + # Server supports pagination. Update the cookie so the next search finds the next page, + # then conduct the next search. + cookie = lc.cookie = pctrls[0].cookie + if cookie: + msgid = conn.search_ext(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, + serverctrls=[lc]) + continue + else: + # No additional results. + break + else: + # Pagintation is not supported. + logger.debug('Pagination is not supported for this LDAP server') + break diff --git a/data/users/federated.py b/data/users/federated.py index 8b6e570cb..683cc0945 100644 --- a/data/users/federated.py +++ b/data/users/federated.py @@ -37,33 +37,6 @@ class FederatedUsers(object): """ If implemented, get_user must be implemented as well. """ return (None, 'Not supported') - def _get_federated_user(self, username, email): - db_user = model.user.verify_federated_login(self._federated_service, username) - if not db_user: - # We must create the user in our db - valid_username = None - for valid_username in generate_valid_usernames(username): - if model.user.is_username_unique(valid_username): - break - - if not valid_username: - logger.error('Unable to pick a username for user: %s', username) - return (None, 'Unable to pick a username. Please report this to your administrator.') - - prompts = model.user.get_default_user_prompts(features) - db_user = model.user.create_federated_user(valid_username, email, self._federated_service, - username, - set_password_notification=False, - email_required=self._requires_email, - prompts=prompts) - else: - # Update the db attributes from the federated service. - if email: - db_user.email = email - db_user.save() - - return (db_user, None) - def link_user(self, username_or_email): (credentials, err_msg) = self.get_user(username_or_email) if credentials is None: @@ -98,3 +71,36 @@ class FederatedUsers(object): return (None, err_msg) return (db_user, None) + + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): + """ Returns an iterator over all the members of the group matching the given lookup args + dictionary. The format of the lookup args dictionary is specific to the implementation. + """ + return (None, 'Not supported') + + def _get_federated_user(self, username, email): + db_user = model.user.verify_federated_login(self._federated_service, username) + if not db_user: + # We must create the user in our db + valid_username = None + for valid_username in generate_valid_usernames(username): + if model.user.is_username_unique(valid_username): + break + + if not valid_username: + logger.error('Unable to pick a username for user: %s', username) + return (None, 'Unable to pick a username. Please report this to your administrator.') + + prompts = model.user.get_default_user_prompts(features) + db_user = model.user.create_federated_user(valid_username, email, self._federated_service, + username, + set_password_notification=False, + email_required=self._requires_email, + prompts=prompts) + else: + # Update the db attributes from the federated service. + if email: + db_user.email = email + db_user.save() + + return (db_user, None) diff --git a/test/test_ldap.py b/test/test_ldap.py index 6e2a5d914..621d1d00e 100644 --- a/test/test_ldap.py +++ b/test/test_ldap.py @@ -34,18 +34,25 @@ def mock_ldap(requires_email=True): 'dc': ['quay', 'io'], 'ou': 'otheremployees' }, + 'cn=AwesomeFolk,dc=quay,dc=io': { + 'dc': ['quay', 'io'], + 'cn': 'AwesomeFolk' + }, 'uid=testy,ou=employees,dc=quay,dc=io': { 'dc': ['quay', 'io'], 'ou': 'employees', - 'uid': 'testy', - 'userPassword': ['password'] + 'uid': ['testy'], + 'userPassword': ['password'], + 'mail': ['bar@baz.com'], + 'memberOf': ['cn=AwesomeFolk,dc=quay,dc=io'], }, 'uid=someuser,ou=employees,dc=quay,dc=io': { 'dc': ['quay', 'io'], 'ou': 'employees', 'uid': ['someuser'], 'userPassword': ['somepass'], - 'mail': ['foo@bar.com'] + 'mail': ['foo@bar.com'], + 'memberOf': ['cn=AwesomeFolk,dc=quay,dc=io'], }, 'uid=nomail,ou=employees,dc=quay,dc=io': { 'dc': ['quay', 'io'], @@ -301,6 +308,25 @@ class TestLDAP(unittest.TestCase): requires_email=False, timeout=5) ldap.query_users('cool') + def test_iterate_group_members(self): + with mock_ldap() as ldap: + (it, err) = ldap.iterate_group_members({'group_dn': 'cn=AwesomeFolk'}, + disable_pagination=True) + self.assertIsNone(err) + + results = list(it) + self.assertEquals(2, len(results)) + + first = results[0][0] + self.assertEquals('testy', first.id) + self.assertEquals('testy', first.username) + self.assertEquals('bar@baz.com', first.email) + + second = results[1][0] + self.assertEquals('someuser', second.id) + self.assertEquals('someuser', second.username) + self.assertEquals('foo@bar.com', second.email) + if __name__ == '__main__': unittest.main() From f5a854c1898289e311db3ecabefe399612028ff7 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Feb 2017 12:01:41 -0500 Subject: [PATCH 02/30] Add TeamSync database and API support Teams can now have a TeamSync entry in the database, indicating how they are synced via an external group. If found, then the user membership of the team cannot be changed via the API. --- data/database.py | 11 +++++++++- data/model/team.py | 31 +++++++++++++++++++++++--- endpoints/api/team.py | 49 +++++++++++++++++++++++++++++++++-------- initdb.py | 3 +++ test/test_api_usage.py | 50 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 13 deletions(-) diff --git a/data/database.py b/data/database.py index 6d7b7e3b2..37b89a7af 100644 --- a/data/database.py +++ b/data/database.py @@ -451,7 +451,7 @@ class User(BaseModel): TagManifest, AccessToken, OAuthAccessToken, BlobUpload, RepositoryNotification, OAuthAuthorizationCode, RepositoryActionCount, TagManifestLabel, Tag, - ManifestLabel, BlobUploading} | beta_classes + ManifestLabel, BlobUploading, TeamSync} | beta_classes delete_instance_filtered(self, User, delete_nullable, skip_transitive_deletes) @@ -526,6 +526,15 @@ class LoginService(BaseModel): name = CharField(unique=True, index=True) +class TeamSync(BaseModel): + team = ForeignKeyField(Team) + + transaction_id = CharField() + last_updated = DateTimeField(default=datetime.now, index=True) + service = ForeignKeyField(LoginService) + config = JSONField() + + class FederatedLogin(BaseModel): user = QuayUserField(allows_robots=True, index=True) service = ForeignKeyField(LoginService) diff --git a/data/model/team.py b/data/model/team.py index 753d00e2f..1d176ebdf 100644 --- a/data/model/team.py +++ b/data/model/team.py @@ -1,9 +1,13 @@ -from data.database import Team, TeamMember, TeamRole, User, TeamMemberInvite, RepositoryPermission +import json + +from peewee import fn + +from data.database import (Team, TeamMember, TeamRole, User, TeamMemberInvite, RepositoryPermission, + TeamSync, LoginService) from data.model import (DataModelException, InvalidTeamException, UserAlreadyInTeam, - InvalidTeamMemberException, user, _basequery) + InvalidTeamMemberException, _basequery) from data.text import prefix_search from util.validation import validate_username -from peewee import fn, JOIN_LEFT_OUTER from util.morecollections import AttrDict @@ -369,3 +373,24 @@ def confirm_team_invite(code, user_obj): team = found.team inviter = found.inviter return (team, inviter) + +def set_team_syncing(team, login_service_name, config): + login_service = LoginService.get(name=login_service_name) + TeamSync.create(team=team, transaction_id='', service=login_service, config=json.dumps(config)) + +def get_team_sync_information(orgname, teamname): + """ Returns the team syncing information for the team with the given name under the organization + with the given name or None if none. + """ + query = (TeamSync + .select(TeamSync, LoginService) + .join(Team) + .join(User) + .switch(TeamSync) + .join(LoginService) + .where(Team.name == teamname, User.organization == True, User.username == orgname)) + + try: + return query.get() + except TeamSync.DoesNotExist: + return None diff --git a/endpoints/api/team.py b/endpoints/api/team.py index 81b779b58..fada006f2 100644 --- a/endpoints/api/team.py +++ b/endpoints/api/team.py @@ -1,19 +1,22 @@ """ Create, list and manage an organization's teams. """ +from functools import wraps + from flask import request import features -from endpoints.api import (resource, nickname, ApiResource, validate_json_request, request_error, - log_action, internal_only, require_scope, path_param, query_param, - truthy_bool, parse_args, require_user_admin, show_if) -from endpoints.exception import Unauthorized, NotFound +from app import avatar, authentication from auth.permissions import AdministerOrganizationPermission, ViewTeamPermission from auth.auth_context import get_authenticated_user from auth import scopes from data import model +from endpoints.api import (resource, nickname, ApiResource, validate_json_request, request_error, + log_action, internal_only, require_scope, path_param, query_param, + truthy_bool, parse_args, require_user_admin, show_if, format_date) +from endpoints.exception import Unauthorized, NotFound, InvalidRequest from util.useremails import send_org_invite_email -from app import avatar +from util.names import parse_robot_username def permission_view(permission): return { @@ -24,7 +27,6 @@ def permission_view(permission): 'role': permission.role.name } - def try_accept_invite(code, user): (team, inviter) = model.team.confirm_team_invite(code, user) @@ -40,7 +42,6 @@ def try_accept_invite(code, user): return team - def handle_addinvite_team(inviter, team, user=None, email=None): requires_invite = features.MAILING and features.REQUIRE_TEAM_INVITE invite = model.team.add_or_invite_to_team(inviter, team, user, email, @@ -82,7 +83,6 @@ def member_view(member, invited=False): 'invited': invited, } - def invite_view(invite): if invite.user: return member_view(invite.user, invited=True) @@ -94,6 +94,26 @@ def invite_view(invite): 'invited': True } +def disallow_for_synced_team(except_robots=False): + """ Disallows the decorated operation for a team that is marked as being synced from an internal + auth provider such as LDAP. If except_robots is True, then the operation is allowed if the + member specified on the operation is a robot account. + """ + def inner(func): + @wraps(func) + def wrapper(self, *args, **kwargs): + # Team syncing can only be enabled if we have a federated service. + if authentication.federated_service: + orgname = kwargs['orgname'] + teamname = kwargs['teamname'] + if model.team.get_team_sync_information(orgname, teamname): + if not except_robots or not parse_robot_username(kwargs.get('membername', '')): + raise InvalidRequest('Cannot call this method on an auth-synced team') + + return func(self, *args, **kwargs) + return wrapper + return inner + @resource('/v1/organization//team/') @path_param('orgname', 'The name of the organization') @@ -214,6 +234,14 @@ class TeamMemberList(ApiResource): 'can_edit': edit_permission.can() } + sync_info = model.team.get_team_sync_information(orgname, teamname) + if sync_info is not None: + data['synced'] = { + 'last_updated': format_date(sync_info.last_updated), + 'service': sync_info.service.name, + 'config': sync_info.config, + } + return data raise Unauthorized() @@ -228,6 +256,7 @@ class TeamMember(ApiResource): @require_scope(scopes.ORG_ADMIN) @nickname('updateOrganizationTeamMember') + @disallow_for_synced_team(except_robots=True) def put(self, orgname, teamname, membername): """ Adds or invites a member to an existing team. """ permission = AdministerOrganizationPermission(orgname) @@ -265,6 +294,7 @@ class TeamMember(ApiResource): @require_scope(scopes.ORG_ADMIN) @nickname('deleteOrganizationTeamMember') + @disallow_for_synced_team(except_robots=True) def delete(self, orgname, teamname, membername): """ Delete a member of a team. If the user is merely invited to join the team, then the invite is removed instead. @@ -308,6 +338,7 @@ class InviteTeamMember(ApiResource): """ Resource for inviting a team member via email address. """ @require_scope(scopes.ORG_ADMIN) @nickname('inviteTeamMemberEmail') + @disallow_for_synced_team() def put(self, orgname, teamname, email): """ Invites an email address to an existing team. """ permission = AdministerOrganizationPermission(orgname) @@ -407,7 +438,7 @@ class TeamMemberInvite(ApiResource): @nickname('declineOrganizationTeamInvite') @require_user_admin def delete(self, code): - """ Delete an existing member of a team. """ + """ Delete an existing invitation to join a team. """ (team, inviter) = model.team.delete_team_invite(code, user_obj=get_authenticated_user()) model.notification.delete_matching_notifications(get_authenticated_user(), 'org_team_invite', diff --git a/initdb.py b/initdb.py index 03bb9131c..f8ce513ab 100644 --- a/initdb.py +++ b/initdb.py @@ -700,6 +700,9 @@ def populate_database(minimal=False, with_storage=False): model.team.add_user_to_team(creatorbot, creators) model.team.add_user_to_team(creatoruser, creators) + synced_team = model.team.create_team('synced', org, 'member', 'Some synced team.') + model.team.set_team_syncing(synced_team, 'ldap', {'group_dn': 'cn=Test-Group,ou=Users'}) + __generate_repository(with_storage, new_user_1, 'superwide', None, False, [], [(10, [], 'latest2'), (2, [], 'latest3'), diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 2f2548df5..52dabbaaa 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -7,6 +7,7 @@ import time import re import json as py_json +from mock import patch from StringIO import StringIO from calendar import timegm from contextlib import contextmanager @@ -77,6 +78,7 @@ from endpoints.api.suconfig import (SuperUserRegistryStatus, SuperUserConfig, Su SuperUserCreateInitialSuperUser) from endpoints.api.manifest import RepositoryManifestLabels, ManageRepositoryManifestLabel from test.test_ssl_util import generate_test_cert +from util.morecollections import AttrDict try: @@ -1592,6 +1594,54 @@ class TestUpdateOrganizationTeamMember(ApiTestCase): self.assertNotEqual(membername, member['name']) + def test_updatemembers_syncedteam(self): + self.login(ADMIN_ACCESS_USER) + + with patch('endpoints.api.team.authentication', AttrDict({'federated_service': 'foobar'})): + # Add the user to a non-synced team, which should succeed. + self.putJsonResponse(TeamMember, + params=dict(orgname=ORGANIZATION, teamname='owners', + membername=READ_ACCESS_USER)) + + # Remove the user from the non-synced team, which should succeed. + self.deleteEmptyResponse(TeamMember, + params=dict(orgname=ORGANIZATION, teamname='owners', + membername=READ_ACCESS_USER)) + + # Attempt to add the user to a synced team, which should fail. + self.putResponse(TeamMember, + params=dict(orgname=ORGANIZATION, teamname='synced', + membername=READ_ACCESS_USER), + expected_code=400) + + # Attempt to remove the user from the synced team, which should fail. + self.deleteResponse(TeamMember, + params=dict(orgname=ORGANIZATION, teamname='synced', + membername=READ_ACCESS_USER), + expected_code=400) + + # Add a robot to the synced team, which should succeed. + self.putJsonResponse(TeamMember, + params=dict(orgname=ORGANIZATION, teamname='synced', + membername=ORGANIZATION + '+coolrobot')) + + # Remove the robot from the non-synced team, which should succeed. + self.deleteEmptyResponse(TeamMember, + params=dict(orgname=ORGANIZATION, teamname='synced', + membername=ORGANIZATION + '+coolrobot')) + + # Invite a team member to a non-synced team, which should succeed. + self.putJsonResponse(InviteTeamMember, + params=dict(orgname=ORGANIZATION, teamname='owners', + email='someguy+new@devtable.com')) + + # Attempt to invite a team member to a synced team, which should fail. + self.putResponse(InviteTeamMember, + params=dict(orgname=ORGANIZATION, teamname='synced', + email='someguy+new@devtable.com'), + expected_code=400) + + class TestAcceptTeamMemberInvite(ApiTestCase): def test_accept(self): self.login(ADMIN_ACCESS_USER) From 1cfc4a834114e4d90dd4feacced2a77cb51c8f28 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Feb 2017 14:23:50 -0500 Subject: [PATCH 03/30] Change max size of LDAP pages and add filtering to reduce attributes returned --- data/users/externalldap.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/data/users/externalldap.py b/data/users/externalldap.py index 6642e7279..cce256f47 100644 --- a/data/users/externalldap.py +++ b/data/users/externalldap.py @@ -12,7 +12,7 @@ logger = logging.getLogger(__name__) _DEFAULT_NETWORK_TIMEOUT = 10.0 # seconds _DEFAULT_TIMEOUT = 10.0 # seconds -_DEFAULT_PAGE_SIZE = 500 +_DEFAULT_PAGE_SIZE = 1000 class LDAPConnectionBuilder(object): @@ -276,13 +276,15 @@ class LDAPUsers(FederatedUsers): cookie='') search_flt = '(memberOf=%s,%s)' % (group_dn, self._base_dn) + attributes = [self._uid_attr, self._email_attr] for user_search_dn in self._user_dns: # Conduct the initial search for users that are a member of the group. if disable_pagination: - msgid = conn.search(user_search_dn, ldap.SCOPE_SUBTREE, search_flt) + msgid = conn.search(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, attrlist=attributes) else: - msgid = conn.search_ext(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, serverctrls=[lc]) + msgid = conn.search_ext(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, serverctrls=[lc], + attrlist=attributes) while True: if disable_pagination: From ecfac817217f22d3a4d174a003423798c950cd5a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Feb 2017 17:10:26 -0500 Subject: [PATCH 04/30] Add check_group_lookup_args and service_metadata to auth providers --- data/users/__init__.py | 12 ++++++++++++ data/users/database.py | 11 +++++++++++ data/users/externalldap.py | 20 ++++++++++++++++++++ data/users/federated.py | 12 ++++++++++++ test/test_ldap.py | 16 ++++++++++++++++ 5 files changed, 71 insertions(+) diff --git a/data/users/__init__.py b/data/users/__init__.py index 669ba7918..7cf3bccef 100644 --- a/data/users/__init__.py +++ b/data/users/__init__.py @@ -186,6 +186,18 @@ class UserAuthentication(object): """ Verifies that the given username and password credentials are valid. """ return self.state.verify_credentials(username_or_email, password) + def check_group_lookup_args(self, group_lookup_args): + """ Verifies that the given group lookup args point to a valid group. Returns a tuple consisting + of a boolean status and an error message (if any). + """ + return self.state.check_group_lookup_args(group_lookup_args) + + def service_metadata(self): + """ Returns a dictionary of extra metadata to present to *superusers* about this auth engine. + For example, LDAP returns the base DN so we can display to the user during sync setup. + """ + return self.state.service_metadata() + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): """ Returns a tuple of an iterator over all the members of the group matching the given lookup args dictionary, or the error that occurred if the initial call failed or is unsupported. diff --git a/data/users/database.py b/data/users/database.py index 4edb0f442..6e5bf9949 100644 --- a/data/users/database.py +++ b/data/users/database.py @@ -28,3 +28,14 @@ class DatabaseUsers(object): """ No need to implement, as we already query for users directly in the database. """ return (None, '', '') + def check_group_lookup_args(self, group_lookup_args): + """ Never used since all groups, by definition, are in the database. """ + return (False, 'Not supported') + + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): + """ Never used since all groups, by definition, are in the database. """ + return (None, 'Not supported') + + def service_metadata(self): + """ Never used since database has no metadata """ + return {} diff --git a/data/users/externalldap.py b/data/users/externalldap.py index cce256f47..a059352af 100644 --- a/data/users/externalldap.py +++ b/data/users/externalldap.py @@ -259,6 +259,26 @@ class LDAPUsers(FederatedUsers): return self._build_user_information(found_response) + def service_metadata(self): + return { + 'base_dn': self._base_dn, + } + + def check_group_lookup_args(self, group_lookup_args, disable_pagination=False): + if not group_lookup_args.get('group_dn'): + return (False, 'Missing group_dn') + + (it, err) = self.iterate_group_members(group_lookup_args, page_size=1, + disable_pagination=disable_pagination) + if err is not None: + return (False, err) + + results = list(it) + if not results: + return (False, 'Group does not exist or is empty') + + return (True, None) + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): try: with self._ldap.get_connection(): diff --git a/data/users/federated.py b/data/users/federated.py index 683cc0945..09d2eb748 100644 --- a/data/users/federated.py +++ b/data/users/federated.py @@ -72,6 +72,18 @@ class FederatedUsers(object): return (db_user, None) + def service_metadata(self): + """ Returns a dictionary of extra metadata to present to *superusers* about this auth engine. + For example, LDAP returns the base DN so we can display to the user during sync setup. + """ + return {} + + def check_group_lookup_args(self, group_lookup_args): + """ Verifies that the given group lookup args point to a valid group. Returns a tuple consisting + of a boolean status and an error message (if any). + """ + return (False, 'Not supported') + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): """ Returns an iterator over all the members of the group matching the given lookup args dictionary. The format of the lookup args dictionary is specific to the implementation. diff --git a/test/test_ldap.py b/test/test_ldap.py index 621d1d00e..2cbd2999a 100644 --- a/test/test_ldap.py +++ b/test/test_ldap.py @@ -327,6 +327,22 @@ class TestLDAP(unittest.TestCase): self.assertEquals('someuser', second.username) self.assertEquals('foo@bar.com', second.email) + def test_check_group_lookup_args(self): + with mock_ldap() as ldap: + (result, err) = ldap.check_group_lookup_args({'group_dn': 'cn=invalid'}, + disable_pagination=True) + self.assertFalse(result) + self.assertIsNotNone(err) + + (result, err) = ldap.check_group_lookup_args({'group_dn': 'cn=AwesomeFolk'}, + disable_pagination=True) + self.assertTrue(result) + self.assertIsNone(err) + + def test_metadata(self): + with mock_ldap() as ldap: + assert 'base_dn' in ldap.service_metadata() + if __name__ == '__main__': unittest.main() From bb20422260ba3c3480032456116e097ea9b8f0f3 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Feb 2017 18:02:53 -0500 Subject: [PATCH 05/30] Fix pagination disabling in LDAP with mockldap Since mockldap doesn't support pagination, just disable it globally --- data/users/externalldap.py | 18 ++++++++++-------- test/test_ldap.py | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/data/users/externalldap.py b/data/users/externalldap.py index a059352af..6ab4b6c6b 100644 --- a/data/users/externalldap.py +++ b/data/users/externalldap.py @@ -66,7 +66,7 @@ class LDAPUsers(FederatedUsers): def __init__(self, ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, allow_tls_fallback=False, secondary_user_rdns=None, requires_email=True, - timeout=None, network_timeout=None): + timeout=None, network_timeout=None, force_no_pagination=False): super(LDAPUsers, self).__init__('ldap', requires_email) self._ldap = LDAPConnectionBuilder(ldap_uri, admin_dn, admin_passwd, allow_tls_fallback, @@ -76,6 +76,7 @@ class LDAPUsers(FederatedUsers): self._email_attr = email_attr self._allow_tls_fallback = allow_tls_fallback self._requires_email = requires_email + self._force_no_pagination = force_no_pagination # Note: user_rdn is a list of RDN pieces (for historical reasons), and secondary_user_rds # is a list of RDN strings. @@ -291,6 +292,7 @@ class LDAPUsers(FederatedUsers): return (self._iterate_members(group_dn, page_size, disable_pagination), None) def _iterate_members(self, group_dn, page_size, disable_pagination): + has_pagination = not(self._force_no_pagination or disable_pagination) with self._ldap.get_connection() as conn: lc = ldap.controls.libldap.SimplePagedResultsControl(criticality=True, size=page_size, cookie='') @@ -300,24 +302,24 @@ class LDAPUsers(FederatedUsers): for user_search_dn in self._user_dns: # Conduct the initial search for users that are a member of the group. - if disable_pagination: - msgid = conn.search(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, attrlist=attributes) - else: + if has_pagination: msgid = conn.search_ext(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, serverctrls=[lc], attrlist=attributes) + else: + msgid = conn.search(user_search_dn, ldap.SCOPE_SUBTREE, search_flt, attrlist=attributes) while True: - if disable_pagination: - _, rdata = conn.result(msgid) - else: + if has_pagination: _, rdata, _, serverctrls = conn.result3(msgid) + else: + _, rdata = conn.result(msgid) # Yield any users found. for userdata in rdata: yield self._build_user_information(userdata[1]) # If pagination is disabled, nothing more to do. - if disable_pagination: + if not has_pagination: break # Filter down the controls with which the server responded, looking for the paging diff --git a/test/test_ldap.py b/test/test_ldap.py index 2cbd2999a..0bb05ab12 100644 --- a/test/test_ldap.py +++ b/test/test_ldap.py @@ -19,7 +19,7 @@ def _create_ldap(requires_email=True): ldap = LDAPUsers('ldap://localhost', base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, secondary_user_rdns=secondary_user_rdns, - requires_email=requires_email) + requires_email=requires_email, force_no_pagination=True) return ldap @contextmanager From a17b6370325c9ac3131a1cae7981efbb5a1b7fef Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Feb 2017 18:19:35 -0500 Subject: [PATCH 06/30] Fix ordering in LDAP test --- test/test_ldap.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/test_ldap.py b/test/test_ldap.py index 0bb05ab12..04b6f4fdb 100644 --- a/test/test_ldap.py +++ b/test/test_ldap.py @@ -318,14 +318,20 @@ class TestLDAP(unittest.TestCase): self.assertEquals(2, len(results)) first = results[0][0] - self.assertEquals('testy', first.id) - self.assertEquals('testy', first.username) - self.assertEquals('bar@baz.com', first.email) - second = results[1][0] - self.assertEquals('someuser', second.id) - self.assertEquals('someuser', second.username) - self.assertEquals('foo@bar.com', second.email) + + if first.id == 'testy': + testy, someuser = first, second + else: + testy, someuser = second, first + + self.assertEquals('testy', testy.id) + self.assertEquals('testy', testy.username) + self.assertEquals('bar@baz.com', testy.email) + + self.assertEquals('someuser', someuser.id) + self.assertEquals('someuser', someuser.username) + self.assertEquals('foo@bar.com', someuser.email) def test_check_group_lookup_args(self): with mock_ldap() as ldap: From 8ea39771400f3014e07e0c313240b191da25f778 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 17 Feb 2017 18:20:23 -0500 Subject: [PATCH 07/30] Add ability to enable, disable and view team syncing in UI and API Also extracts out some common testing infrastructure to make testing APIs easier now using pytest --- data/model/team.py | 6 ++ endpoints/api/team.py | 88 ++++++++++++++++++++++++---- endpoints/api/test/shared.py | 3 - endpoints/api/test/test_security.py | 14 ++++- endpoints/api/test/test_team.py | 35 +++++++++++ static/css/pages/team-view.css | 18 +++++- static/directives/team-view-add.html | 16 +++-- static/js/pages/team-view.js | 80 +++++++++++++++++++++++++ static/partials/team-view.html | 65 ++++++++++++++++++-- 9 files changed, 298 insertions(+), 27 deletions(-) create mode 100644 endpoints/api/test/test_team.py diff --git a/data/model/team.py b/data/model/team.py index 1d176ebdf..52a58b842 100644 --- a/data/model/team.py +++ b/data/model/team.py @@ -375,9 +375,15 @@ def confirm_team_invite(code, user_obj): return (team, inviter) def set_team_syncing(team, login_service_name, config): + """ Sets the given team to sync to the given service using the given config. """ login_service = LoginService.get(name=login_service_name) TeamSync.create(team=team, transaction_id='', service=login_service, config=json.dumps(config)) +def remove_team_syncing(orgname, teamname): + existing = get_team_sync_information(orgname, teamname) + if existing: + existing.delete_instance() + def get_team_sync_information(orgname, teamname): """ Returns the team syncing information for the team with the given name under the organization with the given name or None if none. diff --git a/endpoints/api/team.py b/endpoints/api/team.py index fada006f2..80c2ad5ef 100644 --- a/endpoints/api/team.py +++ b/endpoints/api/team.py @@ -1,5 +1,7 @@ """ Create, list and manage an organization's teams. """ +import json + from functools import wraps from flask import request @@ -7,13 +9,16 @@ from flask import request import features from app import avatar, authentication -from auth.permissions import AdministerOrganizationPermission, ViewTeamPermission +from auth.permissions import (AdministerOrganizationPermission, ViewTeamPermission, + SuperUserPermission) + from auth.auth_context import get_authenticated_user from auth import scopes from data import model from endpoints.api import (resource, nickname, ApiResource, validate_json_request, request_error, log_action, internal_only, require_scope, path_param, query_param, - truthy_bool, parse_args, require_user_admin, show_if, format_date) + truthy_bool, parse_args, require_user_admin, show_if, format_date, + verify_not_prod, require_fresh_login) from endpoints.exception import Unauthorized, NotFound, InvalidRequest from util.useremails import send_org_invite_email from util.names import parse_robot_username @@ -200,6 +205,57 @@ class OrganizationTeam(ApiResource): raise Unauthorized() +@resource('/v1/organization//team//syncing') +@path_param('orgname', 'The name of the organization') +@path_param('teamname', 'The name of the team') +class OrganizationTeamSyncing(ApiResource): + """ Resource for managing syncing of a team by a backing group. """ + @require_scope(scopes.ORG_ADMIN) + @require_scope(scopes.SUPERUSER) + @nickname('enableOrganizationTeamSync') + @verify_not_prod + @require_fresh_login + def post(self, orgname, teamname): + # User must be both the org admin AND a superuser. + if SuperUserPermission().can() and AdministerOrganizationPermission(orgname).can(): + try: + team = model.team.get_organization_team(orgname, teamname) + except model.InvalidTeamException: + raise NotFound() + + config = request.get_json() + + # Ensure that the specified config points to a valid group. + status, err = authentication.check_group_lookup_args(config) + if not status: + raise InvalidRequest('Could not sync to group: %s' % err) + + # Set the team's syncing config. + model.team.set_team_syncing(team, authentication.federated_service, config) + + return team_view(orgname, team) + + raise Unauthorized() + + @require_scope(scopes.ORG_ADMIN) + @require_scope(scopes.SUPERUSER) + @nickname('disableOrganizationTeamSync') + @verify_not_prod + @require_fresh_login + def delete(self, orgname, teamname): + # User must be both the org admin AND a superuser. + if SuperUserPermission().can() and AdministerOrganizationPermission(orgname).can(): + try: + team = model.team.get_organization_team(orgname, teamname) + except model.InvalidTeamException: + raise NotFound() + + model.team.remove_team_syncing(orgname, teamname) + return team_view(orgname, team) + + raise Unauthorized() + + @resource('/v1/organization//team//members') @path_param('orgname', 'The name of the organization') @path_param('teamname', 'The name of the team') @@ -231,16 +287,28 @@ class TeamMemberList(ApiResource): data = { 'name': teamname, 'members': [member_view(m) for m in members] + [invite_view(i) for i in invites], - 'can_edit': edit_permission.can() + 'can_edit': edit_permission.can(), } - sync_info = model.team.get_team_sync_information(orgname, teamname) - if sync_info is not None: - data['synced'] = { - 'last_updated': format_date(sync_info.last_updated), - 'service': sync_info.service.name, - 'config': sync_info.config, - } + if authentication.federated_service: + if SuperUserPermission().can(): + data['can_sync'] = { + 'service': authentication.federated_service, + } + + data['can_sync'].update(authentication.service_metadata()) + + sync_info = model.team.get_team_sync_information(orgname, teamname) + if sync_info is not None: + data['synced'] = { + 'service': sync_info.service.name, + } + + if SuperUserPermission().can(): + data['synced'].update({ + 'last_updated': format_date(sync_info.last_updated), + 'config': json.loads(sync_info.config), + }) return data diff --git a/endpoints/api/test/shared.py b/endpoints/api/test/shared.py index 5b9c2f090..140f6b37e 100644 --- a/endpoints/api/test/shared.py +++ b/endpoints/api/test/shared.py @@ -2,15 +2,12 @@ import datetime import json from contextlib import contextmanager - from data import model from endpoints.api import api CSRF_TOKEN_KEY = '_csrf_token' CSRF_TOKEN = '123csrfforme' - -@contextmanager def client_with_identity(auth_username, client): with client.session_transaction() as sess: if auth_username and auth_username is not None: diff --git a/endpoints/api/test/test_security.py b/endpoints/api/test/test_security.py index 7d094f44c..dafd9fd0c 100644 --- a/endpoints/api/test/test_security.py +++ b/endpoints/api/test/test_security.py @@ -1,14 +1,26 @@ import pytest +from endpoints.api import api +from endpoints.api.team import OrganizationTeamSyncing from endpoints.api.test.shared import client_with_identity, conduct_api_call from endpoints.api.superuser import SuperUserRepositoryBuildLogs, SuperUserRepositoryBuildResource from endpoints.api.superuser import SuperUserRepositoryBuildStatus -from test.fixtures import app, appconfig, database_uri, init_db_path, sqlitedb_file +from endpoints.test.fixtures import app, appconfig, database_uri, init_db_path, sqlitedb_file TEAM_PARAMS = {'orgname': 'buynlarge', 'teamname': 'owners'} BUILD_PARAMS = {'build_uuid': 'test-1234'} @pytest.mark.parametrize('resource,method,params,body,identity,expected', [ + (OrganizationTeamSyncing, 'POST', TEAM_PARAMS, {}, None, 403), + (OrganizationTeamSyncing, 'POST', TEAM_PARAMS, {}, 'freshuser', 403), + (OrganizationTeamSyncing, 'POST', TEAM_PARAMS, {}, 'reader', 403), + (OrganizationTeamSyncing, 'POST', TEAM_PARAMS, {}, 'devtable', 400), + + (OrganizationTeamSyncing, 'DELETE', TEAM_PARAMS, {}, None, 403), + (OrganizationTeamSyncing, 'DELETE', TEAM_PARAMS, {}, 'freshuser', 403), + (OrganizationTeamSyncing, 'DELETE', TEAM_PARAMS, {}, 'reader', 403), + (OrganizationTeamSyncing, 'DELETE', TEAM_PARAMS, {}, 'devtable', 200), + (SuperUserRepositoryBuildLogs, 'GET', BUILD_PARAMS, None, None, 401), (SuperUserRepositoryBuildLogs, 'GET', BUILD_PARAMS, None, 'freshuser', 403), (SuperUserRepositoryBuildLogs, 'GET', BUILD_PARAMS, None, 'reader', 403), diff --git a/endpoints/api/test/test_team.py b/endpoints/api/test/test_team.py new file mode 100644 index 000000000..1f04ed108 --- /dev/null +++ b/endpoints/api/test/test_team.py @@ -0,0 +1,35 @@ +import json + +from mock import patch + +from data import model +from endpoints.api import api +from endpoints.api.test.shared import client_with_identity, conduct_api_call +from endpoints.api.team import OrganizationTeamSyncing +from test.test_ldap import mock_ldap + +TEAM_PARAMS = {'orgname': 'buynlarge', 'teamname': 'owners'} + +def test_team_syncing(client): + with mock_ldap() as ldap: + with patch('endpoints.api.team.authentication', ldap): + cl = client_with_identity('devtable', client) + config = { + 'group_dn': 'cn=AwesomeFolk', + } + + conduct_api_call(cl, OrganizationTeamSyncing, 'POST', TEAM_PARAMS, config) + + # Ensure the team is now synced. + sync_info = model.team.get_team_sync_information(TEAM_PARAMS['orgname'], + TEAM_PARAMS['teamname']) + assert sync_info is not None + assert json.loads(sync_info.config) == config + + # Remove the syncing. + conduct_api_call(cl, OrganizationTeamSyncing, 'DELETE', TEAM_PARAMS, None) + + # Ensure the team is no longer synced. + sync_info = model.team.get_team_sync_information(TEAM_PARAMS['orgname'], + TEAM_PARAMS['teamname']) + assert sync_info is None diff --git a/static/css/pages/team-view.css b/static/css/pages/team-view.css index 86cb7b6de..dad2fccab 100644 --- a/static/css/pages/team-view.css +++ b/static/css/pages/team-view.css @@ -3,6 +3,18 @@ position: relative; } +.team-view .team-sync-table { + margin-bottom: 20px; +} + +.team-view .team-sync-table td { + padding: 6px; +} + +.team-view .team-sync-table td:first-child { + font-weight: bold; +} + .team-view .team-title { vertical-align: middle; margin-right: 10px; @@ -14,10 +26,10 @@ margin-left: 6px; } -.team-view .team-view-header { +.team-view .team-view-header, .team-view .team-sync-header { border-bottom: 1px solid #eee; - margin-bottom: 10px; - padding-bottom: 10px; + margin-bottom: 20px; + padding-bottom: 20px; } .team-view .team-view-header button i.fa { diff --git a/static/directives/team-view-add.html b/static/directives/team-view-add.html index 88c747d2f..91aa154bb 100644 --- a/static/directives/team-view-add.html +++ b/static/directives/team-view-add.html @@ -1,20 +1,26 @@
Inviting team member
- Search by registry username, robot account name or enter an email address to invite - Search by registry username or robot account name + + Search by registry username, robot account name or enter an email address to invite + Search by registry username or robot account name + + + Search by robot account name. Users must be added in {{ syncInfo.service }}. +
diff --git a/static/js/pages/team-view.js b/static/js/pages/team-view.js index 0215ab080..91135163c 100644 --- a/static/js/pages/team-view.js +++ b/static/js/pages/team-view.js @@ -14,12 +14,14 @@ var teamname = $routeParams.teamname; var orgname = $routeParams.orgname; + $scope.context = {}; $scope.orgname = orgname; $scope.teamname = teamname; $scope.addingMember = false; $scope.memberMap = null; $scope.allowEmail = Features.MAILING; $scope.feedback = null; + $scope.allowedEntities = ['user', 'robot']; $rootScope.title = 'Loading...'; @@ -146,6 +148,39 @@ }, ApiService.errorDisplay('Cannot remove team member')); }; + $scope.getServiceName = function(service) { + switch (service) { + case 'ldap': + return 'LDAP'; + + case 'keystone': + return 'Keystone Auth'; + + case 'jwtauthn': + return 'External JWT Auth'; + + default: + return synced.service; + } + }; + + $scope.getAddPlaceholder = function(email, synced) { + var kinds = []; + + if (!synced) { + kinds.push('registered user'); + } + + kinds.push('robot'); + + if (email && !synced) { + kinds.push('email address'); + } + + kind_string = kinds.join(', ') + return 'Add a ' + kind_string + ' to the team'; + }; + $scope.updateForDescription = function(content) { $scope.organization.teams[teamname].description = content; @@ -166,6 +201,48 @@ }); }; + $scope.showEnableSyncing = function() { + $scope.enableSyncingInfo = { + 'service_info': $scope.canSync, + 'config': {} + }; + }; + + $scope.showDisableSyncing = function() { + msg = 'Are you sure you want to disable group syncing on this team? ' + + 'The team will once again become editable.'; + bootbox.confirm(msg, function(result) { + if (result) { + $scope.disableSyncing(); + } + }); + }; + + $scope.disableSyncing = function() { + var params = { + 'orgname': orgname, + 'teamname': teamname + }; + + var errorHandler = ApiService.errorDisplay('Could not disable team syncing'); + ApiService.disableOrganizationTeamSync(null, params).then(function(resp) { + loadMembers(); + }, errorHandler); + }; + + $scope.enableSyncing = function(config, callback) { + var params = { + 'orgname': orgname, + 'teamname': teamname + }; + + var errorHandler = ApiService.errorDisplay('Cannot enable team syncing', callback); + ApiService.enableOrganizationTeamSync(config, params).then(function(resp) { + loadMembers(); + callback(true); + }, errorHandler); + }; + var loadOrganization = function() { $scope.orgResource = ApiService.getOrganizationAsResource({'orgname': orgname}).get(function(org) { $scope.organization = org; @@ -187,6 +264,9 @@ $scope.membersResource = ApiService.getOrganizationTeamMembersAsResource(params).get(function(resp) { $scope.members = resp.members; $scope.canEditMembers = resp.can_edit; + $scope.canSync = resp.can_sync; + $scope.syncInfo = resp.synced; + $scope.allowedEntities = resp.synced ? ['robot'] : ['user', 'robot']; $('.info-icon').popover({ 'trigger': 'hover', diff --git a/static/partials/team-view.html b/static/partials/team-view.html index 5be2d58c5..c3694ba7a 100644 --- a/static/partials/team-view.html +++ b/static/partials/team-view.html @@ -18,6 +18,39 @@
+
+
Directory Synchronization
+

Directory synchronization allows this team's user membership to be backed by a group in {{ getServiceName(canSync.service) }}.

+ +
+ + +
+
+ This team is synchronized with a group in {{ getServiceName(syncInfo.service) }} and its user membership is therefore read-only. +
+ +
+
Directory Synchronization
+ + + + + + + + + +
Bound to group: +
+ {{ syncInfo.config.group_dn }} +
+
Last Updated: at {{ syncInfo.last_updated | amDateFormat:'dddd, MMMM Do YYYY, h:mm:ss a' }}
+ + +
+
+
Team Description
@@ -33,12 +66,15 @@
-
Team Members
+
Team Members
This team has no members.
-
- Click the "Add Team Member" button above to add or invite team members. +
+ Enter a user or robot above to add or invite to the team. +
+
+ This team is synchronized with an external group defined in {{ getServiceName(syncInfo.service) }}. To add a user to this team, add them in the backing group. To add a robot account to this team, enter them above.
@@ -46,7 +82,7 @@ - Team Members + Team Members (defined in {{ getServiceName(syncInfo.service) }}) - + Remove {{ member.name }} @@ -122,6 +158,25 @@
+ +
+
Please note that once team syncing is enabled, the team's user membership from within will be read-only.
+
+
+
+ Enter the distinguished name of the group, relative to {{ enableSyncingInfo.service_info.base_dn }}: + +
+
+
+
+ +