duplicate namespace social login fix (#1910)

Be smarter about locating the users's existing v3 namespace, instead of always making a new one with an incrementing suffix.

No-Issue

Signed-off-by: James Tanner <[email protected]>

galaxy_ng/app/api/v1/viewsets/sync.py

@@ -1,5 +1,3 @@
-import logging
-
 from django.conf import settings
 
 from drf_spectacular.utils import extend_schema
@@ -25,9 +23,6 @@
 )
 
 
-logger = logging.getLogger(__name__)
-
-
 class LegacyRolesSyncViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, LegacyTasksMixin):
     """Load roles from an upstream v1 source."""
 
@@ -45,6 +40,5 @@ def create(self, request):
         serializer = self.get_serializer(data=request.data)
         serializer.is_valid(raise_exception=True)
         kwargs = dict(serializer.validated_data)
-        logger.debug(f'REQUEST kwargs: {kwargs}')
         task_id = self.legacy_dispatch(legacy_sync_from_upstream, kwargs=kwargs)
         return Response({'task': task_id})

galaxy_ng/app/utils/galaxy.py

@@ -54,7 +54,7 @@ def paginated_results(next_url):
     _baseurl = parsed.scheme + '://' + parsed.netloc
     results = []
     while next_url:
-        logger.info(f'fetch {next_url}')
+        logger.info(f'pagination fetch {next_url}')
         rr = safe_fetch(next_url)
         if rr.status_code == 404:
             break
@@ -79,12 +79,10 @@ def paginated_results(next_url):
 
 
 def find_namespace(baseurl=None, name=None, id=None):
-
-    logger.info(f'baseurl1: {baseurl}')
     if baseurl is None or not baseurl:
         baseurl = 'https://old-galaxy.ansible.com'
     baseurl += '/api/v1/namespaces'
-    logger.info(f'baseurl2: {baseurl}')
+    logger.info(f'find_namespace baseurl:{baseurl} name:{name} id:{id}')
 
     parsed = urlparse(baseurl)
     _baseurl = parsed.scheme + '://' + parsed.netloc
@@ -130,6 +128,7 @@ def get_namespace_owners_details(baseurl, ns_id):
     owners = []
     next_owners_url = baseurl + f'/api/v1/namespaces/{ns_id}/owners/'
     while next_owners_url:
+        logger.info(f'fetch {next_owners_url}')
         o_data = safe_fetch(next_owners_url).json()
         for owner in o_data['results']:
             owners.append(owner)
@@ -147,14 +146,12 @@ def upstream_namespace_iterator(
     start_page=None,
     require_content=True,
 ):
-
     """Abstracts the pagination of v2 collections into a generator with error handling."""
-    logger.info(f'baseurl1: {baseurl}')
     if baseurl is None or not baseurl:
         baseurl = 'https://old-galaxy.ansible.com/api/v1/namespaces'
     if not baseurl.rstrip().endswith('/api/v1/namespaces'):
         baseurl = baseurl.rstrip() + '/api/v1/namespaces'
-    logger.info(f'baseurl2: {baseurl}')
+    logger.info(f'upstream_namespace_iterator baseurl:{baseurl}')
 
     # normalize the upstream url
     parsed = urlparse(baseurl)
@@ -226,10 +223,9 @@ def upstream_collection_iterator(
     start_page=None,
 ):
     """Abstracts the pagination of v2 collections into a generator with error handling."""
-    logger.info(f'baseurl1: {baseurl}')
     if baseurl is None or not baseurl:
         baseurl = 'https://old-galaxy.ansible.com/api/v2/collections'
-    logger.info(f'baseurl2: {baseurl}')
+    logger.info(f'upstream_collection_iterator baseurl:{baseurl}')
 
     # normalize the upstream url
     parsed = urlparse(baseurl)
@@ -409,10 +405,9 @@ def upstream_role_iterator(
     start_page=None,
 ):
     """Abstracts the pagination of v1 roles into a generator with error handling."""
