Skip to content

Commit

Permalink
Hide sensitive services on admin (#1931)
Browse files Browse the repository at this point in the history
* hide sensitive services on admin

* fix

* fix

* fix

* fix

* Update app/main/views/service_settings.py

Co-authored-by: Andrew <[email protected]>

---------

Co-authored-by: Andrew <[email protected]>
  • Loading branch information
jzbahrai and andrewleith authored Aug 27, 2024
1 parent 45c9bbd commit 6497e84
Show file tree
Hide file tree
Showing 14 changed files with 68 additions and 11 deletions.
2 changes: 0 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ AWS_REGION=us-east-1
AWS_ACCESS_KEY_ID=
AWS_SECRET_ACCESS_KEY=

SENSITIVE_SERVICES=

REDIS_ENABLED=False

MIXPANEL_PROJECT_TOKEN=<project_token>
Expand Down
2 changes: 1 addition & 1 deletion app/assets/javascripts/main.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion app/assets/javascripts/scheduler.min.js

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ class Config(object):
SECRET_KEY = env.list("SECRET_KEY", [])
SECURITY_EMAIL = os.environ.get("SECURITY_EMAIL", "[email protected]")
SENDING_DOMAIN = os.environ.get("SENDING_DOMAIN", "notification.alpha.canada.ca")
SENSITIVE_SERVICES = os.environ.get("SENSITIVE_SERVICES", "")
SESSION_COOKIE_HTTPONLY = True
SESSION_COOKIE_NAME = "notify_admin_session"
SESSION_COOKIE_SAMESITE = "Lax"
Expand Down
17 changes: 17 additions & 0 deletions app/main/views/service_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,23 @@ def service_switch_live(service_id):
)


@main.route("/services/<service_id>/service-settings/set-sensitive-service", methods=["GET", "POST"])
@user_is_platform_admin
def set_sensitive_service(service_id):
title = _("Set sensitive service")
form = ServiceOnOffSettingForm(name=title, enabled=current_service.sensitive_service)

if form.validate_on_submit():
current_service.update(sensitive_service=form.enabled.data)
return redirect(url_for(".service_settings", service_id=service_id))

return render_template(
"views/service-settings/set-service-setting.html",
title=_("Set sensitive service"),
form=form,
)


@main.route(
"/services/<service_id>/service-settings/switch-upload-document",
methods=["GET", "POST"],
Expand Down
1 change: 1 addition & 0 deletions app/models/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class Service(JSONModel):
"go_live_at",
"sending_domain",
"organisation_notes",
"sensitive_service",
}

TEMPLATE_TYPES = (
Expand Down
5 changes: 3 additions & 2 deletions app/notify_client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ def init_app(self, app):
self.api_key = app.config["ADMIN_CLIENT_SECRET"]
self.route_secret = app.config["ROUTE_SECRET_KEY_1"]
self.waf_secret = app.config["WAF_SECRET"]
self.sensitive_services = [x.strip() for x in app.config["SENSITIVE_SERVICES"].split(",")]

def generate_headers(self, api_token):
headers = {
Expand Down Expand Up @@ -59,7 +58,9 @@ def log_admin_call(self, url, method):
# to prevent cyclical imports
from app import current_service

service_is_sensitive = current_service and any(service == current_service.id for service in self.sensitive_services)
service_is_sensitive = True
if current_service:
service_is_sensitive = current_service.sensitive_service if current_service.sensitive_service else False
if hasattr(current_user, "platform_admin") and current_user.platform_admin:
user = current_user.email_address + "|" + current_user.id
logger.warn("{}Admin API request {} {} {} ".format("Sensitive " if service_is_sensitive else "", method, url, user))
Expand Down
1 change: 1 addition & 0 deletions app/notify_client/service_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def update_service(self, service_id, **kwargs):
"go_live_at",
"sending_domain",
"sms_volume_today",
"sensitive_service",
}

if disallowed_attributes:
Expand Down
17 changes: 17 additions & 0 deletions app/templates/views/service-settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,23 @@ <h2 class="heading-medium">{{ _('Platform admin settings') }}</h2>
)}}
{% endcall %}

