Skip to content

Commit

Permalink
Separate private and public landing requests
Browse files Browse the repository at this point in the history
Private requests must be claimed before use. Public / Anonymous requests
only depend on the public flag, so anoymous users can create private
landing requests and authenticated users can create public requests.

Tests should cover all of these scenarios.
  • Loading branch information
mvdbeek committed Oct 20, 2024
1 parent 2b89fef commit a611aa9
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 23 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ remove-api-schema:
rm _schema.yaml
rm _shed_schema.yaml

update-client-api-schema: client-node-deps build-api-schema
update-client-api-schema: build-api-schema
$(IN_VENV) cd client && npx openapi-typescript ../_schema.yaml > src/api/schema/schema.ts && npx prettier --write src/api/schema/schema.ts
$(IN_VENV) cd client && npx openapi-typescript ../_shed_schema.yaml > ../lib/tool_shed/webapp/frontend/src/schema/schema.ts && npx prettier --write ../lib/tool_shed/webapp/frontend/src/schema/schema.ts
$(MAKE) remove-api-schema
Expand Down
6 changes: 6 additions & 0 deletions client/src/api/schema/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7103,6 +7103,12 @@ export interface components {
CreateWorkflowLandingRequestPayload: {
/** Client Secret */
client_secret?: string | null;
/**
* Public
* @description If workflow landing request is public anyone with the uuid can use the landing request. If not public the request must be claimed before use and additional verification might occur.
* @default false
*/
public: boolean | null;
/** Request State */
request_state?: Record<string, never> | null;
/** Workflow Id */
Expand Down
4 changes: 4 additions & 0 deletions lib/galaxy/exceptions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ class Conflict(MessageException):
err_code = error_codes_by_name["CONFLICT"]


class ItemMustBeClaimed(Conflict):
err_code = error_codes_by_name["MUST_CLAIM"]


class DeprecatedMethod(MessageException):
"""
Method (or a particular form/arg signature) has been removed and won't be available later
Expand Down
5 changes: 5 additions & 0 deletions lib/galaxy/exceptions/error_codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@
"code": 409001,
"message": "Database conflict prevented fulfilling the request."
},
{
"name": "MUST_CLAIM",
"code": 409010,
"message": "Private request must be claimed before use"
},
{
"name": "DEPRECATED_API_CALL",
"code": 410001,
Expand Down
5 changes: 5 additions & 0 deletions lib/galaxy/managers/landing.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from galaxy.exceptions import (
InsufficientPermissionsException,
ItemAlreadyClaimedException,
ItemMustBeClaimed,
ObjectNotFound,
RequestParameterMissingException,
)
Expand Down Expand Up @@ -59,6 +60,7 @@ def create_tool_landing_request(self, payload: CreateToolLandingRequestPayload,
model.request_state = payload.request_state
model.uuid = uuid4()
model.client_secret = payload.client_secret
model.public = payload.public
if user_id:
model.user_id = user_id
self._save(model)
Expand All @@ -77,6 +79,7 @@ def create_workflow_landing_request(self, payload: CreateWorkflowLandingRequestP
model.uuid = uuid4()
model.client_secret = payload.client_secret
model.request_state = payload.request_state
model.public = payload.public
self._save(model)
return self._workflow_response(model)

Expand Down Expand Up @@ -201,6 +204,8 @@ def _workflow_response(self, model: WorkflowLandingRequestModel) -> WorkflowLand
return response_model

def _check_ownership(self, trans: ProvidesUserContext, model: LandingRequestModel):
if not model.public and self._state(model) == LandingRequestState.UNCLAIMED:
raise ItemMustBeClaimed
if model.user_id and model.user_id != trans.user.id:
raise InsufficientPermissionsException()

Expand Down
2 changes: 2 additions & 0 deletions lib/galaxy/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11264,6 +11264,7 @@ class ToolLandingRequest(Base):
tool_version: Mapped[Optional[str]] = mapped_column(String(255), default=None)
request_state: Mapped[Optional[Dict]] = mapped_column(JSONType)
client_secret: Mapped[Optional[str]] = mapped_column(String(255), default=None)
public: Mapped[bool] = mapped_column(Boolean)

user: Mapped[Optional["User"]] = relationship()

Expand All @@ -11284,6 +11285,7 @@ class WorkflowLandingRequest(Base):
client_secret: Mapped[Optional[str]] = mapped_column(String(255), default=None)
workflow_source: Mapped[Optional[str]] = mapped_column(String(255), default=None)
workflow_source_type: Mapped[Optional[str]] = mapped_column(String(255), default=None)
public: Mapped[bool] = mapped_column(Boolean)

user: Mapped[Optional["User"]] = relationship()
stored_workflow: Mapped[Optional["StoredWorkflow"]] = relationship()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Add workflow_source and workflow_source_type column
"""Add workflow_source and workflow_source_type and public column
Revision ID: a99a5b52ccb8
Revises: 7ffd33d5d144
Expand All @@ -10,6 +10,7 @@

from galaxy.model.migrations.util import (
add_column,
column_exists,
drop_column,
transaction,
)
Expand All @@ -20,17 +21,27 @@
branch_labels = None
depends_on = None

table_name = "workflow_landing_request"
column_names = ["workflow_source", "workflow_source_type"]
workflow_table_name = "workflow_landing_request"
tool_table_name = "tool_landing_request"


def drop_if_exists(table_name: str, column_name: str):
if column_exists(table_name, column_name, True):
drop_column(table_name, column_name)


def upgrade():
with transaction():
for column_name in column_names:
add_column(table_name, sa.Column(column_name, sa.String(255), nullable=True))
add_column(workflow_table_name, sa.Column("workflow_source", sa.String(255), nullable=True))
add_column(workflow_table_name, sa.Column("workflow_source_type", sa.String(255), nullable=True))
add_column(workflow_table_name, sa.Column("public", sa.Boolean, nullable=False))
add_column(tool_table_name, sa.Column("public", sa.Boolean, nullable=False))


def downgrade():
with transaction():
for column_name in column_names:
drop_column(table_name, column_name)
drop_column(workflow_table_name, "workflow_source")
drop_column(workflow_table_name, "workflow_source_type")
# For test.galaxyproject.org which was deployed from PR branch that didn't contain public column
drop_if_exists(workflow_table_name, "public")
drop_if_exists(tool_table_name, "public")
5 changes: 5 additions & 0 deletions lib/galaxy/schema/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -3854,13 +3854,18 @@ class CreateToolLandingRequestPayload(Model):
tool_version: Optional[str] = None
request_state: Optional[Dict[str, Any]] = None
client_secret: Optional[str] = None
public: bool = False


class CreateWorkflowLandingRequestPayload(Model):
workflow_id: str
workflow_target_type: Literal["stored_workflow", "workflow", "trs_url"]
request_state: Optional[Dict[str, Any]] = None
client_secret: Optional[str] = None
public: Optional[bool] = Field(
False,
description="If workflow landing request is public anyone with the uuid can use the landing request. If not public the request must be claimed before use and additional verification might occur.",
)


class ClaimLandingPayload(Model):
Expand Down
3 changes: 3 additions & 0 deletions lib/galaxy/webapps/galaxy/api/workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
BaseGalaxyAPIController,
depends,
DependsOnTrans,
DependsOnUser,
IndexQueryTag,
LandingUuidPathParam,
Router,
Expand Down Expand Up @@ -1179,6 +1180,7 @@ def claim_landing(
trans: ProvidesUserContext = DependsOnTrans,
uuid: UUID4 = LandingUuidPathParam,
payload: Optional[ClaimLandingPayload] = Body(...),
user: model.User = DependsOnUser,
) -> WorkflowLandingRequest:
return self.landing_manager.claim_workflow_landing_request(trans, uuid, payload)

Expand All @@ -1187,6 +1189,7 @@ def get_landing(
self,
trans: ProvidesUserContext = DependsOnTrans,
uuid: UUID4 = LandingUuidPathParam,
user: model.User = DependsOnUser,
) -> WorkflowLandingRequest:
return self.landing_manager.get_workflow_landing_request(trans, uuid)

Expand Down
144 changes: 129 additions & 15 deletions lib/galaxy_test/api/test_landing.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
Dict,
)

from galaxy.schema.schema import CreateWorkflowLandingRequestPayload
from galaxy.schema.schema import (
CreateWorkflowLandingRequestPayload,
WorkflowLandingRequest,
)
from galaxy_test.base.populators import (
DatasetPopulator,
skip_without_tool,
Expand All @@ -23,22 +26,85 @@ def setUp(self):
self.workflow_populator = WorkflowPopulator(self.galaxy_interactor)

@skip_without_tool("cat1")
def test_workflow_landing(self):
workflow_id = self.workflow_populator.simple_workflow("test_landing")
workflow_target_type = "stored_workflow"
request_state = _workflow_request_state()
request = CreateWorkflowLandingRequestPayload(
workflow_id=workflow_id,
workflow_target_type=workflow_target_type,
request_state=request_state,
)
def test_create_public_workflow_landing_authenticated_user(self):
request = _get_simple_landing_payload(self.workflow_populator, public=True)
response = self.dataset_populator.create_workflow_landing(request)
assert response.workflow_id == request.workflow_id
assert response.workflow_target_type == request.workflow_target_type

with self._different_user():
# Can use without claim
_can_use_request(self.dataset_populator, response)

with self._different_user(anon=True):
# Cannot claim since can't run workflows
_cannot_use_request(self.dataset_populator, response)

# Should claim of public landing request be denied ?
# Yes, so other user cannot own workflow in between use and invocation submission
# No, since there's no way to delete a landing request. If you accidentally make something
# with a private reference public you can't delete the landing page request.
# TODO: allow deleting landing request, deny claiming public requests ?

with self._different_user():
_can_claim_request(self.dataset_populator, response)
_can_use_request(self.dataset_populator, response)

# Cannot use if other user claimed
_cannot_claim_request(self.dataset_populator, response)

@skip_without_tool("cat1")
def test_create_public_workflow_landing_anonymous_user(self):
# Anonymous user can create public landing request
request = _get_simple_landing_payload(self.workflow_populator, public=True)
with self._different_user(anon=True):
response = self.dataset_populator.create_workflow_landing(request)

with self._different_user():
# Can use without claim
_can_use_request(self.dataset_populator, response)

with self._different_user(anon=True):
# Cannot claim since can't run workflows
_cannot_use_request(self.dataset_populator, response)

with self._different_user():
_can_claim_request(self.dataset_populator, response)
_can_use_request(self.dataset_populator, response)

# Cannot use if other user claimed
_cannot_claim_request(self.dataset_populator, response)

@skip_without_tool("cat1")
def test_create_private_workflow_landing_authenticated_user(self):
request = _get_simple_landing_payload(self.workflow_populator, public=False)
response = self.dataset_populator.create_workflow_landing(request)
assert response.workflow_id == workflow_id
assert response.workflow_target_type == workflow_target_type
with self._different_user():
# Must be claimed first
_cannot_use_request(self.dataset_populator, response, expect_status_code=409)
_can_claim_request(self.dataset_populator, response)
# Can be used after claim by same user
_can_use_request(self.dataset_populator, response)

# other user claimed, so we can't use
_cannot_claim_request(self.dataset_populator, response)
_cannot_use_request(self.dataset_populator, response)

@skip_without_tool("cat1")
def test_create_private_workflow_landing_anonymous_user(self):
request = _get_simple_landing_payload(self.workflow_populator, public=False)
with self._different_user(anon=True):
response = self.dataset_populator.create_workflow_landing(request)
with self._different_user():
# Must be claimed first
_cannot_use_request(self.dataset_populator, response, expect_status_code=409)
_can_claim_request(self.dataset_populator, response)
# Can be used after claim by same user
_can_use_request(self.dataset_populator, response)

response = self.dataset_populator.claim_workflow_landing(response.uuid)
assert response.workflow_id == workflow_id
assert response.workflow_target_type == workflow_target_type
# other user claimed, so we can't use
_cannot_claim_request(self.dataset_populator, response)
_cannot_use_request(self.dataset_populator, response)


def _workflow_request_state() -> Dict[str, Any]:
Expand All @@ -50,3 +116,51 @@ def _workflow_request_state() -> Dict[str, Any]:
"WorkflowInput2": {"src": "url", "url": f"base64://{input_b64_2}", "ext": "txt", "deferred": deferred},
}
return inputs


def _get_simple_landing_payload(workflow_populator: WorkflowPopulator, public: bool = False):
workflow_id = workflow_populator.simple_workflow("test_landing")
if public:
workflow_populator.make_public(workflow_id)
workflow_target_type = "stored_workflow"
request_state = _workflow_request_state()
return CreateWorkflowLandingRequestPayload(
workflow_id=workflow_id,
workflow_target_type=workflow_target_type,
request_state=request_state,
public=public,
)


def _can_claim_request(dataset_populator: DatasetPopulator, request: WorkflowLandingRequest):
response = dataset_populator.claim_workflow_landing(request.uuid)
assert response.workflow_id == request.workflow_id
assert response.workflow_target_type == request.workflow_target_type


def _cannot_claim_request(dataset_populator: DatasetPopulator, request: WorkflowLandingRequest):
exception_encountered = False
try:
_can_claim_request(dataset_populator, request)
except Exception as e:
assert "Request status code (403)" in str(e)
exception_encountered = True
assert exception_encountered, "Expected claim to fail"


def _can_use_request(dataset_populator: DatasetPopulator, request: WorkflowLandingRequest):
response = dataset_populator.use_workflow_landing(request.uuid)
assert response.workflow_id == request.workflow_id
assert response.workflow_target_type == request.workflow_target_type


def _cannot_use_request(
dataset_populator: DatasetPopulator, request: WorkflowLandingRequest, expect_status_code: int = 403
):
exception_encountered = False
try:
_can_use_request(dataset_populator, request)
except Exception as e:
assert f"Request status code ({expect_status_code})" in str(e)
exception_encountered = True
assert exception_encountered, "Expected landing page usage to fail"
6 changes: 6 additions & 0 deletions lib/galaxy_test/base/populators.py
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,12 @@ def claim_workflow_landing(self, uuid: UUID4) -> WorkflowLandingRequest:
api_asserts.assert_status_code_is(claim_response, 200)
return WorkflowLandingRequest.model_validate(claim_response.json())

def use_workflow_landing(self, uuid: UUID4) -> WorkflowLandingRequest:
url = f"workflow_landings/{uuid}"
landing_reponse = self._get(url, {"client_secret": "foobar"})
api_asserts.assert_status_code_is(landing_reponse, 200)
return WorkflowLandingRequest.model_validate(landing_reponse.json())

def create_tool_from_path(self, tool_path: str) -> Dict[str, Any]:
tool_directory = os.path.dirname(os.path.abspath(tool_path))
payload = dict(
Expand Down

0 comments on commit a611aa9

Please sign in to comment.