Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix notifications access from service #154

Closed
wants to merge 14 commits into from
14 changes: 13 additions & 1 deletion app/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
from datetime import datetime

import click
from flask import Flask, render_template
from flask import Flask, redirect, render_template, request

import notifications
import storage
from db import get_db_connection

Expand Down Expand Up @@ -63,6 +64,17 @@ def document_upload():
return f'<form method="post" action="{upload_url}" enctype="multipart/form-data">{additional_fields}<input type="file" name="file"><input type="submit"></form>'


@app.route("/email-notifications", methods=["GET", "POST"])
def email_notifications():
if request.method == "POST":
to = request.form["to"]
subject = "Test notification"
message = "This is a system generated test notification. If you received this email in error, please ignore it."
logger.info("Sending test email to %s", to)
notifications.send_email(to, subject, message)
return f'<form method="post">Send test email to:<input type="email" name="to"><input type="submit"></form>'


@app.route("/secrets")
def secrets():
secret_sauce = os.environ["SECRET_SAUCE"]
Expand Down
38 changes: 38 additions & 0 deletions app/notifications.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import os
import boto3

def send_email(to: str, subject: str, message: str):
pinpoint_client = boto3.client("pinpoint")
app_id = os.environ["AWS_PINPOINT_APP_ID"]

response = pinpoint_client.send_messages(
ApplicationId=app_id,
MessageRequest={
"Addresses": {
to: {
"ChannelType": "EMAIL"
}
},
"MessageConfiguration": {
"EmailMessage": {
"SimpleEmail": {
"Subject": {
"Charset": "UTF-8",
"Data": subject
},
"HtmlPart": {
"Charset": "UTF-8",
"Data": message
},
"TextPart": {
"Charset": "UTF-8",
"Data": message
}
}
}
}
}
)
print(response)

return response
8 changes: 8 additions & 0 deletions app/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
<body>
<main>
<h1>Hello, world</h1>
<ul>
<li><a href="/">Home</a></li>
<li><a href="/health">Health</a></li>
<li><a href="/migrations">Migrations</a></li>
<li><a href="/document-upload">Document Upload</a></li>
<li><a href="/email-notifications">Email Notifications</a></li>
<li><a href="/secrets">Secrets</a></li>
</ul>
</main>
</body>
</html>
5 changes: 4 additions & 1 deletion infra/app/service/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ module "service" {
},
module.app_config.enable_identity_provider ? {
identity_provider_access = module.identity_provider_client[0].access_policy_arn,
} : {}
} : {},
module.app_config.enable_notifications ? {
notifications_access = module.notifications[0].access_policy_arn,
} : {},
)

is_temporary = local.is_temporary
Expand Down
6 changes: 6 additions & 0 deletions infra/modules/network/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ variable "enable_command_execution" {
default = false
}

variable "enable_notifications" {
type = bool
description = "Whether the application(s) in this network need AWS Pinpoint access."
default = false
}

variable "has_database" {
type = bool
description = "Whether the application(s) in this network have a database. Determines whether to create VPC endpoints needed by the database layer."
Expand Down
3 changes: 3 additions & 0 deletions infra/modules/network/vpc_endpoints.tf
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ locals {

# AWS services used by ECS Exec
var.enable_command_execution ? ["ssmmessages"] : [],

# AWS services used by notifications
var.enable_notifications ? ["pinpoint", "email-smtp"] : [],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's called mobile-targeting in IAM but the VPC endpoints are ["pinpoint", "email-smtp"] ??? bah

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i put mobiletargeting at first and got an error saying that doesn't exist.

also, the AWS docs don't mention "email-smtp" anywhere, instead they said to search for smtp and then click on that one, so I had to go to the console and search to find out that email-smtp was what they meant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image https://docs.aws.amazon.com/ses/latest/dg/send-email-set-up-vpc-endpoints.html

amazing right?

)

# S3 and DynamoDB use Gateway VPC endpoints. All other services use Interface VPC endpoints
Expand Down
32 changes: 32 additions & 0 deletions infra/modules/notifications/resources/access_control.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
resource "aws_iam_policy" "access" {
name = "${var.name}-notifications-access"
description = "Policy for calling SendMessages and SendUsersMessages on Pinpoint app ${var.name}"

policy = jsonencode({
Version = "2012-10-17"
Statement = [
# From https://docs.aws.amazon.com/pinpoint/latest/developerguide/permissions-actions.html#permissions-actions-apiactions-messages
{
Effect = "Allow"
Action = [
"mobiletargeting:SendMessages",
"mobiletargeting:SendUsersMessages"
]
Resource = "${aws_pinpoint_app.app.arn}/messages"
},

# From https://docs.aws.amazon.com/pinpoint/latest/developerguide/permissions-ses.html
{
Effect = "Allow"
Action = [
"ses:SendEmail",
"ses:SendRawEmail",
]
Resource = [
var.domain_identity_arn,
"arn:*:ses:*:*:configuration-set/*",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason why this 2nd on isn't an arn as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configuration sets don't actually have arns as far as i could tell. couldn't find it on AWS console, and don't see it in the outputs in the terraform aws_sesv2_configuration_set data source or resource, so you'd have to construct it manually.

and in my opinion i don't think it's actually a big deal to allow the use of any configuration set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow... yeah no arns mentioned here. That's a new one for me!

]
}
]
})
}
4 changes: 4 additions & 0 deletions infra/modules/notifications/resources/outputs.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
output "app_id" {
value = aws_pinpoint_app.app.application_id
}

output "access_policy_arn" {
value = aws_iam_policy.access.arn
}
4 changes: 4 additions & 0 deletions infra/networks/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ locals {
for environment_config in app.environment_configs : true if environment_config.service_config.enable_command_execution == true && environment_config.network_name == var.network_name
])
])

# Whether any of the applications in the network has enabled notifications
enable_notifications = anytrue([for app in local.apps_in_network : app.enable_notifications])
}

terraform {
Expand Down Expand Up @@ -76,6 +79,7 @@ module "network" {
has_database = local.has_database
has_external_non_aws_service = local.has_external_non_aws_service
enable_command_execution = local.enable_command_execution
enable_notifications = local.enable_notifications
}

module "domain" {
Expand Down
Loading