Merge pull request #2813 from coreos-inc/joseph.schorr/QUAY-701/team-sync-nonsuperuser

Add option for self-service team synchronization

config.py

@@ -462,6 +462,9 @@ class DefaultConfig(ImmutableConfig):
   TEAM_RESYNC_STALE_TIME = '30m'
   TEAM_SYNC_WORKER_FREQUENCY = 60 # seconds
 
+  # Feature Flag: If enabled, non-superusers can setup team syncing.
+  FEATURE_NONSUPERUSER_TEAM_SYNCING_SETUP = False
+
   # The default configurable tag expiration time for time machine.
   DEFAULT_TAG_EXPIRATION = '2w'
 

endpoints/api/team.py

@@ -209,6 +209,14 @@ class OrganizationTeam(ApiResource):
     raise Unauthorized()
 
 
+def _syncing_setup_allowed(orgname):
+  """ Returns whether syncing setup is allowed for the current user over the matching org. """
+  if not features.NONSUPERUSER_TEAM_SYNCING_SETUP and not SuperUserPermission().can():
+    return False
+
+  return AdministerOrganizationPermission(orgname).can()
+
+
 @resource('/v1/organization/<orgname>/team/<teamname>/syncing')
 @path_param('orgname', 'The name of the organization')
 @path_param('teamname', 'The name of the team')
@@ -221,8 +229,7 @@ class OrganizationTeamSyncing(ApiResource):
   @verify_not_prod
   @require_fresh_login
   def post(self, orgname, teamname):
-    # User must be both the org admin AND a superuser.
+    if _syncing_setup_allowed(orgname):
-    if SuperUserPermission().can() and AdministerOrganizationPermission(orgname).can():
       try:
         team = model.team.get_organization_team(orgname, teamname)
       except model.InvalidTeamException:
@@ -248,8 +255,7 @@ class OrganizationTeamSyncing(ApiResource):
   @verify_not_prod
   @require_fresh_login
   def delete(self, orgname, teamname):
-    # User must be both the org admin AND a superuser.
+    if _syncing_setup_allowed(orgname):
-    if SuperUserPermission().can() and AdministerOrganizationPermission(orgname).can():
       try:
         team = model.team.get_organization_team(orgname, teamname)
       except model.InvalidTeamException:
@@ -296,7 +302,7 @@ class TeamMemberList(ApiResource):
       }
 
       if features.TEAM_SYNCING and authentication.federated_service:
-        if SuperUserPermission().can() and AdministerOrganizationPermission(orgname).can():
+        if _syncing_setup_allowed(orgname):
           data['can_sync'] = {
             'service': authentication.federated_service,
           }

endpoints/api/test/test_security.py

@@ -1,3 +1,5 @@
+from mock import patch
+
 import pytest
 from flask_principal import AnonymousIdentity
 
@@ -10,7 +12,7 @@ from endpoints.api.signing import RepositorySignatures
 from endpoints.api.search import ConductRepositorySearch
 from endpoints.api.superuser import SuperUserRepositoryBuildLogs, SuperUserRepositoryBuildResource
 from endpoints.api.superuser import SuperUserRepositoryBuildStatus
-from endpoints.test.shared import client_with_identity
+from endpoints.test.shared import client_with_identity, toggle_feature
 
 from test.fixtures import *
 
@@ -69,3 +71,27 @@ NOTIFICATION_PARAMS = {'namespace': 'devtable', 'repository': 'devtable/simple',
 def test_api_security(resource, method, params, body, identity, expected, client):
   with client_with_identity(identity, client) as cl:
     conduct_api_call(cl, resource, method, params, body, expected)
+
+
+@pytest.mark.parametrize('is_superuser', [
+  (True),
+  (False),
+])
+@pytest.mark.parametrize('allow_nonsuperuser', [
+  (True),
+  (False),
+])
+@pytest.mark.parametrize('method, expected', [
+  ('POST', 400),
+  ('DELETE', 200),
+])
+def test_team_sync_security(is_superuser, allow_nonsuperuser, method, expected, client):
+  def is_superuser_method(_):
+    return is_superuser
+
+  with patch('auth.permissions.superusers.is_superuser', is_superuser_method):
+    with toggle_feature('NONSUPERUSER_TEAM_SYNCING_SETUP', allow_nonsuperuser):
+      with client_with_identity('devtable', client) as cl:
+        expect_success = is_superuser or allow_nonsuperuser
+        expected_status = expected if expect_success else 403
+        conduct_api_call(cl, OrganizationTeamSyncing, method, TEAM_PARAMS, {}, expected_status)

endpoints/test/shared.py

@@ -30,6 +30,16 @@ def client_with_identity(auth_username, client):
     sess[CSRF_TOKEN_KEY] = None
 
 
+@contextmanager
+def toggle_feature(name, enabled):
+  """ Context manager which temporarily toggles a feature. """
+  import features
+  previous_value = getattr(features, name)
+  setattr(features, name, enabled)
+  yield
+  setattr(features, name, previous_value)
+
+
 def add_csrf_param(params):
   """ Returns a params dict with the CSRF parameter added. """
   params = params or {}

static/directives/config/config-setup-tool.html

@@ -673,6 +673,18 @@
             </div>
           </td>
           </tr>
+          <tr ng-if="(config.AUTHENTICATION_TYPE == 'LDAP' || config.AUTHENTICATION_TYPE == 'Keystone') && config.FEATURE_TEAM_SYNCING">
+          <td>Self-service team syncing setup:</td>
+          <td>
+            <div class="co-alert co-alert-warning" style="margin-bottom: 20px;">If enabled, this feature will allow *any organization administrator* to read the membership of any {{ config.AUTHENTICATION_TYPE }} group.</div>
+            <div class="config-bool-field" binding="config.FEATURE_NONSUPERUSER_TEAM_SYNCING_SETUP">
+              Allow non-superusers to enable and manage team syncing
+            </div>
+            <div class="help-text">
+            If enabled, non-superusers will be able to enable and manage team sycning on teams under organizations in which they are administrators.
+            </div>
+          </td>
+          </tr>
         </table>
 
         <!--  Keystone Authentication -->