Skip to content

Commit

Permalink
Validate namespace by checking cloud platform repo (#1453)
Browse files Browse the repository at this point in the history
* Validate namespace by checking cloud platform repo

* Fixed formatting errors

* Fixed tests

* Refactored code to check both dev and prod exist before creating app

* Fixed not being able to add capitalised username in restricted tool release

* Made error message more relevant

* Ran black

* Added tests for githubapi

* Ran black

* Ran isort
  • Loading branch information
jamesstottmoj authored Feb 7, 2025
1 parent 7969407 commit 635c46e
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 4 deletions.
12 changes: 12 additions & 0 deletions controlpanel/api/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ def get_repository(self, repo_name: str):

return self._process_response(response)

def get_repository_contents(self, repo_name: str, repo_path: str):
response = requests.get(
self._get_repo_api_url(repo_name=repo_name, api_call=f"contents/{repo_path}"),
headers=self.headers,
)
if response.status_code == 404:
raise RepositoryNotFound(
f"Repository path '{repo_path}' in {repo_name} not found. It may not exist"
)

return self._process_response(response)

def read_app_deploy_info(self, repo_name: str, deploy_file="deploy.json"):
response = requests.get(
self._get_repo_api_url(repo_name=repo_name, api_call=f"contents/{deploy_file}"),
Expand Down
2 changes: 1 addition & 1 deletion controlpanel/frontend/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ def get_target_users(self):
"""
target_list = self.data.get("target_users_list", "")
if target_list:
usernames = set([username.strip() for username in target_list.split(",")])
usernames = set([username.strip().lower() for username in target_list.split(",")])
return User.objects.filter(username__in=usernames)
else:
return []
Expand Down
18 changes: 18 additions & 0 deletions controlpanel/frontend/views/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

# First-party/Local
from controlpanel.api import auth0, cluster
from controlpanel.api.github import GithubAPI, RepositoryNotFound
from controlpanel.api.models import (
App,
AppIPAllowList,
Expand Down Expand Up @@ -198,9 +199,26 @@ def get_success_url(self):
)
return reverse_lazy("manage-app", kwargs={"pk": self.object.pk})

def _check_namespace_in_cloud_platform(self, form_namespace):
"""Throws RepositoryNotFound error if namespace not found in CP repo"""

namespaces = [f"{form_namespace}-dev", f"{form_namespace}-prod"]
cloud_platform_repo_name = "cloud-platform-environments"

for namespace in namespaces:
path = f"namespaces/live.cloud-platform.service.justice.gov.uk/{namespace}"
GithubAPI(self.request.user.github_api_token).get_repository_contents(
cloud_platform_repo_name, path
)

def form_valid(self, form):
try:
self._check_namespace_in_cloud_platform(form.cleaned_data.get("namespace"))

self.object = AppManager().register_app(self.request.user, form.cleaned_data)
except RepositoryNotFound as ex:
form.add_error("namespace", str(ex))
return FormMixin.form_invalid(self, form)
except Exception as ex:
form.add_error("repo_url", str(ex))
return FormMixin.form_invalid(self, form)
Expand Down
79 changes: 79 additions & 0 deletions tests/api/test_github.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Standard library
from unittest.mock import Mock, patch

# Third-party
import pytest

# First-party/Local
from controlpanel.api.github import GithubAPI, RepositoryNotFound


@pytest.fixture()
def requests():
"""
Mock calls to requests
"""
with patch("controlpanel.api.github.requests") as requests:
yield requests


@pytest.fixture()
def request_get_success(requests):
"""
Mock calls to requests
"""
response = Mock()
response.status_code = 200
response.json.return_value = {"repo": "test-repo-name"}
requests.get.return_value = response

yield requests


@pytest.fixture()
def request_get_not_found(requests):
"""
Mock calls to requests
"""
response = Mock()
response.status_code = 404
requests.get.return_value = response

yield requests


def test_get_repository_success(request_get_success):

test_api_token = "abc123"
response = GithubAPI(test_api_token).get_repository("test-repo-name")

assert response["repo"] == "test-repo-name"


def test_get_repository_not_found(request_get_not_found):

with pytest.raises(
RepositoryNotFound, match="Repository 'test-repo-name' not found, it may be private"
):
test_api_token = "abc123"
GithubAPI(test_api_token).get_repository("test-repo-name")


def test_get_repository_contents_success(request_get_success):

test_api_token = "abc123"
response = GithubAPI(test_api_token).get_repository_contents(
"test-repo-name", "some/resource/path"
)

assert response["repo"] == "test-repo-name"


def test_get_repository_contents_not_found(request_get_not_found):

with pytest.raises(
RepositoryNotFound,
match="Repository path 'some/resource/path' in test-repo-name not found. It may not exist",
):
test_api_token = "abc123"
GithubAPI(test_api_token).get_repository_contents("test-repo-name", "some/resource/path")
10 changes: 7 additions & 3 deletions tests/frontend/views/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ def githubapi():
"""
with (
patch("controlpanel.frontend.forms.GithubAPI"),
patch("controlpanel.frontend.views.app.GithubAPI"),
patch("controlpanel.api.cluster.GithubAPI") as GithubAPI,
):
yield GithubAPI.return_value
Expand Down Expand Up @@ -775,7 +776,8 @@ def test_app_settings_permission(client, app, users, repos_with_auth, user, can_
assert "Edit" not in auth_item_ui.text


def test_register_app_with_xacct_policy(client, users):
def test_register_app_with_xacct_policy(client, users, githubapi):
githubapi.get_repository_contents.return_value = {"repo": "test-app-namespace-test"}
test_app_name = "test_app_with_xacct_policy"
assert App.objects.filter(name=test_app_name).count() == 0
client.force_login(users["superuser"])
Expand All @@ -795,7 +797,8 @@ def test_register_app_with_xacct_policy(client, users):
assert response.url == reverse("manage-app", kwargs={"pk": created_app.pk})


def test_register_app_with_creating_datasource(client, users):
def test_register_app_with_creating_datasource(client, users, githubapi):
githubapi.get_repository_contents.return_value = {"repo": "test-app-namespace-test"}
test_app_name = "test_app_with_creating_datasource"
test_bucket_name = "test-bucket"
assert App.objects.filter(name=test_app_name).count() == 0
Expand All @@ -819,7 +822,8 @@ def test_register_app_with_creating_datasource(client, users):
assert response.url == reverse("manage-app", kwargs={"pk": created_app.pk})


def test_register_app_with_existing_datasource(client, users, s3buckets):
def test_register_app_with_existing_datasource(client, users, s3buckets, githubapi):
githubapi.get_repository_contents.return_value = {"repo": "test-app-namespace-test"}
test_app_name = "test_app_with_existing_datasource"
existing_bucket = s3buckets["not_connected"]
user = users["superuser"]
Expand Down

0 comments on commit 635c46e

Please sign in to comment.