Added struct definition to include ClientCert information to API Gateway when using mTLS #342

dum0nt73 · 2020-12-05T20:17:32Z

Issue #337

Added struct definition to include ClientCert information in APIGatewayCustomAuthorizerRequestTypeRequestIdentity and APIGatewayV2HTTPRequestContext for REST and HTTP API Gateway when mTLS is configured.

https://aws.amazon.com/blogs/compute/introducing-mutual-tls-authentication-for-amazon-api-gateway/
https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-mutual-tls.html
https://docs.aws.amazon.com/apigateway/latest/developerguide/rest-api-mutual-tls.html

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…ity and Authentication and ClientCert to APIGatewayV2HTTPRequestContext Struct definitions

Catching up to Current aws commits

codecov-io · 2020-12-05T20:25:20Z

Codecov Report

Merging #342 (92a9843) into master (e5da494) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #342   +/-   ##
=======================================
  Coverage   72.22%   72.22%           
=======================================
  Files          19       19           
  Lines         738      738           
=======================================
  Hits          533      533           
  Misses        138      138           
  Partials       67       67

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5da494...92a9843. Read the comment docs.

harrisonhjones

Thanks for the contribution. Left a few comments / questions.

harrisonhjones · 2020-12-17T18:22:40Z

events/apigw.go

+	Validity      APIGatewayV2HTTPRequestContextAuthenticationClientCertValidity `json:"validity"`
+}
+
+// APIGatewayV2HTTPRequestContextAuthentication contains authentication context information for the request caller including client certificate information if using mTLS..


nit pick: here and elsewhere there is an unnecessary extra . at end of doc comment

Fixed the extra .

harrisonhjones · 2020-12-17T18:23:35Z

events/apigw.go

@@ -189,10 +190,46 @@ type APIGatewayWebsocketProxyRequestContext struct {
 	Status             string                    `json:"status"`
 }

-// APIGatewayCustomAuthorizerRequestTypeRequestIdentity contains identity information for the request caller.
+// APIGatewayCustomAuthorizerRequestTypeRequestIdentityClientCertValidity contains certificate validity information for the request caller if using mTLS..


I usually prefer for definitions to read top to bottom in the order they are used. In this case that would mean that each of your new types would only come after they were first used. Could we make that update here?

Changed the order of definitions to match project conventions.

harrisonhjones · 2020-12-17T18:25:55Z

events/apigw.go

+	NotAfter  string `json:"notAfter"`
+	NotBefore string `json:"notBefore"`


Looks like these are timestamps. I'm not entirely sure what we do elsewhere in this package but maybe these could be time.Times?

I was unable to find a way to convert the time format of the validity dates in the JSON unmarshalling not of the Tag hints I could find seam to work. I don't think writing a custom unmarshalling routine would be appropriate but am open to suggestion.

harrisonhjones · 2020-12-17T18:27:18Z

events/apigw.go

+	Time           string                                               `json:"time"`
+	TimeEpoch      int64                                                `json:"timeEpoch"`
+	HTTP           APIGatewayV2HTTPRequestContextHTTPDescription        `json:"http"`
+	Authentication APIGatewayV2HTTPRequestContextAuthentication         `json:"authentication"`


This field is optional right? Or is it always supplied now by API GW?

Supplied by API GW when mTLS is configured, otherwise it will be ignored since we are just unmarshalling.

harrisonhjones · 2020-12-17T18:28:30Z

events/apigw.go

-	SourceIP string `json:"sourceIp"`
+	APIKey     string                                                         `json:"apiKey"`
+	SourceIP   string                                                         `json:"sourceIp"`
+	ClientCert APIGatewayCustomAuthorizerRequestTypeRequestIdentityClientCert `json:"clientCert"`


This field is optional right? Or is it always supplied now by API GW?

Supplied by API GW when mTLS is configured, otherwise it will be ignored since we are just unmarshalling.

Bring local fork up to date

… update

…uest

queeno · 2021-05-19T14:30:58Z

Are you planning to merge and release this PR soon?

codecov-commenter · 2021-05-22T02:03:33Z

Codecov Report

Merging #342 (87559da) into master (5d64132) will not change coverage.
The diff coverage is n/a.

