Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prettier DRF pages when using trusted proxy #15579

Merged
merged 2 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions awx/api/generics.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from django.db import connection, transaction
from django.db.models.fields.related import OneToOneRel
from django.http import QueryDict
from django.shortcuts import get_object_or_404
from django.shortcuts import get_object_or_404, redirect
from django.template.loader import render_to_string
from django.utils.encoding import smart_str
from django.utils.safestring import mark_safe
Expand All @@ -36,7 +36,7 @@
# django-ansible-base
from ansible_base.rest_filters.rest_framework.field_lookup_backend import FieldLookupBackend
from ansible_base.lib.utils.models import get_all_field_names
from ansible_base.lib.utils.requests import get_remote_host
from ansible_base.lib.utils.requests import get_remote_host, is_proxied_request
from ansible_base.rbac.models import RoleEvaluation, RoleDefinition
from ansible_base.rbac.permission_registry import permission_registry
from ansible_base.jwt_consumer.common.util import validate_x_trusted_proxy_header
Expand Down Expand Up @@ -82,6 +82,12 @@

class LoggedLoginView(auth_views.LoginView):
def get(self, request, *args, **kwargs):
if is_proxied_request():
next = request.GET.get('next', "")

Check warning on line 86 in awx/api/generics.py

View check run for this annotation

Codecov / codecov/patch

awx/api/generics.py#L86

Added line #L86 was not covered by tests
if next:
next = f"?next={next}"
return redirect(f"/{next}")

Check warning on line 89 in awx/api/generics.py

View check run for this annotation

Codecov / codecov/patch

awx/api/generics.py#L88-L89

Added lines #L88 - L89 were not covered by tests
relrod marked this conversation as resolved.
Show resolved Hide resolved

# The django.auth.contrib login form doesn't perform the content
# negotiation we've come to expect from DRF; add in code to catch
# situations where Accept != text/html (or */*) and reply with
Expand All @@ -97,6 +103,15 @@
return super(LoggedLoginView, self).get(request, *args, **kwargs)

def post(self, request, *args, **kwargs):
if is_proxied_request():
relrod marked this conversation as resolved.
Show resolved Hide resolved
# Give a message, saying to login via AAP
return Response(

Check warning on line 108 in awx/api/generics.py

View check run for this annotation

Codecov / codecov/patch

awx/api/generics.py#L108

Added line #L108 was not covered by tests
{
'detail': _('Please log in via Platform Authentication.'),
},
status=status.HTTP_401_UNAUTHORIZED,
)

ret = super(LoggedLoginView, self).post(request, *args, **kwargs)
ip = get_remote_host(request) # request.META.get('REMOTE_ADDR', None)
if request.user.is_authenticated:
Expand All @@ -119,6 +134,12 @@
success_url_allowed_hosts = set(settings.LOGOUT_ALLOWED_HOSTS.split(",")) if settings.LOGOUT_ALLOWED_HOSTS else set()

def dispatch(self, request, *args, **kwargs):
if is_proxied_request():
# 1) We intentionally don't obey ?next= here, just always redirect to platform login
# 2) Hack to prevent rewrites of Location header
qs = "?__gateway_no_rewrite__=1&next=/"
return redirect(f"/api/gateway/v1/logout/{qs}")

Check warning on line 141 in awx/api/generics.py

View check run for this annotation

Codecov / codecov/patch

awx/api/generics.py#L140-L141

Added lines #L140 - L141 were not covered by tests

original_user = getattr(request, 'user', None)
ret = super(LoggedLogoutView, self).dispatch(request, *args, **kwargs)
current_user = getattr(request, 'user', None)
Expand Down
4 changes: 4 additions & 0 deletions awx/settings/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@
'social_django.context_processors.login_redirect',
],
'builtins': ['awx.main.templatetags.swagger'],
'libraries': {
"ansible_base.lib.templatetags.requests": "ansible_base.lib.templatetags.requests",
"ansible_base.lib.templatetags.util": "ansible_base.lib.templatetags.util",
},
},
'DIRS': [
os.path.join(BASE_DIR, 'templates'),
Expand Down
20 changes: 18 additions & 2 deletions awx/templates/rest_framework/api.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
{% extends 'rest_framework/base.html' %}
{% load i18n static %}
{% load i18n static ansible_base.lib.templatetags.requests ansible_base.lib.templatetags.util %}

{% block title %}{{ name }} · {% trans 'AWX REST API' %}{% endblock %}

{% block bootstrap_theme %}
{% is_proxied_request as proxied %}
<link rel="stylesheet" type="text/css" href="{% static 'rest_framework/css/bootstrap.min.css' %}" />
{% if proxied %}
<style>
{# inline_file from DAB #}
{% inline_file "static/api/api.css" True %}
relrod marked this conversation as resolved.
Show resolved Hide resolved
</style>
{% else %}
<link rel="stylesheet" type="text/css" href="{% static 'api/api.css' %}?v={{tower_version}}" />
{% endif %}
{% endblock %}

{% block style %}
Expand All @@ -24,7 +32,6 @@
<span class="icon-bar"></span>
</button>
<a class="navbar-brand" href="{% url 'api:api_root_view' %}">
<img class="logo" src="{% static 'media/logo-header.svg' %}">
<span>{% trans 'REST API' %}</span>
</a>
<a class="navbar-title" href="{{ request.get_full_path }}">
Expand Down Expand Up @@ -74,5 +81,14 @@
<a class="toggle-description js-tooltip" href="#" title="Show/Hide Description"><span class="glyphicon glyphicon-question-sign"></span></a>
</div>
{{ block.super }}

{% is_proxied_request as proxied %}
{% if proxied %}
<script>
{# inline_file from DAB #}
{% inline_file "static/api/api.js" True %}
</script>
{% else %}
<script src="{% static 'api/api.js' %}?v={{tower_version}}"></script>
{% endif %}
{% endblock %}
Loading