mirror of
https://github.com/ansible/awx.git
synced 2026-02-14 17:50:02 -03:30
update the named URL code to properly return 404 vs 403
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user