From 152b44e07f29c9a4c5a18086e66a537303f9351a Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Wed, 27 Feb 2019 18:58:53 +1100 Subject: [PATCH 1/6] Dont decode policies --- iamy/policy.go | 10 ++-------- iamy/policy_test.go | 7 +++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/iamy/policy.go b/iamy/policy.go index 094b73e..325ecc7 100644 --- a/iamy/policy.go +++ b/iamy/policy.go @@ -3,19 +3,13 @@ package iamy import ( "encoding/json" "log" - "net/url" "reflect" "sort" ) -func NewPolicyDocumentFromEncodedJson(encoded string) (*PolicyDocument, error) { - jsonString, err := url.QueryUnescape(encoded) - if err != nil { - return nil, err - } - +func NewPolicyDocumentFromEncodedJson(jsonString string) (*PolicyDocument, error) { var doc PolicyDocument - if err = json.Unmarshal([]byte(jsonString), &doc); err != nil { + if err := json.Unmarshal([]byte(jsonString), &doc); err != nil { return nil, err } diff --git a/iamy/policy_test.go b/iamy/policy_test.go index e12ebc7..a23ad6a 100644 --- a/iamy/policy_test.go +++ b/iamy/policy_test.go @@ -126,3 +126,10 @@ Actual: %#v`, nt.description, nt.input, nt.expected, result) } } } + +func TestNewPolicyDocumentFromEncodedJson(t *testing.T) { + _, err := NewPolicyDocumentFromEncodedJson("{\"Version\":\"2012-10-17\",\"Id\":\"AllowPublicRead\",\"Statement\":[{\"Sid\":\"PublicReadBucketObjects\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:GetObject\",\"Resource\":\"arn:aws:s3:::example.com/*\",\"Condition\":{\"StringEquals\":{\"aws:Referer\":\"%zz\"}}}]}") + if err != nil { + t.Errorf("Error decoding policy %s", err) + } +} From a2fa33185c4660eee13726779609f6d1efc1398b Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Mon, 4 Mar 2019 17:14:47 +1100 Subject: [PATCH 2/6] Print errors I'm not quite sure how to ensure the errors get raised appropriately --- iamy/aws.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/iamy/aws.go b/iamy/aws.go index 79ccdfb..681bb6f 100644 --- a/iamy/aws.go +++ b/iamy/aws.go @@ -120,8 +120,11 @@ func (a *AwsFetcher) fetchIamData() error { }), }, func(resp *iam.GetAccountAuthorizationDetailsOutput, lastPage bool) bool { + // There's a bug here. This doesn't set the variable outside this function scope + // So IAMY just swallows any errors returned from populateIamData() populateIamDataErr = a.populateIamData(resp) if populateIamDataErr != nil { + log.Println("Error Fetching IAM Data") return false } return true From bc42f210b94b1c8a300a17734a091b0b27115e0b Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Mon, 4 Mar 2019 17:15:29 +1100 Subject: [PATCH 3/6] Don't decode S3 bucket policies They're already decoded, as opposed to IAM policies --- iamy/aws.go | 2 +- iamy/policy.go | 20 +++++++++++++++++++- iamy/policy_test.go | 4 ++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/iamy/aws.go b/iamy/aws.go index 681bb6f..e4d14ec 100644 --- a/iamy/aws.go +++ b/iamy/aws.go @@ -92,7 +92,7 @@ func (a *AwsFetcher) fetchS3Data() error { continue } - policyDoc, err := NewPolicyDocumentFromEncodedJson(b.policyJson) + policyDoc, err := NewPolicyDocumentFromJson(b.policyJson) if err != nil { return errors.Wrap(err, "Error creating Policy document") } diff --git a/iamy/policy.go b/iamy/policy.go index 325ecc7..f6d0aca 100644 --- a/iamy/policy.go +++ b/iamy/policy.go @@ -3,15 +3,33 @@ package iamy import ( "encoding/json" "log" + "net/url" "reflect" "sort" ) -func NewPolicyDocumentFromEncodedJson(jsonString string) (*PolicyDocument, error) { +func NewPolicyDocumentFromJson(jsonString string) (*PolicyDocument, error) { var doc PolicyDocument if err := json.Unmarshal([]byte(jsonString), &doc); err != nil { + log.Printf("Error unmarshalling JSON %s %s", err, jsonString) return nil, err } + //log.Printf("Doc %s\nJSON: %s", doc, jsonString) + + return &doc, nil +} + +func NewPolicyDocumentFromEncodedJson(encoded string) (*PolicyDocument, error) { + jsonString, err := url.QueryUnescape(encoded) + if err != nil { + return nil, err + } + var doc PolicyDocument + if err := json.Unmarshal([]byte(jsonString), &doc); err != nil { + log.Printf("Error unmarshalling JSON %s %s", err, jsonString) + return nil, err + } + //log.Printf("Doc %s\nJSON: %s", doc, jsonString) return &doc, nil } diff --git a/iamy/policy_test.go b/iamy/policy_test.go index a23ad6a..449f484 100644 --- a/iamy/policy_test.go +++ b/iamy/policy_test.go @@ -127,8 +127,8 @@ Actual: %#v`, nt.description, nt.input, nt.expected, result) } } -func TestNewPolicyDocumentFromEncodedJson(t *testing.T) { - _, err := NewPolicyDocumentFromEncodedJson("{\"Version\":\"2012-10-17\",\"Id\":\"AllowPublicRead\",\"Statement\":[{\"Sid\":\"PublicReadBucketObjects\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:GetObject\",\"Resource\":\"arn:aws:s3:::example.com/*\",\"Condition\":{\"StringEquals\":{\"aws:Referer\":\"%zz\"}}}]}") +func TestNewPolicyDocumentFromJson(t *testing.T) { + _, err := NewPolicyDocumentFromJson("{\"Version\":\"2012-10-17\",\"Id\":\"AllowPublicRead\",\"Statement\":[{\"Sid\":\"PublicReadBucketObjects\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:GetObject\",\"Resource\":\"arn:aws:s3:::example.com/*\",\"Condition\":{\"StringEquals\":{\"aws:Referer\":\"%zz\"}}}]}") if err != nil { t.Errorf("Error decoding policy %s", err) } From a21f30502b812f82db5e30369c893a29843c0bb5 Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Tue, 5 Mar 2019 10:55:54 +1100 Subject: [PATCH 4/6] DRY up function Remove commented out code --- iamy/policy.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/iamy/policy.go b/iamy/policy.go index f6d0aca..3883858 100644 --- a/iamy/policy.go +++ b/iamy/policy.go @@ -14,7 +14,6 @@ func NewPolicyDocumentFromJson(jsonString string) (*PolicyDocument, error) { log.Printf("Error unmarshalling JSON %s %s", err, jsonString) return nil, err } - //log.Printf("Doc %s\nJSON: %s", doc, jsonString) return &doc, nil } @@ -24,14 +23,8 @@ func NewPolicyDocumentFromEncodedJson(encoded string) (*PolicyDocument, error) { if err != nil { return nil, err } - var doc PolicyDocument - if err := json.Unmarshal([]byte(jsonString), &doc); err != nil { - log.Printf("Error unmarshalling JSON %s %s", err, jsonString) - return nil, err - } - //log.Printf("Doc %s\nJSON: %s", doc, jsonString) - return &doc, nil + return NewPolicyDocumentFromJson(jsonString) } // PolicyDocument represents an AWS policy document. From 4173f0423eb442e36ab47315b1ab4360dca03cc2 Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Tue, 5 Mar 2019 10:57:15 +1100 Subject: [PATCH 5/6] Make test readable --- iamy/policy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iamy/policy_test.go b/iamy/policy_test.go index 449f484..acec793 100644 --- a/iamy/policy_test.go +++ b/iamy/policy_test.go @@ -128,7 +128,7 @@ Actual: %#v`, nt.description, nt.input, nt.expected, result) } func TestNewPolicyDocumentFromJson(t *testing.T) { - _, err := NewPolicyDocumentFromJson("{\"Version\":\"2012-10-17\",\"Id\":\"AllowPublicRead\",\"Statement\":[{\"Sid\":\"PublicReadBucketObjects\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"s3:GetObject\",\"Resource\":\"arn:aws:s3:::example.com/*\",\"Condition\":{\"StringEquals\":{\"aws:Referer\":\"%zz\"}}}]}") + _, err := NewPolicyDocumentFromJson(`{"Version":"2012-10-17","Id":"AllowPublicRead","Statement":[{"Sid":"PublicReadBucketObjects","Effect":"Allow","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::example.com/*","Condition":{"StringEquals":{"aws:Referer":"%zz"}}}]}`) if err != nil { t.Errorf("Error decoding policy %s", err) } From 59eac9c2d936fa9b88f33c07897be56bc635315d Mon Sep 17 00:00:00 2001 From: Patrick Robinson Date: Tue, 5 Mar 2019 10:58:00 +1100 Subject: [PATCH 6/6] Remove comment and log Will fix in another PR --- iamy/aws.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/iamy/aws.go b/iamy/aws.go index e4d14ec..9b8ea04 100644 --- a/iamy/aws.go +++ b/iamy/aws.go @@ -120,11 +120,8 @@ func (a *AwsFetcher) fetchIamData() error { }), }, func(resp *iam.GetAccountAuthorizationDetailsOutput, lastPage bool) bool { - // There's a bug here. This doesn't set the variable outside this function scope - // So IAMY just swallows any errors returned from populateIamData() populateIamDataErr = a.populateIamData(resp) if populateIamDataErr != nil { - log.Println("Error Fetching IAM Data") return false } return true