❗ Current head 87559da differs from pull request most recent head bc8a460. Consider uploading reports for the commit bc8a460 to get more accurate results

@@           Coverage Diff           @@
##           master     #342   +/-   ##
=======================================
  Coverage   72.22%   72.22%           
=======================================
  Files          19       19           
  Lines         738      738           
=======================================
  Hits          533      533           
  Misses        138      138           
  Partials       67       67

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d64132...bc8a460. Read the comment docs.

Catching up to current Master Fork

kpes · 2021-06-28T15:09:43Z

@harrisonhjones @dum0nt73 Any chance this could be merged soon?

dum0nt73 · 2021-07-03T05:32:51Z

@kpes , I have requested the review to have it merged just waiting now. Hopefully they will approve it soon.

carlzogh

LGTM - thanks @dum0nt73

BWPSmith73 and others added 5 commits November 20, 2020 15:02

Added ClientCert to APIGatewayCustomAuthorizerRequestTypeRequestIdent…

1d83b07

…ity and Authentication and ClientCert to APIGatewayV2HTTPRequestContext Struct definitions

Merge pull request #1 from aws/master

283152d

Catching up to Current aws commits

Updated testdata to include new mTLS context objexts to go test passes

411a5fc

Merge branch 'master' of https://github.com/dum0nt73/aws-lambda-go

0e98aee

Updated lambda authorizer test json to includ clientCert

734e78c

harrisonhjones reviewed Dec 17, 2020

View reviewed changes

dum0nt73 and others added 8 commits April 11, 2021 17:02

Merge pull request #2 from aws/master

c6725b7

Bring local fork up to date

Rearrange struct definition order to match AWS Go project conventions

e31c9a1

Merge branch 'master' of https://github.com/dum0nt73/aws-lambda-go

92a9843

Rearrange struct definition order to match AWS Go project conventions

19fe79d

Rearrange struct definition order to match AWS Go project conventions…

ec6fee3

… update

Fixed space in test data file

87559da

Converted Validity start and end from string to time.Time

45be04b

Converted Validity start and end from string to time.Time for HTTPReq…

e2f0235

…uest

Merge branch 'master' into master

36dc632

dum0nt73 and others added 3 commits June 20, 2021 11:09

Merge pull request #3 from aws/master

3329129

Catching up to current Master Fork

Changing Validity notAfter and notBefore back to strings

1c35d15

Merge branch 'master' of https://github.com/dum0nt73/aws-lambda-go

bc8a460

carlzogh approved these changes Jul 5, 2021

View reviewed changes

carlzogh merged commit 159d1c6 into aws:master Jul 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added struct definition to include ClientCert information to API Gateway when using mTLS #342

Added struct definition to include ClientCert information to API Gateway when using mTLS #342

dum0nt73 commented Dec 5, 2020

codecov-io commented Dec 5, 2020 •

edited

Loading

harrisonhjones left a comment

harrisonhjones Dec 17, 2020

dum0nt73 Jun 20, 2021

harrisonhjones Dec 17, 2020

dum0nt73 Jun 20, 2021

harrisonhjones Dec 17, 2020

dum0nt73 Jun 20, 2021

harrisonhjones Dec 17, 2020

dum0nt73 Jun 20, 2021

harrisonhjones Dec 17, 2020

dum0nt73 Jun 20, 2021

queeno commented May 19, 2021

codecov-commenter commented May 22, 2021 •

edited

Loading

kpes commented Jun 28, 2021

dum0nt73 commented Jul 3, 2021

carlzogh left a comment

		NotAfter string `json:"notAfter"`
		NotBefore string `json:"notBefore"`

Added struct definition to include ClientCert information to API Gateway when using mTLS #342

Added struct definition to include ClientCert information to API Gateway when using mTLS #342

Conversation

dum0nt73 commented Dec 5, 2020

codecov-io commented Dec 5, 2020 • edited Loading

Codecov Report

harrisonhjones left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

queeno commented May 19, 2021

codecov-commenter commented May 22, 2021 • edited Loading

Codecov Report

kpes commented Jun 28, 2021

dum0nt73 commented Jul 3, 2021

carlzogh left a comment

Choose a reason for hiding this comment

codecov-io commented Dec 5, 2020 •

edited

Loading

codecov-commenter commented May 22, 2021 •

edited

Loading