Skip to content

fix: resource policies fix for v1.15.1 #1181

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 7, 2019
Merged

fix: resource policies fix for v1.15.1 #1181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
daecd7f
fix: resource policies fix for v1.15.1
praneetap Oct 7, 2019
3f670e0
remove unnecessary boto from swagger.py tests
praneetap Oct 7, 2019
2c0a91e
version bump
praneetap Oct 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion examples/2016-10-31/api_resource_policy/template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ Globals:
"Effect": "Allow",
"Principal": "*",
"Action": "execute-api:Invoke",
"Resource": "execute-api:*/*/*",
"Resource": "execute-api:/Prod/PUT/get",
"Condition": {
"IpAddress": {
"aws:SourceIp": "1.2.3.4"
}
}
}]
# OR you can use the following, they both do the same thing
IpRangeBlacklist: ['1.2.3.4']
Resources:
MyFunction:
Type: AWS::Serverless::Function
Expand Down
2 changes: 1 addition & 1 deletion samtranslator/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '1.15.0'
__version__ = '1.15.1'
13 changes: 3 additions & 10 deletions samtranslator/swagger/swagger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from samtranslator.model.intrinsics import ref
from samtranslator.model.intrinsics import make_conditional, fnSub
from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException
from samtranslator.translator.arn_generator import ArnGenerator


class SwaggerEditor(object):
Expand Down Expand Up @@ -891,15 +890,9 @@ def _get_method_path_uri_list(self, path, api_id, stage):

for m in methods:
method = '*' if (m.lower() == self._X_ANY_METHOD or m.lower() == 'any') else m.upper()

# RestApiId can be a simple string or intrinsic function like !Ref. Using Fn::Sub will handle both cases
resource = '${__ApiId__}/' + '${__Stage__}/' + method + path
partition = ArnGenerator.get_partition_name(None)
if partition is None:
partition = "aws"
source_arn = fnSub(ArnGenerator.generate_arn(partition=partition, service='execute-api', resource=resource),
{"__ApiId__": api_id, "__Stage__": stage})
uri_list.extend([source_arn])
resource = "execute-api:/${__Stage__}/" + method + path
resource = fnSub(resource, {"__Stage__": stage})
uri_list.extend([resource])
return uri_list

def _add_ip_resource_policy_for_method(self, ip_list, conditional, resource_list):
Expand Down
104 changes: 48 additions & 56 deletions tests/swagger/test_swagger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,6 @@ def test_must_add_custom_statements(self):

self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))

@patch("boto3.session.Session.region_name", "eu-west-2")
def test_must_add_iam_allow(self):
## fails
resourcePolicy = {
Expand All @@ -1438,14 +1437,14 @@ def test_must_add_iam_allow(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Allow',
Expand All @@ -1457,7 +1456,6 @@ def test_must_add_iam_allow(self):

self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))

@patch("boto3.session.Session.region_name", "eu-west-2")
def test_must_add_iam_deny(self):

resourcePolicy = {
Expand All @@ -1474,14 +1472,14 @@ def test_must_add_iam_deny(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Deny',
Expand All @@ -1493,7 +1491,6 @@ def test_must_add_iam_deny(self):

self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))

@patch("boto3.session.Session.region_name", "eu-west-2")
def test_must_add_ip_allow(self):

resourcePolicy = {
Expand All @@ -1510,14 +1507,14 @@ def test_must_add_ip_allow(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Allow',
Expand All @@ -1527,14 +1524,14 @@ def test_must_add_ip_allow(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Deny',
Expand All @@ -1549,7 +1546,6 @@ def test_must_add_ip_allow(self):

self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))

@patch("boto3.session.Session.region_name", "eu-west-2")
def test_must_add_ip_deny(self):

resourcePolicy = {
Expand All @@ -1566,14 +1562,14 @@ def test_must_add_ip_deny(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Allow',
Expand All @@ -1583,14 +1579,14 @@ def test_must_add_ip_deny(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Deny',
Expand All @@ -1605,7 +1601,6 @@ def test_must_add_ip_deny(self):

self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))

@patch("boto3.session.Session.region_name", "eu-west-2")
def test_must_add_vpc_allow(self):

resourcePolicy = {
Expand All @@ -1624,14 +1619,14 @@ def test_must_add_vpc_allow(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Allow',
Expand All @@ -1641,14 +1636,14 @@ def test_must_add_vpc_allow(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Deny',
Expand All @@ -1663,14 +1658,14 @@ def test_must_add_vpc_allow(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Deny',
Expand All @@ -1686,7 +1681,6 @@ def test_must_add_vpc_allow(self):

self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))

@patch("boto3.session.Session.region_name", "eu-west-2")
def test_must_add_vpc_deny(self):

resourcePolicy = {
Expand All @@ -1704,14 +1698,14 @@ def test_must_add_vpc_deny(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Allow',
Expand All @@ -1721,14 +1715,14 @@ def test_must_add_vpc_deny(self):
'Action': 'execute-api:Invoke',
'Resource': [ {
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Deny',
Expand All @@ -1744,9 +1738,7 @@ def test_must_add_vpc_deny(self):

self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))

@patch("boto3.session.Session.region_name", "eu-west-2")
def test_must_add_iam_allow_and_custom(self):
## fails
resourcePolicy = {
'AwsAccountWhitelist': [
'123456'
Expand All @@ -1769,14 +1761,14 @@ def test_must_add_iam_allow_and_custom(self):
'Action': 'execute-api:Invoke',
'Resource': [{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/PUT/foo',
{'__Stage__': 'prod'}
]
},
{
'Fn::Sub': [
'arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/GET/foo',
{'__Stage__': 'prod', '__ApiId__': '123'}
'execute-api:/${__Stage__}/GET/foo',
{'__Stage__': 'prod'}
]
}],
'Effect': 'Allow',
Expand Down
25 changes: 12 additions & 13 deletions tests/translator/output/api_with_aws_account_blacklist.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,26 @@
]
}
},
"ServerlessRestApiDeploymentaf499b09d8": {
"Type": "AWS::ApiGateway::Deployment",
"ServerlessRestApiProdStage": {
"Type": "AWS::ApiGateway::Stage",
"Properties": {
"DeploymentId": {
"Ref": "ServerlessRestApiDeployment58caafe4ca"
},
"RestApiId": {
"Ref": "ServerlessRestApi"
},
"Description": "RestApi deployment id: af499b09d83319b6e1cf6d2f1611824b4c0bd7a9",
"StageName": "Stage"
"StageName": "Prod"
}
},
"ServerlessRestApiProdStage": {
"Type": "AWS::ApiGateway::Stage",
"ServerlessRestApiDeployment58caafe4ca": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"DeploymentId": {
"Ref": "ServerlessRestApiDeploymentaf499b09d8"
},
"RestApiId": {
"Ref": "ServerlessRestApi"
},
"StageName": "Prod"
"Description": "RestApi deployment id: 58caafe4ca843d2d259f522ee6c408cfd9523ea1",
"StageName": "Stage"
}
},
"MyFunctionRole": {
Expand Down Expand Up @@ -100,10 +100,9 @@
"Resource": [
{
"Fn::Sub": [
"arn:aws:execute-api:${AWS::Region}:${AWS::AccountId}:${__ApiId__}/${__Stage__}/PUT/get",
"execute-api:/${__Stage__}/PUT/get",
{
"__Stage__": "Prod",
"__ApiId__": "ServerlessRestApi"
"__Stage__": "Prod"
}
]
}
Expand Down
Loading