Merge branch 'downstream' into devel

awx/main/access.py

@@ -2479,13 +2479,16 @@ class NotificationAccess(BaseAccess):
 
 class LabelAccess(BaseAccess):
     '''
-    I can see/use a Label if I have permission to associated organization
+    I can see/use a Label if I have permission to associated organization, or to a JT that the label is on
     '''
     model = Label
     prefetch_related = ('modified_by', 'created_by', 'organization',)
 
     def filtered_queryset(self):
-        return self.model.objects.all()
+        return self.model.objects.filter(
+            Q(organization__in=Organization.accessible_pk_qs(self.user, 'read_role')) |
+            Q(unifiedjobtemplate_labels__in=UnifiedJobTemplate.accessible_pk_qs(self.user, 'read_role'))
+        )
 
     @check_superuser
     def can_add(self, data):

awx/main/credential_plugins/aim.py

@@ -1,4 +1,4 @@
-from .plugin import CredentialPlugin, CertFiles
+from .plugin import CredentialPlugin, CertFiles, raise_for_status
 
 from urllib.parse import quote, urlencode, urljoin
 
@@ -82,8 +82,9 @@ def aim_backend(**kwargs):
             timeout=30,
             cert=cert,
             verify=verify,
+            allow_redirects=False,
         )
-    res.raise_for_status()
+    raise_for_status(res)
     return res.json()['Content']
 
 

awx/main/credential_plugins/conjur.py

@@ -1,4 +1,4 @@
-from .plugin import CredentialPlugin, CertFiles
+from .plugin import CredentialPlugin, CertFiles, raise_for_status
 
 import base64
 from urllib.parse import urljoin, quote
@@ -58,7 +58,8 @@ def conjur_backend(**kwargs):
 
     auth_kwargs = {
         'headers': {'Content-Type': 'text/plain'},
-        'data': api_key
+        'data': api_key,
+        'allow_redirects': False,
     }
 
     with CertFiles(cacert) as cert:
@@ -68,11 +69,12 @@ def conjur_backend(**kwargs):
             urljoin(url, '/'.join(['authn', account, username, 'authenticate'])),
             **auth_kwargs
         )
-    resp.raise_for_status()
+    raise_for_status(resp)
     token = base64.b64encode(resp.content).decode('utf-8')
 
     lookup_kwargs = {
         'headers': {'Authorization': 'Token token="{}"'.format(token)},
+        'allow_redirects': False,
     }
 
     # https://www.conjur.org/api.html#secrets-retrieve-a-secret-get
@@ -88,7 +90,7 @@ def conjur_backend(**kwargs):
     with CertFiles(cacert) as cert:
         lookup_kwargs['verify'] = cert
         resp = requests.get(path, timeout=30, **lookup_kwargs)
-    resp.raise_for_status()
+    raise_for_status(resp)
     return resp.text
 
 

awx/main/credential_plugins/hashivault.py

@@ -3,7 +3,7 @@ import os
 import pathlib
 from urllib.parse import urljoin
 
-from .plugin import CredentialPlugin, CertFiles
+from .plugin import CredentialPlugin, CertFiles, raise_for_status
 
 import requests
 from django.utils.translation import ugettext_lazy as _
@@ -145,7 +145,10 @@ def kv_backend(**kwargs):
     cacert = kwargs.get('cacert', None)
     api_version = kwargs['api_version']
 
-    request_kwargs = {'timeout': 30}
+    request_kwargs = {
+        'timeout': 30,
+        'allow_redirects': False,
+    }
 
     sess = requests.Session()
     sess.headers['Authorization'] = 'Bearer {}'.format(token)
@@ -175,7 +178,7 @@ def kv_backend(**kwargs):
     with CertFiles(cacert) as cert:
         request_kwargs['verify'] = cert
         response = sess.get(request_url, **request_kwargs)
-    response.raise_for_status()
+    raise_for_status(response)
 
     json = response.json()
     if api_version == 'v2':
