Skip to content

Commit

Permalink
Backport fix related to merging of redirection URL query string param…
Browse files Browse the repository at this point in the history
…eters.
  • Loading branch information
GrahamDumpleton committed Aug 22, 2024
1 parent 0f012f2 commit c48b04f
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 24 deletions.
1 change: 1 addition & 0 deletions project-docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Educates
:maxdepth: 2
:caption: Release Notes:

release-notes/version-2.7.4
release-notes/version-2.7.3
release-notes/version-2.7.2
release-notes/version-2.7.1
Expand Down
19 changes: 19 additions & 0 deletions project-docs/release-notes/version-2.7.4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Version 2.7.4
=============

Upcoming Changes
----------------

For details on significant changes in future versions, including feature
deprecations and removals which may necessitate updates to existing workshops,
see [Upcoming changes](upcoming-changes).

Backported Changes
------------------

* When an index URL was supplied to the training portal in the `TrainingPortal`
resource, or via the REST API, if the URL had query string parameters, the
query string param added by the training platform with the notification
message, was not being merged with the existing set of query string parameters
and was instead being added to the value of the last query string parameter in
the URL.
33 changes: 27 additions & 6 deletions training-portal/src/project/apps/workshops/views/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from ..manager.sessions import retrieve_session_for_user
from ..manager.locking import resources_lock
from ..models import TrainingPortal, Environment, EnvironmentState, SessionState
from .helpers import update_query_params


@login_required
Expand All @@ -56,12 +57,22 @@ def environment(request, name):
instance = Environment.objects.get(name=name)
except Environment.DoesNotExist:
if index_url:
return redirect(index_url + "?notification=workshop-invalid")
return redirect(
update_query_params(index_url, {"notification": "workshop-invalid"})
)

if not request.user.is_staff and settings.PORTAL_INDEX:
return redirect(settings.PORTAL_INDEX + "?notification=workshop-invalid")
return redirect(
update_query_params(
settings.PORTAL_INDEX, {"notification": "workshop-invalid"}
)
)

return redirect(reverse("workshops_catalog") + "?notification=workshop-invalid")
return redirect(
update_query_params(
reverse("workshops_catalog"), {"notification": "workshop-invalid"}
)
)

# Retrieve a session for the user for this workshop environment.

Expand All @@ -71,12 +82,22 @@ def environment(request, name):
return redirect("workshops_session", name=session.name)

if index_url:
return redirect(index_url + "?notification=session-unavailable")
return redirect(
update_query_params(index_url, {"notification": "session-unavailable"})
)

if not request.user.is_staff and settings.PORTAL_INDEX:
return redirect(settings.PORTAL_INDEX + "?notification=session-unavailable")
return redirect(
update_query_params(
settings.PORTAL_INDEX, {"notification": "session-unavailable"}
)
)

return redirect(reverse("workshops_catalog") + "?notification=session-unavailable")
return redirect(
update_query_params(
reverse("workshops_catalog"), {"notification": "session-unavailable"}
)
)


@require_http_methods(["GET"])
Expand Down
46 changes: 46 additions & 0 deletions training-portal/src/project/apps/workshops/views/helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
"""Helper functions for dealing with URLs."""

from urllib.parse import urlparse, parse_qs, urlencode, urlunparse, urljoin


def update_query_params(url, params):
"""Update the query parameters of a URL with the given parameters. If the URL
is malformed or cannot be parsed, the original URL is returned."""

try:
# Parse the URL.

parsed_url = urlparse(url)

# Handle URLs with no scheme or netloc (e.g., paths like '/page').

if not parsed_url.scheme and not parsed_url.netloc:
# Treat it as a relative path URL.

base_url = "http://dummy" # Temporary base for relative path handling
parsed_url = urlparse(urljoin(base_url, url))

# Parse existing query parameters.

query_params = parse_qs(parsed_url.query)

# Update or add the new parameters.