-    logger.info(f'baseurl1: {baseurl}')
     if baseurl is None or not baseurl:
         baseurl = 'https://old-galaxy.ansible.com/api/v1/roles'
-    logger.info(f'baseurl2: {baseurl}')
+    logger.info(f'upstream_role_iterator baseurl:{baseurl}')
 
     # normalize the upstream url
     parsed = urlparse(baseurl)

galaxy_ng/social/__init__.py

@@ -63,16 +63,26 @@ def do_auth(self, access_token, *args, **kwargs):
         v3_namespace, v3_namespace_created = \
             self.handle_v3_namespace(auth_response, email, login, gid)
 
+        logger.info(
+            f'v3 namespace {v3_namespace} created:{v3_namespace_created}'
+            + f' for github login {login}'
+        )
+
         # create a legacynamespace and bind to the v3 namespace?
-        legacy_namespace, legacy_namespace_created = \
-            self._ensure_legacynamespace(login, v3_namespace)
+        if v3_namespace:
+            legacy_namespace, legacy_namespace_created = \
+                self._ensure_legacynamespace(login, v3_namespace)
+            logger.info(
+                f'legacy namespace {legacy_namespace} created:{legacy_namespace_created}'
+                + f' for github login {login}'
+            )
 
         return auth_response
 
     def handle_v3_namespace(self, session_user, session_email, session_login, github_id):
 
