diff --git a/airlock/forms.py b/airlock/forms.py index ec633c3c..3c31d833 100644 --- a/airlock/forms.py +++ b/airlock/forms.py @@ -1,6 +1,11 @@ from django import forms +class TokenLoginForm(forms.Form): + user = forms.CharField() + token = forms.CharField() + + class AddFileForm(forms.Form): filegroup = forms.ChoiceField(required=False) new_filegroup = forms.CharField(required=False) diff --git a/airlock/views.py b/airlock/views.py deleted file mode 100644 index 5659f09f..00000000 --- a/airlock/views.py +++ /dev/null @@ -1,316 +0,0 @@ -import requests -from django import forms -from django.conf import settings -from django.contrib import messages -from django.core.exceptions import PermissionDenied -from django.http import Http404 -from django.shortcuts import redirect -from django.template.response import TemplateResponse -from django.urls import reverse -from django.views.decorators.http import require_http_methods - -from airlock import login_api -from airlock.api import Status, UrlPath -from airlock.file_browser_api import get_request_tree, get_workspace_tree -from airlock.forms import AddFileForm -from local_db.api import LocalDBProvider - - -api = LocalDBProvider() - - -def login_exempt(view): - view.login_exempt = True - return view - - -class TokenLoginForm(forms.Form): - user = forms.CharField() - token = forms.CharField() - - -@login_exempt -def login(request): - default_next_url = reverse("workspace_index") - - if request.method != "POST": - next_url = request.GET.get("next", default_next_url) - if request.user is not None: - return redirect(next_url) - token_login_form = TokenLoginForm() - else: - next_url = request.POST.get("next", default_next_url) - token_login_form = TokenLoginForm(request.POST) - user_data = get_user_data_or_set_form_errors(token_login_form) - # If `user_data` is None then the form object will have the relevant errors - if user_data is not None: - # TODO: the current code expects an `id` field but the API doesn't return - # one; we should work out what we're doing here - user_data["id"] = user_data["username"] - request.session["user"] = user_data - return redirect(next_url) - - return TemplateResponse( - request, - "login.html", - { - "next_url": next_url, - "token_login_form": token_login_form, - "dev_users_file": settings.AIRLOCK_DEV_USERS_FILE, - }, - ) - - -def get_user_data_or_set_form_errors(form): - if not form.is_valid(): - return - try: - return login_api.get_user_data( - user=form.cleaned_data["user"], - token=form.cleaned_data["token"], - ) - except login_api.LoginError as exc: - form.add_error("token", str(exc)) - - -def logout(request): - """ - User information is held in the session. On logout, remove - session data and redirect to the home page. - """ - request.session.flush() - return redirect(reverse("home")) - - -@login_exempt # for now -def index(request): - return TemplateResponse(request, "index.html") - - -def validate_workspace(user, workspace_name): - """Ensure the workspace exists and the current user has permissions to access it.""" - try: - workspace = api.get_workspace(workspace_name) - except api.WorkspaceNotFound: - raise Http404() - - if user is None or not user.has_permission(workspace_name): - raise PermissionDenied() - - return workspace - - -def validate_release_request(user, request_id): - """Ensure the release request exists for this workspace.""" - try: - release_request = api.get_release_request(request_id) - except api.ReleaseRequestNotFound: - raise Http404() - - # check user permissions for this workspace - validate_workspace(user, release_request.workspace) - - return release_request - - -def workspace_index(request): - workspaces = api.get_workspaces_for_user(request.user) - return TemplateResponse(request, "workspaces.html", {"workspaces": workspaces}) - - -def workspace_view(request, workspace_name: str, path: str = ""): - workspace = validate_workspace(request.user, workspace_name) - - tree = get_workspace_tree(workspace, path) - - try: - path_item = tree.get_path(path) - except tree.PathNotFound: - raise Http404() - - is_directory_url = path.endswith("/") or path == "" - if path_item.is_directory() != is_directory_url: - return redirect(path_item.url()) - - current_request = api.get_current_request(workspace_name, request.user) - - # 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 - # 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(): - form = AddFileForm(release_request=current_request) - - return TemplateResponse( - request, - "file_browser/index.html", - { - "workspace": workspace, - "root": tree, - "path_item": path_item, - "context": "workspace", - "title": f"Files for workspace {workspace_name}", - "request_file_url": reverse( - "workspace_add_file", - kwargs={"workspace_name": workspace_name}, - ), - "current_request": current_request, - "form": form, - }, - ) - - -@require_http_methods(["POST"]) -def workspace_add_file_to_request(request, workspace_name): - workspace = validate_workspace(request.user, workspace_name) - relpath = UrlPath(request.POST["path"]) - try: - workspace.abspath(relpath) - except api.FileNotFound: - raise Http404() - - release_request = api.get_current_request(workspace_name, request.user, create=True) - form = AddFileForm(request.POST, release_request=release_request) - if form.is_valid(): - group_name = request.POST.get("new_filegroup") or request.POST.get("filegroup") - try: - api.add_file_to_request(release_request, relpath, request.user, group_name) - except api.APIException as err: - # This exception is raised if the file has already been added - # (to any group on the request) - messages.error(request, str(err)) - else: - messages.success( - request, f"File has been added to request (file group '{group_name}')" - ) - else: - for error in form.errors.values(): - messages.error(request, error) - - # Redirect to the file in the workspace - return redirect(workspace.get_url(relpath)) - - -def request_index(request): - authored_requests = api.get_requests_authored_by_user(request.user) - - outstanding_requests = [] - if request.user.output_checker: - outstanding_requests = api.get_outstanding_requests_for_review(request.user) - - return TemplateResponse( - request, - "requests.html", - { - "authored_requests": authored_requests, - "outstanding_requests": outstanding_requests, - }, - ) - - -def request_view(request, request_id: str, path: str = ""): - release_request = validate_release_request(request.user, request_id) - - tree = get_request_tree(release_request, path) - try: - path_item = tree.get_path(path) - except tree.PathNotFound: - raise Http404() - - is_directory_url = path.endswith("/") or path == "" - if path_item.is_directory() != is_directory_url: - return redirect(path_item.url()) - - is_author = release_request.author == request.user.username - # hack for testing w/o having to switch users - if "is_author" in request.GET: # pragma: nocover - is_author = request.GET["is_author"].lower() == "true" - - request_submit_url = reverse( - "request_submit", - kwargs={"request_id": request_id}, - ) - request_reject_url = reverse( - "request_reject", - kwargs={"request_id": request_id}, - ) - release_files_url = reverse( - "request_release_files", - kwargs={"request_id": request_id}, - ) - - context = { - "workspace": api.get_workspace(release_request.workspace), - "release_request": release_request, - "root": tree, - "path_item": path_item, - "context": "request", - "title": f"Request for {release_request.workspace} by {release_request.author}", - # TODO file these in from user/models - "is_author": is_author, - "is_output_checker": request.user.output_checker, - "request_submit_url": request_submit_url, - "request_reject_url": request_reject_url, - "release_files_url": release_files_url, - } - - return TemplateResponse(request, "file_browser/index.html", context) - - -@require_http_methods(["POST"]) -def request_submit(request, request_id): - release_request = validate_release_request(request.user, request_id) - - try: - api.set_status(release_request, Status.SUBMITTED, request.user) - except api.RequestPermissionDenied as exc: - raise PermissionDenied(str(exc)) - - messages.success(request, "Request has been submitted") - return redirect(release_request.get_url()) - - -@require_http_methods(["POST"]) -def request_reject(request, request_id): - release_request = validate_release_request(request.user, request_id) - - try: - api.set_status(release_request, Status.REJECTED, request.user) - except api.RequestPermissionDenied as exc: - raise PermissionDenied(str(exc)) - - messages.error(request, "Request has been rejected") - return redirect(release_request.get_url()) - - -@require_http_methods(["POST"]) -def request_release_files(request, request_id): - release_request = validate_release_request(request.user, request_id) - - try: - # For now, we just implicitly approve when release files is requested - api.set_status(release_request, Status.APPROVED, request.user) - api.release_files(release_request, request.user) - except api.RequestPermissionDenied as exc: - raise PermissionDenied(str(exc)) - except requests.HTTPError as err: - if settings.DEBUG: - return TemplateResponse( - request, - "jobserver-error.html", - { - "response": err.response, - "type": err.response.headers["Content-Type"], - }, - ) - - if err.response.status_code == 403: - raise PermissionDenied() from None - raise - - messages.success(request, "Files have been released to jobs.opensafely.org") - return redirect(release_request.get_url()) diff --git a/airlock/views/__init__.py b/airlock/views/__init__.py new file mode 100644 index 00000000..ebb77606 --- /dev/null +++ b/airlock/views/__init__.py @@ -0,0 +1,14 @@ +from .auth import login, logout +from .index import index +from .request import ( + request_index, + request_reject, + request_release_files, + request_submit, + request_view, +) +from .workspace import ( + workspace_add_file_to_request, + workspace_index, + workspace_view, +) diff --git a/airlock/views/auth.py b/airlock/views/auth.py new file mode 100644 index 00000000..69d20934 --- /dev/null +++ b/airlock/views/auth.py @@ -0,0 +1,62 @@ +from django.conf import settings +from django.shortcuts import redirect +from django.template.response import TemplateResponse +from django.urls import reverse + +from airlock import login_api +from airlock.forms import TokenLoginForm + +from .helpers import login_exempt + + +@login_exempt +def login(request): + default_next_url = reverse("workspace_index") + + if request.method != "POST": + next_url = request.GET.get("next", default_next_url) + if request.user is not None: + return redirect(next_url) + token_login_form = TokenLoginForm() + else: + next_url = request.POST.get("next", default_next_url) + token_login_form = TokenLoginForm(request.POST) + user_data = get_user_data_or_set_form_errors(token_login_form) + # If `user_data` is None then the form object will have the relevant errors + if user_data is not None: + # TODO: the current code expects an `id` field but the API doesn't return + # one; we should work out what we're doing here + user_data["id"] = user_data["username"] + request.session["user"] = user_data + return redirect(next_url) + + return TemplateResponse( + request, + "login.html", + { + "next_url": next_url, + "token_login_form": token_login_form, + "dev_users_file": settings.AIRLOCK_DEV_USERS_FILE, + }, + ) + + +def get_user_data_or_set_form_errors(form): + if not form.is_valid(): + return + try: + return login_api.get_user_data( + user=form.cleaned_data["user"], + token=form.cleaned_data["token"], + ) + except login_api.LoginError as exc: + form.add_error("token", str(exc)) + + +def logout(request): + """ + User information is held in the session. On logout, remove + session data and redirect to the home page. + """ + request.session.flush() + return redirect(reverse("home")) diff --git a/airlock/views/helpers.py b/airlock/views/helpers.py new file mode 100644 index 00000000..5d01354e --- /dev/null +++ b/airlock/views/helpers.py @@ -0,0 +1,38 @@ +from django.core.exceptions import PermissionDenied +from django.http import Http404 + +from local_db.api import LocalDBProvider + + +api = LocalDBProvider() + + +def login_exempt(view): + view.login_exempt = True + return view + + +def validate_workspace(user, workspace_name): + """Ensure the workspace exists and the current user has permissions to access it.""" + try: + workspace = api.get_workspace(workspace_name) + except api.WorkspaceNotFound: + raise Http404() + + if user is None or not user.has_permission(workspace_name): + raise PermissionDenied() + + return workspace + + +def validate_release_request(user, request_id): + """Ensure the release request exists for this workspace.""" + try: + release_request = api.get_release_request(request_id) + except api.ReleaseRequestNotFound: + raise Http404() + + # check user permissions for this workspace + validate_workspace(user, release_request.workspace) + + return release_request diff --git a/airlock/views/index.py b/airlock/views/index.py new file mode 100644 index 00000000..ad6f47eb --- /dev/null +++ b/airlock/views/index.py @@ -0,0 +1,8 @@ +from django.template.response import TemplateResponse + +from .helpers import login_exempt + + +@login_exempt # for now +def index(request): + return TemplateResponse(request, "index.html") diff --git a/airlock/views/request.py b/airlock/views/request.py new file mode 100644 index 00000000..52503c93 --- /dev/null +++ b/airlock/views/request.py @@ -0,0 +1,139 @@ +import requests +from django.conf import settings +from django.contrib import messages +from django.core.exceptions import PermissionDenied +from django.http import Http404 +from django.shortcuts import redirect +from django.template.response import TemplateResponse +from django.urls import reverse +from django.views.decorators.http import require_http_methods + +from airlock.api import Status +from airlock.file_browser_api import get_request_tree +from local_db.api import LocalDBProvider + +from .helpers import validate_release_request + + +api = LocalDBProvider() + + +def request_index(request): + authored_requests = api.get_requests_authored_by_user(request.user) + + outstanding_requests = [] + if request.user.output_checker: + outstanding_requests = api.get_outstanding_requests_for_review(request.user) + + return TemplateResponse( + request, + "requests.html", + { + "authored_requests": authored_requests, + "outstanding_requests": outstanding_requests, + }, + ) + + +def request_view(request, request_id: str, path: str = ""): + release_request = validate_release_request(request.user, request_id) + + tree = get_request_tree(release_request, path) + try: + path_item = tree.get_path(path) + except tree.PathNotFound: + raise Http404() + + is_directory_url = path.endswith("/") or path == "" + if path_item.is_directory() != is_directory_url: + return redirect(path_item.url()) + + is_author = release_request.author == request.user.username + # hack for testing w/o having to switch users + if "is_author" in request.GET: # pragma: nocover + is_author = request.GET["is_author"].lower() == "true" + + request_submit_url = reverse( + "request_submit", + kwargs={"request_id": request_id}, + ) + request_reject_url = reverse( + "request_reject", + kwargs={"request_id": request_id}, + ) + release_files_url = reverse( + "request_release_files", + kwargs={"request_id": request_id}, + ) + + context = { + "workspace": api.get_workspace(release_request.workspace), + "release_request": release_request, + "root": tree, + "path_item": path_item, + "context": "request", + "title": f"Request for {release_request.workspace} by {release_request.author}", + # TODO file these in from user/models + "is_author": is_author, + "is_output_checker": request.user.output_checker, + "request_submit_url": request_submit_url, + "request_reject_url": request_reject_url, + "release_files_url": release_files_url, + } + + return TemplateResponse(request, "file_browser/index.html", context) + + +@require_http_methods(["POST"]) +def request_submit(request, request_id): + release_request = validate_release_request(request.user, request_id) + + try: + api.set_status(release_request, Status.SUBMITTED, request.user) + except api.RequestPermissionDenied as exc: + raise PermissionDenied(str(exc)) + + messages.success(request, "Request has been submitted") + return redirect(release_request.get_url()) + + +@require_http_methods(["POST"]) +def request_reject(request, request_id): + release_request = validate_release_request(request.user, request_id) + + try: + api.set_status(release_request, Status.REJECTED, request.user) + except api.RequestPermissionDenied as exc: + raise PermissionDenied(str(exc)) + + messages.error(request, "Request has been rejected") + return redirect(release_request.get_url()) + + +@require_http_methods(["POST"]) +def request_release_files(request, request_id): + release_request = validate_release_request(request.user, request_id) + + try: + # For now, we just implicitly approve when release files is requested + api.set_status(release_request, Status.APPROVED, request.user) + api.release_files(release_request, request.user) + except api.RequestPermissionDenied as exc: + raise PermissionDenied(str(exc)) + except requests.HTTPError as err: + if settings.DEBUG: + return TemplateResponse( + request, + "jobserver-error.html", + { + "response": err.response, + "type": err.response.headers["Content-Type"], + }, + ) + + if err.response.status_code == 403: + raise PermissionDenied() from None + raise + + messages.success(request, "Files have been released to jobs.opensafely.org") + return redirect(release_request.get_url()) diff --git a/airlock/views/workspace.py b/airlock/views/workspace.py new file mode 100644 index 00000000..6cd23282 --- /dev/null +++ b/airlock/views/workspace.py @@ -0,0 +1,98 @@ +from django.contrib import messages +from django.http import Http404 +from django.shortcuts import redirect +from django.template.response import TemplateResponse +from django.urls import reverse +from django.views.decorators.http import require_http_methods + +from airlock.api import UrlPath +from airlock.file_browser_api import get_workspace_tree +from airlock.forms import AddFileForm +from local_db.api import LocalDBProvider + +from .helpers import validate_workspace + + +api = LocalDBProvider() + + +def workspace_index(request): + workspaces = api.get_workspaces_for_user(request.user) + return TemplateResponse(request, "workspaces.html", {"workspaces": workspaces}) + + +def workspace_view(request, workspace_name: str, path: str = ""): + workspace = validate_workspace(request.user, workspace_name) + + tree = get_workspace_tree(workspace, path) + + try: + path_item = tree.get_path(path) + except tree.PathNotFound: + raise Http404() + + is_directory_url = path.endswith("/") or path == "" + if path_item.is_directory() != is_directory_url: + return redirect(path_item.url()) + + current_request = api.get_current_request(workspace_name, request.user) + + # 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 + # 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(): + form = AddFileForm(release_request=current_request) + + return TemplateResponse( + request, + "file_browser/index.html", + { + "workspace": workspace, + "root": tree, + "path_item": path_item, + "context": "workspace", + "title": f"Files for workspace {workspace_name}", + "request_file_url": reverse( + "workspace_add_file", + kwargs={"workspace_name": workspace_name}, + ), + "current_request": current_request, + "form": form, + }, + ) + + +@require_http_methods(["POST"]) +def workspace_add_file_to_request(request, workspace_name): + workspace = validate_workspace(request.user, workspace_name) + relpath = UrlPath(request.POST["path"]) + try: + workspace.abspath(relpath) + except api.FileNotFound: + raise Http404() + + release_request = api.get_current_request(workspace_name, request.user, create=True) + form = AddFileForm(request.POST, release_request=release_request) + if form.is_valid(): + group_name = request.POST.get("new_filegroup") or request.POST.get("filegroup") + try: + api.add_file_to_request(release_request, relpath, request.user, group_name) + except api.APIException as err: + # This exception is raised if the file has already been added + # (to any group on the request) + messages.error(request, str(err)) + else: + messages.success( + request, f"File has been added to request (file group '{group_name}')" + ) + else: + for error in form.errors.values(): + messages.error(request, error) + + # Redirect to the file in the workspace + return redirect(workspace.get_url(relpath)) diff --git a/pyproject.toml b/pyproject.toml index 9d46f9a0..9c8c7b98 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,3 +40,6 @@ template_extensions = "html" exclude_blocks = [ "\\/\\w+" ] + +[tool.ruff.lint.per-file-ignores] +"airlock/views/__init__.py" = ["F401"] diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py index 7e540408..0453dfdf 100644 --- a/tests/functional/test_e2e.py +++ b/tests/functional/test_e2e.py @@ -121,7 +121,6 @@ def test_e2e_release_files(page, live_server, dev_users, release_files_stubber): # get the request ID for the just-created request, for later reference request_id = url_regex.match(page.url).groups()[0] - # Add this when the view-by-groups is ready. # Find the filegroup in the tree # Note: `get_by_role`` gets all links on the page; `locator` searches # for elements with the filegroup class; the `scope` pseudoselector diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py new file mode 100644 index 00000000..729bde0d --- /dev/null +++ b/tests/integration/conftest.py @@ -0,0 +1,22 @@ +import pytest + +from airlock.users import User + + +@pytest.fixture +def client_with_user(client): + def _client(session_user): + session_user = {"id": 1, "username": "test", **session_user} + session = client.session + session["user"] = session_user + session.save() + client.user = User.from_session(session) + return client + + return _client + + +@pytest.fixture +def client_with_permission(client_with_user): + output_checker = {"output_checker": True} + yield client_with_user(output_checker) diff --git a/tests/integration/views/__init__.py b/tests/integration/views/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/integration/views/test_index.py b/tests/integration/views/test_index.py new file mode 100644 index 00000000..39f2f702 --- /dev/null +++ b/tests/integration/views/test_index.py @@ -0,0 +1,9 @@ +# import pytest + + +# pytestmark = pytest.mark.django_db + + +def test_index(client): + response = client.get("/") + assert "Hello World" in response.rendered_content diff --git a/tests/integration/test_views.py b/tests/integration/views/test_request.py similarity index 52% rename from tests/integration/test_views.py rename to tests/integration/views/test_request.py index 4a3bf345..06e1202c 100644 --- a/tests/integration/test_views.py +++ b/tests/integration/views/test_request.py @@ -2,7 +2,6 @@ import pytest import requests -from django.contrib import messages from airlock.api import Status from airlock.users import User @@ -12,298 +11,6 @@ pytestmark = pytest.mark.django_db -def test_index(client): - response = client.get("/") - assert "Hello World" in response.rendered_content - - -@pytest.fixture -def client_with_user(client): - def _client(session_user): - session_user = {"id": 1, "username": "test", **session_user} - session = client.session - session["user"] = session_user - session.save() - client.user = User.from_session(session) - return client - - return _client - - -@pytest.fixture -def client_with_permission(client_with_user): - output_checker = {"output_checker": True} - yield client_with_user(output_checker) - - -def test_workspace_view(client_with_permission): - factories.write_workspace_file("workspace", "file.txt") - - response = client_with_permission.get("/workspaces/view/workspace/") - assert "file.txt" in response.rendered_content - assert "release-request-button" not in response.rendered_content - - -def test_workspace_view_with_existing_request_for_user( - client_with_permission, -): - user = User.from_session(client_with_permission.session) - factories.write_workspace_file("workspace", "file.txt") - release_request = factories.create_release_request("workspace", user=user) - factories.create_filegroup( - release_request, group_name="default_group", filepaths=["file.txt"] - ) - response = client_with_permission.get("/workspaces/view/workspace/") - assert "current-request-button" in response.rendered_content - - -def test_workspace_does_not_exist(client_with_permission): - response = client_with_permission.get("/workspaces/view/bad/") - assert response.status_code == 404 - - -def test_workspace_view_with_directory(client_with_permission): - factories.write_workspace_file("workspace", "some_dir/file.txt") - response = client_with_permission.get("/workspaces/view/workspace/some_dir/") - assert response.status_code == 200 - assert "file.txt" in response.rendered_content - - -def test_workspace_view_with_file(client_with_permission): - factories.write_workspace_file("workspace", "file.txt", "foobar") - response = client_with_permission.get("/workspaces/view/workspace/file.txt") - assert response.status_code == 200 - assert "foobar" in response.rendered_content - - -def test_workspace_view_with_html_file(client_with_permission): - factories.write_workspace_file( - "workspace", "file.html", "foobar" - ) - response = client_with_permission.get("/workspaces/view/workspace/file.html") - assert "foobar" in response.rendered_content - - -def test_workspace_view_with_csv_file(client_with_permission): - factories.write_workspace_file("workspace", "file.csv", "header1,header2\nFoo,Bar") - response = client_with_permission.get("/workspaces/view/workspace/file.csv") - for content in ["foobar" + ) + response = client_with_permission.get("/workspaces/view/workspace/file.html") + assert "foobar" in response.rendered_content + + +def test_workspace_view_with_csv_file(client_with_permission): + factories.write_workspace_file("workspace", "file.csv", "header1,header2\nFoo,Bar") + response = client_with_permission.get("/workspaces/view/workspace/file.csv") + for content in ["