{% call row() %}
{% set txt = _('Set sensitive service') %}
{{ text_field(txt)}}
{% if current_service.sensitive_service %}
{% set value = _('On') %}
{{ text_field(value) }}
{% else %}
{% set value = _('Off') %}
{{ text_field(value) }}
{% endif %}
{{ edit_field(
change_txt,
url_for('.set_sensitive_service', service_id=current_service.id),
for=txt
) }}
{% endcall %}

{% endcall %}

</div>
Expand Down
3 changes: 2 additions & 1 deletion app/translations/csv/fr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -1973,4 +1973,5 @@
"Provide an accessible description of your logo","Fournissez une description accessible de votre logo"
"Category","Catégorie"
"Does the category still apply?","La catégorie s'applique-t-elle toujours?"
"Review your activity","Examinez votre activité"
"Review your activity","Examinez votre activité"
"Set sensitive service","Définir un service sensible"
2 changes: 2 additions & 0 deletions tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ def service_json(
sending_domain=None,
go_live_user=None,
organisation_notes="",
sensitive_service=None,
):
if users is None:
users = []
Expand Down Expand Up @@ -238,6 +239,7 @@ def service_json(
"sending_domain": sending_domain,
"go_live_user": go_live_user,
"organisation_notes": organisation_notes,
"sensitive_service": sensitive_service,
}
service: Service = dotdict(service_dict)
return service
Expand Down
15 changes: 15 additions & 0 deletions tests/app/main/views/test_service_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -4291,3 +4291,18 @@ def test_update_service_data_retention_populates_form(
assert response.status_code == 200
page = BeautifulSoup(response.data.decode("utf-8"), "html.parser")
assert page.find("input", attrs={"name": "days_of_retention"})["value"] == "5"


class TestSettingSensitiveService:

def test_should_redirect_after_change_service_name(
self,
client_request,
mock_update_service,
service_one,
platform_admin_user,
):
client_request.login(platform_admin_user, service_one)
client_request.post(
"main.set_sensitive_service", service_id=SERVICE_ONE_ID, _data={"sensitive_service": False}, _expected_status=200
)
5 changes: 2 additions & 3 deletions tests/app/notify_client/test_notify_admin_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,10 @@ def test_non_sensitive_logging_enabled_for_admin_users(app_, platform_admin_user

@pytest.mark.parametrize("method", ["put", "post", "delete"])
def test_sensitive_logging_enabled_for_admin_users(app_, platform_admin_user, method, caplog):
sensitive_service = Service(service_json(id_="ss1111"))
sensitive_service = Service(service_json(id_="ss1111", sensitive_service=True))

api_client = NotifyAdminAPIClient()
with set_config(app_, "SENSITIVE_SERVICES", "222, ss1111,33333"):
api_client.init_app(app_)
api_client.init_app(app_)

with app_.test_request_context(), app_.test_client() as client:
client.login(platform_admin_user)
Expand Down
6 changes: 6 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,11 @@ def mock_service_name_is_unique(mocker):
return mocker.patch("app.service_api_client.is_service_name_unique", return_value=True)


@pytest.fixture(scope="function")
def mock_set_sensitive_service(mocker):
return mocker.patch("app.service_api_client.update_service", return_value=True)


@pytest.fixture(scope="function")
def mock_service_email_from_is_not_unique(mocker):
return mocker.patch("app.service_api_client.is_service_email_from_unique", return_value=False)
Expand Down Expand Up @@ -724,6 +729,7 @@ def _update(service_id, **kwargs):
"email_from",
"sms_sender",
"permissions",
"sensitive_service",
]
},
)
Expand Down

0 comments on commit 6497e84

Please sign in to comment.