@@ -198,7 +201,10 @@ def ssh_backend(**kwargs):
     role = kwargs['role']
     cacert = kwargs.get('cacert', None)
 
-    request_kwargs = {'timeout': 30}
+    request_kwargs = {
+        'timeout': 30,
+        'allow_redirects': False,
+    }
 
     request_kwargs['json'] = {'public_key': kwargs['public_key']}
     if kwargs.get('valid_principals'):
@@ -215,7 +221,7 @@ def ssh_backend(**kwargs):
         request_kwargs['verify'] = cert
         resp = sess.post(request_url, **request_kwargs)
 
-    resp.raise_for_status()
+    raise_for_status(resp)
     return resp.json()['data']['signed_key']
 
 

awx/main/credential_plugins/plugin.py

@@ -3,9 +3,19 @@ import tempfile
 
 from collections import namedtuple
 
+from requests.exceptions import HTTPError
+
 CredentialPlugin = namedtuple('CredentialPlugin', ['name', 'inputs', 'backend'])
 
 
+def raise_for_status(resp):
+    resp.raise_for_status()
+    if resp.status_code >= 300:
+        exc = HTTPError()
+        setattr(exc, 'response', resp)
+        raise exc
+
+
 class CertFiles():
     """
     A context manager used for writing a certificate and (optional) key

awx/main/middleware.py

@@ -14,7 +14,7 @@ from django.conf import settings
 from django.contrib.auth.models import User
 from django.db.migrations.executor import MigrationExecutor
 from django.db import connection
-from django.shortcuts import get_object_or_404, redirect
+from django.shortcuts import redirect
 from django.apps import apps
 from django.utils.deprecation import MiddlewareMixin
 from django.utils.translation import ugettext_lazy as _
@@ -148,7 +148,21 @@ class URLModificationMiddleware(MiddlewareMixin):
     def _named_url_to_pk(cls, node, resource, named_url):
         kwargs = {}
         if node.populate_named_url_query_kwargs(kwargs, named_url):
-            return str(get_object_or_404(node.model, **kwargs).pk)
+            match = node.model.objects.filter(**kwargs).first()
+            if match:
+                return str(match.pk)
+            else:
+                # if the name does *not* resolve to any actual resource,
+                # we should still attempt to route it through so that 401s are
+                # respected
+                # using "zero" here will cause the URL regex to match e.g.,
+                # /api/v2/users/<integer>/, but it also means that anonymous
+                # users will go down the path of having their credentials
+                # verified; in this way, *anonymous* users will that visit
+                # /api/v2/users/invalid-username/ *won't* see a 404, they'll
+                # see a 401 as if they'd gone to /api/v2/users/0/
+                #
+                return '0'
         if resource == 'job_templates' and '++' not in named_url:
             # special case for deprecated job template case
             # will not raise a 404 on its own
@@ -178,6 +192,7 @@ class URLModificationMiddleware(MiddlewareMixin):
             old_path = request.path_info
         new_path = self._convert_named_url(old_path)
         if request.path_info != new_path:
+            request.environ['awx.named_url_rewritten'] = request.path
             request.path = request.path.replace(request.path_info, new_path)
             request.path_info = new_path
 

awx/main/notifications/grafana_backend.py

@@ -94,8 +94,8 @@ class GrafanaBackend(AWXBaseEmailBackend, CustomNotificationBase):
                               headers=grafana_headers,
                               verify=(not self.grafana_no_verify_ssl))
             if r.status_code >= 400:
-                logger.error(smart_text(_("Error sending notification grafana: {}").format(r.text)))
+                logger.error(smart_text(_("Error sending notification grafana: {}").format(r.status_code)))
                 if not self.fail_silently:
-                    raise Exception(smart_text(_("Error sending notification grafana: {}").format(r.text)))
+                    raise Exception(smart_text(_("Error sending notification grafana: {}").format(r.status_code)))
             sent_messages += 1
         return sent_messages

awx/main/notifications/mattermost_backend.py

@@ -46,8 +46,8 @@ class MattermostBackend(AWXBaseEmailBackend, CustomNotificationBase):
             r = requests.post("{}".format(m.recipients()[0]),
                               json=payload, verify=(not self.mattermost_no_verify_ssl))
             if r.status_code >= 400:
-                logger.error(smart_text(_("Error sending notification mattermost: {}").format(r.text)))
+                logger.error(smart_text(_("Error sending notification mattermost: {}").format(r.status_code)))
                 if not self.fail_silently:
-                    raise Exception(smart_text(_("Error sending notification mattermost: {}").format(r.text)))
+                    raise Exception(smart_text(_("Error sending notification mattermost: {}").format(r.status_code)))
             sent_messages += 1
         return sent_messages

awx/main/notifications/rocketchat_backend.py

@@ -46,9 +46,9 @@ class RocketChatBackend(AWXBaseEmailBackend, CustomNotificationBase):
 
             if r.status_code >= 400:
                 logger.error(smart_text(
-                    _("Error sending notification rocket.chat: {}").format(r.text)))
+                    _("Error sending notification rocket.chat: {}").format(r.status_code)))
                 if not self.fail_silently:
                     raise Exception(smart_text(
-                        _("Error sending notification rocket.chat: {}").format(r.text)))
+                        _("Error sending notification rocket.chat: {}").format(r.status_code)))
             sent_messages += 1
         return sent_messages

awx/main/notifications/webhook_backend.py

@@ -72,8 +72,8 @@ class WebhookBackend(AWXBaseEmailBackend, CustomNotificationBase):
                               headers=self.headers,
                               verify=(not self.disable_ssl_verification))
             if r.status_code >= 400:
-                logger.error(smart_text(_("Error sending notification webhook: {}").format(r.text)))
+                logger.error(smart_text(_("Error sending notification webhook: {}").format(r.status_code)))
                 if not self.fail_silently:
-                    raise Exception(smart_text(_("Error sending notification webhook: {}").format(r.text)))
+                    raise Exception(smart_text(_("Error sending notification webhook: {}").format(r.status_code)))
             sent_messages += 1
         return sent_messages

awx/main/tests/functional/test_named_url.py

@@ -219,3 +219,27 @@ def test_credential(get, admin_user, credentialtype_ssh):
     url = reverse('api:credential_detail', kwargs={'pk': test_cred.pk})
     response = get(url, user=admin_user, expect=200)
     assert response.data['related']['named_url'].endswith('/test_cred++Machine+ssh++/')
+
+
+@pytest.mark.django_db
+def test_403_vs_404(get):
+    cindy = User.objects.create(
+        username='cindy',
+        password='test_user',
+        is_superuser=False
+    )
+    bob = User.objects.create(
+        username='bob',
+        password='test_user',
+        is_superuser=False
+    )
+
+    # bob cannot see cindy, pk lookup should be a 403
+    url = reverse('api:user_detail', kwargs={'pk': cindy.pk})
+    get(url, user=bob, expect=403)
+
+    # bob cannot see cindy, username lookup should be a 404
+    get('/api/v2/users/cindy/', user=bob, expect=404)
+
+    get(f'/api/v2/users/{cindy.pk}/', expect=401)
+    get('/api/v2/users/cindy/', expect=404)

awx/main/tests/functional/test_rbac_label.py

@@ -20,8 +20,19 @@ def test_label_get_queryset_su(label, user):
 
 
 @pytest.mark.django_db
-def test_label_access(label, user):
+def test_label_read_access(label, user):
     access = LabelAccess(user('user', False))
+    assert not access.can_read(label)
+    label.organization.member_role.members.add(user('user', False))
+    assert access.can_read(label)
+
+
+@pytest.mark.django_db
+def test_label_jt_read_access(label, user, job_template):
+    access = LabelAccess(user('user', False))
+    assert not access.can_read(label)
+    job_template.read_role.members.add(user('user', False))
+    job_template.labels.add(label)
     assert access.can_read(label)