Skip to content
Open
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
13 changes: 13 additions & 0 deletions app/schemas/workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import base64
from datetime import datetime
from enum import StrEnum
from typing import Any, Literal
Expand Down Expand Up @@ -99,6 +100,18 @@ class WorkflowUserDetails(BaseModel):
user_email: str = Field(..., description="Email address of the user")
ip_address: str = Field(..., description="IP address of the user")

def get_encoded_account_details(self) -> str:
"""Return the `-A` account string for cluster job submission.

Encodes email and IP as base64 (colon-joined) so the account string
never leaks the user's raw email/IP into cluster logs or job metadata.
"""
encoded_email = base64.b64encode(self.user_email.encode()).decode()
if not self.ip_address:
return encoded_email
encoded_ip = base64.b64encode(self.ip_address.encode()).decode()
return f"{encoded_email}:{encoded_ip}"


class WispsFormData(WorkflowFormData):
"""Form data for WISPS workflows (interaction-screening, bulk-prediction)."""
Expand Down
9 changes: 2 additions & 7 deletions app/services/bindflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import Any

from ..schemas.workflows import WorkflowUserDetails
from .cluster_utils import GADI_PROJECT, encode_ip
from .cluster_utils import GADI_PROJECT
from .workflow_config_fetcher import fetch_workflow_config


