Skip to content

Commit

Permalink
Feat/notify user on apikey revoke (#2036)
Browse files Browse the repository at this point in the history
* task(migration): add revoke_api_key email

* feat(api-key-revoke): notify service users by email when key is revoked

* chore: formatting

* chore(api_key_revoke): fix typo in method name

* test(api_key_revoke): ensure user is notify when key is revoked

* chore: update migration name to avoid upstream collision

* chore: fix incorrect revision name

---------

Co-authored-by: William B <[email protected]>
  • Loading branch information
andrewleith and whabanks authored Nov 27, 2023
1 parent d0e1554 commit 0f3cd44
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 35 deletions.
48 changes: 15 additions & 33 deletions app/api_key/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
get_last_send_for_api_key,
get_total_notifications_sent_for_api_key,
)
from app.dao.services_dao import dao_fetch_service_by_id
from app.errors import InvalidRequest, register_errors
from app.service.sender import send_notification_to_service_users

api_key_blueprint = Blueprint("api_key", __name__)
register_errors(api_key_blueprint)
Expand Down Expand Up @@ -72,38 +74,18 @@ def get_api_keys_ranked(n_days_back):
return jsonify(data=data)


def send_api_key_revokation_email(service_id, api_key_name, api_key_information):
"""
TODO: this function if not ready yet. It needs a template to be created.
email = email_data_request_schema.load(request.get_json())
users_to_send_to = dao_fetch_active_users_for_service(service_id)
template = dao_get_template_by_id(current_app.config["API_KEY_REVOKED_TEMPLATE_ID"]) # this template currently doesn't exist
service = Service.query.get(current_app.config["NOTIFY_SERVICE_ID"])
users_service = Service.query.get(service_id)
for user_to_send_to in users_to_send_to:
saved_notification = persist_notification(
template_id=template.id,
template_version=template.version,
recipient=email["email"],
service=service,
personalisation={
"user_name": user_to_send_to.name,
"api_key_name": api_key_name,
"service_name": users_service.name,
"api_key_information": api_key_information,
},
notification_type=template.template_type,
api_key_id=None,
key_type=KEY_TYPE_NORMAL,
reply_to_text=service.get_default_reply_to_email_address(),
)
send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY)
"""
return
def send_api_key_revocation_email(service_id, api_key_name, api_key_information):
service = dao_fetch_service_by_id(service_id)
send_notification_to_service_users(
service_id=service_id,
template_id=current_app.config["APIKEY_REVOKE_TEMPLATE_ID"],
personalisation={
"service_name": service.name,
"public_location": api_key_information["url"],
"key_name": api_key_name,
},
include_user_fields=["name"],
)


@sre_tools_blueprint.route("/api-key-revoke", methods=["POST"])
Expand Down Expand Up @@ -168,6 +150,6 @@ def revoke_api_keys():
)

# Step 4
send_api_key_revokation_email(api_key.service_id, api_key.name, api_key_data)
send_api_key_revocation_email(api_key.service_id, api_key.name, api_key_data)

return jsonify(result="ok"), 201
1 change: 1 addition & 0 deletions app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ class Config(object):
NEAR_DAILY_EMAIL_LIMIT_TEMPLATE_ID = "9aa60ad7-2d7f-46f0-8cbe-2bac3d4d77d8"
REACHED_DAILY_EMAIL_LIMIT_TEMPLATE_ID = "ee036547-e51b-49f1-862b-10ea982cfceb"
DAILY_EMAIL_LIMIT_UPDATED_TEMPLATE_ID = "97dade64-ea8d-460f-8a34-900b74ee5eb0"
APIKEY_REVOKE_TEMPLATE_ID = "a0a4e7b8-8a6a-4eaa-9f4e-9c3a5b2dbcf3"

# Allowed service IDs able to send HTML through their templates.
ALLOW_HTML_SERVICE_IDS: List[str] = [id.strip() for id in os.getenv("ALLOW_HTML_SERVICE_IDS", "").split(",")]
Expand Down
2 changes: 1 addition & 1 deletion migrations/versions/0440_add_index_n_history_comp.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from alembic import op

revision = "0440_add_index_n_history_2"
revision = "0440_add_index_n_history_comp"
down_revision = "0439_add_index_n_history"


