Unverified Commit fda88e48 authored by David López's avatar David López Committed by GitHub
Browse files

Merge pull request #14057 from mvdbeek/backport_session_fix

[21.09][Backport] Add sessioncookie with ``SameSite=None; secure`` for tool_runner path
parents 83612406 6679571b
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -1245,6 +1245,12 @@ class NavigatesGalaxy(HasDriver):
        self.driver.execute_script("arguments[0].scrollIntoView(true);", tool_element)
        tool_link.wait_for_and_click()

    def datasource_tool_open(self, tool_id):
        tool_link = self.components.tool_panel.data_source_tool_link(tool_id=tool_id)
        tool_element = tool_link.wait_for_present()
        self.driver.execute_script("arguments[0].scrollIntoView(true);", tool_element)
        tool_link.wait_for_and_click()

    def tool_parameter_div(self, expanded_parameter_id):
        return self.components.tool_form.parameter_div(parameter=expanded_parameter_id).wait_for_clickable()

+5 −4
Original line number Diff line number Diff line
@@ -331,6 +331,7 @@ tool_panel:
  selectors:
    tool_link: 'a[href$$="tool_runner?tool_id=${tool_id}"]'
    outer_tool_link: '.toolTitle a[href$$="tool_runner?tool_id=${tool_id}"]'
    data_source_tool_link: 'a[href$$="tool_runner/data_source_redirect?tool_id=${tool_id}"]'
    search: '.search-query'
    workflow_names: '#internal-workflows .toolTitle'
    views_button: '.tool-panel-dropdown'
+33 −2
Original line number Diff line number Diff line
@@ -9,7 +9,10 @@ import socket
import string
import time
from http.cookies import CookieError
from typing import Any, Dict
from typing import (
    Any,
    Dict,
)
from urllib.parse import urlparse

import mako.lookup
@@ -63,6 +66,8 @@ UCSC_SERVERS = (
    'hgw8.soe.ucsc.edu',
)

TOOL_RUNNER_SESSION_COOKIE = "galaxytoolrunnersession"


class WebApplication(base.WebApplication):
    """
@@ -379,14 +384,40 @@ class GalaxyWebTransaction(base.DefaultWebTransaction, context.ProvidesHistoryCo
            if name in self.response.cookies:
                return self.response.cookies[name].value
            else:
                if name not in self.request.cookies and TOOL_RUNNER_SESSION_COOKIE in self.request.cookies:
                    # TOOL_RUNNER_SESSION_COOKIE value is the encoded galaxysession cookie.
                    # We decode it here and pretend it's the galaxysession
                    tool_runner_path = url_for(controller="tool_runner")
                    if self.request.path.startswith(tool_runner_path):
                        return self.security.decode_guid(self.request.cookies[TOOL_RUNNER_SESSION_COOKIE].value)
                return self.request.cookies[name].value
        except Exception:
            return None

    def set_cookie(self, value, name='galaxysession', path='/', age=90, version='1'):
    def set_cookie(self, value, name="galaxysession", path="/", age=90, version="1"):
        self._set_cookie(value, name=name, path=path, age=age, version=version)
        if name == "galaxysession":
            # Set an extra sessioncookie that will only be sent and be accepted on the tool_runner path.
            # Use the id_secret to encode the sessioncookie, so if a malicious site
            # obtains the sessioncookie they can only run tools.
            self._set_cookie(
                value,
                name=TOOL_RUNNER_SESSION_COOKIE,
                path=url_for(controller="tool_runner"),
                age=age,
                version=version,
                encode_value=True,
            )
            tool_runner_cookie = self.response.cookies[TOOL_RUNNER_SESSION_COOKIE]
            tool_runner_cookie["SameSite"] = "None"
            tool_runner_cookie["secure"] = True

    def _set_cookie(self, value, name="galaxysession", path="/", age=90, version="1", encode_value=False):
        """Convenience method for setting a session cookie"""
        # The galaxysession cookie value must be a high entropy 128 bit random number encrypted
        # using a server secret key.  Any other value is invalid and could pose security issues.
        if encode_value:
            value = self.security.encode_guid(value)
        self.response.cookies[name] = unicodify(value)
        self.response.cookies[name]['path'] = path
        self.response.cookies[name]['max-age'] = 3600 * 24 * age  # 90 days
+27 −0
Original line number Diff line number Diff line
import base64
from urllib.parse import urljoin

from requests import get

@@ -27,3 +28,29 @@ class AuthenticationApiTestCase(ApiTestCase):
        random_api_url = self._api_url("users", use_key=False)
        random_api_response = get(random_api_url, params=dict(key=auth_dict["api_key"]))
        self._assert_status_code_is(random_api_response, 200)

    def test_tool_runner_session_cookie_handling(self):
        response = get(self.url)
        tool_runner_session_cookie = response.cookies["galaxytoolrunnersession"]
        galaxy_session_cookie = response.cookies["galaxysession"]
        assert tool_runner_session_cookie != galaxy_session_cookie
        root_response = get(self.url, cookies={"galaxytoolrunnersession": tool_runner_session_cookie})
        root_response.raise_for_status()
        # Browser will only send cookie to /tool_runner path, but let's make sure it isn't accepted.
        # Galaxy responds with a new session and sessioncookie in that case.
        # (We might want to redirect to the login page instead if require_login is set?)
        assert root_response.cookies["galaxysession"] != galaxy_session_cookie
        tool_runner_response = get(
            urljoin(self.url, "tool_runner?tool_id=test_data_source"),
            cookies={"galaxytoolrunnersession": tool_runner_session_cookie},
        )
        tool_runner_response.raise_for_status()
        # Verify that we're not returning the sessioncookie
        assert "galaxysession" not in tool_runner_response.cookies
        # Make sure history for original session received job
        current_history_json_response = get(
            urljoin(self.url, "history/current_history_json"), cookies={"galaxysession": galaxy_session_cookie}
        )
        current_history_json_response.raise_for_status()
        current_history = current_history_json_response.json()
        assert current_history["contents_active"]["active"] == 1
+29 −0
Original line number Diff line number Diff line
from galaxy_test.base.populators import skip_if_site_down
from .framework import (
    managed_history,
    selenium_test,
    SeleniumTestCase,
    UsesHistoryItemAssertions,
)


class DataSourceTestCase(SeleniumTestCase, UsesHistoryItemAssertions):

    ensure_registered = True

    @selenium_test
    @managed_history
    @skip_if_site_down("https://genome.ucsc.edu/cgi-bin/hgTables")
    def test_ucsc_table_direct1_data_source(self):
        self.home()
        self.datasource_tool_open("ucsc_table_direct1")
        self.screenshot("ucsc_table_browser_first_page")
        checkbox = self.wait_for_selector("#checkboxGalaxy")
        assert checkbox.get_attribute("checked") == "true"
        submit_button = self.wait_for_selector("#hgta_doTopSubmit")
        submit_button.click()
        self.screenshot("ucsc_table_browser_second_page")
        self.wait_for_selector("#hgta_doGalaxyQuery").click()
        self.history_panel_wait_for_hid_ok(1)
        # Make sure we're still logged in (xref https://github.com/galaxyproject/galaxy/issues/11374)
        self.components.masthead.logged_in_only.wait_for_visible()
Loading