Expand All @@ -31,11 +31,6 @@ def get_bindflow_config_text(
"""Read bindflow base config and append a process override block with runtime values."""
base = fetch_workflow_config(config_file_path)

account = (
f"{user_details.user_email}:{encode_ip(user_details.ip_address)}"
if user_details.ip_address
else user_details.user_email
)
cluster_opts = f"-A {account}"
cluster_opts = f"-A {user_details.get_encoded_account_details()}"
override = f'\nprocess {{\n clusterOptions = "{cluster_opts}"\n}}\n'
return base + override
6 changes: 0 additions & 6 deletions app/services/cluster_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@

from __future__ import annotations

import base64
import os

GADI_PROJECT: str = os.getenv("GADI_PROJECT", "yz52")


def encode_ip(ip_address: str) -> str:
"""Return the base64 encoding of an IP address string."""
return base64.b64encode(ip_address.encode()).decode()
8 changes: 1 addition & 7 deletions app/services/proteindj_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from typing import Any

from ..schemas.workflows import WorkflowUserDetails
from .cluster_utils import encode_ip
from .workflow_config_fetcher import fetch_workflow_config


Expand Down Expand Up @@ -43,11 +42,6 @@ def get_proteindj_config_text(
"""Read proteindj base config and append a process override block with runtime values."""
base = fetch_workflow_config(config_file_path)

account = (
f"{user_details.user_email}:{encode_ip(user_details.ip_address)}"
if user_details.ip_address
else user_details.user_email
)
cluster_opts = f"-A {account}"
cluster_opts = f"-A {user_details.get_encoded_account_details()}"
override = f'\nprocess {{\n clusterOptions = "{cluster_opts}"\n}}\n'
return base + override
9 changes: 2 additions & 7 deletions app/services/proteinfold_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import Any

from ..schemas.workflows import WorkflowUserDetails
from .cluster_utils import GADI_PROJECT, encode_ip
from .cluster_utils import GADI_PROJECT
from .workflow_config_fetcher import fetch_workflow_config


Expand All @@ -29,11 +29,6 @@ def get_proteinfold_config_text(
"""Read proteinfold base config and append a process override block with runtime values."""
base = fetch_workflow_config(config_file_path)

account = (
f"{user_details.user_email}:{encode_ip(user_details.ip_address)}"
if user_details.ip_address
else user_details.user_email
)
cluster_opts = f"-P {GADI_PROJECT} -A {account}"
cluster_opts = f"-P {GADI_PROJECT} -A {user_details.get_encoded_account_details()}"
override = f'\nprocess {{\n clusterOptions = "{cluster_opts}"\n}}\n'
return base + override
9 changes: 2 additions & 7 deletions app/services/wisps_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import Any, Literal

from ..schemas.workflows import WorkflowUserDetails
from .cluster_utils import GADI_PROJECT, encode_ip
from .cluster_utils import GADI_PROJECT
from .workflow_config_fetcher import fetch_workflow_config

WispsMode = Literal["g1-g2", "manual"]
Expand Down Expand Up @@ -49,11 +49,6 @@ def get_wisps_config_text(
"""
base = fetch_workflow_config(config_file_path)

account = (
f"{user_details.user_email}:{encode_ip(user_details.ip_address)}"
if user_details.ip_address
else user_details.user_email
)
cluster_opts = f"-P {GADI_PROJECT} -A {account}"
cluster_opts = f"-P {GADI_PROJECT} -A {user_details.get_encoded_account_details()}"
override = f'\nprocess {{\n clusterOptions = "{cluster_opts}"\n}}\n'
return base + override
8 changes: 4 additions & 4 deletions tests/test_proteindj_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,20 +221,20 @@ def test_get_proteindj_config_text_appends_process_block():
assert "clusterOptions" in result


def test_get_proteindj_config_text_contains_email_and_encoded_ip():
def test_get_proteindj_config_text_contains_encoded_email_and_encoded_ip():
with patch("builtins.open", mock_open(read_data="base_config")):
result = get_proteindj_config_text("/fake/proteindj.config", user_details=_USER_DETAILS)
assert "user@ex.com" in result
assert "dXNlckBleC5jb20=" in result
assert "MS4yLjMuNA==" in result


def test_get_proteindj_config_text_without_ip_uses_email_only():
def test_get_proteindj_config_text_without_ip_uses_encoded_email_only():
with patch("builtins.open", mock_open(read_data="base_config")):
result = get_proteindj_config_text(
"/fake/proteindj.config",
user_details=_USER_DETAILS.model_copy(update={"ip_address": ""}),
)
assert "-A user@ex.com" in result
assert "-A dXNlckBleC5jb20=" in result
assert ":" not in result.split("clusterOptions = ")[1]


Expand Down
8 changes: 4 additions & 4 deletions tests/test_proteinfold_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,23 +547,23 @@ def test_get_proteinfold_config_text_appends_process_block():
assert "clusterOptions" in result


def test_get_proteinfold_config_text_contains_email_and_encoded_ip():
def test_get_proteinfold_config_text_contains_encoded_email_and_encoded_ip():
with patch("builtins.open", mock_open(read_data="base_config")):
result = get_proteinfold_config_text(
"/fake/proteinfold.config",
user_details=_USER_DETAILS,
)
assert "user@ex.com" in result
assert "dXNlckBleC5jb20=" in result
assert "MS4yLjMuNA==" in result


def test_get_proteinfold_config_text_without_ip_uses_email_only():
def test_get_proteinfold_config_text_without_ip_uses_encoded_email_only():
with patch("builtins.open", mock_open(read_data="base_config")):
result = get_proteinfold_config_text(
"/fake/proteinfold.config",
user_details=_USER_DETAILS.model_copy(update={"ip_address": ""}),
)
assert "-A user@ex.com" in result
assert "-A dXNlckBleC5jb20=" in result
assert ":" not in result.split("clusterOptions = ")[1]


Expand Down
11 changes: 11 additions & 0 deletions tests/test_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
WorkflowLaunchForm,
WorkflowLaunchPayload,
WorkflowLaunchResponse,
WorkflowUserDetails,
map_pipeline_status_to_ui,
)

Expand Down Expand Up @@ -462,3 +463,13 @@ def test_interaction_screening_request_extra_fields_forbidden():
runId="run-1",
extra="bad",
)


def test_get_encoded_account_details_encodes_email_and_ip():
details = WorkflowUserDetails(user_email="user@example.com", ip_address="1.2.3.4")
assert details.get_encoded_account_details() == "dXNlckBleGFtcGxlLmNvbQ==:MS4yLjMuNA=="


def test_get_encoded_account_details_without_ip_encodes_email_only():
details = WorkflowUserDetails(user_email="user@example.com", ip_address="")
assert details.get_encoded_account_details() == "dXNlckBleGFtcGxlLmNvbQ=="
6 changes: 3 additions & 3 deletions tests/test_services_bindflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def test_get_bindflow_config_text_interpolates_email():
"/fake/bindflow.config",
user_details=_user_details("alice@example.com"),
)
assert "-A alice@example.com" in result
assert "-A YWxpY2VAZXhhbXBsZS5jb20=" in result


def test_get_bindflow_config_text_without_ip_address_omits_encoding():
Expand All @@ -154,7 +154,7 @@ def test_get_bindflow_config_text_without_ip_address_omits_encoding():
"/fake/bindflow.config",
user_details=_user_details("user@example.com"),
)
assert "-A user@example.com" in result
assert "-A dXNlckBleGFtcGxlLmNvbQ==" in result
assert ":" not in result.split("clusterOptions = ")[1]


Expand All @@ -164,7 +164,7 @@ def test_get_bindflow_config_text_with_ip_address_appends_encoded_ip():
"/fake/bindflow.config",
user_details=_user_details("user@example.com", ip_address="1.2.3.4"),
)
assert "-A user@example.com:MS4yLjMuNA==" in result
assert "-A dXNlckBleGFtcGxlLmNvbQ==:MS4yLjMuNA==" in result


def test_get_bindflow_config_text_url_fetching():
Expand Down
15 changes: 0 additions & 15 deletions tests/test_services_cluster_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,9 @@
# pylint: disable=missing-function-docstring
from __future__ import annotations

import base64
import importlib

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have some simple unit tests of encoding email addresses, to show the expected input and output

from app.services import cluster_utils
from app.services.cluster_utils import encode_ip


def test_encode_ip_returns_base64_of_ip_string():
assert encode_ip("1.2.3.4") == base64.b64encode(b"1.2.3.4").decode()


def test_encode_ip_empty_string():
assert encode_ip("") == ""


def test_encode_ip_ipv6_address():
ip = "2001:db8::1"
assert encode_ip(ip) == base64.b64encode(ip.encode()).decode()


def test_gadi_project_defaults_to_yz52(monkeypatch):
Expand Down
8 changes: 4 additions & 4 deletions tests/test_wisps_coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,23 +262,23 @@ def test_get_wisps_config_text_appends_process_block():
assert "clusterOptions" in result


def test_get_wisps_config_text_contains_email_and_encoded_ip():
def test_get_wisps_config_text_contains_encoded_email_and_encoded_ip():
with patch("builtins.open", mock_open(read_data="base_config")):
result = get_wisps_config_text(
config_file_path="/fake/path.config",
user_details=_USER_DETAILS,
)
assert "user@ex.com" in result
assert "dXNlckBleC5jb20=" in result
assert "MS4yLjMuNA==" in result


def test_get_wisps_config_text_without_ip_uses_email_only():
def test_get_wisps_config_text_without_ip_uses_encoded_email_only():
with patch("builtins.open", mock_open(read_data="base_config")):
result = get_wisps_config_text(
config_file_path="/fake/path.config",
user_details=_USER_DETAILS.model_copy(update={"ip_address": ""}),
)
assert "-A user@ex.com" in result
assert "-A dXNlckBleC5jb20=" in result
assert ":" not in result.split("clusterOptions = ")[1]


Expand Down