Skip to content

Commit

Permalink
Disable add-file-to-request button for output-checkers
Browse files Browse the repository at this point in the history
if they do not have explicit permission for the workspace.
  • Loading branch information
rebkwok committed Mar 6, 2024
1 parent 96da8c9 commit 4152bcc
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 4 deletions.
6 changes: 5 additions & 1 deletion airlock/templates/file_browser/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,14 @@
</form>
{% /card %}
{% /modal %}
{% else %}
{% elif file_in_request %}
{% #button type="button" disabled=True tooltip="This file has already been added to the current request" id="add-file-modal-button-disabled" %}
Add File to Request
{% /button %}
{% else %}
{% #button type="button" disabled=True tooltip="You do not have permission to add this file to a request" id="add-file-modal-button-disabled" %}
Add File to Request
{% /button %}
{% endif %}
{% elif is_author %}
<form action="" method="POST">
Expand Down
7 changes: 6 additions & 1 deletion airlock/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,13 @@ def from_session(cls, session_data):
def has_permission(self, workspace_name):
return (
# Output checkers can view all workspaces
self.output_checker or workspace_name in self.workspaces
self.output_checker or self.can_create_request(workspace_name)
)

def can_create_request(self, workspace_name):
# Only users with explict access to the workspace can create release
# requests.
return workspace_name in self.workspaces

def is_authenticated(self):
return True
10 changes: 8 additions & 2 deletions airlock/views/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,18 @@ def workspace_view(request, workspace_name: str, path: str = ""):

# Only include the AddFileForm if this pathitem is a file that
# can be added to a request - i.e. it is a file and it's not
# already on the curent request for the user
# already on the curent request for the user, and the user
# is allowed to add it to a request (if they are an output-checker they
# are allowed to view all workspaces, but not necessarily create
# requests for them.)
# Currently we can just rely on checking the relpath against
# the files on the request. In future we'll likely also need to
# check file metadata to allow updating a file if the original has
# changed.
form = None
if current_request is None or path_item.relpath not in current_request.file_set():
file_in_request = current_request and path_item.relpath in current_request.file_set()
if request.user.can_create_request(workspace_name) and (
current_request is None or not file_in_request):
form = AddFileForm(release_request=current_request)

return TemplateResponse(
Expand All @@ -62,6 +67,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
kwargs={"workspace_name": workspace_name},
),
"current_request": current_request,
"file_in_request": file_in_request,
"form": form,
},
)
Expand Down
15 changes: 15 additions & 0 deletions tests/integration/views/test_workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ def test_workspace_view_redirects_to_file(client_with_permission):
assert response.headers["Location"] == "/workspaces/view/workspace/file.txt"


@pytest.mark.parametrize(
"user,can_see_form",
[
({"workspaces": ["workspace"], "output_checker": True}, True),
({"workspaces": ["workspace"], "output_checker": False}, True),
({"workspaces": [], "output_checker": True}, False),
]
)
def test_workspace_view_file_add_to_request(client_with_user, user, can_see_form):
client = client_with_user(user)
factories.write_workspace_file("workspace", "file.txt")
response = client.get("/workspaces/view/workspace/file.txt")
assert (response.context["form"] is None) == (not can_see_form)


def test_workspace_view_index_no_user(client):
workspace = factories.create_workspace("workspace")
(workspace.root() / "some_dir").mkdir(parents=True)
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,25 @@ def test_session_user_has_permission(output_checker, workspaces, has_permission)
}
user = User.from_session(mock_session)
assert user.has_permission("test") == has_permission


@pytest.mark.parametrize(
"output_checker,workspaces,can_create_request",
[
(True, [], False),
(True, ["other", "other1"], False),
(False, ["test", "other", "other1"], True),
(False, ["other", "other1"], False),
],
)
def test_session_user_can_create_request(output_checker, workspaces, can_create_request):
mock_session = {
"user": {
"id": 1,
"username": "test",
"workspaces": workspaces,
"output_checker": output_checker,
}
}
user = User.from_session(mock_session)
assert user.can_create_request("test") == can_create_request

0 comments on commit 4152bcc

Please sign in to comment.