query_params.update({key: [value] for key, value in params.items()})

# Reconstruct the URL with the updated query string.

updated_query = urlencode(query_params, doseq=True)
updated_url = urlunparse(parsed_url._replace(query=updated_query))

# If the URL was originally a path, strip out the dummy scheme and netloc.

if parsed_url.scheme == "http" and parsed_url.netloc == "dummy":
return updated_url.replace("http://dummy", "")

return updated_url

except Exception: # pylint: disable=broad-except
# In case of any parsing errors, return the original URL.

return url
63 changes: 45 additions & 18 deletions training-portal/src/project/apps/workshops/views/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
from ..manager.sessions import update_session_status, create_request_resources
from ..manager.analytics import report_analytics_event
from ..models import TrainingPortal, SessionState
from .helpers import update_query_params


@login_required(login_url="/")
Expand All @@ -67,25 +68,35 @@ def session(request, name):

if not instance:
if index_url:
return redirect(index_url + "?notification=session-invalid")
return redirect(
update_query_params(index_url, {"notification": "session-invalid"})
)

if not request.user.is_staff and settings.PORTAL_INDEX:
return redirect(settings.PORTAL_INDEX + "?notification=session-invalid")

return redirect(reverse("workshops_catalog") + "?notification=session-invalid")
return redirect(
update_query_params(
settings.PORTAL_INDEX, {"notification": "session-invalid"}
)
)

return redirect(
update_query_params(
reverse("workshops_catalog"), {"notification": "session-invalid"}
)
)

context["session"] = instance
context["session_owner"] = instance.owner and instance.owner.get_username() or ""

context[
"session_url"
] = f"{settings.INGRESS_PROTOCOL}://{instance.name}.{settings.INGRESS_DOMAIN}"
context["session_url"] = (
f"{settings.INGRESS_PROTOCOL}://{instance.name}.{settings.INGRESS_DOMAIN}"
)

portal_url = f"{settings.INGRESS_PROTOCOL}://{settings.PORTAL_HOSTNAME}"

context[
"restart_url"
] = f"{portal_url}/workshops/session/{instance.name}/delete/?notification=startup-timeout"
context["restart_url"] = (
f"{portal_url}/workshops/session/{instance.name}/delete/?notification=startup-timeout"
)
context["startup_timeout"] = instance.environment.overdue.total_seconds()

try:
Expand Down Expand Up @@ -217,12 +228,22 @@ def session_delete(request, name):

if not instance:
if index_url:
return redirect(index_url + "?notification=session-invalid")
return redirect(
update_query_params(index_url, {"notification": "session-invalid"})
)

if not request.user.is_staff and settings.PORTAL_INDEX:
return redirect(settings.PORTAL_INDEX + "?notification=session-invalid")

return redirect(reverse("workshops_catalog") + "?notification=session-invalid")
return redirect(
update_query_params(
settings.PORTAL_INDEX, {"notification": "session-invalid"}
)
)

return redirect(
update_query_params(
reverse("workshops_catalog"), {"notification": "session-invalid"}
)
)

# Mark the instance as stopping now so that it will not be picked up
# by the user again if they attempt to create a new session immediately.
Expand All @@ -239,12 +260,18 @@ def session_delete(request, name):
notification = "session-deleted"

if index_url:
return redirect(index_url + f"?notification={notification}")
return redirect(update_query_params(index_url, {"notification": notification}))

if not request.user.is_staff and settings.PORTAL_INDEX:
return redirect(settings.PORTAL_INDEX + f"?notification={notification}")

return redirect(reverse("workshops_catalog") + f"?notification={notification}")
return redirect(
update_query_params(settings.PORTAL_INDEX, {"notification": notification})
)

return redirect(
update_query_params(
reverse("workshops_catalog"), {"notification": notification}
)
)


@protected_resource()
Expand Down

0 comments on commit c48b04f

Please sign in to comment.