-        logger.debug(
-            f'HANDLING V3 NAMESPACE session_user:{session_user}'
+        logger.info(
+            f'handle_v3_namespace(1): session_user:{session_user}'
             + f' session_email:{session_email} session_login:{session_login}'
         )
 
@@ -81,16 +91,23 @@ def handle_v3_namespace(self, session_user, session_email, session_login, github
         # first make the namespace name ...
         namespace_name = self.transform_namespace_name(session_login)
 
-        logger.debug(f'TRANSFORMED NAME: {namespace_name}')
+        logger.info(
+            f'handle_v3_namespace(2): session_login:{session_login}'
+            + f' transformed_name:{namespace_name}'
+        )
 
         if not self.validate_namespace_name(namespace_name):
-            logger.debug(f'DID NOT VALIDATE NAMESPACE NAME: {namespace_name}')
+            logger.info(
+                f'handle_v3_namespace(3): session_login:{session_login}'
+                + f' transformed_name:{namespace_name} validated:False')
             return False, False
 
         # does the namespace already exist?
         found_namespace = Namespace.objects.filter(name=namespace_name).first()
-
-        logger.debug(f'FOUND NAMESPACE: {found_namespace}')
+        logger.info(
+            f'handle_v3_namespace(4): session_login:{session_login}'
+            + f' transformed_name:{namespace_name} found:{found_namespace}'
+        )
 
         # is it owned by this userid?
         if found_namespace:
@@ -99,21 +116,65 @@ def handle_v3_namespace(self, session_user, session_email, session_login, github
             logger.debug(f'FOUND EXISTING OWNERS: {owners}')
 
             if session_user in owners:
+                logger.info(
+                    f'handle_v3_namespace(5): session_login:{session_login}'
+                    f' is owner of {found_namespace}'
+                )
                 return found_namespace, False
+            else:
+                logger.info(
+                    f'handle_v3_namespace(6): session_login:{session_login}'
+                    + f' is NOT owner of {found_namespace}'
+                )
+
+        # FIXME - make one from the transformed name?
+        if not found_namespace:
+            namespace, namespace_created = self._ensure_namespace(namespace_name, session_user)
+            logger.info(
+                f'handle_v3_namespace(7): session_login:{session_login}'
+                + f' created:{namespace_created} namespace:{namespace}'
+            )
+            return namespace, namespace_created
+
+        # short circuit if the user does own at least one namespace ...
+        owned_namespaces = rbac.get_owned_v3_namespaces(session_user)
+        logger.info(
+            f'handle_v3_namespace(8): session_login:{session_login} owns {owned_namespaces}'
+        )
+
+        if owned_namespaces:
+            # does one resemble the desired namespace name?
+            owned_namespaces = sorted(owned_namespaces)
+            for ns in owned_namespaces:
+                if ns.name.startswith(namespace_name):
+                    logger.info(
+                        f'handle_v3_namespace(9): session_login:{session_login} returning {ns}'
+                    )
+                    return ns, False
+
+            logger.info(
+                f'handle_v3_namespace(10): session_login:{session_login}'
+                + f' no candidates among {owned_namespaces}'
+            )
+            return None, False
 
         # should always have a namespace ...
         if found_namespace:
-            logger.debug(
-                f'GENERATING A NEW NAMESPACE NAME SINCE USER DOES NOT OWN {found_namespace}'
-            )
             namespace_name = self.generate_available_namespace_name(session_login, github_id)
-            logger.debug(f'FINAL NAMESPACE NAME {namespace_name}')
+            logger.info(
+                f'handle_v3_namespace(11): session_login:{session_login}'
+                + f' new namespace name {namespace_name}'
+            )
 
         # create a v3 namespace?
         namespace, namespace_created = self._ensure_namespace(namespace_name, session_user)
+        logger.info(
+            f'handle_v3_namespace(12): session_login:{session_login}'
+            + f' namespace:{namespace} created:{namespace_created}'
+        )
 
         owned = rbac.get_owned_v3_namespaces(session_user)
-        logger.debug(f'NS OWNED BY {session_user}: {owned}')
+        logger.info(f'handle_v3_namespace(13): session_login:{session_login} owns {owned}')
 
         return namespace, namespace_created
 

galaxy_ng/tests/integration/api/test_community.py

@@ -166,7 +166,11 @@ def test_me_social_with_v1_synced_user(ansible_config):
     )
 
     # v1 sync the user's roles and namespace ...
-    pargs = json.dumps({"github_user": username, "limit": 1}).encode('utf-8')
+    pargs = json.dumps({
+        "github_user": username,
+        "limit": 1,
+        "baseurl": "https://old-galaxy.ansible.com"
+    }).encode('utf-8')
     resp = admin_client('/api/v1/sync/', method='POST', args=pargs)
     wait_for_v1_task(resp=resp, api_client=admin_client)
 
@@ -430,6 +434,7 @@ def test_list_collections_social(ansible_config):
         validate_json(instance=resp.json(), schema=schema_objectlist)
 
 
+@pytest.mark.skip(reason='switchover is complete')
 @pytest.mark.deployment_community
 def test_v1_sync_with_user_and_limit(ansible_config):
     """" Tests if v1 sync accepts a user&limit arg """

galaxy_ng/tests/integration/cli/test_community_sync.py

@@ -58,7 +58,7 @@ def test_community_collection_download_count_sync(ansible_config):
     remotes = dict((x['name'], x) for x in resp['results'])
     community_remote_config = {
         'name': 'community',
-        'url': 'https://galaxy.ansible.com/',
+        'url': 'https://old-galaxy.ansible.com/',
         'sync_dependencies': False,
         'requirements_file': json.dumps({'collections': ['.'.join(list(sync_collection))]}),
     }

galaxy_ng/tests/integration/community/test_community_namespace_rbac.py

@@ -241,15 +241,35 @@ def test_social_user_with_reclaimed_login(ansible_config):
     #   the "bar" namespace will not belong to them
     #   do they own -any- namespaces?
 
+    admin_config = ansible_config("admin")
+    admin_client = get_client(
+        config=admin_config,
+        request_token=False,
+        require_auth=True
+    )
+
     ga = GithubAdminClient()
     ga.delete_user(login='Wilk42')
     ga.delete_user(login='sean-m-sullivan')
     cleanup_social_user('Wilk42', ansible_config)
     cleanup_social_user('sean-m-sullivan', ansible_config)
+    cleanup_social_user('sean_m_sullivan', ansible_config)
     cleanup_social_user('[email protected]', ansible_config)
 
     default_cfg = extract_default_config(ansible_config)
 
+    nsmap = {}
+    next_url = '/api/v3/namespaces/'
+    while next_url:
+        resp = admin_client(next_url)
+        nsmap.update(dict((x['name'], x) for x in resp['data']))
+        next_url = resp['links']['next']
+    for nsname, nsdata in nsmap.items():
+        if not nsname.lower().startswith('sean_m') and not nsname.lower().startswith('wilk42'):
+            continue
+        cleanup_social_user(nsname, ansible_config)
+        cleanup_namespace(nsname, api_client=admin_client)
+
     old_login = 'Wilk42'
     email = '[email protected]'
     user_a = ga.create_user(login=old_login, email=email)
@@ -258,7 +278,6 @@ def test_social_user_with_reclaimed_login(ansible_config):
 
     # login once to make user
     with SocialGithubClient(config=user_a) as client:
-
         a_resp = client.get('_ui/v1/me/')
         ns_resp = client.get('_ui/v1/my-namespaces/')
         ns_ds = ns_resp.json()
@@ -480,3 +499,66 @@ def test_rbac_utils_get_owned_v3_namespaces(ansible_config):
 @pytest.mark.deployment_community
 def test_community_tools_urls(ansible_config):
     pass
+
+
+@pytest.mark.deployment_community
+def test_social_auth_no_duplicated_namespaces(ansible_config):
+
+    # https://issues.redhat.com/browse/AAH-2729
+
+    ga = GithubAdminClient()
+    ga.delete_user(login='Wilk42')
+    ga.delete_user(login='sean-m-sullivan')
+    cleanup_social_user('sean-m-sullivan', ansible_config)
+    cleanup_social_user('Wilk42', ansible_config)
+    cleanup_social_user('wilk42', ansible_config)
+    cleanup_social_user('[email protected]', ansible_config)
+    cleanup_social_user('[email protected]', ansible_config)
+
+    default_cfg = extract_default_config(ansible_config)
+
+    admin_config = ansible_config("admin")
+    admin_client = get_client(
+        config=admin_config,
+        request_token=False,
+        require_auth=True
+    )
+
+    # find all the sean_m* namespaces and clean them up ...
+    nsmap = {}
+    next_url = '/api/v3/namespaces/'
+    while next_url:
+        resp = admin_client(next_url)
+        nsmap.update(dict((x['name'], x) for x in resp['data']))
+        next_url = resp['links']['next']
+    for nsname, nsdata in nsmap.items():
+        if not nsname.startswith('sean_m') and not nsname.startswith('wilk42'):
+            continue
+        cleanup_social_user(nsname, ansible_config)
+        cleanup_namespace(nsname, api_client=admin_client)
+
+    # make sean_m_sullivan namespace with no owners?
+    api_prefix = admin_client.config.get("api_prefix").rstrip("/")
+    payload = {'name': 'sean_m_sullivan', 'groups': []}
+    resp = admin_client(f'{api_prefix}/v3/namespaces/', args=payload, method='POST')
+
+    # make sean-m-sullivan on github
+    ga.delete_user(login='sean-m-sullivan')
+    sean = ga.create_user(login='sean-m-sullivan', email='[email protected]', uid=30054029)
+    sean['username'] = sean['login']
+    sean.update(default_cfg)
+
+    # login 10 times ...
+    for x in range(0, 10):
+        with SocialGithubClient(config=sean) as client:
+            client.get('_ui/v1/me/')
+
+    # check to make sure only the one ns was created ...
+    nsmap = {}
+    next_url = '/api/v3/namespaces/'
+    while next_url:
+        resp = admin_client(next_url)
+        nsmap.update(dict((x['name'], x) for x in resp['data']))
+        next_url = resp['links']['next']
+    sean_namespaces = sorted([x for x in nsmap.keys() if x.startswith('sean_m')])
+    assert sean_namespaces == ['sean_m_sullivan', 'sean_m_sullivan0']