Expand Down
103 changes: 103 additions & 0 deletions migrations/versions/0441_add_apikey_revoke_email.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""
Revision ID: 0441_add_apikey_revoke_email
Revises: 0440_add_index_n_history_comp
Create Date: 2022-09-21 00:00:00
"""
from datetime import datetime

from alembic import op
from flask import current_app

revision = "0441_add_apikey_revoke_email"
down_revision = "0440_add_index_n_history_comp"

apikey_revoke_template_id = current_app.config["APIKEY_REVOKE_TEMPLATE_ID"]


def upgrade():
template_insert = """
INSERT INTO templates (id, name, template_type, created_at, content, archived, service_id, subject,
created_by_id, version, process_type, hidden)
VALUES ('{}', '{}', '{}', '{}', '{}', False, '{}', '{}', '{}', 1, '{}', false)
"""
template_history_insert = """
INSERT INTO templates_history (id, name, template_type, created_at, content, archived, service_id, subject,
created_by_id, version, process_type, hidden)
VALUES ('{}', '{}', '{}', '{}', '{}', False, '{}', '{}', '{}', 1, '{}', false)
"""

apikey_revoke_limit_content = "\n".join(
[
"[[fr]]",
"(La version française suit)",
"[[/fr]]",
"",
"[[en]]",
"Hello,",
"",
"We discovered that an API key for service **''((service_name))''** is publicly available. GC Notify detected the key at ((public_location)). To protect GC Notify’s security, we revoked **''((key_name))''**.",
"",
"If you have questions or concerns, contact us.",
"",
"The GC Notify team",
"[[/en]]",
"",
"---",
"",
"[[fr]]",
"Bonjour,",
"",
"Nous avons découvert qu’une clé API du service **''((service_name))''** était à la disposition du public. Notification GC a détecté la clé à l’adresse suivante : ((public_location)). Pour la sécurité de Notification GC, nous avons révoqué **''((key_name))''**.",
"",
"Pour toutes questions, contactez-nous.",
"",
"L’équipe Notification GC",
"[[/fr]]",
]
)

templates = [
{
"id": apikey_revoke_template_id,
"name": "API Key revoke EMAIL",
"subject": "We revoked your API key | Nous avons révoqué votre clé API",
"content": apikey_revoke_limit_content,
},
]

for template in templates:
op.execute(
template_insert.format(
template["id"],
template["name"],
"email",
datetime.utcnow(),
template["content"],
current_app.config["NOTIFY_SERVICE_ID"],
template["subject"],
current_app.config["NOTIFY_USER_ID"],
"normal",
)
)

op.execute(
template_history_insert.format(
template["id"],
template["name"],
"email",
datetime.utcnow(),
template["content"],
current_app.config["NOTIFY_SERVICE_ID"],
template["subject"],
current_app.config["NOTIFY_USER_ID"],
"normal",
)
)


def downgrade():
op.execute("DELETE FROM notifications WHERE template_id = '{}'".format(apikey_revoke_template_id))
op.execute("DELETE FROM notification_history WHERE template_id = '{}'".format(apikey_revoke_template_id))
op.execute("DELETE FROM template_redacted WHERE template_id = '{}'".format(apikey_revoke_template_id))
op.execute("DELETE FROM templates_history WHERE id = '{}'".format(apikey_revoke_template_id))
op.execute("DELETE FROM templates WHERE id = '{}'".format(apikey_revoke_template_id))
6 changes: 5 additions & 1 deletion tests/app/api_key/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ def test_get_api_keys_ranked(admin_request, notify_db, notify_db_session):


class TestApiKeyRevocation:
def test_revoke_api_keys_with_valid_auth(self, client, notify_db, notify_db_session, mocker):
def test_revoke_api_keys_with_valid_auth_revokes_and_notifies_user(self, client, notify_db, notify_db_session, mocker):
notify_users = mocker.patch("app.api_key.rest.send_api_key_revocation_email")

service = create_service(service_name="Service 1")
api_key_1 = create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name="Key 1")
unsigned_secret = get_unsigned_secret(api_key_1.id)
Expand All @@ -104,6 +106,8 @@ def test_revoke_api_keys_with_valid_auth(self, client, notify_db, notify_db_sess
assert api_key_1.compromised_key_info["source"] == "cds-tester"
assert api_key_1.compromised_key_info["time_of_revocation"]

notify_users.assert_called_once()

def test_revoke_api_keys_fails_with_no_auth(self, client, notify_db, notify_db_session, mocker):
service = create_service(service_name="Service 1")
api_key_1 = create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name="Key 1")
Expand Down

0 comments on commit 0f3cd44

Please sign in to comment.